Merge pull request #1949 from flant/use-constants-for-errors

Use constants to form oauth2 error responses
dexidp · Jan 19, 2021 · a55de6c · a55de6c
2 parents 3650fe2 + bb503db
commit a55de6c
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 14 deletions.
diff --git a/server/oauth2.go b/server/oauth2.go
@@ -485,13 +485,13 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques
 		}
 	}
 	if !hasOpenIDScope {
-		return nil, newErr("invalid_scope", `Missing required scope(s) ["openid"].`)
+		return nil, newErr(errInvalidScope, `Missing required scope(s) ["openid"].`)
 	}
 	if len(unrecognized) > 0 {
-		return nil, newErr("invalid_scope", "Unrecognized scope(s) %q", unrecognized)
+		return nil, newErr(errInvalidScope, "Unrecognized scope(s) %q", unrecognized)
 	}
 	if len(invalidScopes) > 0 {
-		return nil, newErr("invalid_scope", "Client can't request scope(s) %q", invalidScopes)
+		return nil, newErr(errInvalidScope, "Client can't request scope(s) %q", invalidScopes)
 	}
 
 	var rt struct {
@@ -509,7 +509,7 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques
 		case responseTypeToken:
 			rt.token = true
 		default:
-			return nil, newErr("invalid_request", "Invalid response type %q", responseType)
+			return nil, newErr(errInvalidRequest, "Invalid response type %q", responseType)
 		}
 
 		if !s.supportedResponseTypes[responseType] {
@@ -518,28 +518,28 @@ func (s *Server) parseAuthorizationRequest(r *http.Request) (*storage.AuthReques
 	}
 
 	if len(responseTypes) == 0 {
-		return nil, newErr("invalid_requests", "No response_type provided")
+		return nil, newErr(errInvalidRequest, "No response_type provided")
 	}
 
 	if rt.token && !rt.code && !rt.idToken {
 		// "token" can't be provided by its own.
 		//
 		// https://openid.net/specs/openid-connect-core-1_0.html#Authentication
-		return nil, newErr("invalid_request", "Response type 'token' must be provided with type 'id_token' and/or 'code'")
+		return nil, newErr(errInvalidRequest, "Response type 'token' must be provided with type 'id_token' and/or 'code'")
 	}
 	if !rt.code {
 		// Either "id_token token" or "id_token" has been provided which implies the
 		// implicit flow. Implicit flow requires a nonce value.
 		//
 		// https://openid.net/specs/openid-connect-core-1_0.html#ImplicitAuthRequest
 		if nonce == "" {
-			return nil, newErr("invalid_request", "Response type 'token' requires a 'nonce' value.")
+			return nil, newErr(errInvalidRequest, "Response type 'token' requires a 'nonce' value.")
 		}
 	}
 	if rt.token {
 		if redirectURI == redirectURIOOB {
 			err := fmt.Sprintf("Cannot use response type 'token' with redirect_uri '%s'.", redirectURIOOB)
-			return nil, newErr("invalid_request", err)
+			return nil, newErr(errInvalidRequest, err)
 		}
 	}
 

diff --git a/server/oauth2_test.go b/server/oauth2_test.go
@@ -10,6 +10,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/require"
 	"gopkg.in/square/go-jose.v2"
 
 	"github.com/dexidp/dex/storage"
@@ -26,7 +27,8 @@ func TestParseAuthorizationRequest(t *testing.T) {
 
 		queryParams map[string]string
 
-		wantErr bool
+		wantErr    bool
+		exactError *authErr
 	}{
 		{
 			name: "normal request",
@@ -269,6 +271,29 @@ func TestParseAuthorizationRequest(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "No response type",
+			clients: []storage.Client{
+				{
+					ID:           "bar",
+					RedirectURIs: []string{"https://example.com/bar"},
+				},
+			},
+			supportedResponseTypes: []string{"code"},
+			queryParams: map[string]string{
+				"client_id":             "bar",
+				"redirect_uri":          "https://example.com/bar",
+				"code_challenge":        "123",
+				"code_challenge_method": "plain",
+				"scope":                 "openid email profile",
+			},
+			wantErr: true,
+			exactError: &authErr{
+				RedirectURI: "https://example.com/bar",
+				Type:        "invalid_request",
+				Description: "No response_type provided",
+			},
+		},
 	}
 
 	for _, tc := range tests {
@@ -294,12 +319,15 @@ func TestParseAuthorizationRequest(t *testing.T) {
 			} else {
 				req = httptest.NewRequest("GET", httpServer.URL+"/auth?"+params.Encode(), nil)
 			}
+
 			_, err := server.parseAuthorizationRequest(req)
-			if err != nil && !tc.wantErr {
-				t.Errorf("%s: %v", tc.name, err)
-			}
-			if err == nil && tc.wantErr {
-				t.Errorf("%s: expected error", tc.name)
+			if tc.wantErr {
+				require.Error(t, err)
+				if tc.exactError != nil {
+					require.Equal(t, tc.exactError, err)
+				}
+			} else {
+				require.NoError(t, err)
 			}
 		}()
 	}