Use StatusForbidden to prevent infinite redirects

oauth2-proxy · Nov 18, 2020 · e7a94d2 · e7a94d2
1 parent 849139a
commit e7a94d2
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 22 deletions.
diff --git a/oauthproxy.go b/oauthproxy.go
@@ -934,14 +934,14 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
 func (p *OAuthProxy) AuthOnly(rw http.ResponseWriter, req *http.Request) {
 	session, err := p.getAuthenticatedSession(rw, req)
 	if err != nil {
-		http.Error(rw, "unauthorized request", http.StatusUnauthorized)
+		http.Error(rw, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
 		return
 	}
 
 	// Allow secondary group restrictions based on the `allowed_group` or
 	// `allowed_groups` querystring parameter
 	if !checkAllowedGroups(req, session) {
-		http.Error(rw, "unauthorized request", http.StatusUnauthorized)
+		http.Error(rw, http.StatusText(http.StatusForbidden), http.StatusForbidden)
 		return
 	}
 

diff --git a/oauthproxy_test.go b/oauthproxy_test.go
@@ -1189,7 +1189,7 @@ func TestAuthOnlyEndpointUnauthorizedOnNoCookieSetError(t *testing.T) {
 	test.proxy.ServeHTTP(test.rw, test.req)
 	assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
 	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) {
@@ -1209,7 +1209,7 @@ func TestAuthOnlyEndpointUnauthorizedOnExpiration(t *testing.T) {
 	test.proxy.ServeHTTP(test.rw, test.req)
 	assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
 	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) {
@@ -1228,7 +1228,7 @@ func TestAuthOnlyEndpointUnauthorizedOnEmailValidationFailure(t *testing.T) {
 	test.proxy.ServeHTTP(test.rw, test.req)
 	assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
 	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) {
@@ -2651,84 +2651,84 @@ func TestAuthOnlyAllowedGroups(t *testing.T) {
 		allowedGroups      []string
 		groups             []string
 		querystring        string
-		expectUnauthorized bool
+		expectedStatusCode int
 	}{
 		{
 			name:               "NoAllowedGroups",
 			allowedGroups:      []string{},
 			groups:             []string{},
 			querystring:        "",
-			expectUnauthorized: false,
+			expectedStatusCode: http.StatusAccepted,
 		},
 		{
 			name:               "NoAllowedGroupsUserHasGroups",
 			allowedGroups:      []string{},
 			groups:             []string{"a", "b"},
 			querystring:        "",
-			expectUnauthorized: false,
+			expectedStatusCode: http.StatusAccepted,
 		},
 		{
 			name:               "UserInAllowedGroup",
 			allowedGroups:      []string{"a"},
 			groups:             []string{"a", "b"},
 			querystring:        "",
-			expectUnauthorized: false,
+			expectedStatusCode: http.StatusAccepted,
 		},
 		{
 			name:               "UserNotInAllowedGroup",
 			allowedGroups:      []string{"a"},
 			groups:             []string{"c"},
 			querystring:        "",
-			expectUnauthorized: true,
+			expectedStatusCode: http.StatusUnauthorized,
 		},
 		{
 			name:               "UserInQuerystringGroup",
 			allowedGroups:      []string{"a", "b"},
 			groups:             []string{"a", "c"},
 			querystring:        "?allowed_group=a",
-			expectUnauthorized: false,
+			expectedStatusCode: http.StatusAccepted,
 		},
 		{
 			name:               "UserInOnlyQuerystringGroup",
 			allowedGroups:      []string{},
 			groups:             []string{"a", "c"},
 			querystring:        "?allowed_groups=a,b",
-			expectUnauthorized: false,
+			expectedStatusCode: http.StatusAccepted,
 		},
 		{
 			name:               "UserInMultiParamQuerystringGroup",
 			allowedGroups:      []string{"a", "b"},
 			groups:             []string{"b"},
 			querystring:        "?allowed_group=a&allowed_group=b",
-			expectUnauthorized: false,
+			expectedStatusCode: http.StatusAccepted,
 		},
 		{
 			name:               "UserInDelimitedQuerystringGroup",
 			allowedGroups:      []string{"a", "b", "c"},
 			groups:             []string{"c"},
 			querystring:        "?allowed_groups=a,c",
-			expectUnauthorized: false,
+			expectedStatusCode: http.StatusAccepted,
 		},
 		{
 			name:               "UserNotInQuerystringGroup",
 			allowedGroups:      []string{},
 			groups:             []string{"c"},
 			querystring:        "?allowed_group=a&allowed_group=b",
-			expectUnauthorized: true,
+			expectedStatusCode: http.StatusForbidden,
 		},
 		{
 			name:               "UserInConfigGroupNotInQuerystringGroup",
 			allowedGroups:      []string{"a", "b", "c"},
 			groups:             []string{"c"},
 			querystring:        "?allowed_group=a&allowed_group=b",
-			expectUnauthorized: true,
+			expectedStatusCode: http.StatusForbidden,
 		},
 		{
 			name:               "UserInQuerystringGroupNotInConfigGroup",
 			allowedGroups:      []string{"a", "b"},
 			groups:             []string{"c"},
 			querystring:        "?allowed_groups=b,c",
-			expectUnauthorized: true,
+			expectedStatusCode: http.StatusUnauthorized,
 		},
 	}
 
@@ -2756,11 +2756,7 @@ func TestAuthOnlyAllowedGroups(t *testing.T) {
 
 			test.proxy.ServeHTTP(test.rw, test.req)
 
-			if tt.expectUnauthorized {
-				assert.Equal(t, http.StatusUnauthorized, test.rw.Code)
-			} else {
-				assert.Equal(t, http.StatusAccepted, test.rw.Code)
-			}
+			assert.Equal(t, tt.expectedStatusCode, test.rw.Code)
 		})
 	}
 }