fix: openid ignores missing redirect uri

Fixes an issue where Authorize Requests which were intended for an OpenID Connect 1.0 client would incorrectly be allowed when missing the redirect URI when it's required by the specification.

Author: james-d-elliott

authorize_request_handler.go

@@ -161,6 +161,17 @@ func (f *Fosite) validateAuthorizeRedirectURI(_ *http.Request, request *Authoriz
 	// Fetch redirect URI from request
 	rawRedirURI := request.Form.Get("redirect_uri")
 
+	// This ensures that the 'redirect_uri' parameter is present for OpenID Connect 1.0 authorization requests as per:
+	//
+	// Authorization Code Flow - https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
+	// Implicit Flow - https://openid.net/specs/openid-connect-core-1_0.html#ImplicitAuthRequest
+	// Hybrid Flow - https://openid.net/specs/openid-connect-core-1_0.html#HybridAuthRequest
+	//
+	// Note: as per the Hybrid Flow documentation the Hybrid Flow has the same requirements as the Authorization Code Flow.
+	if len(rawRedirURI) == 0 && request.GetRequestedScopes().Has("openid") {
+		return errorsx.WithStack(ErrInvalidRequest.WithHint("The 'redirect_uri' parameter is required when using OpenID Connect 1.0."))
+	}
+
 	// Validate redirect uri
 	redirectURI, err := MatchRedirectURIWithClientRedirectURIs(rawRedirURI, request.Client)
 	if err != nil {
@@ -172,14 +183,18 @@ func (f *Fosite) validateAuthorizeRedirectURI(_ *http.Request, request *Authoriz
 	return nil
 }
 
+func (f *Fosite) parseAuthorizeScope(_ *http.Request, request *AuthorizeRequest) error {
+	request.SetRequestedScopes(RemoveEmpty(strings.Split(request.Form.Get("scope"), " ")))
+
+	return nil
+}
+
 func (f *Fosite) validateAuthorizeScope(ctx context.Context, _ *http.Request, request *AuthorizeRequest) error {
-	scope := RemoveEmpty(strings.Split(request.Form.Get("scope"), " "))
-	for _, permission := range scope {
+	for _, permission := range request.GetRequestedScopes() {
 		if !f.Config.GetScopeStrategy(ctx)(request.Client.GetScopes(), permission) {
 			return errorsx.WithStack(ErrInvalidScope.WithHintf("The OAuth 2.0 Client is not allowed to request scope '%s'.", permission))
 		}
 	}
-	request.SetRequestedScopes(scope)
 
 	return nil
 }
@@ -356,27 +371,31 @@ func (f *Fosite) newAuthorizeRequest(ctx context.Context, r *http.Request, isPAR
 		return request, err
 	}
 
-	if err := f.validateAuthorizeRedirectURI(r, request); err != nil {
+	if err = f.parseAuthorizeScope(r, request); err != nil {
+		return request, err
+	}
+
+	if err = f.validateAuthorizeRedirectURI(r, request); err != nil {
 		return request, err
 	}
 
-	if err := f.validateAuthorizeScope(ctx, r, request); err != nil {
+	if err = f.validateAuthorizeScope(ctx, r, request); err != nil {
 		return request, err
 	}
 
-	if err := f.validateAuthorizeAudience(ctx, r, request); err != nil {
+	if err = f.validateAuthorizeAudience(ctx, r, request); err != nil {
 		return request, err
 	}
 
 	if len(request.Form.Get("registration")) > 0 {
 		return request, errorsx.WithStack(ErrRegistrationNotSupported)
 	}
 
-	if err := f.validateResponseTypes(r, request); err != nil {
+	if err = f.validateResponseTypes(r, request); err != nil {
 		return request, err
 	}
 
-	if err := f.validateResponseMode(r, request); err != nil {
+	if err = f.validateResponseMode(r, request); err != nil {
 		return request, err
 	}
 

integration/oidc_explicit_test.go

@@ -55,6 +55,18 @@ func TestOpenIDConnectExplicitFlow(t *testing.T) {
 			},
 			authStatusCode: http.StatusOK,
 		},
+		{
+			session:     newIDSession(&jwt.IDTokenClaims{Subject: "peter"}),
+			description: "should fail registered single redirect uri but no redirect uri in request",
+			setup: func(oauthClient *oauth2.Config) string {
+				oauthClient.Scopes = []string{"openid"}
+				oauthClient.RedirectURL = ""
+
+				return oauthClient.AuthCodeURL("12345678901234567890") + "&nonce=11234123"
+			},
+			authStatusCode: http.StatusBadRequest,
+			expectAuthErr:  `{"error":"invalid_request","error_description":"The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. The 'redirect_uri' parameter is required when using OpenID Connect 1.0."}`,
+		},
 		{
 			session:     newIDSession(&jwt.IDTokenClaims{Subject: "peter"}),
 			description: "should fail because nonce is not long enough",
@@ -88,6 +100,21 @@ func TestOpenIDConnectExplicitFlow(t *testing.T) {
 			},
 			authStatusCode: http.StatusOK,
 		},
+		{
+			session: newIDSession(&jwt.IDTokenClaims{
+				Subject:     "peter",
+				RequestedAt: time.Now().UTC(),
+				AuthTime:    time.Now().Add(time.Second).UTC(),
+			}),
+			description: "should not pass missing redirect uri",
+			setup: func(oauthClient *oauth2.Config) string {
+				oauthClient.RedirectURL = ""
+				oauthClient.Scopes = []string{"openid"}
+				return oauthClient.AuthCodeURL("12345678901234567890") + "&nonce=1234567890&prompt=login"
+			},
+			expectAuthErr:  `{"error":"invalid_request","error_description":"The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. The 'redirect_uri' parameter is required when using OpenID Connect 1.0."}`,
+			authStatusCode: http.StatusBadRequest,
+		},
 		{
 			session: newIDSession(&jwt.IDTokenClaims{
 				Subject:     "peter",
@@ -121,7 +148,7 @@ func TestOpenIDConnectExplicitFlow(t *testing.T) {
 			defer ts.Close()
 
 			oauthClient := newOAuth2Client(ts)
-			store.Clients["my-client"].(*goauth2.DefaultClient).RedirectURIs[0] = ts.URL + "/callback"
+			store.Clients["my-client"].(*goauth2.DefaultClient).RedirectURIs = []string{ts.URL + "/callback"}
 
 			resp, err := http.Get(c.setup(oauthClient))
 			require.NoError(t, err)

integration/oidc_implicit_hybrid_test.go

@@ -107,6 +107,7 @@ func TestOIDCImplicitFlow(t *testing.T) {
 			var callbackURL *url.URL
 
 			authURL := strings.Replace(oauthClient.AuthCodeURL(state), "response_type=code", "response_type="+c.responseType, -1) + "&nonce=" + c.nonce
+
 			client := &http.Client{
 				CheckRedirect: func(req *http.Request, via []*http.Request) error {
 					callbackURL = req.URL