Use StatusForbidden to prevent infinite redirects

This commit is contained in:
Nick Meves 2020-11-17 19:03:41 -08:00
parent 23b2355f85
commit 44d83e5f95
No known key found for this signature in database
GPG Key ID: 93BA8A3CEDCDD1CF
2 changed files with 18 additions and 22 deletions

@ -930,14 +930,14 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
func (p *OAuthProxy) AuthOnly(rw http.ResponseWriter, req *http.Request) { func (p *OAuthProxy) AuthOnly(rw http.ResponseWriter, req *http.Request) {
session, err := p.getAuthenticatedSession(rw, req) session, err := p.getAuthenticatedSession(rw, req)
if err != nil { if err != nil {
http.Error(rw, "unauthorized request", http.StatusUnauthorized) http.Error(rw, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
return return
} }
// Allow secondary group restrictions based on the `allowed_group` or // Allow secondary group restrictions based on the `allowed_group` or
// `allowed_groups` querystring parameter // `allowed_groups` querystring parameter
if !checkAllowedGroups(req, session) { if !checkAllowedGroups(req, session) {
http.Error(rw, "unauthorized request", http.StatusUnauthorized) http.Error(rw, http.StatusText(http.StatusForbidden), http.StatusForbidden)
return return
} }

@ -1236,7 +1236,7 @@ func TestAuthOnlyEndpointUnauthorizedOnNoCookieSetError(t *testing.T) {
test.proxy.ServeHTTP(test.rw, test.req) test.proxy.ServeHTTP(test.rw, test.req)
assert.Equal(t, http.StatusUnauthorized, test.rw.Code) assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
bodyBytes, _ := ioutil.ReadAll(test.rw.Body) bodyBytes, _ := ioutil.ReadAll(test.rw.Body)
assert.Equal(t, "unauthorized request\n", string(bodyBytes)) assert.Equal(t, "Unauthorized\n", string(bodyBytes))
} }
func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) { func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) {
@ -1256,7 +1256,7 @@ func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) {
test.proxy.ServeHTTP(test.rw, test.req) test.proxy.ServeHTTP(test.rw, test.req)
assert.Equal(t, http.StatusUnauthorized, test.rw.Code) assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
bodyBytes, _ := ioutil.ReadAll(test.rw.Body) bodyBytes, _ := ioutil.ReadAll(test.rw.Body)
assert.Equal(t, "unauthorized request\n", string(bodyBytes)) assert.Equal(t, "Unauthorized\n", string(bodyBytes))
} }
func TestAuthOnlyEndpointUnauthorizedOnEmailValidationFailure(t *testing.T) { func TestAuthOnlyEndpointUnauthorizedOnEmailValidationFailure(t *testing.T) {
@ -1275,7 +1275,7 @@ func TestAuthOnlyEndpointUnauthorizedOnEmailValidationFailure(t *testing.T) {
test.proxy.ServeHTTP(test.rw, test.req) test.proxy.ServeHTTP(test.rw, test.req)
assert.Equal(t, http.StatusUnauthorized, test.rw.Code) assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
bodyBytes, _ := ioutil.ReadAll(test.rw.Body) bodyBytes, _ := ioutil.ReadAll(test.rw.Body)
assert.Equal(t, "unauthorized request\n", string(bodyBytes)) assert.Equal(t, "Unauthorized\n", string(bodyBytes))
} }
func TestAuthOnlyEndpointSetXAuthRequestHeaders(t *testing.T) { func TestAuthOnlyEndpointSetXAuthRequestHeaders(t *testing.T) {
@ -2698,84 +2698,84 @@ func TestAuthOnlyAllowedGroups(t *testing.T) {
allowedGroups []string allowedGroups []string
groups []string groups []string
querystring string querystring string
expectUnauthorized bool expectedStatusCode int
}{ }{
{ {
name: "NoAllowedGroups", name: "NoAllowedGroups",
allowedGroups: []string{}, allowedGroups: []string{},
groups: []string{}, groups: []string{},
querystring: "", querystring: "",
expectUnauthorized: false, expectedStatusCode: http.StatusAccepted,
}, },
{ {
name: "NoAllowedGroupsUserHasGroups", name: "NoAllowedGroupsUserHasGroups",
allowedGroups: []string{}, allowedGroups: []string{},
groups: []string{"a", "b"}, groups: []string{"a", "b"},
querystring: "", querystring: "",
expectUnauthorized: false, expectedStatusCode: http.StatusAccepted,
}, },
{ {
name: "UserInAllowedGroup", name: "UserInAllowedGroup",
allowedGroups: []string{"a"}, allowedGroups: []string{"a"},
groups: []string{"a", "b"}, groups: []string{"a", "b"},
querystring: "", querystring: "",
expectUnauthorized: false, expectedStatusCode: http.StatusAccepted,
}, },
{ {
name: "UserNotInAllowedGroup", name: "UserNotInAllowedGroup",
allowedGroups: []string{"a"}, allowedGroups: []string{"a"},
groups: []string{"c"}, groups: []string{"c"},
querystring: "", querystring: "",
expectUnauthorized: true, expectedStatusCode: http.StatusUnauthorized,
}, },
{ {
name: "UserInQuerystringGroup", name: "UserInQuerystringGroup",
allowedGroups: []string{"a", "b"}, allowedGroups: []string{"a", "b"},
groups: []string{"a", "c"}, groups: []string{"a", "c"},
querystring: "?allowed_group=a", querystring: "?allowed_group=a",
expectUnauthorized: false, expectedStatusCode: http.StatusAccepted,
}, },
{ {
name: "UserInOnlyQuerystringGroup", name: "UserInOnlyQuerystringGroup",
allowedGroups: []string{}, allowedGroups: []string{},
groups: []string{"a", "c"}, groups: []string{"a", "c"},
querystring: "?allowed_groups=a,b", querystring: "?allowed_groups=a,b",
expectUnauthorized: false, expectedStatusCode: http.StatusAccepted,
}, },
{ {
name: "UserInMultiParamQuerystringGroup", name: "UserInMultiParamQuerystringGroup",
allowedGroups: []string{"a", "b"}, allowedGroups: []string{"a", "b"},
groups: []string{"b"}, groups: []string{"b"},
querystring: "?allowed_group=a&allowed_group=b", querystring: "?allowed_group=a&allowed_group=b",
expectUnauthorized: false, expectedStatusCode: http.StatusAccepted,
}, },
{ {
name: "UserInDelimitedQuerystringGroup", name: "UserInDelimitedQuerystringGroup",
allowedGroups: []string{"a", "b", "c"}, allowedGroups: []string{"a", "b", "c"},
groups: []string{"c"}, groups: []string{"c"},
querystring: "?allowed_groups=a,c", querystring: "?allowed_groups=a,c",
expectUnauthorized: false, expectedStatusCode: http.StatusAccepted,
}, },
{ {
name: "UserNotInQuerystringGroup", name: "UserNotInQuerystringGroup",
allowedGroups: []string{}, allowedGroups: []string{},
groups: []string{"c"}, groups: []string{"c"},
querystring: "?allowed_group=a&allowed_group=b", querystring: "?allowed_group=a&allowed_group=b",
expectUnauthorized: true, expectedStatusCode: http.StatusForbidden,
}, },
{ {
name: "UserInConfigGroupNotInQuerystringGroup", name: "UserInConfigGroupNotInQuerystringGroup",
allowedGroups: []string{"a", "b", "c"}, allowedGroups: []string{"a", "b", "c"},
groups: []string{"c"}, groups: []string{"c"},
querystring: "?allowed_group=a&allowed_group=b", querystring: "?allowed_group=a&allowed_group=b",
expectUnauthorized: true, expectedStatusCode: http.StatusForbidden,
}, },
{ {
name: "UserInQuerystringGroupNotInConfigGroup", name: "UserInQuerystringGroupNotInConfigGroup",
allowedGroups: []string{"a", "b"}, allowedGroups: []string{"a", "b"},
groups: []string{"c"}, groups: []string{"c"},
querystring: "?allowed_groups=b,c", querystring: "?allowed_groups=b,c",
expectUnauthorized: true, expectedStatusCode: http.StatusUnauthorized,
}, },
} }
@ -2803,11 +2803,7 @@ func TestAuthOnlyAllowedGroups(t *testing.T) {
test.proxy.ServeHTTP(test.rw, test.req) test.proxy.ServeHTTP(test.rw, test.req)
if tc.expectUnauthorized { assert.Equal(t, tc.expectedStatusCode, test.rw.Code)
assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
} else {
assert.Equal(t, http.StatusAccepted, test.rw.Code)
}
}) })
} }
} }