diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index 8fbcf5751..d74ad0947 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -7,11 +7,11 @@ package auth import ( "fmt" "net/http" - - "github.com/ory/fosite/handler/openid" + "time" "github.com/ory/fosite" - + "github.com/ory/fosite/handler/openid" + "github.com/ory/fosite/token/jwt" "golang.org/x/oauth2" "k8s.io/klog/v2" @@ -55,7 +55,22 @@ func NewHandler( return err } - _, err = oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &openid.DefaultSession{}) + // Grant the openid scope (for now) if they asked for it so that `NewAuthorizeResponse` will perform its OIDC validations. + for _, scope := range authorizeRequester.GetRequestedScopes() { + if scope == "openid" { + authorizeRequester.GrantScope(scope) + } + } + + now := time.Now() + _, err = oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, &openid.DefaultSession{ + Claims: &jwt.IDTokenClaims{ + // Temporary claim values to allow `NewAuthorizeResponse` to perform other OIDC validations. + Subject: "none", + AuthTime: now, + RequestedAt: now, + }, + }) if err != nil { klog.InfoS("authorize response error", "err", err, "details", fosite.ErrorToRFC6749Error(err).ToValues()) oauthHelper.WriteAuthorizeError(w, authorizeRequester, err) diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 1da217a96..399605e39 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -51,6 +51,13 @@ func TestAuthorizationEndpoint(t *testing.T) { } `) + fositePromptHasNoneAndOtherValueErrorQuery = map[string]string{ + "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\n\nParameter \"prompt\" was set to \"none\", but contains other values as well which is not allowed.", + "error_hint": "Parameter \"prompt\" was set to \"none\", but contains other values as well which is not allowed.", + "state": "some-state-value", + } + fositeMissingCodeChallengeErrorQuery = map[string]string{ "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\n\nThis client must include a code_challenge when performing the authorize code flow, but it is missing.", @@ -118,6 +125,7 @@ func TestAuthorizationEndpoint(t *testing.T) { Clients: map[string]fosite.Client{oidc.PinnipedCLIOIDCClient().ID: oidc.PinnipedCLIOIDCClient()}, AuthorizeCodes: map[string]storage.StoreAuthorizeCode{}, PKCES: map[string]fosite.Requester{}, + IDSessions: map[string]fosite.Requester{}, } hmacSecret := []byte("some secret - must have at least 32 bytes") require.GreaterOrEqual(t, len(hmacSecret), 32, "fosite requires that hmac secrets have at least 32 bytes") @@ -415,6 +423,22 @@ func TestAuthorizationEndpoint(t *testing.T) { wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeMissingCodeChallengeMethodErrorQuery), wantBodyString: "", }, + { + // This is just one of the many OIDC validations run by fosite. This test is to ensure that we are running + // through that part of the fosite library. + name: "prompt param is not allowed to have none and another legal value at the same time", + issuer: issuer, + idpListGetter: newIDPListGetter(upstreamOIDCIdentityProvider), + generateState: happyStateGenerator, + generatePKCE: happyPKCEGenerator, + generateNonce: happyNonceGenerator, + method: http.MethodGet, + path: modifiedHappyGetRequestPath(map[string]string{"prompt": "none login"}), + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositePromptHasNoneAndOtherValueErrorQuery), + wantBodyString: "", + }, { name: "state does not have enough entropy", issuer: issuer, @@ -526,9 +550,6 @@ func TestAuthorizationEndpoint(t *testing.T) { rsp := httptest.NewRecorder() subject.ServeHTTP(rsp, req) - t.Logf("response: %#v", rsp) - t.Logf("body: %q", rsp.Body.String()) - require.Equal(t, test.wantStatus, rsp.Code) requireEqualContentType(t, rsp.Header().Get("Content-Type"), test.wantContentType) diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index a03c86d04..8d319c1d1 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -44,7 +44,7 @@ func FositeOauth2Helper(oauthStore interface{}, hmacSecretOfLengthAtLeast32 []by nil, // hasher, defaults to using BCrypt when nil. Used for hashing client secrets. compose.OAuth2AuthorizeExplicitFactory, // compose.OAuth2RefreshTokenGrantFactory, - // compose.OpenIDConnectExplicitFactory, + compose.OpenIDConnectExplicitFactory, // compose.OpenIDConnectRefreshFactory, compose.OAuth2PKCEFactory, ) diff --git a/internal/oidc/provider/manager/manager.go b/internal/oidc/provider/manager/manager.go index fbee5db41..229dd3751 100644 --- a/internal/oidc/provider/manager/manager.go +++ b/internal/oidc/provider/manager/manager.go @@ -74,6 +74,7 @@ func (m *Manager) SetProviders(oidcProviders ...*provider.OIDCProvider) { Clients: map[string]fosite.Client{oidc.PinnipedCLIOIDCClient().ID: oidc.PinnipedCLIOIDCClient()}, AuthorizeCodes: map[string]storage.StoreAuthorizeCode{}, PKCES: map[string]fosite.Requester{}, + IDSessions: map[string]fosite.Requester{}, } oauthHelper := oidc.FositeOauth2Helper(oauthStore, []byte("some secret - must have at least 32 bytes")) // TODO replace this secret