fix: require redirect_uri for OpenID Connect calls

authorize_request_handler.go

@@ -165,6 +165,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 {
@@ -176,14 +187,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
 }
@@ -363,27 +378,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
 	}
 

handler/openid/flow_explicit_auth.go

@@ -47,6 +47,18 @@ func (c *OpenIDConnectExplicitHandler) HandleAuthorizeEndpointRequest(ctx contex
 		return errorsx.WithStack(fosite.ErrMisconfiguration.WithDebug("The authorization code has not been issued yet, indicating a broken code configuration."))
 	}
 
+	// 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.
+	rawRedirectURI := ar.GetRequestForm().Get("redirect_uri")
+	if len(rawRedirectURI) == 0 {
+		return errorsx.WithStack(fosite.ErrInvalidRequest.WithHint("The 'redirect_uri' parameter is required when using OpenID Connect 1.0."))
+	}
+
 	if err := c.OpenIDConnectRequestValidator.ValidatePrompt(ctx, ar); err != nil {
 		return err
 	}

handler/openid/flow_explicit_auth_test.go

@@ -6,6 +6,7 @@ package openid
 import (
 	"context"
 	"fmt"
+	"net/url"
 	"testing"
 
 	"github.com/ory/fosite/internal/gen"
@@ -55,6 +56,9 @@ func TestExplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
 	session := NewDefaultSession()
 	session.Claims.Subject = "foo"
 	areq.Session = session
+	areq.Form = url.Values{
+		"redirect_uri": {"https://foobar.com"},
+	}
 
 	for k, c := range []struct {
 		description string
@@ -110,6 +114,16 @@ func TestExplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
 				return h
 			},
 		},
+		{
+			description: "should fail because redirect url is missing",
+			setup: func() OpenIDConnectExplicitHandler {
+				areq.Form.Del("redirect_uri")
+				h, store := makeOpenIDConnectExplicitHandler(ctrl, fosite.MinParameterEntropy)
+				store.EXPECT().CreateOpenIDConnectSession(gomock.Any(), "codeexample", gomock.Eq(areq.Sanitize(oidcParameters))).AnyTimes().Return(nil)
+				return h
+			},
+			expectErr: fosite.ErrInvalidRequest,
+		},
 	} {
 		t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
 			h := c.setup()

handler/openid/flow_hybrid.go

@@ -63,6 +63,18 @@ func (c *OpenIDConnectHybridHandler) HandleAuthorizeEndpointRequest(ctx context.
 		return errorsx.WithStack(fosite.ErrInsufficientEntropy.WithHintf("Parameter 'nonce' is set but does not satisfy the minimum entropy of %d characters.", c.Config.GetMinParameterEntropy(ctx)))
 	}
 
+	// 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.
+	rawRedirectURI := ar.GetRequestForm().Get("redirect_uri")
+	if len(rawRedirectURI) == 0 {
+		return errorsx.WithStack(fosite.ErrInvalidRequest.WithHint("The 'redirect_uri' parameter is required when using OpenID Connect 1.0."))
+	}
+
 	sess, ok := ar.GetSession().(Session)
 	if !ok {
 		return errorsx.WithStack(ErrInvalidSession)

handler/openid/flow_hybrid_test.go

@@ -93,6 +93,7 @@ func TestHybrid_HandleAuthorizeEndpointRequest(t *testing.T) {
 
 	aresp := fosite.NewAuthorizeResponse()
 	areq := fosite.NewAuthorizeRequest()
+	areq.Form = url.Values{"redirect_uri": {"https://foobar.com"}}
 
 	for k, c := range []struct {
 		description string
@@ -116,7 +117,10 @@ func TestHybrid_HandleAuthorizeEndpointRequest(t *testing.T) {
 		{
 			description: "should fail because nonce set but too short",
 			setup: func() OpenIDConnectHybridHandler {
-				areq.Form = url.Values{"nonce": {"short"}}
+				areq.Form = url.Values{
+					"redirect_uri": {"https://foobar.com"},
+					"nonce":        {"short"},
+				}
 				areq.ResponseTypes = fosite.Arguments{"token", "code"}
 				areq.Client = &fosite.DefaultClient{
 					GrantTypes:    fosite.Arguments{"authorization_code", "implicit"},
@@ -131,7 +135,10 @@ func TestHybrid_HandleAuthorizeEndpointRequest(t *testing.T) {
 		{
 			description: "should fail because nonce set but too short for non-default min entropy",
 			setup: func() OpenIDConnectHybridHandler {
-				areq.Form = url.Values{"nonce": {"some-foobar-nonce-win"}}
+				areq.Form = url.Values{
+					"nonce":        {"some-foobar-nonce-win"},
+					"redirect_uri": {"https://foobar.com"},
+				}
 				areq.ResponseTypes = fosite.Arguments{"token", "code"}
 				areq.Client = &fosite.DefaultClient{
 					GrantTypes:    fosite.Arguments{"authorization_code", "implicit"},
@@ -146,7 +153,10 @@ func TestHybrid_HandleAuthorizeEndpointRequest(t *testing.T) {
 		{
 			description: "should fail because session not given",
 			setup: func() OpenIDConnectHybridHandler {
-				areq.Form = url.Values{"nonce": {"long-enough"}}
+				areq.Form = url.Values{
+					"nonce":        {"long-enough"},
+					"redirect_uri": {"https://foobar.com"},
+				}
 				areq.ResponseTypes = fosite.Arguments{"token", "code"}
 				areq.Client = &fosite.DefaultClient{
 					GrantTypes:    fosite.Arguments{"authorization_code", "implicit"},
@@ -181,7 +191,11 @@ func TestHybrid_HandleAuthorizeEndpointRequest(t *testing.T) {
 		{
 			description: "should pass with exact one state parameter in response",
 			setup: func() OpenIDConnectHybridHandler {
-				areq.Form = url.Values{"nonce": {"long-enough"}, "state": {""}}
+				areq.Form = url.Values{
+					"redirect_uri": {"https://foobar.com"},
+					"nonce":        {"long-enough"},
+					"state":        {""},
+				}
 				areq.Client = &fosite.DefaultClient{
 					GrantTypes:    fosite.Arguments{"authorization_code", "implicit"},
 					ResponseTypes: fosite.Arguments{"token", "code", "id_token"},
@@ -258,12 +272,21 @@ func TestHybrid_HandleAuthorizeEndpointRequest(t *testing.T) {
 				internal.RequireEqualTime(t, time.Now().Add(time.Hour).UTC(), areq.GetSession().GetExpiresAt(fosite.AuthorizeCode), time.Second)
 			},
 		},
+		{
+			description: "should fail if redirect_uri is missing",
+			setup: func() OpenIDConnectHybridHandler {
+				areq.Form.Del("redirect_uri")
+				return makeOpenIDConnectHybridHandler(fosite.MinParameterEntropy)
+			},
+			expectErr: fosite.ErrInvalidRequest,
+		},
 		{
 			description: "should pass with custom client lifespans",
 			setup: func() OpenIDConnectHybridHandler {
 				aresp = fosite.NewAuthorizeResponse()
 				areq = fosite.NewAuthorizeRequest()
 				areq.Form.Set("nonce", "some-foobar-nonce-win")
+				areq.Form.Set("redirect_uri", "https://foobar.com")
 				areq.ResponseTypes = fosite.Arguments{"token", "code", "id_token"}
 				areq.Client = &fosite.DefaultClientWithCustomTokenLifespans{
 					DefaultClient: &fosite.DefaultClient{

handler/openid/flow_implicit.go

@@ -48,6 +48,18 @@ func (c *OpenIDConnectImplicitHandler) HandleAuthorizeEndpointRequest(ctx contex
 	//	return errorsx.WithStack(fosite.ErrInvalidGrant.WithDebug("The client is not allowed to use response type token and id_token"))
 	//}
 
+	// 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.
+	rawRedirectURI := ar.GetRequestForm().Get("redirect_uri")
+	if len(rawRedirectURI) == 0 {
+		return errorsx.WithStack(fosite.ErrInvalidRequest.WithHint("The 'redirect_uri' parameter is required when using OpenID Connect 1.0."))
+	}
+
 	if nonce := ar.GetRequestForm().Get("nonce"); len(nonce) == 0 {
 		return errorsx.WithStack(fosite.ErrInvalidRequest.WithHint("Parameter 'nonce' must be set when using the OpenID Connect Implicit Flow."))
 	} else if len(nonce) < c.Config.GetMinParameterEntropy(ctx) {

handler/openid/flow_implicit_test.go

@@ -67,6 +67,9 @@ func TestImplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
 
 	aresp := fosite.NewAuthorizeResponse()
 	areq := fosite.NewAuthorizeRequest()
+	areq.Form = url.Values{
+		"redirect_uri": {"https://foobar.com"},
+	}
 	areq.Session = new(fosite.DefaultSession)
 
 	for k, c := range []struct {
@@ -150,7 +153,10 @@ func TestImplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
 		{
 			description: "should not do anything because request requirements are not met",
 			setup: func() OpenIDConnectImplicitHandler {
-				areq.Form = url.Values{"nonce": {"short"}}
+				areq.Form = url.Values{
+					"nonce":        {"short"},
+					"redirect_uri": {"https://foobar.com"},
+				}
 				areq.ResponseTypes = fosite.Arguments{"id_token"}
 				areq.RequestedScope = fosite.Arguments{"openid"}
 				areq.Client = &fosite.DefaultClient{
@@ -165,7 +171,10 @@ func TestImplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
 		{
 			description: "should fail because session not set",
 			setup: func() OpenIDConnectImplicitHandler {
-				areq.Form = url.Values{"nonce": {"long-enough"}}
+				areq.Form = url.Values{
+					"nonce":        {"long-enough"},
+					"redirect_uri": {"https://foobar.com"},
+				}
 				areq.ResponseTypes = fosite.Arguments{"id_token"}
 				areq.RequestedScope = fosite.Arguments{"openid"}
 				areq.Client = &fosite.DefaultClient{
@@ -282,6 +291,14 @@ func TestImplicit_HandleAuthorizeEndpointRequest(t *testing.T) {
 				assert.NotEmpty(t, aresp.GetParameters().Get("access_token"))
 			},
 		},
+		{
+			description: "should fail without redirect_uri",
+			setup: func() OpenIDConnectImplicitHandler {
+				areq.Form.Del("redirect_uri")
+				return makeOpenIDConnectImplicitHandler(4)
+			},
+			expectErr: fosite.ErrInvalidRequest,
+		},
 	} {
 		t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
 			h := c.setup()

integration/oidc_explicit_test.go

@@ -56,6 +56,30 @@ 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 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",
@@ -66,6 +90,21 @@ func TestOpenIDConnectExplicitFlow(t *testing.T) {
 			authStatusCode: http.StatusOK,
 			expectTokenErr: "insufficient_entropy",
 		},
+		{
+			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"}),
 			description: "should fail because state is not long enough",
@@ -89,6 +128,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",
@@ -122,7 +176,8 @@ func TestOpenIDConnectExplicitFlow(t *testing.T) {
 			defer ts.Close()
 
 			oauthClient := newOAuth2Client(ts)
-			fositeStore.Clients["my-client"].(*fosite.DefaultClient).RedirectURIs[0] = ts.URL + "/callback"
+
+			fositeStore.Clients["my-client"].(*fosite.DefaultClient).RedirectURIs = []string{ts.URL + "/callback"}
 
 			resp, err := http.Get(c.setup(oauthClient))
 			require.NoError(t, err)