From 16115f273002a8aecace86551bf141868c0ea471 Mon Sep 17 00:00:00 2001 From: arekkas Date: Sun, 22 Jul 2018 21:37:26 +0200 Subject: [PATCH 1/4] cmd: Resolves panic when network fails Signed-off-by: arekkas --- cmd/cli/handler_helper.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/cli/handler_helper.go b/cmd/cli/handler_helper.go index 6ff6e86e863..5842837c3bf 100644 --- a/cmd/cli/handler_helper.go +++ b/cmd/cli/handler_helper.go @@ -31,7 +31,11 @@ import ( func checkResponse(response *hydra.APIResponse, err error, expectedStatusCode int) { if response != nil { - pkg.Must(err, "Command failed because calling \"%s %s\" resulted in error \"%s\" occurred.\n%s\n", response.Request.Method, response.RequestURL, err, response.Payload) + var method string + if response.Response != nil && response.Response.Request != nil { + method = response.Request.Method + } + pkg.Must(err, "Command failed because calling \"%s %s\" resulted in error \"%s\" occurred.\n%s\n", method, response.RequestURL, err, response.Payload) } else { pkg.Must(err, "Command failed because error \"%s\" occurred and no response is available.\n", err) } From eb70025ea57bf10d9c79becd56ec7720d4139d5a Mon Sep 17 00:00:00 2001 From: arekkas Date: Sun, 22 Jul 2018 21:48:28 +0200 Subject: [PATCH 2/4] oauth2: Adds JWT Access Token strategy This patch adds the (experimental) ability to issue JSON Web Tokens instead of ORY Hydra's opaque access tokens. Please be aware that this feature has had little real-world and unit testing and may not be suitable for production. Simple integration tests using the JWT strategy have been added to ensure functionality. To use the new JWT strategy, set environment variable `OAUTH2_ACCESS_TOKEN_STRATEGY` to `jwt`. For example: `export OAUTH2_ACCESS_TOKEN_STRATEGY=jwt`. Please be aware that we (ORY) do not recommend using the JWT strategy for various reasons. If you can, use the default and recommended "opaque" strategy instead. Closes #248 Signed-off-by: arekkas --- .circleci/config.yml | 3 +- client/client.go | 2 +- cmd/cli/handler_test.go | 44 +++++----- cmd/root.go | 3 + cmd/serve.go | 6 ++ cmd/server/handler_jwk_factory.go | 16 +++- cmd/server/handler_oauth2_factory.go | 63 +++++++++++--- cmd/server/handler_test.go | 3 +- config/config.go | 1 + jwk/handler.go | 47 ++++++++--- jwk/handler_test.go | 10 ++- oauth2/fosite_store_sql.go | 18 ++++ oauth2/handler.go | 39 ++++++++- oauth2/handler_struct.go | 3 +- oauth2/handler_test.go | 6 +- oauth2/introspector_test.go | 6 +- oauth2/oauth2_auth_code_test.go | 20 ++--- oauth2/oauth2_client_credentials_test.go | 16 ++-- oauth2/revocator_test.go | 4 +- oauth2/session.go | 28 +++++++ scripts/test-e2e-jwt.sh | 93 +++++++++++++++++++++ scripts/{test-e2e.sh => test-e2e-opaque.sh} | 0 22 files changed, 339 insertions(+), 92 deletions(-) create mode 100755 scripts/test-e2e-jwt.sh rename scripts/{test-e2e.sh => test-e2e-opaque.sh} (100%) diff --git a/.circleci/config.yml b/.circleci/config.yml index e8ee19c3eba..8e27e3a916b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -38,7 +38,8 @@ jobs: - run: go install github.com/ory/hydra/test/mock-lcp - run: go-acc -o coverage.txt ./... - run: go test -race -short $(go list ./... | grep -v cmd) - - run: ./scripts/test-e2e.sh + - run: ./scripts/test-e2e-jwt.sh + - run: ./scripts/test-e2e-opaque.sh - run: test -z "$CIRCLE_PR_NUMBER" && goveralls -service=circle-ci -coverprofile=coverage.txt -repotoken=$COVERALLS_REPO_TOKEN || echo "forks are not allowed to push to coveralls" swagger: diff --git a/client/client.go b/client/client.go index 78e7836aed5..120aff10281 100644 --- a/client/client.go +++ b/client/client.go @@ -145,7 +145,7 @@ type Client struct { } func (c *Client) GetID() string { - return c.ID + return c.ClientID } func (c *Client) GetRedirectURIs() []string { diff --git a/cmd/cli/handler_test.go b/cmd/cli/handler_test.go index 74c3338fbef..eb2f2b36ce1 100644 --- a/cmd/cli/handler_test.go +++ b/cmd/cli/handler_test.go @@ -21,35 +21,31 @@ package cli import ( - "flag" - "log" "testing" - "github.com/jmoiron/sqlx" "github.com/ory/hydra/config" - "github.com/ory/sqlcon/dockertest" ) -var db *sqlx.DB - -func TestMain(m *testing.M) { - runner := dockertest.Register() - - flag.Parse() - if !testing.Short() { - dockertest.Parallel([]func(){ - func() { - var err error - db, err = dockertest.ConnectToTestPostgreSQL() - if err != nil { - log.Fatalf("Unable to connect to database: %s", err) - } - }, - }) - } - - runner.Exit(m.Run()) -} +//var db *sqlx.DB +// +//func TestMain(m *testing.M) { +// runner := dockertest.Register() +// +// flag.Parse() +// if !testing.Short() { +// dockertest.Parallel([]func(){ +// func() { +// var err error +// db, err = dockertest.ConnectToTestPostgreSQL() +// if err != nil { +// log.Fatalf("Unable to connect to database: %s", err) +// } +// }, +// }) +// } +// +// runner.Exit(m.Run()) +//} func TestNewHandler(t *testing.T) { _ = NewHandler(&config.Config{}) diff --git a/cmd/root.go b/cmd/root.go index 1c4b61c70f0..187191232ad 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -134,6 +134,9 @@ func initConfig() { viper.BindEnv("CLUSTER_URL") viper.SetDefault("CLUSTER_URL", "") + viper.BindEnv("OAUTH2_ACCESS_TOKEN_STRATEGY") + viper.SetDefault("OAUTH2_ACCESS_TOKEN_STRATEGY", "opaque") + viper.BindEnv("PORT") viper.SetDefault("PORT", 4444) diff --git a/cmd/serve.go b/cmd/serve.go index 119e73a8577..8d374192586 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -129,6 +129,12 @@ OAUTH2 CONTROLS codes and similar errors. Defaults to OAUTH2_SHARE_ERROR_DEBUG=false +- OAUTH2_ACCESS_TOKEN_STRATEGY: Sets the Access Token Strategy. Defaults to "opaque" which is the recommended strategy + for usage with ORY Hydra. If set to "jwt", then Access Tokens will be a signed JSON Web Token. The public key + for verifying the token can be obtained from "./well-known/jwks.json". Please note that the "jwt" strategy is currently + in BETA and not recommended for production just yet. + Defaults to OAUTH2_ACCESS_TOKEN_STRATEGY="opaque" + OPENID CONNECT CONTROLS =============== diff --git a/cmd/server/handler_jwk_factory.go b/cmd/server/handler_jwk_factory.go index 3672ddc8f96..1d8d8ac7ca8 100644 --- a/cmd/server/handler_jwk_factory.go +++ b/cmd/server/handler_jwk_factory.go @@ -25,6 +25,7 @@ import ( "github.com/ory/herodot" "github.com/ory/hydra/config" "github.com/ory/hydra/jwk" + "github.com/ory/hydra/oauth2" "github.com/ory/sqlcon" ) @@ -60,12 +61,19 @@ func newJWKHandler(c *config.Config, router *httprouter.Router) *jwk.Handler { ctx := c.Context() w := herodot.NewJSONWriter(c.GetLogger()) w.ErrorEnhancer = writerErrorEnhancer + var wellKnown []string - expectDependency(c.GetLogger(), ctx.KeyManager) - h := &jwk.Handler{ - H: w, - Manager: ctx.KeyManager, + if c.OAuth2AccessTokenStrategy == "jwt" { + wellKnown = append(wellKnown, oauth2.OAuth2JWTKeyName) } + + expectDependency(c.GetLogger(), ctx.KeyManager) + h := jwk.NewHandler( + ctx.KeyManager, + nil, + w, + wellKnown, + ) h.SetRoutes(router) return h } diff --git a/cmd/server/handler_oauth2_factory.go b/cmd/server/handler_oauth2_factory.go index fc46fedf315..1aa5134aab4 100644 --- a/cmd/server/handler_oauth2_factory.go +++ b/cmd/server/handler_oauth2_factory.go @@ -30,6 +30,7 @@ import ( "github.com/julienschmidt/httprouter" "github.com/ory/fosite" "github.com/ory/fosite/compose" + foauth2 "github.com/ory/fosite/handler/oauth2" "github.com/ory/fosite/handler/openid" "github.com/ory/herodot" "github.com/ory/hydra/client" @@ -52,7 +53,7 @@ func injectFositeStore(c *config.Config, clients client.Manager) { break case *sqlcon.SQLConnection: expectDependency(c.GetLogger(), con.GetDatabase()) - store = oauth2.NewFositeSQLStore(clients, con.GetDatabase(), c.GetLogger(), c.GetAccessTokenLifespan()) + store = oauth2.NewFositeSQLStore(clients, con.GetDatabase(), c.GetLogger(), c.GetAccessTokenLifespan(), c.OAuth2AccessTokenStrategy == "jwt") break case *config.PluginConnection: var err error @@ -99,11 +100,38 @@ func newOAuth2Provider(c *config.Config) fosite.OAuth2Provider { } oidcStrategy := &openid.DefaultStrategy{JWTStrategy: jwtStrategy} + var coreStrategy foauth2.CoreStrategy + hmacStrategy := compose.NewOAuth2HMACStrategy(fc, c.GetSystemSecret()) + if c.OAuth2AccessTokenStrategy == "jwt" { + kid := uuid.New() + if _, err := createOrGetJWK(c, oauth2.OAuth2JWTKeyName, kid, "private"); err != nil { + c.GetLogger().WithError(err).Fatalf(`Could not fetch private signing key for OAuth 2.0 Access Tokens - did you forget to run "hydra migrate sql" or forget to set the SYSTEM_SECRET?`) + } + + if _, err := createOrGetJWK(c, oauth2.OAuth2JWTKeyName, kid, "public"); err != nil { + c.GetLogger().WithError(err).Fatalf(`Could not fetch public signing key for OAuth 2.0 Access Tokens - did you forget to run "hydra migrate sql" or forget to set the SYSTEM_SECRET?`) + } + + jwtStrategy, err := jwk.NewRS256JWTStrategy(c.Context().KeyManager, oauth2.OAuth2JWTKeyName) + if err != nil { + c.GetLogger().WithError(err).Fatalf("Unable to refresh Access Token signing keys.") + } + + coreStrategy = &foauth2.DefaultJWTStrategy{ + JWTStrategy: jwtStrategy, + HMACSHAStrategy: hmacStrategy, + } + } else if c.OAuth2AccessTokenStrategy == "opaque" { + coreStrategy = hmacStrategy + } else { + c.GetLogger().Fatalf(`Environment variable OAUTH2_ACCESS_TOKEN_STRATEGY is set to "%s" but only "opaque" and "jwt" are valid values.`, c.OAuth2AccessTokenStrategy) + } + return compose.Compose( fc, store, &compose.CommonStrategy{ - CoreStrategy: compose.NewOAuth2HMACStrategy(fc, c.GetSystemSecret()), + CoreStrategy: coreStrategy, OpenIDConnectTokenStrategy: oidcStrategy, JWTStrategy: jwtStrategy, }, @@ -148,12 +176,20 @@ func newOAuth2Handler(c *config.Config, router *httprouter.Router, cm consent.Ma errorURL, err := url.Parse(c.ErrorURL) pkg.Must(err, "Could not parse error url %s.", errorURL) - jwtStrategy, err := jwk.NewRS256JWTStrategy(c.Context().KeyManager, oauth2.OpenIDConnectKeyName) + openIDJWTStrategy, err := jwk.NewRS256JWTStrategy(c.Context().KeyManager, oauth2.OpenIDConnectKeyName) pkg.Must(err, "Could not fetch private signing key for OpenID Connect - did you forget to run \"hydra migrate sql\" or forget to set the SYSTEM_SECRET?") - oidcStrategy := &openid.DefaultStrategy{JWTStrategy: jwtStrategy} + oidcStrategy := &openid.DefaultStrategy{JWTStrategy: openIDJWTStrategy} w := herodot.NewJSONWriter(c.GetLogger()) w.ErrorEnhancer = writerErrorEnhancer + var accessTokenJWTStrategy *jwk.RS256JWTStrategy + + if c.OAuth2AccessTokenStrategy == "jwt" { + accessTokenJWTStrategy, err = jwk.NewRS256JWTStrategy(c.Context().KeyManager, oauth2.OAuth2JWTKeyName) + if err != nil { + c.GetLogger().WithError(err).Fatalf("Unable to refresh Access Token signing keys.") + } + } handler := &oauth2.Handler{ ScopesSupported: c.OpenIDDiscoveryScopesSupported, @@ -170,15 +206,16 @@ func newOAuth2Handler(c *config.Config, router *httprouter.Router, cm consent.Ma oidcStrategy, openid.NewOpenIDConnectRequestValidator(nil, oidcStrategy), ), - Storage: c.Context().FositeStore, - ErrorURL: *errorURL, - H: w, - AccessTokenLifespan: c.GetAccessTokenLifespan(), - CookieStore: sessions.NewCookieStore(c.GetCookieSecret()), - IssuerURL: c.Issuer, - L: c.GetLogger(), - JWTStrategy: jwtStrategy, - IDTokenLifespan: c.GetIDTokenLifespan(), + Storage: c.Context().FositeStore, + ErrorURL: *errorURL, + H: w, + AccessTokenLifespan: c.GetAccessTokenLifespan(), + CookieStore: sessions.NewCookieStore(c.GetCookieSecret()), + IssuerURL: c.Issuer, + L: c.GetLogger(), + OpenIDJWTStrategy: openIDJWTStrategy, + AccessTokenJWTStrategy: accessTokenJWTStrategy, + IDTokenLifespan: c.GetIDTokenLifespan(), } handler.SetRoutes(router) diff --git a/cmd/server/handler_test.go b/cmd/server/handler_test.go index f49a954dd6c..1f0ebc0ba22 100644 --- a/cmd/server/handler_test.go +++ b/cmd/server/handler_test.go @@ -31,7 +31,8 @@ func TestStart(t *testing.T) { router := httprouter.New() h := &Handler{ Config: &config.Config{ - DatabaseURL: "memory", + DatabaseURL: "memory", + OAuth2AccessTokenStrategy: "opaque", }, } h.registerRoutes(router) diff --git a/config/config.go b/config/config.go index 1758b280c23..11b1d47f22b 100644 --- a/config/config.go +++ b/config/config.go @@ -78,6 +78,7 @@ type Config struct { OpenIDDiscoveryScopesSupported string `mapstructure:"OIDC_DISCOVERY_SCOPES_SUPPORTED" yaml:"-"` OpenIDDiscoveryUserinfoEndpoint string `mapstructure:"OIDC_DISCOVERY_USERINFO_ENDPOINT" yaml:"-"` SendOAuth2DebugMessagesToClients bool `mapstructure:"OAUTH2_SHARE_ERROR_DEBUG" yaml:"-"` + OAuth2AccessTokenStrategy string `mapstructure:"OAUTH2_ACCESS_TOKEN_STRATEGY" yaml:"-"` ForceHTTP bool `yaml:"-"` BuildVersion string `yaml:"-"` diff --git a/jwk/handler.go b/jwk/handler.go index ca7343f2e93..1ab30a458fc 100644 --- a/jwk/handler.go +++ b/jwk/handler.go @@ -38,9 +38,24 @@ const ( ) type Handler struct { - Manager Manager - Generators map[string]KeyGenerator - H herodot.Writer + Manager Manager + Generators map[string]KeyGenerator + H herodot.Writer + WellKnownKeys []string +} + +func NewHandler( + manager Manager, + generators map[string]KeyGenerator, + h herodot.Writer, + wellKnownKeys []string, +) *Handler { + return &Handler{ + Manager: manager, + Generators: generators, + H: h, + WellKnownKeys: append(wellKnownKeys, IDTokenKeyName), + } } func (h *Handler) GetGenerators() map[string]KeyGenerator { @@ -92,19 +107,25 @@ func (h *Handler) SetRoutes(r *httprouter.Router) { // 403: genericError // 500: genericError func (h *Handler) WellKnown(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - keys, err := h.Manager.GetKeySet(IDTokenKeyName) - if err != nil { - h.H.WriteError(w, r, err) - return - } + var jwks jose.JSONWebKeySet - keys, err = FindKeysByPrefix(keys, "public") - if err != nil { - h.H.WriteError(w, r, err) - return + for _, set := range h.WellKnownKeys { + keys, err := h.Manager.GetKeySet(set) + if err != nil { + h.H.WriteError(w, r, err) + return + } + + keys, err = FindKeysByPrefix(keys, "public") + if err != nil { + h.H.WriteError(w, r, err) + return + } + + jwks.Keys = append(jwks.Keys, keys.Keys...) } - h.H.Write(w, r, keys) + h.H.Write(w, r, &jwks) } // swagger:route GET /keys/{set}/{kid} jsonWebKey getJsonWebKey diff --git a/jwk/handler_test.go b/jwk/handler_test.go index a9535c58b25..b814bfc19bc 100644 --- a/jwk/handler_test.go +++ b/jwk/handler_test.go @@ -41,10 +41,12 @@ func init() { router := httprouter.New() IDKS, _ = testGenerator.Generate("test-id", "sig") - h := Handler{ - Manager: &MemoryManager{}, - H: herodot.NewJSONWriter(nil), - } + h := NewHandler( + &MemoryManager{}, + nil, + herodot.NewJSONWriter(nil), + []string{}, + ) h.Manager.AddKeySet(IDTokenKeyName, IDKS) h.SetRoutes(router) testServer = httptest.NewServer(router) diff --git a/oauth2/fosite_store_sql.go b/oauth2/fosite_store_sql.go index 277ef35c203..fb2f40f9a63 100644 --- a/oauth2/fosite_store_sql.go +++ b/oauth2/fosite_store_sql.go @@ -22,6 +22,7 @@ package oauth2 import ( "context" + "crypto/sha512" "database/sql" "encoding/json" "fmt" @@ -43,18 +44,21 @@ type FositeSQLStore struct { DB *sqlx.DB L logrus.FieldLogger AccessTokenLifespan time.Duration + HashSignature bool } func NewFositeSQLStore(m client.Manager, db *sqlx.DB, l logrus.FieldLogger, accessTokenLifespan time.Duration, + hashSignature bool, ) *FositeSQLStore { return &FositeSQLStore{ Manager: m, L: l, DB: db, AccessTokenLifespan: accessTokenLifespan, + HashSignature: hashSignature, } } @@ -253,7 +257,17 @@ func (s *sqlData) toRequest(session fosite.Session, cm client.Manager, logger lo return r, nil } +// hashSignature prevents errors where the signature is longer than 128 characters (and thus doesn't fit into the pk). +func (s *FositeSQLStore) hashSignature(signature, table string) string { + if table == sqlTableAccess && s.HashSignature { + return fmt.Sprintf("%x", sha512.Sum384([]byte(signature))) + } + return signature +} + func (s *FositeSQLStore) createSession(signature string, requester fosite.Requester, table string) error { + signature = s.hashSignature(signature, table) + data, err := sqlSchemaFromRequest(signature, requester, s.L) if err != nil { return err @@ -272,6 +286,8 @@ func (s *FositeSQLStore) createSession(signature string, requester fosite.Reques } func (s *FositeSQLStore) findSessionBySignature(signature string, session fosite.Session, table string) (fosite.Requester, error) { + signature = s.hashSignature(signature, table) + var d sqlData if err := s.DB.Get(&d, s.DB.Rebind(fmt.Sprintf("SELECT * FROM hydra_oauth2_%s WHERE signature=?", table)), signature); err == sql.ErrNoRows { return nil, errors.Wrap(fosite.ErrNotFound, "") @@ -291,6 +307,8 @@ func (s *FositeSQLStore) findSessionBySignature(signature string, session fosite } func (s *FositeSQLStore) deleteSession(signature string, table string) error { + signature = s.hashSignature(signature, table) + if _, err := s.DB.Exec(s.DB.Rebind(fmt.Sprintf("DELETE FROM hydra_oauth2_%s WHERE signature=?", table)), signature); err != nil { return sqlcon.HandleError(err) } diff --git a/oauth2/handler.go b/oauth2/handler.go index 5e0274241ce..5caf3b4b191 100644 --- a/oauth2/handler.go +++ b/oauth2/handler.go @@ -36,11 +36,13 @@ import ( "github.com/ory/hydra/client" "github.com/ory/hydra/consent" "github.com/ory/hydra/pkg" + "github.com/pborman/uuid" "github.com/pkg/errors" ) const ( OpenIDConnectKeyName = "hydra.openid.id-token" + OAuth2JWTKeyName = "hydra.jwt.access-token" DefaultConsentPath = "/oauth2/fallbacks/consent" DefaultErrorPath = "/oauth2/fallbacks/error" @@ -272,13 +274,13 @@ func (h *Handler) UserinfoHandler(w http.ResponseWriter, r *http.Request, _ http delete(interim, "exp") delete(interim, "jti") - keyID, err := h.JWTStrategy.GetPublicKeyID() + keyID, err := h.OpenIDJWTStrategy.GetPublicKeyID() if err != nil { h.H.WriteError(w, r, err) return } - token, _, err := h.JWTStrategy.Generate(jwt2.MapClaims(interim), &jwt.Headers{ + token, _, err := h.OpenIDJWTStrategy.Generate(jwt2.MapClaims(interim), &jwt.Headers{ Extra: map[string]interface{}{ "kid": keyID, }, @@ -473,7 +475,23 @@ func (h *Handler) TokenHandler(w http.ResponseWriter, r *http.Request, _ httprou } if accessRequest.GetGrantTypes().Exact("client_credentials") { + var accessTokenKeyID string + if h.AccessTokenJWTStrategy != nil { + accessTokenKeyID, err = h.AccessTokenJWTStrategy.GetPublicKeyID() + if err != nil { + pkg.LogError(err, h.L) + h.OAuth2.WriteAccessError(w, accessRequest, err) + return + } + } + session.Subject = accessRequest.GetClient().GetID() + session.ClientID = accessRequest.GetClient().GetID() + session.JTI = uuid.New() + session.KID = accessTokenKeyID + session.DefaultSession.Claims.Issuer = strings.TrimRight(h.IssuerURL, "/") + "/" + session.DefaultSession.Claims.IssuedAt = time.Now().UTC() + for _, scope := range accessRequest.GetRequestedScopes() { if h.ScopeStrategy(accessRequest.GetClient().GetScopes(), scope) { accessRequest.GrantScope(scope) @@ -533,13 +551,23 @@ func (h *Handler) AuthHandler(w http.ResponseWriter, r *http.Request, _ httprout authorizeRequest.GrantScope(scope) } - keyID, err := h.JWTStrategy.GetPublicKeyID() + openIDKeyID, err := h.OpenIDJWTStrategy.GetPublicKeyID() if err != nil { pkg.LogError(err, h.L) h.writeAuthorizeError(w, authorizeRequest, err) return } + var accessTokenKeyID string + if h.AccessTokenJWTStrategy != nil { + accessTokenKeyID, err = h.AccessTokenJWTStrategy.GetPublicKeyID() + if err != nil { + pkg.LogError(err, h.L) + h.writeAuthorizeError(w, authorizeRequest, err) + return + } + } + authorizeRequest.SetID(session.Challenge) // done @@ -557,12 +585,15 @@ func (h *Handler) AuthHandler(w http.ResponseWriter, r *http.Request, _ httprout Extra: session.Session.IDToken, }, // required for lookup on jwk endpoint - Headers: &jwt.Headers{Extra: map[string]interface{}{"kid": keyID}}, + Headers: &jwt.Headers{Extra: map[string]interface{}{"kid": openIDKeyID}}, Subject: session.ConsentRequest.Subject, }, Extra: session.Session.AccessToken, // Here, we do not include the client because it's typically not the audience. Audience: []string{}, + JTI: uuid.New(), + KID: accessTokenKeyID, + ClientID: authorizeRequest.GetClient().GetID(), }) if err != nil { pkg.LogError(err, h.L) diff --git a/oauth2/handler_struct.go b/oauth2/handler_struct.go index facc7bc17ad..37a1c6e2d99 100644 --- a/oauth2/handler_struct.go +++ b/oauth2/handler_struct.go @@ -47,7 +47,8 @@ type Handler struct { IDTokenLifespan time.Duration CookieStore sessions.Store - JWTStrategy *jwk.RS256JWTStrategy + OpenIDJWTStrategy *jwk.RS256JWTStrategy + AccessTokenJWTStrategy *jwk.RS256JWTStrategy L logrus.FieldLogger diff --git a/oauth2/handler_test.go b/oauth2/handler_test.go index cbc0aee103c..38da9675688 100644 --- a/oauth2/handler_test.go +++ b/oauth2/handler_test.go @@ -149,9 +149,9 @@ func TestUserinfo(t *testing.T) { jwtStrategy, err := jwk.NewRS256JWTStrategy(jm, oauth2.OpenIDConnectKeyName) h := &oauth2.Handler{ - OAuth2: op, - H: herodot.NewJSONWriter(logrus.New()), - JWTStrategy: jwtStrategy, + OAuth2: op, + H: herodot.NewJSONWriter(logrus.New()), + OpenIDJWTStrategy: jwtStrategy, } router := httprouter.New() h.SetRoutes(router) diff --git a/oauth2/introspector_test.go b/oauth2/introspector_test.go index 45167f7b1c5..4c50fa2a8bd 100644 --- a/oauth2/introspector_test.go +++ b/oauth2/introspector_test.go @@ -71,9 +71,9 @@ func TestIntrospectorSDK(t *testing.T) { compose.OAuth2AuthorizeExplicitFactory, compose.OAuth2TokenIntrospectionFactory, ), - H: herodot.NewJSONWriter(l), - IssuerURL: "foobariss", - JWTStrategy: jwtStrategy, + H: herodot.NewJSONWriter(l), + IssuerURL: "foobariss", + OpenIDJWTStrategy: jwtStrategy, } handler.SetRoutes(router) server := httptest.NewServer(router) diff --git a/oauth2/oauth2_auth_code_test.go b/oauth2/oauth2_auth_code_test.go index b1401eeea88..a4df50712cf 100644 --- a/oauth2/oauth2_auth_code_test.go +++ b/oauth2/oauth2_auth_code_test.go @@ -169,7 +169,7 @@ func TestAuthCodeWithDefaultStrategy(t *testing.T) { H: herodot.NewJSONWriter(l), ScopeStrategy: fosite.ExactScopeStrategy, IDTokenLifespan: time.Minute, IssuerURL: ts.URL, ForcedHTTP: true, L: l, - JWTStrategy: jwtStrategy, + OpenIDJWTStrategy: jwtStrategy, } handler.SetRoutes(router) @@ -626,15 +626,15 @@ func TestAuthCodeWithMockStrategy(t *testing.T) { compose.OAuth2TokenRevocationFactory, compose.OAuth2TokenIntrospectionFactory, ), - Consent: consentStrategy, - CookieStore: sessions.NewCookieStore([]byte("foo-secret")), - ForcedHTTP: true, - L: l, - H: herodot.NewJSONWriter(l), - ScopeStrategy: fosite.HierarchicScopeStrategy, - IDTokenLifespan: time.Minute, - IssuerURL: ts.URL, - JWTStrategy: jwtStrategy, + Consent: consentStrategy, + CookieStore: sessions.NewCookieStore([]byte("foo-secret")), + ForcedHTTP: true, + L: l, + H: herodot.NewJSONWriter(l), + ScopeStrategy: fosite.HierarchicScopeStrategy, + IDTokenLifespan: time.Minute, + IssuerURL: ts.URL, + OpenIDJWTStrategy: jwtStrategy, } handler.SetRoutes(router) diff --git a/oauth2/oauth2_client_credentials_test.go b/oauth2/oauth2_client_credentials_test.go index a722744bafa..0322ef8104c 100644 --- a/oauth2/oauth2_client_credentials_test.go +++ b/oauth2/oauth2_client_credentials_test.go @@ -64,14 +64,14 @@ func TestClientCredentials(t *testing.T) { compose.OAuth2TokenIntrospectionFactory, ), //Consent: consentStrategy, - CookieStore: sessions.NewCookieStore([]byte("foo-secret")), - ForcedHTTP: true, - ScopeStrategy: fosite.HierarchicScopeStrategy, - IDTokenLifespan: time.Minute, - H: herodot.NewJSONWriter(l), - L: l, - IssuerURL: ts.URL, - JWTStrategy: jwtStrategy, + CookieStore: sessions.NewCookieStore([]byte("foo-secret")), + ForcedHTTP: true, + ScopeStrategy: fosite.HierarchicScopeStrategy, + IDTokenLifespan: time.Minute, + H: herodot.NewJSONWriter(l), + L: l, + IssuerURL: ts.URL, + OpenIDJWTStrategy: jwtStrategy, } handler.SetRoutes(router) diff --git a/oauth2/revocator_test.go b/oauth2/revocator_test.go index eb9769f15ba..da98970103b 100644 --- a/oauth2/revocator_test.go +++ b/oauth2/revocator_test.go @@ -79,8 +79,8 @@ func TestRevoke(t *testing.T) { compose.OAuth2TokenIntrospectionFactory, compose.OAuth2TokenRevocationFactory, ), - H: herodot.NewJSONWriter(nil), - JWTStrategy: jwtStrategy, + H: herodot.NewJSONWriter(nil), + OpenIDJWTStrategy: jwtStrategy, } router := httprouter.New() diff --git a/oauth2/session.go b/oauth2/session.go index ef045594f5d..592cad7c594 100644 --- a/oauth2/session.go +++ b/oauth2/session.go @@ -28,9 +28,13 @@ import ( ) type Session struct { + // JSON fields are needed for store serialization *openid.DefaultSession `json:"idToken"` Audience []string Extra map[string]interface{} `json:"extra"` + JTI string + KID string + ClientID string } func NewSession(subject string) *Session { @@ -45,6 +49,30 @@ func NewSession(subject string) *Session { } } +func (s *Session) GetJWTClaims() *jwt.JWTClaims { + claims := &jwt.JWTClaims{ + Subject: s.Subject, + Audience: s.Audience, + JTI: s.JTI, + IssuedAt: s.DefaultSession.Claims.IssuedAt, + Issuer: s.DefaultSession.Claims.Issuer, + NotBefore: s.DefaultSession.Claims.IssuedAt, + ExpiresAt: s.GetExpiresAt(fosite.AccessToken), + Extra: s.Extra, + // These are set by the DefaultJWTStrategy + // Scope: s.Scope, + } + + claims.Extra["client_id"] = s.ClientID + return claims +} + +func (s *Session) GetJWTHeader() *jwt.Headers { + return &jwt.Headers{ + Extra: map[string]interface{}{"kid": s.KID}, + } +} + func (s *Session) Clone() fosite.Session { if s == nil { return nil diff --git a/scripts/test-e2e-jwt.sh b/scripts/test-e2e-jwt.sh new file mode 100755 index 00000000000..a883963ad64 --- /dev/null +++ b/scripts/test-e2e-jwt.sh @@ -0,0 +1,93 @@ +#!/usr/bin/env bash + +set -euo pipefail + +cd "$( dirname "${BASH_SOURCE[0]}" )/.." + +killall hydra || true +killall mock-lcp || true +killall mock-cb || true + +export HYDRA_URL=http://127.0.0.1:4444/ +export OAUTH2_CLIENT_ID=foobar +export OAUTH2_CLIENT_SECRET=bazbar +export OAUTH2_ISSUER_URL=http://127.0.0.1:4444/ +export LOG_LEVEL=debug +export REDIRECT_URL=http://127.0.0.1:4445/callback +export AUTH2_SCOPE=openid,offline + +go install . +go install ./test/mock-client +go install ./test/mock-lcp +go install ./test/mock-cb + +DATABASE_URL=memory \ + OAUTH2_CONSENT_URL=http://127.0.0.1:3000/consent \ + OAUTH2_LOGIN_URL=http://127.0.0.1:3000/login \ + OAUTH2_ERROR_URL=http://127.0.0.1:3000/error \ + OAUTH2_SHARE_ERROR_DEBUG=true \ + OAUTH2_ACCESS_TOKEN_STRATEGY=jwt \ + hydra serve --dangerous-force-http --disable-telemetry & + +PORT=3000 mock-lcp & + +PORT=4445 mock-cb & + +while ! echo exit | nc 127.0.0.1 4444; do sleep 1; done +while ! echo exit | nc 127.0.0.1 4445; do sleep 1; done +while ! echo exit | nc 127.0.0.1 3000; do sleep 1; done + + +hydra clients create \ + --endpoint http://127.0.0.1:4444 \ + --id $OAUTH2_CLIENT_ID \ + --secret $OAUTH2_CLIENT_SECRET \ + --response-types token,code,id_token \ + --grant-types refresh_token,authorization_code,client_credentials \ + --scope openid,offline \ + --callbacks http://127.0.0.1:4445/callback + +token=$(hydra token client) + +hydra token introspect "$token" + +## Authenticate but do not remember user +cookie=$(OAUTH2_EXTRA="&mockLogin=accept&mockConsent=accept" \ + mock-client) +export AUTH_COOKIE=$cookie + +## Must fail because prompt=none but no session was remembered +if OAUTH2_EXTRA="&mockLogin=accept&mockConsent=accept&prompt=none" \ + mock-client; then + echo "should have failed" + exit 1 +fi + +# Authenticate and remember login (but not consent) +cookie=$(OAUTH2_EXTRA="&mockLogin=accept&mockConsent=accept&rememberLogin=yes" \ + mock-client) +export AUTH_COOKIE=$cookie + +## Must fail because prompt=none but consent was not yet stored +if OAUTH2_EXTRA="&mockLogin=accept&mockConsent=accept&prompt=none" \ + mock-client; then + echo "should have failed" + exit 1 +fi + +# Remember consent +cookie=$(OAUTH2_EXTRA="&mockLogin=accept&mockConsent=accept&rememberConsent=yes" \ + mock-client) + +## Prompt none should work now because cookie was set +OAUTH2_EXTRA="&mockLogin=accept&mockConsent=accept&prompt=none" \ + mock-client + +hydra clients delete $OAUTH2_CLIENT_ID + +kill %1 +kill %2 +kill %3 +exit 0 + +sleep 5 diff --git a/scripts/test-e2e.sh b/scripts/test-e2e-opaque.sh similarity index 100% rename from scripts/test-e2e.sh rename to scripts/test-e2e-opaque.sh From d3b4e779446a34b0f74cf7bb7550fdd4c871c135 Mon Sep 17 00:00:00 2001 From: arekkas Date: Sun, 22 Jul 2018 23:12:22 +0200 Subject: [PATCH 3/4] client: Deprecate field `id`, now only `client_id` is to be used Signed-off-by: arekkas --- client/client.go | 3 -- client/client_test.go | 2 +- client/handler.go | 1 - client/manager_0_sql_migrations_test.go | 2 +- client/manager_memory.go | 15 +++--- client/manager_sql.go | 5 +- client/manager_test_helpers.go | 18 +++---- client/sdk_test.go | 11 +--- client/validator.go | 6 +-- client/validator_test.go | 14 ++--- consent/manager_memory.go | 12 ++--- consent/manager_test.go | 8 +-- consent/sdk_test.go | 4 +- consent/sql_helper_test.go | 4 +- consent/strategy_default_test.go | 60 +++++++++++----------- docker-compose.yml | 2 +- docs/api.swagger.json | 5 -- integration/sql_schema_test.go | 2 +- oauth2/fosite_store_test.go | 2 +- oauth2/fosite_store_test_helpers.go | 12 ++--- oauth2/handler_test.go | 6 +-- oauth2/oauth2_auth_code_test.go | 10 ++-- oauth2/oauth2_client_credentials_test.go | 2 +- oauth2/session.go | 4 ++ sdk/go/hydra/swagger/docs/OAuth2Client.md | 1 - sdk/go/hydra/swagger/o_auth2_client.go | 3 -- sdk/js/swagger/docs/OAuth2Client.md | 1 - sdk/js/swagger/src/model/OAuth2Client.js | 8 --- sdk/php/swagger/docs/Model/OAuth2Client.md | 1 - sdk/php/swagger/lib/Model/OAuth2Client.php | 27 ---------- 30 files changed, 93 insertions(+), 158 deletions(-) diff --git a/client/client.go b/client/client.go index 120aff10281..85438bab78e 100644 --- a/client/client.go +++ b/client/client.go @@ -36,9 +36,6 @@ type Client struct { // ClientID is the id for this client. ClientID string `json:"client_id"` - // ID is an alisa for client_id. - ID string `json:"id"` - // Name is the human-readable string name of the client to be presented to the // end-user during authorization. Name string `json:"client_name"` diff --git a/client/client_test.go b/client/client_test.go index 37fa26092a4..a79f98f3ab9 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -32,7 +32,7 @@ var _ fosite.Client = new(Client) func TestClient(t *testing.T) { c := &Client{ - ID: "foo", + ClientID: "foo", RedirectURIs: []string{"foo"}, Scope: "foo bar", TokenEndpointAuthMethod: "none", diff --git a/client/handler.go b/client/handler.go index 5fbc4f3b9ad..1f833fc97dd 100644 --- a/client/handler.go +++ b/client/handler.go @@ -152,7 +152,6 @@ func (h *Handler) Update(w http.ResponseWriter, r *http.Request, ps httprouter.P secret = c.Secret } - c.ID = ps.ByName("id") c.ClientID = ps.ByName("id") if err := h.Validator.Validate(&c); err != nil { h.H.WriteError(w, r, err) diff --git a/client/manager_0_sql_migrations_test.go b/client/manager_0_sql_migrations_test.go index 2565281b3ec..48f49ddd991 100644 --- a/client/manager_0_sql_migrations_test.go +++ b/client/manager_0_sql_migrations_test.go @@ -154,7 +154,7 @@ func TestMigrations(t *testing.T) { s := &client.SQLManager{DB: db, Hasher: &fosite.BCrypt{WorkFactor: 4}} c, err := s.GetConcreteClient(key) require.NoError(t, err) - assert.EqualValues(t, c.ID, key) + assert.EqualValues(t, c.GetID(), key) }) } diff --git a/client/manager_memory.go b/client/manager_memory.go index af5e072b0e4..ee5771cb65f 100644 --- a/client/manager_memory.go +++ b/client/manager_memory.go @@ -53,8 +53,7 @@ func (m *MemoryManager) GetConcreteClient(id string) (*Client, error) { defer m.RUnlock() for _, c := range m.Clients { - if c.ID == id { - c.ClientID = c.ID + if c.GetID() == id { return &c, nil } } @@ -67,7 +66,7 @@ func (m *MemoryManager) GetClient(_ context.Context, id string) (fosite.Client, } func (m *MemoryManager) UpdateClient(c *Client) error { - o, err := m.GetClient(context.Background(), c.ID) + o, err := m.GetClient(context.Background(), c.GetID()) if err != nil { return err } @@ -88,7 +87,7 @@ func (m *MemoryManager) UpdateClient(c *Client) error { m.Lock() defer m.Unlock() for k, f := range m.Clients { - if f.GetID() == c.ID { + if f.GetID() == c.GetID() { m.Clients[k] = *c } } @@ -109,13 +108,12 @@ func (m *MemoryManager) Authenticate(id string, secret []byte) (*Client, error) return nil, errors.WithStack(err) } - c.ClientID = c.ID return c, nil } func (m *MemoryManager) CreateClient(c *Client) error { - if _, err := m.GetConcreteClient(c.ID); err == nil { - return errors.Errorf("Client %s already exists", c.ID) + if _, err := m.GetConcreteClient(c.GetID()); err == nil { + return errors.Errorf("Client %s already exists", c.GetID()) } m.Lock() @@ -152,8 +150,7 @@ func (m *MemoryManager) GetClients(limit, offset int) (clients map[string]Client start, end := pagination.Index(limit, offset, len(m.Clients)) for _, c := range m.Clients[start:end] { - c.ClientID = c.ID - clients[c.ID] = c + clients[c.GetID()] = c } return clients, nil diff --git a/client/manager_sql.go b/client/manager_sql.go index 04f1147e8cb..4adf1c46cf0 100644 --- a/client/manager_sql.go +++ b/client/manager_sql.go @@ -213,7 +213,7 @@ func sqlDataFromClient(d *Client) (*sqlData, error) { } return &sqlData{ - ID: d.ID, + ID: d.GetID(), Name: d.Name, Secret: d.Secret, RedirectURIs: strings.Join(d.RedirectURIs, "|"), @@ -239,7 +239,6 @@ func sqlDataFromClient(d *Client) (*sqlData, error) { func (d *sqlData) ToClient() (*Client, error) { c := &Client{ - ID: d.ID, ClientID: d.ID, Name: d.Name, Secret: d.Secret, @@ -301,7 +300,7 @@ func (m *SQLManager) GetClient(_ context.Context, id string) (fosite.Client, err } func (m *SQLManager) UpdateClient(c *Client) error { - o, err := m.GetClient(context.Background(), c.ID) + o, err := m.GetClient(context.Background(), c.GetID()) if err != nil { return errors.WithStack(err) } diff --git a/client/manager_test_helpers.go b/client/manager_test_helpers.go index 335cdf6fe95..70f58c02a56 100644 --- a/client/manager_test_helpers.go +++ b/client/manager_test_helpers.go @@ -34,14 +34,14 @@ func TestHelperClientAutoGenerateKey(k string, m Storage) func(t *testing.T) { return func(t *testing.T) { t.Parallel() c := &Client{ - ID: "foo", + ClientID: "foo", Secret: "secret", RedirectURIs: []string{"http://redirect"}, TermsOfServiceURI: "foo", } assert.NoError(t, m.CreateClient(c)) //assert.NotEmpty(t, c.ID) - assert.NoError(t, m.DeleteClient(c.ID)) + assert.NoError(t, m.DeleteClient(c.GetID())) } } @@ -49,7 +49,7 @@ func TestHelperClientAuthenticate(k string, m Manager) func(t *testing.T) { return func(t *testing.T) { t.Parallel() m.CreateClient(&Client{ - ID: "1234321", + ClientID: "1234321", Secret: "secret", RedirectURIs: []string{"http://redirect"}, }) @@ -59,7 +59,7 @@ func TestHelperClientAuthenticate(k string, m Manager) func(t *testing.T) { c, err = m.Authenticate("1234321", []byte("secret")) require.NoError(t, err) - assert.Equal(t, "1234321", c.ID) + assert.Equal(t, "1234321", c.GetID()) } } @@ -70,7 +70,7 @@ func TestHelperCreateGetDeleteClient(k string, m Storage) func(t *testing.T) { assert.NotNil(t, err) c := &Client{ - ID: "1234", + ClientID: "1234", Name: "name", Secret: "secret", RedirectURIs: []string{"http://redirect", "http://redirect1"}, @@ -100,7 +100,7 @@ func TestHelperCreateGetDeleteClient(k string, m Storage) func(t *testing.T) { } assert.NoError(t, m.CreateClient(&Client{ - ID: "2-1234", + ClientID: "2-1234", Name: "name", Secret: "secret", RedirectURIs: []string{"http://redirect"}, @@ -116,8 +116,8 @@ func TestHelperCreateGetDeleteClient(k string, m Storage) func(t *testing.T) { ds, err := m.GetClients(100, 0) assert.NoError(t, err) assert.Len(t, ds, 2) - assert.NotEqual(t, ds["1234"].ID, ds["2-1234"].ID) - assert.NotEqual(t, ds["1234"].ID, ds["2-1234"].ClientID) + assert.NotEqual(t, ds["1234"].ClientID, ds["2-1234"].ClientID) + assert.NotEqual(t, ds["1234"].ClientID, ds["2-1234"].ClientID) //test if SecretExpiresAt was set properly assert.Equal(t, ds["1234"].SecretExpiresAt, 0) @@ -132,7 +132,7 @@ func TestHelperCreateGetDeleteClient(k string, m Storage) func(t *testing.T) { assert.Len(t, ds, 0) err = m.UpdateClient(&Client{ - ID: "2-1234", + ClientID: "2-1234", Name: "name-new", Secret: "secret-new", RedirectURIs: []string{"http://redirect/new"}, diff --git a/client/sdk_test.go b/client/sdk_test.go index 61ce0dcecb2..1166481395a 100644 --- a/client/sdk_test.go +++ b/client/sdk_test.go @@ -81,7 +81,6 @@ func TestClientSDK(t *testing.T) { t.Run("case=client is created and updated", func(t *testing.T) { createClient := createTestClient("") compareClient := createClient - compareClient.Id = compareClient.ClientId createClient.ClientSecretExpiresAt = 10 // returned client is correct on Create @@ -114,7 +113,6 @@ func TestClientSDK(t *testing.T) { // create another client updateClient := createTestClient("foo") result, response, err = c.UpdateOAuth2Client(createClient.ClientId, updateClient) - updateClient.Id = updateClient.ClientId require.NoError(t, err) require.EqualValues(t, http.StatusOK, response.StatusCode, "%s", response.Payload) assert.EqualValues(t, updateClient, *result) @@ -125,7 +123,6 @@ func TestClientSDK(t *testing.T) { result, response, err = c.GetOAuth2Client(updateClient.ClientId) require.NoError(t, err) require.EqualValues(t, http.StatusOK, response.StatusCode, "%s", response.Payload) - compareClient.ClientId = compareClient.Id assert.EqualValues(t, compareClient, *result) // client can not be found after being deleted @@ -163,7 +160,7 @@ func TestClientSDK(t *testing.T) { client: hydra.OAuth2Client{}, }, { - client: hydra.OAuth2Client{Id: "set-properly-1"}, + client: hydra.OAuth2Client{ClientId: "set-properly-1"}, expectID: "set-properly-1", }, { @@ -176,13 +173,10 @@ func TestClientSDK(t *testing.T) { require.NoError(t, err) require.EqualValues(t, http.StatusCreated, response.StatusCode, "%s", response.Payload) - assert.NotEmpty(t, result.Id) assert.NotEmpty(t, result.ClientId) - assert.EqualValues(t, result.Id, result.ClientId) - id := result.Id + id := result.ClientId if tc.expectID != "" { - assert.EqualValues(t, tc.expectID, result.Id) assert.EqualValues(t, tc.expectID, result.ClientId) id = tc.expectID } @@ -191,7 +185,6 @@ func TestClientSDK(t *testing.T) { require.NoError(t, err) require.EqualValues(t, http.StatusOK, response.StatusCode, "%s", response.Payload) - assert.EqualValues(t, id, result.Id) assert.EqualValues(t, id, result.ClientId) }) } diff --git a/client/validator.go b/client/validator.go index e5e73642d5a..8db7dbca7bb 100644 --- a/client/validator.go +++ b/client/validator.go @@ -49,11 +49,7 @@ func NewValidator( func (v *Validator) Validate(c *Client) error { id := uuid.New() - c.ID = stringsx.Coalesce(c.ID, c.ClientID, id) - c.ClientID = stringsx.Coalesce(c.ClientID, c.ID, id) - if c.ID != c.ClientID { - return errors.WithStack(fosite.ErrInvalidRequest.WithHint("Field id and client_id must match.")) - } + c.ClientID = stringsx.Coalesce(c.ClientID, id) if c.TokenEndpointAuthMethod == "" { c.TokenEndpointAuthMethod = "client_secret_basic" diff --git a/client/validator_test.go b/client/validator_test.go index 68990592924..59173fe8483 100644 --- a/client/validator_test.go +++ b/client/validator_test.go @@ -44,30 +44,26 @@ func TestValidate(t *testing.T) { in: new(Client), check: func(t *testing.T, c *Client) { assert.NotEmpty(t, c.ClientID) - assert.NotEmpty(t, c.ID) - assert.Equal(t, c.ID, c.ClientID) + assert.NotEmpty(t, c.GetID()) + assert.Equal(t, c.GetID(), c.ClientID) }, }, { - in: &Client{ID: "foo"}, + in: &Client{ClientID: "foo"}, check: func(t *testing.T, c *Client) { - assert.Equal(t, c.ID, c.ClientID) + assert.Equal(t, c.GetID(), c.ClientID) }, }, { in: &Client{ClientID: "foo"}, check: func(t *testing.T, c *Client) { - assert.Equal(t, c.ID, c.ClientID) + assert.Equal(t, c.GetID(), c.ClientID) }, }, { in: &Client{ClientID: "foo", UserinfoSignedResponseAlg: "foo"}, expectErr: true, }, - { - in: &Client{ClientID: "foo", ID: "bar"}, - expectErr: true, - }, { in: &Client{ClientID: "foo", TokenEndpointAuthMethod: "private_key_jwt"}, expectErr: true, diff --git a/consent/manager_memory.go b/consent/manager_memory.go index 3928b3bf028..02784458615 100644 --- a/consent/manager_memory.go +++ b/consent/manager_memory.go @@ -69,7 +69,7 @@ func (m *MemoryManager) RevokeUserClientConsentSession(user, client string) erro var found bool for k, c := range m.handledConsentRequests { - if c.ConsentRequest.Subject == user && (client == "" || (client != "" && c.ConsentRequest.Client.ID == client)) { + if c.ConsentRequest.Subject == user && (client == "" || (client != "" && c.ConsentRequest.Client.GetID() == client)) { delete(m.handledConsentRequests, k) delete(m.consentRequests, k) if err := m.store.RevokeAccessToken(nil, c.Challenge); errors.Cause(err) == fosite.ErrNotFound { @@ -124,7 +124,7 @@ func (m *MemoryManager) GetConsentRequest(challenge string) (*ConsentRequest, er m.m["consentRequests"].RLock() defer m.m["consentRequests"].RUnlock() if c, ok := m.consentRequests[challenge]; ok { - c.Client.ClientID = c.Client.ID + c.Client.ClientID = c.Client.GetID() return &c, nil } return nil, errors.WithStack(pkg.ErrNotFound) @@ -151,7 +151,7 @@ func (m *MemoryManager) VerifyAndInvalidateConsentRequest(verifier string) (*Han return nil, err } - c.Client.ClientID = c.Client.ID + c.Client.ClientID = c.Client.GetID() h.ConsentRequest = &c return &h, nil } @@ -195,7 +195,7 @@ func (m *MemoryManager) FindPreviouslyGrantedConsentRequests(client string, subj continue } - cr.Client.ClientID = cr.Client.ID + cr.Client.ClientID = cr.Client.GetID() c.ConsentRequest = cr rs = append(rs, c) } @@ -246,7 +246,7 @@ func (m *MemoryManager) GetAuthenticationRequest(challenge string) (*Authenticat m.m["authRequests"].RLock() defer m.m["authRequests"].RUnlock() if c, ok := m.authRequests[challenge]; ok { - c.Client.ClientID = c.Client.ID + c.Client.ClientID = c.Client.GetID() return &c, nil } return nil, errors.WithStack(pkg.ErrNotFound) @@ -273,7 +273,7 @@ func (m *MemoryManager) VerifyAndInvalidateAuthenticationRequest(verifier string return nil, err } - c.Client.ClientID = c.Client.ID + c.Client.ClientID = c.Client.GetID() h.AuthenticationRequest = &c return &h, nil } diff --git a/consent/manager_test.go b/consent/manager_test.go index 1dc79f64d51..a5c9c619e2a 100644 --- a/consent/manager_test.go +++ b/consent/manager_test.go @@ -53,7 +53,7 @@ func mockConsentRequest(key string, remember bool, rememberFor int, hasError boo Display: "popup" + key, }, RequestedAt: time.Now().UTC().Add(-time.Hour), - Client: &client.Client{ID: "client" + key}, + Client: &client.Client{ClientID: "client" + key}, Subject: "subject" + key, RequestURL: "https://request-url/path" + key, Skip: skip, @@ -100,7 +100,7 @@ func mockAuthRequest(key string, authAt bool) (c *AuthenticationRequest, h *Hand Display: "popup" + key, }, RequestedAt: time.Now().UTC().Add(-time.Hour), - Client: &client.Client{ID: "client" + key}, + Client: &client.Client{ClientID: "client" + key}, Subject: "subject" + key, RequestURL: "https://request-url/path" + key, Skip: true, @@ -482,7 +482,7 @@ func TestManagers(t *testing.T) { } func compareAuthenticationRequest(t *testing.T, a, b *AuthenticationRequest) { - assert.EqualValues(t, a.Client.ID, b.Client.ID) + assert.EqualValues(t, a.Client.GetID(), b.Client.GetID()) assert.EqualValues(t, a.Challenge, b.Challenge) assert.EqualValues(t, *a.OpenIDConnectContext, *b.OpenIDConnectContext) assert.EqualValues(t, a.Subject, b.Subject) @@ -494,7 +494,7 @@ func compareAuthenticationRequest(t *testing.T, a, b *AuthenticationRequest) { } func compareConsentRequest(t *testing.T, a, b *ConsentRequest) { - assert.EqualValues(t, a.Client.ID, b.Client.ID) + assert.EqualValues(t, a.Client.GetID(), b.Client.GetID()) assert.EqualValues(t, a.Challenge, b.Challenge) assert.EqualValues(t, *a.OpenIDConnectContext, *b.OpenIDConnectContext) assert.EqualValues(t, a.Subject, b.Subject) diff --git a/consent/sdk_test.go b/consent/sdk_test.go index 0c5dd28cce4..8790cddf9e6 100644 --- a/consent/sdk_test.go +++ b/consent/sdk_test.go @@ -119,12 +119,12 @@ func compareSDKLoginRequest(t *testing.T, expected *AuthenticationRequest, got * assert.EqualValues(t, expected.Challenge, got.Challenge) assert.EqualValues(t, expected.Subject, got.Subject) assert.EqualValues(t, expected.Skip, got.Skip) - assert.EqualValues(t, expected.Client.ID, got.Client.Id) + assert.EqualValues(t, expected.Client.GetID(), got.Client.ClientId) } func compareSDKConsentRequest(t *testing.T, expected *ConsentRequest, got *swagger.ConsentRequest) { assert.EqualValues(t, expected.Challenge, got.Challenge) assert.EqualValues(t, expected.Subject, got.Subject) assert.EqualValues(t, expected.Skip, got.Skip) - assert.EqualValues(t, expected.Client.ID, got.Client.Id) + assert.EqualValues(t, expected.Client.GetID(), got.Client.ClientId) } diff --git a/consent/sql_helper_test.go b/consent/sql_helper_test.go index c66f9739384..7da3ca20616 100644 --- a/consent/sql_helper_test.go +++ b/consent/sql_helper_test.go @@ -46,7 +46,7 @@ func TestSQLAuthenticationConverter(t *testing.T) { }, AuthenticatedAt: time.Now().UTC().Add(-time.Minute), RequestedAt: time.Now().UTC().Add(-time.Hour), - Client: &client.Client{ID: "client"}, + Client: &client.Client{ClientID: "client"}, Subject: "subject", RequestURL: "https://request-url/path", Skip: true, @@ -101,7 +101,7 @@ func TestSQLConsentConverter(t *testing.T) { IDTokenHintClaims: map[string]interface{}{"foo": "bar"}, }, RequestedAt: time.Now().UTC().Add(-time.Hour), - Client: &client.Client{ID: "client"}, + Client: &client.Client{ClientID: "client"}, Subject: "subject", RequestURL: "https://request-url/path", Skip: true, diff --git a/consent/strategy_default_test.go b/consent/strategy_default_test.go index 6027eebe290..a7e9b37b400 100644 --- a/consent/strategy_default_test.go +++ b/consent/strategy_default_test.go @@ -143,7 +143,7 @@ func TestStrategy(t *testing.T) { }{ { d: "This should fail because a login verifier was given that doesn't exist in the store", - req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ID: "client-id"}}}, + req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}}}, lv: "invalid", expectErrType: []error{fosite.ErrAccessDenied}, expectErr: []bool{true}, @@ -151,7 +151,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should fail because a consent verifier was given but no login verifier", - req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ID: "client-id"}}}, + req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}}}, lv: "", cv: "invalid", expectErrType: []error{fosite.ErrAccessDenied}, @@ -162,7 +162,7 @@ func TestStrategy(t *testing.T) { d: "This should fail because the request was redirected but the login endpoint doesn't do anything (like redirecting back)", req: fosite.AuthorizeRequest{ Request: fosite.Request{ - Client: &client.Client{ID: "client-id"}, + Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}, }, }, @@ -189,7 +189,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should fail because the request was redirected but the login endpoint rejected the request", - req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { lr, res, err := apiClient.RejectLoginRequest(r.URL.Query().Get("login_challenge"), swagger.RejectRequest{ @@ -211,7 +211,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should fail because no cookie jar / invalid csrf", - req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, lph: passAuthentication(apiClient, false), cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { @@ -226,7 +226,7 @@ func TestStrategy(t *testing.T) { { d: "This should fail because consent endpoints idles after login was granted - but consent endpoint should be called because cookie jar exists", jar: newCookieJar(), - req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, lph: passAuthentication(apiClient, false), other: "display=page&ui_locales=de+en&acr_values=1+2", cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { @@ -253,14 +253,14 @@ func TestStrategy(t *testing.T) { d: "This should fail because consent verifier was set but does not exist", jar: newCookieJar(), cv: "invalid", - req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, expectFinalStatusCode: http.StatusForbidden, expectErrType: []error{fosite.ErrAccessDenied}, expectErr: []bool{true}, }, { d: "This should fail because consent endpoints denies the request after login was granted", - req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: newCookieJar(), lph: passAuthentication(apiClient, false), cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { @@ -284,7 +284,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should pass because login and consent have been granted", - req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: newCookieJar(), lph: passAuthentication(apiClient, false), cph: passAuthorization(apiClient, false), @@ -304,7 +304,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should pass because login and consent have been granted, this time we remember the decision", - req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ, lph: passAuthentication(apiClient, true), cph: passAuthorization(apiClient, true), @@ -324,7 +324,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should fail because prompt=none, client is public, and redirection scheme is not HTTPS but a custom scheme", - req: fosite.AuthorizeRequest{RedirectURI: mustParseURL(t, "custom://redirection-scheme/path"), Request: fosite.Request{Client: &client.Client{TokenEndpointAuthMethod: "none", ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{RedirectURI: mustParseURL(t, "custom://redirection-scheme/path"), Request: fosite.Request{Client: &client.Client{TokenEndpointAuthMethod: "none", ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, prompt: "none", jar: persistentCJ, lph: passAuthentication(apiClient, false), @@ -335,7 +335,7 @@ func TestStrategy(t *testing.T) { // This test is disabled because it breaks OIDC Conformity Tests //{ // d: "This should pass but require consent because it's not an authorization_code flow", - // req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + // req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, // jar: persistentCJ, // lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { // return func(w http.ResponseWriter, r *http.Request) { @@ -397,7 +397,7 @@ func TestStrategy(t *testing.T) { //}, { d: "This should fail at login screen because subject from accept does not match subject from session", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ, lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { @@ -425,7 +425,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should pass and confirm previous authentication and consent because it is a authorization_code", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id", Secret: "should-not-be-included"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id", Secret: "should-not-be-included"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ, lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { @@ -489,7 +489,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should pass and require re-authentication although session is set (because prompt=login)", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ, prompt: "login+consent", lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { @@ -549,7 +549,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should pass and require re-authentication although session is set (because max_age=1)", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ, maxAge: "1", setup: func() { @@ -612,7 +612,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should fail because max_age=1 but prompt=none", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ, setup: func() { time.Sleep(time.Second * 2) @@ -625,7 +625,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should fail because skip is true and remember as well when doing login", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ, lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { @@ -652,7 +652,7 @@ func TestStrategy(t *testing.T) { }, { d: "This fail because skip is true and remember as well when doing consent", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ, lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { @@ -703,7 +703,7 @@ func TestStrategy(t *testing.T) { { d: "This should fail because prompt is none but no auth session exists", prompt: "none", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: newCookieJar(), expectFinalStatusCode: http.StatusBadRequest, expectErrType: []error{fosite.ErrLoginRequired}, @@ -712,7 +712,7 @@ func TestStrategy(t *testing.T) { { d: "This should fail because prompt is none and consent is missing a permission which requires re-authorization of the app", prompt: "none", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a", "this-scope-has-not-been-granted-before"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a", "this-scope-has-not-been-granted-before"}}}, jar: persistentCJ, lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { @@ -740,7 +740,7 @@ func TestStrategy(t *testing.T) { }, { d: "This pass and properly require authentication as well as authorization because prompt is set to login and consent - although previous session exists", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ, prompt: "login+consent", lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { @@ -790,7 +790,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should fail because id_token_hint does not match authentication session and prompt is none", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ, prompt: "none", idTokenHint: fooUserIDToken, @@ -800,7 +800,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should pass and require authentication because id_token_hint does not match subject from session", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ, idTokenHint: fooUserIDToken, lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { @@ -828,7 +828,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should pass and require authentication because id_token_hint does not match subject from session", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ, idTokenHint: fooUserIDToken, lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { @@ -890,7 +890,7 @@ func TestStrategy(t *testing.T) { // checks revoking sessions { d: "This should pass as regularly and create a new session", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ2, lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { @@ -913,7 +913,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should pass and also revoke the session cookie", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ2, prompt: "login", lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { @@ -937,7 +937,7 @@ func TestStrategy(t *testing.T) { }, // these two tests depend on one another { d: "This should require re-authentication because the session was revoked in the previous test", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: persistentCJ2, lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { @@ -968,7 +968,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should require re-authentication because the session does not exist in the store", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: nonexistentCJ, lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { @@ -999,7 +999,7 @@ func TestStrategy(t *testing.T) { }, { d: "This should fail because the user from the ID token does not match the user from the accept login request", - req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}}, + req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, Scopes: []string{"scope-a"}}}, jar: newCookieJar(), idTokenHint: fooUserIDToken, lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { diff --git a/docker-compose.yml b/docker-compose.yml index f394e848051..2fe66c7746a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -39,7 +39,6 @@ services: - hydra-migrate ports: - "4444:4444" - - "4445:4445" command: serve --dangerous-force-http environment: @@ -52,6 +51,7 @@ services: # - DATABASE_URL=mysql://root:secret@tcp(mysqld:3306)/mysql?parseTime=true - SYSTEM_SECRET=youReallyNeedToChangeThis - OAUTH2_SHARE_ERROR_DEBUG=1 + - OAUTH2_ACCESS_TOKEN_STRATEGY=jwt restart: unless-stopped consent: diff --git a/docs/api.swagger.json b/docs/api.swagger.json index 49fe67181ac..eff6f931bdd 100644 --- a/docs/api.swagger.json +++ b/docs/api.swagger.json @@ -2326,11 +2326,6 @@ }, "x-go-name": "GrantTypes" }, - "id": { - "description": "ID is an alisa for client_id.", - "type": "string", - "x-go-name": "ID" - }, "jwks": { "$ref": "#/definitions/JSONWebKeySet" }, diff --git a/integration/sql_schema_test.go b/integration/sql_schema_test.go index b814426dcca..105682e7c34 100644 --- a/integration/sql_schema_test.go +++ b/integration/sql_schema_test.go @@ -76,7 +76,7 @@ func TestSQLSchema(t *testing.T) { require.NoError(t, jm.AddKey("integration-test-foo", jwk.First(p1))) require.NoError(t, pm.Create(&ladon.DefaultPolicy{ID: "integration-test-foo", Resources: []string{"foo"}, Actions: []string{"bar"}, Subjects: []string{"baz"}, Effect: "allow"})) - require.NoError(t, cm.CreateClient(&client.Client{ID: "integration-test-foo"})) + require.NoError(t, cm.CreateClient(&client.Client{ClientID: "integration-test-foo"})) require.NoError(t, crm.CreateAuthenticationSession(&consent.AuthenticationSession{ ID: "foo", AuthenticatedAt: time.Now(), diff --git a/oauth2/fosite_store_test.go b/oauth2/fosite_store_test.go index 03e4425bb2f..97d24485dec 100644 --- a/oauth2/fosite_store_test.go +++ b/oauth2/fosite_store_test.go @@ -40,7 +40,7 @@ import ( var fositeStores = map[string]pkg.FositeStorer{} var clientManager = &client.MemoryManager{ - Clients: []client.Client{{ID: "foobar"}}, + Clients: []client.Client{{ClientID: "foobar"}}, Hasher: &fosite.BCrypt{}, } var databases = make(map[string]*sqlx.DB) diff --git a/oauth2/fosite_store_test_helpers.go b/oauth2/fosite_store_test_helpers.go index 8a536fa7f17..00572ee05eb 100644 --- a/oauth2/fosite_store_test_helpers.go +++ b/oauth2/fosite_store_test_helpers.go @@ -36,7 +36,7 @@ import ( var defaultRequest = fosite.Request{ RequestedAt: time.Now().UTC().Round(time.Second), - Client: &client.Client{ID: "foobar"}, + Client: &client.Client{ClientID: "foobar"}, Scopes: fosite.Arguments{"fa", "ba"}, GrantedScopes: fosite.Arguments{"fa", "ba"}, Form: url.Values{"foo": []string{"bar", "baz"}}, @@ -92,10 +92,10 @@ func TestHelperRevokeRefreshToken(m pkg.FositeStorer) func(t *testing.T) { _, err := m.GetRefreshTokenSession(ctx, "1111", &fosite.DefaultSession{}) assert.NotNil(t, err) - err = m.CreateRefreshTokenSession(ctx, "1111", &fosite.Request{ID: id, Client: &client.Client{ID: "foobar"}, RequestedAt: time.Now().UTC().Round(time.Second), Session: &fosite.DefaultSession{}}) + err = m.CreateRefreshTokenSession(ctx, "1111", &fosite.Request{ID: id, Client: &client.Client{ClientID: "foobar"}, RequestedAt: time.Now().UTC().Round(time.Second), Session: &fosite.DefaultSession{}}) require.NoError(t, err) - err = m.CreateRefreshTokenSession(ctx, "1122", &fosite.Request{ID: id, Client: &client.Client{ID: "foobar"}, RequestedAt: time.Now().UTC().Round(time.Second), Session: &fosite.DefaultSession{}}) + err = m.CreateRefreshTokenSession(ctx, "1122", &fosite.Request{ID: id, Client: &client.Client{ClientID: "foobar"}, RequestedAt: time.Now().UTC().Round(time.Second), Session: &fosite.DefaultSession{}}) require.NoError(t, err) _, err = m.GetRefreshTokenSession(ctx, "1111", &fosite.DefaultSession{}) @@ -184,7 +184,7 @@ var flushRequests = []*fosite.Request{ { ID: "flush-1", RequestedAt: time.Now().Round(time.Second), - Client: &client.Client{ID: "foobar"}, + Client: &client.Client{ClientID: "foobar"}, Scopes: fosite.Arguments{"fa", "ba"}, GrantedScopes: fosite.Arguments{"fa", "ba"}, Form: url.Values{"foo": []string{"bar", "baz"}}, @@ -193,7 +193,7 @@ var flushRequests = []*fosite.Request{ { ID: "flush-2", RequestedAt: time.Now().Round(time.Second).Add(-(lifespan + time.Minute)), - Client: &client.Client{ID: "foobar"}, + Client: &client.Client{ClientID: "foobar"}, Scopes: fosite.Arguments{"fa", "ba"}, GrantedScopes: fosite.Arguments{"fa", "ba"}, Form: url.Values{"foo": []string{"bar", "baz"}}, @@ -202,7 +202,7 @@ var flushRequests = []*fosite.Request{ { ID: "flush-3", RequestedAt: time.Now().Round(time.Second).Add(-(lifespan + time.Hour)), - Client: &client.Client{ID: "foobar"}, + Client: &client.Client{ClientID: "foobar"}, Scopes: fosite.Arguments{"fa", "ba"}, GrantedScopes: fosite.Arguments{"fa", "ba"}, Form: url.Values{"foo": []string{"bar", "baz"}}, diff --git a/oauth2/handler_test.go b/oauth2/handler_test.go index 38da9675688..95b97f4b754 100644 --- a/oauth2/handler_test.go +++ b/oauth2/handler_test.go @@ -56,7 +56,7 @@ var flushRequests = []*fosite.Request{ { ID: "flush-1", RequestedAt: time.Now().Round(time.Second), - Client: &client.Client{ID: "foobar"}, + Client: &client.Client{ClientID: "foobar"}, Scopes: fosite.Arguments{"fa", "ba"}, GrantedScopes: fosite.Arguments{"fa", "ba"}, Form: url.Values{"foo": []string{"bar", "baz"}}, @@ -65,7 +65,7 @@ var flushRequests = []*fosite.Request{ { ID: "flush-2", RequestedAt: time.Now().Round(time.Second).Add(-(lifespan + time.Minute)), - Client: &client.Client{ID: "foobar"}, + Client: &client.Client{ClientID: "foobar"}, Scopes: fosite.Arguments{"fa", "ba"}, GrantedScopes: fosite.Arguments{"fa", "ba"}, Form: url.Values{"foo": []string{"bar", "baz"}}, @@ -74,7 +74,7 @@ var flushRequests = []*fosite.Request{ { ID: "flush-3", RequestedAt: time.Now().Round(time.Second).Add(-(lifespan + time.Hour)), - Client: &client.Client{ID: "foobar"}, + Client: &client.Client{ClientID: "foobar"}, Scopes: fosite.Arguments{"fa", "ba"}, GrantedScopes: fosite.Arguments{"fa", "ba"}, Form: url.Values{"foo": []string{"bar", "baz"}}, diff --git a/oauth2/oauth2_auth_code_test.go b/oauth2/oauth2_auth_code_test.go index a4df50712cf..8c31c23d093 100644 --- a/oauth2/oauth2_auth_code_test.go +++ b/oauth2/oauth2_auth_code_test.go @@ -179,13 +179,13 @@ func TestAuthCodeWithDefaultStrategy(t *testing.T) { api := httptest.NewServer(apiRouter) client := hc.Client{ - ID: "e2e-app-client" + km, Secret: "secret", RedirectURIs: []string{ts.URL + "/callback"}, + ClientID: "e2e-app-client" + km, Secret: "secret", RedirectURIs: []string{ts.URL + "/callback"}, ResponseTypes: []string{"id_token", "code", "token"}, GrantTypes: []string{"implicit", "refresh_token", "authorization_code", "password", "client_credentials"}, Scope: "hydra offline openid", } oauthConfig := &oauth2.Config{ - ClientID: client.ID, ClientSecret: client.Secret, + ClientID: client.GetID(), ClientSecret: client.Secret, Endpoint: oauth2.Endpoint{AuthURL: ts.URL + "/oauth2/auth", TokenURL: ts.URL + "/oauth2/token"}, RedirectURL: client.RedirectURIs[0], Scopes: []string{"hydra", "offline", "openid"}, } @@ -225,7 +225,7 @@ func TestAuthCodeWithDefaultStrategy(t *testing.T) { require.EqualValues(t, http.StatusOK, res.StatusCode) assert.False(t, rr.Skip) assert.Empty(t, rr.Subject) - assert.EqualValues(t, client.ID, rr.Client.ClientId) + assert.EqualValues(t, client.GetID(), rr.Client.ClientId) assert.EqualValues(t, client.GrantTypes, rr.Client.GrantTypes) assert.EqualValues(t, client.LogoURI, rr.Client.LogoUri) assert.EqualValues(t, client.RedirectURIs, rr.Client.RedirectUris) @@ -249,7 +249,7 @@ func TestAuthCodeWithDefaultStrategy(t *testing.T) { require.EqualValues(t, http.StatusOK, res.StatusCode) assert.False(t, rr.Skip) assert.EqualValues(t, "user-a", rr.Subject) - assert.EqualValues(t, client.ID, rr.Client.ClientId) + assert.EqualValues(t, client.GetID(), rr.Client.ClientId) assert.EqualValues(t, client.GrantTypes, rr.Client.GrantTypes) assert.EqualValues(t, client.LogoURI, rr.Client.LogoUri) assert.EqualValues(t, client.RedirectURIs, rr.Client.RedirectUris) @@ -645,7 +645,7 @@ func TestAuthCodeWithMockStrategy(t *testing.T) { m := sync.Mutex{} store.CreateClient(&hc.Client{ - ID: "app-client", + ClientID: "app-client", Secret: "secret", RedirectURIs: []string{ts.URL + "/callback"}, ResponseTypes: []string{"id_token", "code", "token"}, diff --git a/oauth2/oauth2_client_credentials_test.go b/oauth2/oauth2_client_credentials_test.go index 0322ef8104c..f84b690854b 100644 --- a/oauth2/oauth2_client_credentials_test.go +++ b/oauth2/oauth2_client_credentials_test.go @@ -77,7 +77,7 @@ func TestClientCredentials(t *testing.T) { handler.SetRoutes(router) require.NoError(t, store.CreateClient(&hc.Client{ - ID: "app-client", + ClientID: "app-client", Secret: "secret", RedirectURIs: []string{ts.URL + "/callback"}, ResponseTypes: []string{"token"}, diff --git a/oauth2/session.go b/oauth2/session.go index 592cad7c594..8681ee3b508 100644 --- a/oauth2/session.go +++ b/oauth2/session.go @@ -63,6 +63,10 @@ func (s *Session) GetJWTClaims() *jwt.JWTClaims { // Scope: s.Scope, } + if claims.Extra == nil { + claims.Extra = map[string]interface{}{} + } + claims.Extra["client_id"] = s.ClientID return claims } diff --git a/sdk/go/hydra/swagger/docs/OAuth2Client.md b/sdk/go/hydra/swagger/docs/OAuth2Client.md index 30914c136d2..b84f51bd5f1 100644 --- a/sdk/go/hydra/swagger/docs/OAuth2Client.md +++ b/sdk/go/hydra/swagger/docs/OAuth2Client.md @@ -10,7 +10,6 @@ Name | Type | Description | Notes **ClientUri** | **string** | ClientURI is an URL string of a web page providing information about the client. If present, the server SHOULD display this URL to the end-user in a clickable fashion. | [optional] [default to null] **Contacts** | **[]string** | Contacts is a array of strings representing ways to contact people responsible for this client, typically email addresses. | [optional] [default to null] **GrantTypes** | **[]string** | GrantTypes is an array of grant types the client is allowed to use. | [optional] [default to null] -**Id** | **string** | ID is an alisa for client_id. | [optional] [default to null] **Jwks** | [**JsonWebKeySet**](JSONWebKeySet.md) | | [optional] [default to null] **JwksUri** | **string** | URL for the Client's JSON Web Key Set [JWK] document. If the Client signs requests to the Server, it contains the signing key(s) the Server uses to validate signatures from the Client. The JWK Set MAY also contain the Client's encryption keys(s), which are used by the Server to encrypt responses to the Client. When both signing and encryption keys are made available, a use (Key Use) parameter value is REQUIRED for all keys in the referenced JWK Set to indicate each key's intended usage. Although some algorithms allow the same key to be used for both signatures and encryption, doing so is NOT RECOMMENDED, as it is less secure. The JWK x5c parameter MAY be used to provide X.509 representations of keys provided. When used, the bare key values MUST still be present and MUST match those in the certificate. | [optional] [default to null] **LogoUri** | **string** | LogoURI is an URL string that references a logo for the client. | [optional] [default to null] diff --git a/sdk/go/hydra/swagger/o_auth2_client.go b/sdk/go/hydra/swagger/o_auth2_client.go index 55b324ed33a..97440e6dd0d 100644 --- a/sdk/go/hydra/swagger/o_auth2_client.go +++ b/sdk/go/hydra/swagger/o_auth2_client.go @@ -33,9 +33,6 @@ type OAuth2Client struct { // GrantTypes is an array of grant types the client is allowed to use. GrantTypes []string `json:"grant_types,omitempty"` - // ID is an alisa for client_id. - Id string `json:"id,omitempty"` - Jwks JsonWebKeySet `json:"jwks,omitempty"` // URL for the Client's JSON Web Key Set [JWK] document. If the Client signs requests to the Server, it contains the signing key(s) the Server uses to validate signatures from the Client. The JWK Set MAY also contain the Client's encryption keys(s), which are used by the Server to encrypt responses to the Client. When both signing and encryption keys are made available, a use (Key Use) parameter value is REQUIRED for all keys in the referenced JWK Set to indicate each key's intended usage. Although some algorithms allow the same key to be used for both signatures and encryption, doing so is NOT RECOMMENDED, as it is less secure. The JWK x5c parameter MAY be used to provide X.509 representations of keys provided. When used, the bare key values MUST still be present and MUST match those in the certificate. diff --git a/sdk/js/swagger/docs/OAuth2Client.md b/sdk/js/swagger/docs/OAuth2Client.md index e1e77281f82..85894657b1f 100644 --- a/sdk/js/swagger/docs/OAuth2Client.md +++ b/sdk/js/swagger/docs/OAuth2Client.md @@ -10,7 +10,6 @@ Name | Type | Description | Notes **clientUri** | **String** | ClientURI is an URL string of a web page providing information about the client. If present, the server SHOULD display this URL to the end-user in a clickable fashion. | [optional] **contacts** | **[String]** | Contacts is a array of strings representing ways to contact people responsible for this client, typically email addresses. | [optional] **grantTypes** | **[String]** | GrantTypes is an array of grant types the client is allowed to use. | [optional] -**id** | **String** | ID is an alisa for client_id. | [optional] **jwks** | [**JSONWebKeySet**](JSONWebKeySet.md) | | [optional] **jwksUri** | **String** | URL for the Client's JSON Web Key Set [JWK] document. If the Client signs requests to the Server, it contains the signing key(s) the Server uses to validate signatures from the Client. The JWK Set MAY also contain the Client's encryption keys(s), which are used by the Server to encrypt responses to the Client. When both signing and encryption keys are made available, a use (Key Use) parameter value is REQUIRED for all keys in the referenced JWK Set to indicate each key's intended usage. Although some algorithms allow the same key to be used for both signatures and encryption, doing so is NOT RECOMMENDED, as it is less secure. The JWK x5c parameter MAY be used to provide X.509 representations of keys provided. When used, the bare key values MUST still be present and MUST match those in the certificate. | [optional] **logoUri** | **String** | LogoURI is an URL string that references a logo for the client. | [optional] diff --git a/sdk/js/swagger/src/model/OAuth2Client.js b/sdk/js/swagger/src/model/OAuth2Client.js index cc51885935e..7b62b0d20d8 100644 --- a/sdk/js/swagger/src/model/OAuth2Client.js +++ b/sdk/js/swagger/src/model/OAuth2Client.js @@ -98,9 +98,6 @@ 'String' ]) } - if (data.hasOwnProperty('id')) { - obj['id'] = ApiClient.convertToType(data['id'], 'String') - } if (data.hasOwnProperty('jwks')) { obj['jwks'] = JSONWebKeySet.constructFromObject(data['jwks']) } @@ -204,11 +201,6 @@ * @member {Array.} grant_types */ exports.prototype['grant_types'] = undefined - /** - * ID is an alisa for client_id. - * @member {String} id - */ - exports.prototype['id'] = undefined /** * @member {module:model/JSONWebKeySet} jwks */ diff --git a/sdk/php/swagger/docs/Model/OAuth2Client.md b/sdk/php/swagger/docs/Model/OAuth2Client.md index 3a6d1495980..1e84e7077fa 100644 --- a/sdk/php/swagger/docs/Model/OAuth2Client.md +++ b/sdk/php/swagger/docs/Model/OAuth2Client.md @@ -10,7 +10,6 @@ Name | Type | Description | Notes **client_uri** | **string** | ClientURI is an URL string of a web page providing information about the client. If present, the server SHOULD display this URL to the end-user in a clickable fashion. | [optional] **contacts** | **string[]** | Contacts is a array of strings representing ways to contact people responsible for this client, typically email addresses. | [optional] **grant_types** | **string[]** | GrantTypes is an array of grant types the client is allowed to use. | [optional] -**id** | **string** | ID is an alisa for client_id. | [optional] **jwks** | [**\Hydra\SDK\Model\JSONWebKeySet**](JSONWebKeySet.md) | | [optional] **jwks_uri** | **string** | URL for the Client's JSON Web Key Set [JWK] document. If the Client signs requests to the Server, it contains the signing key(s) the Server uses to validate signatures from the Client. The JWK Set MAY also contain the Client's encryption keys(s), which are used by the Server to encrypt responses to the Client. When both signing and encryption keys are made available, a use (Key Use) parameter value is REQUIRED for all keys in the referenced JWK Set to indicate each key's intended usage. Although some algorithms allow the same key to be used for both signatures and encryption, doing so is NOT RECOMMENDED, as it is less secure. The JWK x5c parameter MAY be used to provide X.509 representations of keys provided. When used, the bare key values MUST still be present and MUST match those in the certificate. | [optional] **logo_uri** | **string** | LogoURI is an URL string that references a logo for the client. | [optional] diff --git a/sdk/php/swagger/lib/Model/OAuth2Client.php b/sdk/php/swagger/lib/Model/OAuth2Client.php index 9babf2d55aa..5edc64755b8 100644 --- a/sdk/php/swagger/lib/Model/OAuth2Client.php +++ b/sdk/php/swagger/lib/Model/OAuth2Client.php @@ -61,7 +61,6 @@ class OAuth2Client implements ArrayAccess 'client_uri' => 'string', 'contacts' => 'string[]', 'grant_types' => 'string[]', - 'id' => 'string', 'jwks' => '\Hydra\SDK\Model\JSONWebKeySet', 'jwks_uri' => 'string', 'logo_uri' => 'string', @@ -90,7 +89,6 @@ class OAuth2Client implements ArrayAccess 'client_uri' => null, 'contacts' => null, 'grant_types' => null, - 'id' => null, 'jwks' => null, 'jwks_uri' => null, 'logo_uri' => null, @@ -129,7 +127,6 @@ public static function swaggerFormats() 'client_uri' => 'client_uri', 'contacts' => 'contacts', 'grant_types' => 'grant_types', - 'id' => 'id', 'jwks' => 'jwks', 'jwks_uri' => 'jwks_uri', 'logo_uri' => 'logo_uri', @@ -159,7 +156,6 @@ public static function swaggerFormats() 'client_uri' => 'setClientUri', 'contacts' => 'setContacts', 'grant_types' => 'setGrantTypes', - 'id' => 'setId', 'jwks' => 'setJwks', 'jwks_uri' => 'setJwksUri', 'logo_uri' => 'setLogoUri', @@ -189,7 +185,6 @@ public static function swaggerFormats() 'client_uri' => 'getClientUri', 'contacts' => 'getContacts', 'grant_types' => 'getGrantTypes', - 'id' => 'getId', 'jwks' => 'getJwks', 'jwks_uri' => 'getJwksUri', 'logo_uri' => 'getLogoUri', @@ -244,7 +239,6 @@ public function __construct(array $data = null) $this->container['client_uri'] = isset($data['client_uri']) ? $data['client_uri'] : null; $this->container['contacts'] = isset($data['contacts']) ? $data['contacts'] : null; $this->container['grant_types'] = isset($data['grant_types']) ? $data['grant_types'] : null; - $this->container['id'] = isset($data['id']) ? $data['id'] : null; $this->container['jwks'] = isset($data['jwks']) ? $data['jwks'] : null; $this->container['jwks_uri'] = isset($data['jwks_uri']) ? $data['jwks_uri'] : null; $this->container['logo_uri'] = isset($data['logo_uri']) ? $data['logo_uri'] : null; @@ -440,27 +434,6 @@ public function setGrantTypes($grant_types) return $this; } - /** - * Gets id - * @return string - */ - public function getId() - { - return $this->container['id']; - } - - /** - * Sets id - * @param string $id ID is an alisa for client_id. - * @return $this - */ - public function setId($id) - { - $this->container['id'] = $id; - - return $this; - } - /** * Gets jwks * @return \Hydra\SDK\Model\JSONWebKeySet From 388ee2769093758f3bf20a9803ef36218e33d7a6 Mon Sep 17 00:00:00 2001 From: arekkas Date: Mon, 23 Jul 2018 16:40:32 +0200 Subject: [PATCH 4/4] oauth2: Add and enhance access/refresh token tests This patch introduces more tests for code and refresh flows and the JWT strategy. Signed-off-by: arekkas --- client/manager_0_sql_migrations_test.go | 6 + client/manager_test.go | 6 + consent/manager_test.go | 6 + jwk/manager_0_sql_migrations_test.go | 6 + jwk/manager_test.go | 8 + oauth2/fosite_store_test.go | 6 + oauth2/handler.go | 3 - oauth2/oauth2_auth_code_test.go | 1463 ++++++++++++---------- oauth2/oauth2_client_credentials_test.go | 135 +- oauth2/oauth2_helper_test.go | 6 +- oauth2/session.go | 20 +- 11 files changed, 972 insertions(+), 693 deletions(-) diff --git a/client/manager_0_sql_migrations_test.go b/client/manager_0_sql_migrations_test.go index 48f49ddd991..97bb5f91210 100644 --- a/client/manager_0_sql_migrations_test.go +++ b/client/manager_0_sql_migrations_test.go @@ -23,6 +23,7 @@ package client_test import ( "fmt" "log" + "sync" "testing" "github.com/jmoiron/sqlx" @@ -116,6 +117,7 @@ var migrations = map[string]*migrate.MemoryMigrationSource{ } func TestMigrations(t *testing.T) { + var m sync.Mutex var dbs = map[string]*sqlx.DB{} if testing.Short() { return @@ -127,14 +129,18 @@ func TestMigrations(t *testing.T) { if err != nil { log.Fatalf("Could not connect to database: %v", err) } + m.Lock() dbs["postgres"] = db + m.Unlock() }, func() { db, err := dockertest.ConnectToTestMySQL() if err != nil { log.Fatalf("Could not connect to database: %v", err) } + m.Lock() dbs["mysql"] = db + m.Unlock() }, }) diff --git a/client/manager_test.go b/client/manager_test.go index 467b4631445..50ab85399ba 100644 --- a/client/manager_test.go +++ b/client/manager_test.go @@ -24,6 +24,7 @@ import ( "flag" "fmt" "log" + "sync" "testing" _ "github.com/go-sql-driver/mysql" @@ -35,6 +36,7 @@ import ( ) var clientManagers = map[string]Manager{} +var m sync.Mutex func init() { clientManagers["memory"] = NewMemoryManager(&fosite.BCrypt{}) @@ -61,7 +63,9 @@ func connectToMySQL() { } s := &SQLManager{DB: db, Hasher: &fosite.BCrypt{WorkFactor: 4}} + m.Lock() clientManagers["mysql"] = s + m.Unlock() } func connectToPG() { @@ -71,7 +75,9 @@ func connectToPG() { } s := &SQLManager{DB: db, Hasher: &fosite.BCrypt{WorkFactor: 4}} + m.Lock() clientManagers["postgres"] = s + m.Unlock() } func TestCreateGetDeleteClient(t *testing.T) { diff --git a/consent/manager_test.go b/consent/manager_test.go index a5c9c619e2a..bfa5ef936b7 100644 --- a/consent/manager_test.go +++ b/consent/manager_test.go @@ -24,6 +24,7 @@ import ( "flag" "fmt" "log" + "sync" "testing" "time" @@ -39,6 +40,7 @@ import ( "github.com/stretchr/testify/require" ) +var m sync.Mutex var clientManager = client.NewMemoryManager(&fosite.BCrypt{WorkFactor: 8}) var fositeManager = oauth2.NewFositeMemoryStore(clientManager, time.Hour) var managers = map[string]Manager{ @@ -152,7 +154,9 @@ func connectToPostgres(managers map[string]Manager, c client.Manager) { return } + m.Lock() managers["postgres"] = s + m.Unlock() } func connectToMySQL(managers map[string]Manager, c client.Manager) { @@ -168,7 +172,9 @@ func connectToMySQL(managers map[string]Manager, c client.Manager) { return } + m.Lock() managers["mysql"] = s + m.Unlock() } func TestMain(m *testing.M) { diff --git a/jwk/manager_0_sql_migrations_test.go b/jwk/manager_0_sql_migrations_test.go index cc992149dd6..fcd3bd99f34 100644 --- a/jwk/manager_0_sql_migrations_test.go +++ b/jwk/manager_0_sql_migrations_test.go @@ -23,6 +23,7 @@ package jwk_test import ( "fmt" "log" + "sync" "testing" "github.com/jmoiron/sqlx" @@ -76,6 +77,7 @@ var migrations = &migrate.MemoryMigrationSource{ } func TestMigrations(t *testing.T) { + var m sync.Mutex var dbs = map[string]*sqlx.DB{} if testing.Short() { return @@ -87,14 +89,18 @@ func TestMigrations(t *testing.T) { if err != nil { log.Fatalf("Could not connect to database: %v", err) } + m.Lock() dbs["postgres"] = db + m.Unlock() }, func() { db, err := dockertest.ConnectToTestMySQL() if err != nil { log.Fatalf("Could not connect to database: %v", err) } + m.Lock() dbs["mysql"] = db + m.Unlock() }, }) diff --git a/jwk/manager_test.go b/jwk/manager_test.go index 43369efe521..102adbe6631 100644 --- a/jwk/manager_test.go +++ b/jwk/manager_test.go @@ -24,6 +24,7 @@ import ( "flag" "fmt" "log" + "sync" "testing" _ "github.com/go-sql-driver/mysql" @@ -37,6 +38,7 @@ var managers = map[string]Manager{ "memory": new(MemoryManager), } +var m sync.Mutex var testGenerator = &RS256Generator{} var encryptionKey, _ = RandomBytes(32) @@ -62,7 +64,10 @@ func connectToPG() { } s := &SQLManager{DB: db, Cipher: &AEAD{Key: encryptionKey}} + + m.Lock() managers["postgres"] = s + m.Unlock() } func connectToMySQL() { @@ -72,7 +77,10 @@ func connectToMySQL() { } s := &SQLManager{DB: db, Cipher: &AEAD{Key: encryptionKey}} + + m.Lock() managers["mysql"] = s + m.Unlock() } func TestManagerKey(t *testing.T) { diff --git a/oauth2/fosite_store_test.go b/oauth2/fosite_store_test.go index 97d24485dec..4dd568db1a2 100644 --- a/oauth2/fosite_store_test.go +++ b/oauth2/fosite_store_test.go @@ -24,6 +24,7 @@ import ( "flag" "fmt" "log" + "sync" "testing" "time" @@ -44,6 +45,7 @@ var clientManager = &client.MemoryManager{ Hasher: &fosite.BCrypt{}, } var databases = make(map[string]*sqlx.DB) +var m sync.Mutex func init() { fositeStores["memory"] = NewFositeMemoryStore(nil, time.Hour) @@ -74,8 +76,10 @@ func connectToPG() { log.Fatalf("Could not create postgres schema: %v", err) } + m.Lock() databases["postgres"] = db fositeStores["postgres"] = s + m.Unlock() } func connectToMySQL() { @@ -89,8 +93,10 @@ func connectToMySQL() { log.Fatalf("Could not create postgres schema: %v", err) } + m.Lock() databases["mysql"] = db fositeStores["mysql"] = s + m.Unlock() } func TestCreateGetDeleteAuthorizeCodes(t *testing.T) { diff --git a/oauth2/handler.go b/oauth2/handler.go index 5caf3b4b191..bb025edf504 100644 --- a/oauth2/handler.go +++ b/oauth2/handler.go @@ -36,7 +36,6 @@ import ( "github.com/ory/hydra/client" "github.com/ory/hydra/consent" "github.com/ory/hydra/pkg" - "github.com/pborman/uuid" "github.com/pkg/errors" ) @@ -487,7 +486,6 @@ func (h *Handler) TokenHandler(w http.ResponseWriter, r *http.Request, _ httprou session.Subject = accessRequest.GetClient().GetID() session.ClientID = accessRequest.GetClient().GetID() - session.JTI = uuid.New() session.KID = accessTokenKeyID session.DefaultSession.Claims.Issuer = strings.TrimRight(h.IssuerURL, "/") + "/" session.DefaultSession.Claims.IssuedAt = time.Now().UTC() @@ -591,7 +589,6 @@ func (h *Handler) AuthHandler(w http.ResponseWriter, r *http.Request, _ httprout Extra: session.Session.AccessToken, // Here, we do not include the client because it's typically not the audience. Audience: []string{}, - JTI: uuid.New(), KID: accessTokenKeyID, ClientID: authorizeRequest.GetClient().GetID(), }) diff --git a/oauth2/oauth2_auth_code_test.go b/oauth2/oauth2_auth_code_test.go index 8c31c23d093..8a984ed2a9a 100644 --- a/oauth2/oauth2_auth_code_test.go +++ b/oauth2/oauth2_auth_code_test.go @@ -24,6 +24,7 @@ import ( "bytes" "encoding/json" "fmt" + "io/ioutil" "net/http" "net/http/cookiejar" "net/http/httptest" @@ -33,10 +34,12 @@ import ( "testing" "time" + djwt "github.com/dgrijalva/jwt-go" "github.com/gorilla/sessions" "github.com/julienschmidt/httprouter" "github.com/ory/fosite" "github.com/ory/fosite/compose" + foauth2 "github.com/ory/fosite/handler/oauth2" "github.com/ory/fosite/handler/openid" "github.com/ory/fosite/token/jwt" "github.com/ory/herodot" @@ -92,58 +95,618 @@ type clientCreator interface { // - [x] If `id_token_hint` is handled properly // - [x] What happens if `id_token_hint` does not match the value from the handled authentication request ("accept login") func TestAuthCodeWithDefaultStrategy(t *testing.T) { - var m sync.Mutex - l := logrus.New() - l.Level = logrus.DebugLevel - var lph, cph func(w http.ResponseWriter, r *http.Request) - lp := mockProvider(&lph) - cp := mockProvider(&cph) - jwts := &jwt.RS256JWTStrategy{ - PrivateKey: pkg.MustINSECURELOWENTROPYRSAKEYFORTEST(), - } - hasher := &fosite.BCrypt{ - WorkFactor: 4, - } + for _, strat := range []struct { + d string + s foauth2.CoreStrategy + }{ + { + d: "opaque", + s: oauth2OpqaueStrategy, + }, + { + d: "jwt", + s: oauth2JWTStrategy, + }, + } { + t.Run("strategy="+strat.d, func(t *testing.T) { + var m sync.Mutex + l := logrus.New() + l.Level = logrus.DebugLevel + var lph, cph func(w http.ResponseWriter, r *http.Request) + lp := mockProvider(&lph) + cp := mockProvider(&cph) + jwts := &jwt.RS256JWTStrategy{ + PrivateKey: pkg.MustINSECURELOWENTROPYRSAKEYFORTEST(), + } + hasher := &fosite.BCrypt{ + WorkFactor: 4, + } - fooUserIDToken, _, err := jwts.Generate((jwt.IDTokenClaims{ - Subject: "foouser", - ExpiresAt: time.Now().Add(time.Hour), - IssuedAt: time.Now(), - }).ToMapClaims(), jwt.NewHeaders()) - require.NoError(t, err) + fooUserIDToken, _, err := jwts.Generate((jwt.IDTokenClaims{ + Subject: "foouser", + ExpiresAt: time.Now().Add(time.Hour), + IssuedAt: time.Now(), + }).ToMapClaims(), jwt.NewHeaders()) + require.NoError(t, err) + + // we create a new fositeStore here because the old one + + for km, fs := range fositeStores { + t.Run("manager="+km, func(t *testing.T) { + var cm consent.Manager + switch km { + case "memory": + cm = consent.NewMemoryManager(fs) + fs.(*FositeMemoryStore).Manager = hc.NewMemoryManager(hasher) + case "mysql": + fallthrough + case "postgres": + scm := consent.NewSQLManager(databases[km], fs.(*FositeSQLStore).Manager, fs) + _, err := scm.CreateSchemas() + require.NoError(t, err) + + _, err = (fs.(*FositeSQLStore)).CreateSchemas() + require.NoError(t, err) + + cm = scm + } + + router := httprouter.New() + ts := httptest.NewServer(router) + cookieStore := sessions.NewCookieStore([]byte("foo-secret")) + + consentStrategy := consent.NewStrategy( + lp.URL, cp.URL, ts.URL, "/oauth2/auth", cm, + cookieStore, + fosite.ExactScopeStrategy, false, time.Hour, jwts, + openid.NewOpenIDConnectRequestValidator(nil, jwts), + ) + + jm := &jwk.MemoryManager{Keys: map[string]*jose.JSONWebKeySet{}} + keys, err := (&jwk.RS256Generator{}).Generate("", "sig") + require.NoError(t, err) + require.NoError(t, jm.AddKeySet(OpenIDConnectKeyName, keys)) + jwtStrategy, err := jwk.NewRS256JWTStrategy(jm, OpenIDConnectKeyName) + + handler := &Handler{ + OAuth2: compose.Compose( + fc, fs, strat.s, hasher, + compose.OAuth2AuthorizeExplicitFactory, + compose.OAuth2AuthorizeImplicitFactory, + compose.OAuth2ClientCredentialsGrantFactory, + compose.OAuth2RefreshTokenGrantFactory, + compose.OpenIDConnectExplicitFactory, + compose.OpenIDConnectHybridFactory, + compose.OpenIDConnectImplicitFactory, + compose.OAuth2TokenRevocationFactory, + compose.OAuth2TokenIntrospectionFactory, + ), + Consent: consentStrategy, + CookieStore: cookieStore, + H: herodot.NewJSONWriter(l), + ScopeStrategy: fosite.ExactScopeStrategy, + IDTokenLifespan: time.Minute, IssuerURL: ts.URL, ForcedHTTP: true, L: l, + OpenIDJWTStrategy: jwtStrategy, + } + handler.SetRoutes(router) + + apiHandler := consent.NewHandler(herodot.NewJSONWriter(l), cm) + apiRouter := httprouter.New() + apiHandler.SetRoutes(apiRouter) + api := httptest.NewServer(apiRouter) + + client := hc.Client{ + ClientID: "e2e-app-client" + km + strat.d, Secret: "secret", RedirectURIs: []string{ts.URL + "/callback"}, + ResponseTypes: []string{"id_token", "code", "token"}, + GrantTypes: []string{"implicit", "refresh_token", "authorization_code", "password", "client_credentials"}, + Scope: "hydra offline openid", + } + oauthConfig := &oauth2.Config{ + ClientID: client.GetID(), ClientSecret: client.Secret, + Endpoint: oauth2.Endpoint{AuthURL: ts.URL + "/oauth2/auth", TokenURL: ts.URL + "/oauth2/token"}, + RedirectURL: client.RedirectURIs[0], Scopes: []string{"hydra", "offline", "openid"}, + } + + require.NoError(t, fs.(clientCreator).CreateClient(&client)) + apiClient := swagger.NewOAuth2ApiWithBasePath(api.URL) + + var callbackHandler *httprouter.Handle + router.GET("/callback", func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { + (*callbackHandler)(w, r, ps) + }) + + persistentCJ := newCookieJar() + var code string + for k, tc := range []struct { + authURL string + cj http.CookieJar + d string + cb func(t *testing.T) httprouter.Handle + expectOAuthAuthError bool + expectOAuthTokenError bool + lph, cph func(t *testing.T) func(w http.ResponseWriter, r *http.Request) + setup func() + expectRefreshToken bool + expectIDToken bool + + assertAccessToken func(*testing.T, string) + }{ + { + // First we need to create a persistent session in order to check if the other things work + // as expected + d: "Creates a persisting session for the next test cases", + authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=login+consent&max_age=1", + cj: persistentCJ, + lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + rr, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge")) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + assert.False(t, rr.Skip) + assert.Empty(t, rr.Subject) + assert.EqualValues(t, client.GetID(), rr.Client.ClientId) + assert.EqualValues(t, client.GrantTypes, rr.Client.GrantTypes) + assert.EqualValues(t, client.LogoURI, rr.Client.LogoUri) + assert.EqualValues(t, client.RedirectURIs, rr.Client.RedirectUris) + assert.EqualValues(t, r.URL.Query().Get("login_challenge"), rr.Challenge) + assert.EqualValues(t, []string{"hydra", "offline", "openid"}, rr.RequestedScope) + assert.EqualValues(t, oauthConfig.AuthCodeURL("some-hardcoded-state")+"&prompt=login+consent&max_age=1", rr.RequestUrl) + + v, res, err := apiClient.AcceptLoginRequest(r.URL.Query().Get("login_challenge"), swagger.AcceptLoginRequest{ + Subject: "user-a", Remember: true, RememberFor: 0, + }) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + require.NotEmpty(t, v.RedirectTo) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + } + }, + cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + rr, res, err := apiClient.GetConsentRequest(r.URL.Query().Get("consent_challenge")) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + assert.False(t, rr.Skip) + assert.EqualValues(t, "user-a", rr.Subject) + assert.EqualValues(t, client.GetID(), rr.Client.ClientId) + assert.EqualValues(t, client.GrantTypes, rr.Client.GrantTypes) + assert.EqualValues(t, client.LogoURI, rr.Client.LogoUri) + assert.EqualValues(t, client.RedirectURIs, rr.Client.RedirectUris) + assert.EqualValues(t, []string{"hydra", "offline", "openid"}, rr.RequestedScope) + assert.EqualValues(t, r.URL.Query().Get("consent_challenge"), rr.Challenge) + assert.EqualValues(t, oauthConfig.AuthCodeURL("some-hardcoded-state")+"&prompt=login+consent&max_age=1", rr.RequestUrl) + + v, res, err := apiClient.AcceptConsentRequest(r.URL.Query().Get("consent_challenge"), swagger.AcceptConsentRequest{ + GrantScope: []string{"hydra", "offline", "openid"}, Remember: true, RememberFor: 0, + Session: swagger.ConsentRequestSession{ + AccessToken: map[string]interface{}{"foo": "bar"}, + IdToken: map[string]interface{}{"bar": "baz"}, + }, + }) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + require.NotEmpty(t, v.RedirectTo) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + } + }, + cb: func(t *testing.T) httprouter.Handle { + return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { + code = r.URL.Query().Get("code") + require.NotEmpty(t, code) + w.WriteHeader(http.StatusOK) + } + }, + expectOAuthAuthError: false, + expectOAuthTokenError: false, + expectIDToken: true, + expectRefreshToken: true, + assertAccessToken: func(t *testing.T, token string) { + if strat.d != "jwt" { + return + } + + body, err := djwt.DecodeSegment(strings.Split(token, ".")[1]) + require.NoError(t, err) + + data := map[string]interface{}{} + require.NoError(t, json.Unmarshal(body, &data)) + + assert.EqualValues(t, "e2e-app-client"+km+strat.d, data["client_id"]) + assert.EqualValues(t, "user-a", data["sub"]) + assert.NotEmpty(t, data["iss"]) + assert.NotEmpty(t, data["jti"]) + assert.NotEmpty(t, data["exp"]) + assert.NotEmpty(t, data["iat"]) + assert.NotEmpty(t, data["nbf"]) + assert.EqualValues(t, data["nbf"], data["iat"]) + assert.EqualValues(t, []interface{}{"hydra", "offline", "openid"}, data["scp"]) + assert.EqualValues(t, "bar", data["foo"]) + }, + }, + { + d: "checks if authenticatedAt/requestedAt is properly forwarded across the lifecycle by checking if prompt=none works", + setup: func() { + // In order to check if authenticatedAt/requestedAt works, we'll sleep first in order to ensure that authenticatedAt is in the past + // if handled correctly. + time.Sleep(time.Second * 2) + }, + authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=none&max_age=60", + cj: persistentCJ, + lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + rr, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge")) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + assert.True(t, rr.Skip) + assert.EqualValues(t, "user-a", rr.Subject) + + v, res, err := apiClient.AcceptLoginRequest(r.URL.Query().Get("login_challenge"), swagger.AcceptLoginRequest{Subject: "user-a"}) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + require.NotEmpty(t, v.RedirectTo) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + } + }, + cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + rr, res, err := apiClient.GetConsentRequest(r.URL.Query().Get("consent_challenge")) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + assert.True(t, rr.Skip) + assert.EqualValues(t, "user-a", rr.Subject) + + v, res, err := apiClient.AcceptConsentRequest(r.URL.Query().Get("consent_challenge"), swagger.AcceptConsentRequest{ + GrantScope: []string{"hydra", "offline"}, + Session: swagger.ConsentRequestSession{ + AccessToken: map[string]interface{}{"foo": "bar"}, + IdToken: map[string]interface{}{"bar": "baz"}, + }, + }) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + require.NotEmpty(t, v.RedirectTo) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + } + }, + cb: func(t *testing.T) httprouter.Handle { + return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { + code = r.URL.Query().Get("code") + require.NotEmpty(t, code) + w.WriteHeader(http.StatusOK) + } + }, + expectOAuthAuthError: false, + expectOAuthTokenError: false, + expectIDToken: false, + expectRefreshToken: true, + assertAccessToken: func(t *testing.T, token string) { + if strat.d != "jwt" { + return + } + + body, err := djwt.DecodeSegment(strings.Split(token, ".")[1]) + require.NoError(t, err) + + data := map[string]interface{}{} + require.NoError(t, json.Unmarshal(body, &data)) + + assert.EqualValues(t, "e2e-app-client"+km+strat.d, data["client_id"]) + assert.EqualValues(t, "user-a", data["sub"]) + assert.NotEmpty(t, data["iss"]) + assert.NotEmpty(t, data["jti"]) + assert.NotEmpty(t, data["exp"]) + assert.NotEmpty(t, data["iat"]) + assert.NotEmpty(t, data["nbf"]) + assert.EqualValues(t, data["nbf"], data["iat"]) + assert.EqualValues(t, []interface{}{"hydra", "offline"}, data["scp"]) + assert.EqualValues(t, "bar", data["foo"]) + }, + }, + { + d: "checks if prompt=none fails when no session is set", + authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=none", + cj: newCookieJar(), + expectOAuthAuthError: true, + expectOAuthTokenError: false, + }, + { + d: "checks if authenticatedAt/requestedAt is properly forwarded across the lifecycle by checking if prompt=none fails when maxAge is reached", + authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=none&max_age=1", + cj: persistentCJ, + expectOAuthAuthError: true, + expectOAuthTokenError: false, + }, + { + d: "checks if authenticatedAt/requestedAt is properly forwarded across the lifecycle by checking if prompt=login works", + authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=login", + cj: persistentCJ, + lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + rr, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge")) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + assert.False(t, rr.Skip) + assert.EqualValues(t, "", rr.Subject) + + v, res, err := apiClient.AcceptLoginRequest(r.URL.Query().Get("login_challenge"), swagger.AcceptLoginRequest{Subject: "user-a"}) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + require.NotEmpty(t, v.RedirectTo) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + } + }, + cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + rr, res, err := apiClient.GetConsentRequest(r.URL.Query().Get("consent_challenge")) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + assert.True(t, rr.Skip) + assert.EqualValues(t, "user-a", rr.Subject) + + v, res, err := apiClient.AcceptConsentRequest(r.URL.Query().Get("consent_challenge"), swagger.AcceptConsentRequest{ + GrantScope: []string{"hydra", "openid"}, + Session: swagger.ConsentRequestSession{ + AccessToken: map[string]interface{}{"foo": "bar"}, + IdToken: map[string]interface{}{"bar": "baz"}, + }, + }) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + require.NotEmpty(t, v.RedirectTo) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + } + }, + cb: func(t *testing.T) httprouter.Handle { + return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { + code = r.URL.Query().Get("code") + require.NotEmpty(t, code) + w.WriteHeader(http.StatusOK) + } + }, + expectOAuthAuthError: false, + expectOAuthTokenError: false, + expectIDToken: true, + expectRefreshToken: false, + }, + { + d: "requires re-authentication when id_token_hint is set to a user (\"foouser\") but the session is \"user-a\" and it also fails because the user id from the log in endpoint is not foouser", + authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&id_token_hint=" + fooUserIDToken, + cj: persistentCJ, + lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + rr, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge")) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + assert.False(t, rr.Skip) + assert.EqualValues(t, "", rr.Subject) + assert.Empty(t, rr.Client.ClientSecret) + + v, res, err := apiClient.AcceptLoginRequest(r.URL.Query().Get("login_challenge"), swagger.AcceptLoginRequest{Subject: "user-a"}) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + require.NotEmpty(t, v.RedirectTo) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + } + }, + cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + rr, res, err := apiClient.GetConsentRequest(r.URL.Query().Get("consent_challenge")) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + assert.True(t, rr.Skip) + assert.EqualValues(t, "user-a", rr.Subject) + assert.Empty(t, rr.Client.ClientSecret) + + v, res, err := apiClient.AcceptConsentRequest(r.URL.Query().Get("consent_challenge"), swagger.AcceptConsentRequest{ + GrantScope: []string{"hydra", "openid"}, + Session: swagger.ConsentRequestSession{ + AccessToken: map[string]interface{}{"foo": "bar"}, + IdToken: map[string]interface{}{"bar": "baz"}, + }, + }) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + require.NotEmpty(t, v.RedirectTo) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + } + }, + cb: func(t *testing.T) httprouter.Handle { + return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { + code = r.URL.Query().Get("code") + require.Empty(t, code) + require.EqualValues(t, "login_required", r.URL.Query().Get("error"), r.URL.Query().Get("error_debug")) + w.WriteHeader(http.StatusOK) + } + }, + expectOAuthAuthError: true, + expectOAuthTokenError: false, + expectIDToken: true, + expectRefreshToken: false, + assertAccessToken: func(t *testing.T, token string) { + if strat.d != "jwt" { + return + } + + body, err := djwt.DecodeSegment(strings.Split(token, ".")[1]) + require.NoError(t, err) + + data := map[string]interface{}{} + require.NoError(t, json.Unmarshal(body, &data)) + + assert.EqualValues(t, "e2e-app-client"+km+strat.d, data["client_id"]) + assert.EqualValues(t, "user-a", data["sub"]) + assert.NotEmpty(t, data["iss"]) + assert.NotEmpty(t, data["jti"]) + assert.NotEmpty(t, data["exp"]) + assert.NotEmpty(t, data["iat"]) + assert.NotEmpty(t, data["nbf"]) + assert.EqualValues(t, data["nbf"], data["iat"]) + assert.EqualValues(t, []interface{}{"hydra", "openid"}, data["scp"]) + assert.EqualValues(t, "bar", data["foo"]) + }, + }, + { + d: "should not cause issues if max_age is very low and consent takes a long time", + authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&max_age=3", + //cj: persistentCJ, + lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + _, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge")) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + + time.Sleep(time.Second * 5) + + v, res, err := apiClient.AcceptLoginRequest(r.URL.Query().Get("login_challenge"), swagger.AcceptLoginRequest{Subject: "user-a"}) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + require.NotEmpty(t, v.RedirectTo) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + } + }, + cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + _, res, err := apiClient.GetConsentRequest(r.URL.Query().Get("consent_challenge")) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + + time.Sleep(time.Second * 5) + + v, res, err := apiClient.AcceptConsentRequest(r.URL.Query().Get("consent_challenge"), swagger.AcceptConsentRequest{ + GrantScope: []string{"hydra", "openid"}, + Session: swagger.ConsentRequestSession{ + AccessToken: map[string]interface{}{"foo": "bar"}, + IdToken: map[string]interface{}{"bar": "baz"}, + }, + }) + require.NoError(t, err) + require.EqualValues(t, http.StatusOK, res.StatusCode) + require.NotEmpty(t, v.RedirectTo) + http.Redirect(w, r, v.RedirectTo, http.StatusFound) + } + }, + cb: func(t *testing.T) httprouter.Handle { + return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { + code = r.URL.Query().Get("code") + require.NotEmpty(t, code) + w.WriteHeader(http.StatusOK) + } + }, + expectOAuthAuthError: false, + expectOAuthTokenError: false, + expectIDToken: true, + expectRefreshToken: false, + }, + } { + t.Run(fmt.Sprintf("case=%d/description=%s", k, tc.d), func(t *testing.T) { + m.Lock() + defer m.Unlock() + + code = "" + + if tc.setup != nil { + tc.setup() + } + + if tc.lph != nil { + lph = tc.lph(t) + } else { + lph = noopHandlerDefaultStrategy(t) + } + + if tc.cph != nil { + cph = tc.cph(t) + } else { + cph = noopHandlerDefaultStrategy(t) + } + + if tc.cb == nil { + tc.cb = noopHandler + } + + cb := tc.cb(t) + callbackHandler = &cb + + req, err := http.NewRequest("GET", tc.authURL, nil) + require.NoError(t, err) + + if tc.cj == nil { + tc.cj = newCookieJar() + } + + resp, err := (&http.Client{Jar: tc.cj}).Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + if tc.expectOAuthAuthError { + require.Empty(t, code) + return + } - // we create a new fositeStore here because the old one - - for km, fs := range fositeStores { - t.Run("manager="+km, func(t *testing.T) { - var cm consent.Manager - switch km { - case "memory": - cm = consent.NewMemoryManager(fs) - fs.(*FositeMemoryStore).Manager = hc.NewMemoryManager(hasher) - case "mysql": - fallthrough - case "postgres": - scm := consent.NewSQLManager(databases[km], fs.(*FositeSQLStore).Manager, fs) - _, err := scm.CreateSchemas() - require.NoError(t, err) - - _, err = (fs.(*FositeSQLStore)).CreateSchemas() - require.NoError(t, err) - - cm = scm + require.NotEmpty(t, code) + + token, err := oauthConfig.Exchange(oauth2.NoContext, code) + + if tc.expectOAuthTokenError { + require.Error(t, err) + return + } + + require.NoError(t, err, code) + assert.NotEmpty(t, token.AccessToken) + if tc.expectRefreshToken { + require.NotEmpty(t, token.RefreshToken) + } else { + require.Empty(t, token.RefreshToken) + } + if tc.expectIDToken { + require.NotEmpty(t, token.Extra("id_token")) + } else { + require.Empty(t, token.Extra("id_token")) + } + if tc.assertAccessToken != nil { + tc.assertAccessToken(t, token.AccessToken) + } + }) + } + }) } + }) + } +} +// TestAuthCodeWithMockStrategy runs the authorize_code flow against various ConsentStrategy scenarios. +// For that purpose, the consent strategy is mocked so all scenarios can be applied properly. This test suite checks: +// +// - [x] should pass request if strategy passes +// - [x] should fail because prompt=none and max_age > auth_time +// - [x] should pass because prompt=none and max_age < auth_time +// - [x] should fail because prompt=none but auth_time suggests recent authentication +// - [x] should fail because consent strategy fails +// - [x] should pass with prompt=login when authentication time is recent +// - [x] should fail with prompt=login when authentication time is in the past +func TestAuthCodeWithMockStrategy(t *testing.T) { + for _, strat := range []struct { + d string + s foauth2.CoreStrategy + }{ + { + d: "opaque", + s: oauth2OpqaueStrategy, + }, + { + d: "jwt", + s: oauth2JWTStrategy, + }, + } { + t.Run("strategy="+strat.d, func(t *testing.T) { + consentStrategy := &consentMock{} router := httprouter.New() ts := httptest.NewServer(router) - cookieStore := sessions.NewCookieStore([]byte("foo-secret")) + store := NewFositeMemoryStore(hc.NewMemoryManager(hasher), time.Second) - consentStrategy := consent.NewStrategy( - lp.URL, cp.URL, ts.URL, "/oauth2/auth", cm, - cookieStore, - fosite.ExactScopeStrategy, false, time.Hour, jwts, - openid.NewOpenIDConnectRequestValidator(nil, jwts), - ) + l := logrus.New() + l.Level = logrus.DebugLevel jm := &jwk.MemoryManager{Keys: map[string]*jose.JSONWebKeySet{}} keys, err := (&jwk.RS256Generator{}).Generate("", "sig") @@ -153,7 +716,10 @@ func TestAuthCodeWithDefaultStrategy(t *testing.T) { handler := &Handler{ OAuth2: compose.Compose( - fc, fs, oauth2Strategy, hasher, + fc, + store, + strat.s, + nil, compose.OAuth2AuthorizeExplicitFactory, compose.OAuth2AuthorizeImplicitFactory, compose.OAuth2ClientCredentialsGrantFactory, @@ -164,381 +730,189 @@ func TestAuthCodeWithDefaultStrategy(t *testing.T) { compose.OAuth2TokenRevocationFactory, compose.OAuth2TokenIntrospectionFactory, ), - Consent: consentStrategy, - CookieStore: cookieStore, - H: herodot.NewJSONWriter(l), - ScopeStrategy: fosite.ExactScopeStrategy, - IDTokenLifespan: time.Minute, IssuerURL: ts.URL, ForcedHTTP: true, L: l, + Consent: consentStrategy, + CookieStore: sessions.NewCookieStore([]byte("foo-secret")), + ForcedHTTP: true, + L: l, + H: herodot.NewJSONWriter(l), + ScopeStrategy: fosite.HierarchicScopeStrategy, + IDTokenLifespan: time.Minute, + IssuerURL: ts.URL, OpenIDJWTStrategy: jwtStrategy, } handler.SetRoutes(router) - apiHandler := consent.NewHandler(herodot.NewJSONWriter(l), cm) - apiRouter := httprouter.New() - apiHandler.SetRoutes(apiRouter) - api := httptest.NewServer(apiRouter) + var callbackHandler *httprouter.Handle + router.GET("/callback", func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { + (*callbackHandler)(w, r, ps) + }) + m := sync.Mutex{} - client := hc.Client{ - ClientID: "e2e-app-client" + km, Secret: "secret", RedirectURIs: []string{ts.URL + "/callback"}, + store.CreateClient(&hc.Client{ + ClientID: "app-client", + Secret: "secret", + RedirectURIs: []string{ts.URL + "/callback"}, ResponseTypes: []string{"id_token", "code", "token"}, GrantTypes: []string{"implicit", "refresh_token", "authorization_code", "password", "client_credentials"}, - Scope: "hydra offline openid", - } + Scope: "hydra.* offline openid", + }) + oauthConfig := &oauth2.Config{ - ClientID: client.GetID(), ClientSecret: client.Secret, - Endpoint: oauth2.Endpoint{AuthURL: ts.URL + "/oauth2/auth", TokenURL: ts.URL + "/oauth2/token"}, - RedirectURL: client.RedirectURIs[0], Scopes: []string{"hydra", "offline", "openid"}, + ClientID: "app-client", + ClientSecret: "secret", + Endpoint: oauth2.Endpoint{ + AuthURL: ts.URL + "/oauth2/auth", + TokenURL: ts.URL + "/oauth2/token", + }, + RedirectURL: ts.URL + "/callback", + Scopes: []string{"hydra.*", "offline", "openid"}, } - require.NoError(t, fs.(clientCreator).CreateClient(&client)) - apiClient := swagger.NewOAuth2ApiWithBasePath(api.URL) - - var callbackHandler *httprouter.Handle - router.GET("/callback", func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - (*callbackHandler)(w, r, ps) - }) - - persistentCJ := newCookieJar() var code string for k, tc := range []struct { - authURL string - cj http.CookieJar - d string - cb func(t *testing.T) httprouter.Handle - expectOAuthAuthError bool - expectOAuthTokenError bool - lph, cph func(t *testing.T) func(w http.ResponseWriter, r *http.Request) - setup func() - expectRefreshToken bool - expectIDToken bool + cj http.CookieJar + d string + cb func(t *testing.T) httprouter.Handle + authURL string + shouldPassConsentStrategy bool + expectOAuthAuthError bool + expectOAuthTokenError bool + authTime time.Time + requestTime time.Time + assertAccessToken func(*testing.T, string) }{ { - // First we need to create a persistent session in order to check if the other things work - // as expected - d: "Creates a persisting session for the next test cases", - authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=login+consent&max_age=1", - cj: persistentCJ, - lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { - rr, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge")) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - assert.False(t, rr.Skip) - assert.Empty(t, rr.Subject) - assert.EqualValues(t, client.GetID(), rr.Client.ClientId) - assert.EqualValues(t, client.GrantTypes, rr.Client.GrantTypes) - assert.EqualValues(t, client.LogoURI, rr.Client.LogoUri) - assert.EqualValues(t, client.RedirectURIs, rr.Client.RedirectUris) - assert.EqualValues(t, r.URL.Query().Get("login_challenge"), rr.Challenge) - assert.EqualValues(t, []string{"hydra", "offline", "openid"}, rr.RequestedScope) - assert.EqualValues(t, oauthConfig.AuthCodeURL("some-hardcoded-state")+"&prompt=login+consent&max_age=1", rr.RequestUrl) - - v, res, err := apiClient.AcceptLoginRequest(r.URL.Query().Get("login_challenge"), swagger.AcceptLoginRequest{ - Subject: "user-a", Remember: true, RememberFor: 0, - }) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - require.NotEmpty(t, v.RedirectTo) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) - } - }, - cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { - rr, res, err := apiClient.GetConsentRequest(r.URL.Query().Get("consent_challenge")) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - assert.False(t, rr.Skip) - assert.EqualValues(t, "user-a", rr.Subject) - assert.EqualValues(t, client.GetID(), rr.Client.ClientId) - assert.EqualValues(t, client.GrantTypes, rr.Client.GrantTypes) - assert.EqualValues(t, client.LogoURI, rr.Client.LogoUri) - assert.EqualValues(t, client.RedirectURIs, rr.Client.RedirectUris) - assert.EqualValues(t, []string{"hydra", "offline", "openid"}, rr.RequestedScope) - assert.EqualValues(t, r.URL.Query().Get("consent_challenge"), rr.Challenge) - assert.EqualValues(t, oauthConfig.AuthCodeURL("some-hardcoded-state")+"&prompt=login+consent&max_age=1", rr.RequestUrl) - - v, res, err := apiClient.AcceptConsentRequest(r.URL.Query().Get("consent_challenge"), swagger.AcceptConsentRequest{ - GrantScope: []string{"hydra", "offline", "openid"}, Remember: true, RememberFor: 0, - Session: swagger.ConsentRequestSession{ - AccessToken: map[string]interface{}{"foo": "bar"}, - IdToken: map[string]interface{}{"bar": "baz"}, - }, - }) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - require.NotEmpty(t, v.RedirectTo) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) - } - }, + d: "should pass request if strategy passes", + authURL: oauthConfig.AuthCodeURL("some-foo-state"), + shouldPassConsentStrategy: true, cb: func(t *testing.T) httprouter.Handle { return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { code = r.URL.Query().Get("code") require.NotEmpty(t, code) - w.WriteHeader(http.StatusOK) + w.Write([]byte(r.URL.Query().Get("code"))) } }, - expectOAuthAuthError: false, - expectOAuthTokenError: false, - expectIDToken: true, - expectRefreshToken: true, - }, - { - d: "checks if authenticatedAt/requestedAt is properly forwarded across the lifecycle by checking if prompt=none works", - setup: func() { - // In order to check if authenticatedAt/requestedAt works, we'll sleep first in order to ensure that authenticatedAt is in the past - // if handled correctly. - time.Sleep(time.Second * 2) - }, - authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=none&max_age=60", - cj: persistentCJ, - lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { - rr, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge")) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - assert.True(t, rr.Skip) - assert.EqualValues(t, "user-a", rr.Subject) - - v, res, err := apiClient.AcceptLoginRequest(r.URL.Query().Get("login_challenge"), swagger.AcceptLoginRequest{Subject: "user-a"}) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - require.NotEmpty(t, v.RedirectTo) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) - } - }, - cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { - rr, res, err := apiClient.GetConsentRequest(r.URL.Query().Get("consent_challenge")) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - assert.True(t, rr.Skip) - assert.EqualValues(t, "user-a", rr.Subject) - - v, res, err := apiClient.AcceptConsentRequest(r.URL.Query().Get("consent_challenge"), swagger.AcceptConsentRequest{ - GrantScope: []string{"hydra", "offline"}, - Session: swagger.ConsentRequestSession{ - AccessToken: map[string]interface{}{"foo": "bar"}, - IdToken: map[string]interface{}{"bar": "baz"}, - }, - }) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - require.NotEmpty(t, v.RedirectTo) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) + assertAccessToken: func(t *testing.T, token string) { + if strat.d != "jwt" { + return } + + body, err := djwt.DecodeSegment(strings.Split(token, ".")[1]) + require.NoError(t, err) + + data := map[string]interface{}{} + require.NoError(t, json.Unmarshal(body, &data)) + + assert.EqualValues(t, "app-client", data["client_id"]) + assert.EqualValues(t, "foo", data["sub"]) + assert.NotEmpty(t, data["iss"]) + assert.NotEmpty(t, data["jti"]) + assert.NotEmpty(t, data["exp"]) + assert.NotEmpty(t, data["iat"]) + assert.NotEmpty(t, data["nbf"]) + assert.EqualValues(t, data["nbf"], data["iat"]) + assert.EqualValues(t, []interface{}{"offline", "openid", "hydra.*"}, data["scp"]) }, + }, + { + d: "should fail because prompt=none and max_age > auth_time", + authURL: oauthConfig.AuthCodeURL("some-foo-state") + "&prompt=none&max_age=1", + authTime: time.Now().UTC().Add(-time.Minute), + requestTime: time.Now().UTC(), + shouldPassConsentStrategy: true, cb: func(t *testing.T) httprouter.Handle { return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { code = r.URL.Query().Get("code") - require.NotEmpty(t, code) - w.WriteHeader(http.StatusOK) + err := r.URL.Query().Get("error") + require.Empty(t, code) + require.EqualValues(t, fosite.ErrLoginRequired.Error(), err) } }, - expectOAuthAuthError: false, - expectOAuthTokenError: false, - expectIDToken: false, - expectRefreshToken: true, - }, - { - d: "checks if prompt=none fails when no session is set", - authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=none", - cj: newCookieJar(), - expectOAuthAuthError: true, - expectOAuthTokenError: false, + expectOAuthAuthError: true, }, { - d: "checks if authenticatedAt/requestedAt is properly forwarded across the lifecycle by checking if prompt=none fails when maxAge is reached", - authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=none&max_age=1", - cj: persistentCJ, - expectOAuthAuthError: true, - expectOAuthTokenError: false, - }, - { - d: "checks if authenticatedAt/requestedAt is properly forwarded across the lifecycle by checking if prompt=login works", - authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=login", - cj: persistentCJ, - lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { - rr, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge")) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - assert.False(t, rr.Skip) - assert.EqualValues(t, "", rr.Subject) - - v, res, err := apiClient.AcceptLoginRequest(r.URL.Query().Get("login_challenge"), swagger.AcceptLoginRequest{Subject: "user-a"}) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - require.NotEmpty(t, v.RedirectTo) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) - } - }, - cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { - rr, res, err := apiClient.GetConsentRequest(r.URL.Query().Get("consent_challenge")) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - assert.True(t, rr.Skip) - assert.EqualValues(t, "user-a", rr.Subject) - - v, res, err := apiClient.AcceptConsentRequest(r.URL.Query().Get("consent_challenge"), swagger.AcceptConsentRequest{ - GrantScope: []string{"hydra", "openid"}, - Session: swagger.ConsentRequestSession{ - AccessToken: map[string]interface{}{"foo": "bar"}, - IdToken: map[string]interface{}{"bar": "baz"}, - }, - }) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - require.NotEmpty(t, v.RedirectTo) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) - } - }, + d: "should pass because prompt=none and max_age < auth_time", + authURL: oauthConfig.AuthCodeURL("some-foo-state") + "&prompt=none&max_age=3600", + authTime: time.Now().UTC().Add(-time.Minute), + requestTime: time.Now().UTC(), + shouldPassConsentStrategy: true, cb: func(t *testing.T) httprouter.Handle { return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { code = r.URL.Query().Get("code") require.NotEmpty(t, code) - w.WriteHeader(http.StatusOK) + w.Write([]byte(r.URL.Query().Get("code"))) } }, - expectOAuthAuthError: false, - expectOAuthTokenError: false, - expectIDToken: true, - expectRefreshToken: false, }, { - d: "requires re-authentication when id_token_hint is set to a user (\"foouser\") but the session is \"user-a\" and it also fails because the user id from the log in endpoint is not foouser", - authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&id_token_hint=" + fooUserIDToken, - cj: persistentCJ, - lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { - rr, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge")) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - assert.False(t, rr.Skip) - assert.EqualValues(t, "", rr.Subject) - assert.Empty(t, rr.Client.ClientSecret) - - v, res, err := apiClient.AcceptLoginRequest(r.URL.Query().Get("login_challenge"), swagger.AcceptLoginRequest{Subject: "user-a"}) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - require.NotEmpty(t, v.RedirectTo) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) - } - }, - cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { - rr, res, err := apiClient.GetConsentRequest(r.URL.Query().Get("consent_challenge")) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - assert.True(t, rr.Skip) - assert.EqualValues(t, "user-a", rr.Subject) - assert.Empty(t, rr.Client.ClientSecret) - - v, res, err := apiClient.AcceptConsentRequest(r.URL.Query().Get("consent_challenge"), swagger.AcceptConsentRequest{ - GrantScope: []string{"hydra", "openid"}, - Session: swagger.ConsentRequestSession{ - AccessToken: map[string]interface{}{"foo": "bar"}, - IdToken: map[string]interface{}{"bar": "baz"}, - }, - }) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - require.NotEmpty(t, v.RedirectTo) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) - } - }, + d: "should fail because prompt=none but auth_time suggests recent authentication", + authURL: oauthConfig.AuthCodeURL("some-foo-state") + "&prompt=none", + authTime: time.Now().UTC().Add(-time.Minute), + requestTime: time.Now().UTC().Add(-time.Hour), + shouldPassConsentStrategy: true, cb: func(t *testing.T) httprouter.Handle { return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { code = r.URL.Query().Get("code") + err := r.URL.Query().Get("error") require.Empty(t, code) - require.EqualValues(t, "login_required", r.URL.Query().Get("error"), r.URL.Query().Get("error_debug")) - w.WriteHeader(http.StatusOK) + require.EqualValues(t, fosite.ErrLoginRequired.Error(), err) } }, - expectOAuthAuthError: true, - expectOAuthTokenError: false, - expectIDToken: true, - expectRefreshToken: false, + expectOAuthAuthError: true, }, { - d: "should not cause issues if max_age is very low and consent takes a long time", - authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&max_age=3", - //cj: persistentCJ, - lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { - _, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge")) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - - time.Sleep(time.Second * 5) - - v, res, err := apiClient.AcceptLoginRequest(r.URL.Query().Get("login_challenge"), swagger.AcceptLoginRequest{Subject: "user-a"}) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - require.NotEmpty(t, v.RedirectTo) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) + d: "should fail because consent strategy fails", + authURL: oauthConfig.AuthCodeURL("some-foo-state"), + expectOAuthAuthError: true, + shouldPassConsentStrategy: false, + cb: func(t *testing.T) httprouter.Handle { + return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { + require.Empty(t, r.URL.Query().Get("code")) + assert.Equal(t, fosite.ErrRequestForbidden.Error(), r.URL.Query().Get("error")) } }, - cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) { - return func(w http.ResponseWriter, r *http.Request) { - _, res, err := apiClient.GetConsentRequest(r.URL.Query().Get("consent_challenge")) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - - time.Sleep(time.Second * 5) - - v, res, err := apiClient.AcceptConsentRequest(r.URL.Query().Get("consent_challenge"), swagger.AcceptConsentRequest{ - GrantScope: []string{"hydra", "openid"}, - Session: swagger.ConsentRequestSession{ - AccessToken: map[string]interface{}{"foo": "bar"}, - IdToken: map[string]interface{}{"bar": "baz"}, - }, - }) - require.NoError(t, err) - require.EqualValues(t, http.StatusOK, res.StatusCode) - require.NotEmpty(t, v.RedirectTo) - http.Redirect(w, r, v.RedirectTo, http.StatusFound) + }, + { + d: "should pass with prompt=login when authentication time is recent", + authURL: oauthConfig.AuthCodeURL("some-foo-state") + "&prompt=login", + authTime: time.Now().UTC().Add(-time.Second), + requestTime: time.Now().UTC().Add(-time.Minute), + shouldPassConsentStrategy: true, + cb: func(t *testing.T) httprouter.Handle { + return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { + code = r.URL.Query().Get("code") + require.NotEmpty(t, code) + w.Write([]byte(r.URL.Query().Get("code"))) } }, + }, + { + d: "should fail with prompt=login when authentication time is in the past", + authURL: oauthConfig.AuthCodeURL("some-foo-state") + "&prompt=login", + authTime: time.Now().UTC().Add(-time.Minute), + requestTime: time.Now().UTC(), + expectOAuthAuthError: true, + shouldPassConsentStrategy: true, cb: func(t *testing.T) httprouter.Handle { return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { code = r.URL.Query().Get("code") - require.NotEmpty(t, code) - w.WriteHeader(http.StatusOK) + require.Empty(t, code) + assert.Equal(t, fosite.ErrLoginRequired.Error(), r.URL.Query().Get("error")) } }, - expectOAuthAuthError: false, - expectOAuthTokenError: false, - expectIDToken: true, - expectRefreshToken: false, }, } { t.Run(fmt.Sprintf("case=%d/description=%s", k, tc.d), func(t *testing.T) { m.Lock() defer m.Unlock() - - code = "" - - if tc.setup != nil { - tc.setup() - } - - if tc.lph != nil { - lph = tc.lph(t) - } else { - lph = noopHandlerDefaultStrategy(t) - } - - if tc.cph != nil { - cph = tc.cph(t) - } else { - cph = noopHandlerDefaultStrategy(t) - } - if tc.cb == nil { tc.cb = noopHandler } + consentStrategy.deny = !tc.shouldPassConsentStrategy + consentStrategy.authTime = tc.authTime + consentStrategy.requestTime = tc.requestTime + cb := tc.cb(t) callbackHandler = &cb @@ -567,308 +941,127 @@ func TestAuthCodeWithDefaultStrategy(t *testing.T) { return } - require.NoError(t, err, code) - assert.NotEmpty(t, token.AccessToken) - if tc.expectRefreshToken { - require.NotEmpty(t, token.RefreshToken) - } else { - require.Empty(t, token.RefreshToken) - } - if tc.expectIDToken { - require.NotEmpty(t, token.Extra("id_token")) - } else { - require.Empty(t, token.Extra("id_token")) + if tc.assertAccessToken != nil { + tc.assertAccessToken(t, token.AccessToken) } - }) - } - }) - } -} - -// TestAuthCodeWithMockStrategy runs the authorize_code flow against various ConsentStrategy scenarios. -// For that purpose, the consent strategy is mocked so all scenarios can be applied properly. This test suite checks: -// -// - [x] should pass request if strategy passes -// - [x] should fail because prompt=none and max_age > auth_time -// - [x] should pass because prompt=none and max_age < auth_time -// - [x] should fail because prompt=none but auth_time suggests recent authentication -// - [x] should fail because consent strategy fails -// - [x] should pass with prompt=login when authentication time is recent -// - [x] should fail with prompt=login when authentication time is in the past -func TestAuthCodeWithMockStrategy(t *testing.T) { - consentStrategy := &consentMock{} - router := httprouter.New() - ts := httptest.NewServer(router) - store := NewFositeMemoryStore(hc.NewMemoryManager(hasher), time.Second) - l := logrus.New() - l.Level = logrus.DebugLevel - - jm := &jwk.MemoryManager{Keys: map[string]*jose.JSONWebKeySet{}} - keys, err := (&jwk.RS256Generator{}).Generate("", "sig") - require.NoError(t, err) - require.NoError(t, jm.AddKeySet(OpenIDConnectKeyName, keys)) - jwtStrategy, err := jwk.NewRS256JWTStrategy(jm, OpenIDConnectKeyName) - - handler := &Handler{ - OAuth2: compose.Compose( - fc, - store, - oauth2Strategy, - nil, - compose.OAuth2AuthorizeExplicitFactory, - compose.OAuth2AuthorizeImplicitFactory, - compose.OAuth2ClientCredentialsGrantFactory, - compose.OAuth2RefreshTokenGrantFactory, - compose.OpenIDConnectExplicitFactory, - compose.OpenIDConnectHybridFactory, - compose.OpenIDConnectImplicitFactory, - compose.OAuth2TokenRevocationFactory, - compose.OAuth2TokenIntrospectionFactory, - ), - Consent: consentStrategy, - CookieStore: sessions.NewCookieStore([]byte("foo-secret")), - ForcedHTTP: true, - L: l, - H: herodot.NewJSONWriter(l), - ScopeStrategy: fosite.HierarchicScopeStrategy, - IDTokenLifespan: time.Minute, - IssuerURL: ts.URL, - OpenIDJWTStrategy: jwtStrategy, - } - handler.SetRoutes(router) - - var callbackHandler *httprouter.Handle - router.GET("/callback", func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) { - (*callbackHandler)(w, r, ps) - }) - m := sync.Mutex{} - - store.CreateClient(&hc.Client{ - ClientID: "app-client", - Secret: "secret", - RedirectURIs: []string{ts.URL + "/callback"}, - ResponseTypes: []string{"id_token", "code", "token"}, - GrantTypes: []string{"implicit", "refresh_token", "authorization_code", "password", "client_credentials"}, - Scope: "hydra.* offline openid", - }) - - oauthConfig := &oauth2.Config{ - ClientID: "app-client", - ClientSecret: "secret", - Endpoint: oauth2.Endpoint{ - AuthURL: ts.URL + "/oauth2/auth", - TokenURL: ts.URL + "/oauth2/token", - }, - RedirectURL: ts.URL + "/callback", - Scopes: []string{"hydra.*", "offline", "openid"}, - } - - var code string - for k, tc := range []struct { - cj http.CookieJar - d string - cb func(t *testing.T) httprouter.Handle - authURL string - shouldPassConsentStrategy bool - expectOAuthAuthError bool - expectOAuthTokenError bool - authTime time.Time - requestTime time.Time - }{ - { - d: "should pass request if strategy passes", - authURL: oauthConfig.AuthCodeURL("some-foo-state"), - shouldPassConsentStrategy: true, - cb: func(t *testing.T) httprouter.Handle { - return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { - code = r.URL.Query().Get("code") - require.NotEmpty(t, code) - w.Write([]byte(r.URL.Query().Get("code"))) - } - }, - }, - { - d: "should fail because prompt=none and max_age > auth_time", - authURL: oauthConfig.AuthCodeURL("some-foo-state") + "&prompt=none&max_age=1", - authTime: time.Now().UTC().Add(-time.Minute), - requestTime: time.Now().UTC(), - shouldPassConsentStrategy: true, - cb: func(t *testing.T) httprouter.Handle { - return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { - code = r.URL.Query().Get("code") - err := r.URL.Query().Get("error") - require.Empty(t, code) - require.EqualValues(t, fosite.ErrLoginRequired.Error(), err) - } - }, - expectOAuthAuthError: true, - }, - { - d: "should pass because prompt=none and max_age < auth_time", - authURL: oauthConfig.AuthCodeURL("some-foo-state") + "&prompt=none&max_age=3600", - authTime: time.Now().UTC().Add(-time.Minute), - requestTime: time.Now().UTC(), - shouldPassConsentStrategy: true, - cb: func(t *testing.T) httprouter.Handle { - return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { - code = r.URL.Query().Get("code") - require.NotEmpty(t, code) - w.Write([]byte(r.URL.Query().Get("code"))) - } - }, - }, - { - d: "should fail because prompt=none but auth_time suggests recent authentication", - authURL: oauthConfig.AuthCodeURL("some-foo-state") + "&prompt=none", - authTime: time.Now().UTC().Add(-time.Minute), - requestTime: time.Now().UTC().Add(-time.Hour), - shouldPassConsentStrategy: true, - cb: func(t *testing.T) httprouter.Handle { - return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { - code = r.URL.Query().Get("code") - err := r.URL.Query().Get("error") - require.Empty(t, code) - require.EqualValues(t, fosite.ErrLoginRequired.Error(), err) - } - }, - expectOAuthAuthError: true, - }, - { - d: "should fail because consent strategy fails", - authURL: oauthConfig.AuthCodeURL("some-foo-state"), - expectOAuthAuthError: true, - shouldPassConsentStrategy: false, - cb: func(t *testing.T) httprouter.Handle { - return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { - require.Empty(t, r.URL.Query().Get("code")) - assert.Equal(t, fosite.ErrRequestForbidden.Error(), r.URL.Query().Get("error")) - } - }, - }, - { - d: "should pass with prompt=login when authentication time is recent", - authURL: oauthConfig.AuthCodeURL("some-foo-state") + "&prompt=login", - authTime: time.Now().UTC().Add(-time.Second), - requestTime: time.Now().UTC().Add(-time.Minute), - shouldPassConsentStrategy: true, - cb: func(t *testing.T) httprouter.Handle { - return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { - code = r.URL.Query().Get("code") - require.NotEmpty(t, code) - w.Write([]byte(r.URL.Query().Get("code"))) - } - }, - }, - { - d: "should fail with prompt=login when authentication time is in the past", - authURL: oauthConfig.AuthCodeURL("some-foo-state") + "&prompt=login", - authTime: time.Now().UTC().Add(-time.Minute), - requestTime: time.Now().UTC(), - expectOAuthAuthError: true, - shouldPassConsentStrategy: true, - cb: func(t *testing.T) httprouter.Handle { - return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { - code = r.URL.Query().Get("code") - require.Empty(t, code) - assert.Equal(t, fosite.ErrLoginRequired.Error(), r.URL.Query().Get("error")) - } - }, - }, - } { - t.Run(fmt.Sprintf("case=%d/description=%s", k, tc.d), func(t *testing.T) { - m.Lock() - defer m.Unlock() - if tc.cb == nil { - tc.cb = noopHandler - } - - consentStrategy.deny = !tc.shouldPassConsentStrategy - consentStrategy.authTime = tc.authTime - consentStrategy.requestTime = tc.requestTime + require.NoError(t, err, code) - cb := tc.cb(t) - callbackHandler = &cb + t.Run("case=userinfo", func(t *testing.T) { + var makeRequest = func(req *http.Request) *http.Response { + resp, err = http.DefaultClient.Do(req) + require.NoError(t, err) + return resp + } - req, err := http.NewRequest("GET", tc.authURL, nil) - require.NoError(t, err) + var testSuccess = func(response *http.Response) { + defer resp.Body.Close() - if tc.cj == nil { - tc.cj = newCookieJar() - } + require.Equal(t, http.StatusOK, resp.StatusCode) - resp, err := (&http.Client{Jar: tc.cj}).Do(req) - require.NoError(t, err) - defer resp.Body.Close() + var claims map[string]interface{} + require.NoError(t, json.NewDecoder(resp.Body).Decode(&claims)) + assert.Equal(t, "foo", claims["sub"]) + } - if tc.expectOAuthAuthError { - require.Empty(t, code) - return - } + req, err = http.NewRequest("GET", ts.URL+"/userinfo", nil) + req.Header.Add("Authorization", "bearer "+token.AccessToken) + testSuccess(makeRequest(req)) - require.NotEmpty(t, code) + req, err = http.NewRequest("POST", ts.URL+"/userinfo", nil) + req.Header.Add("Authorization", "bearer "+token.AccessToken) + testSuccess(makeRequest(req)) - token, err := oauthConfig.Exchange(oauth2.NoContext, code) + req, err = http.NewRequest("POST", ts.URL+"/userinfo", bytes.NewBuffer([]byte("access_token="+token.AccessToken))) + req.Header.Add("Content-Type", "application/x-www-form-urlencoded") + testSuccess(makeRequest(req)) - if tc.expectOAuthTokenError { - require.Error(t, err) - return - } + req, err = http.NewRequest("GET", ts.URL+"/userinfo", nil) + req.Header.Add("Authorization", "bearer asdfg") + resp := makeRequest(req) + require.Equal(t, http.StatusUnauthorized, resp.StatusCode) + }) + t.Logf("Got token: %s", token.AccessToken) - require.NoError(t, err, code) + time.Sleep(time.Second * 2) - t.Run("case=userinfo", func(t *testing.T) { - var makeRequest = func(req *http.Request) *http.Response { - resp, err = http.DefaultClient.Do(req) + res, err := testRefresh(t, token, ts.URL) require.NoError(t, err) - return resp - } - - var testSuccess = func(response *http.Response) { - defer resp.Body.Close() + assert.Equal(t, http.StatusOK, res.StatusCode) - require.Equal(t, http.StatusOK, resp.StatusCode) - - var claims map[string]interface{} - require.NoError(t, json.NewDecoder(resp.Body).Decode(&claims)) - assert.Equal(t, "foo", claims["sub"]) - } - - req, err = http.NewRequest("GET", ts.URL+"/userinfo", nil) - req.Header.Add("Authorization", "bearer "+token.AccessToken) - testSuccess(makeRequest(req)) + body, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) - req, err = http.NewRequest("POST", ts.URL+"/userinfo", nil) - req.Header.Add("Authorization", "bearer "+token.AccessToken) - testSuccess(makeRequest(req)) + var refreshedToken oauth2.Token + require.NoError(t, json.Unmarshal(body, &refreshedToken)) - req, err = http.NewRequest("POST", ts.URL+"/userinfo", bytes.NewBuffer([]byte("access_token="+token.AccessToken))) - req.Header.Add("Content-Type", "application/x-www-form-urlencoded") - testSuccess(makeRequest(req)) + t.Logf("Got refresh token: %s", refreshedToken.AccessToken) - req, err = http.NewRequest("GET", ts.URL+"/userinfo", nil) - req.Header.Add("Authorization", "bearer asdfg") - resp := makeRequest(req) - require.Equal(t, http.StatusUnauthorized, resp.StatusCode) - }) + if tc.assertAccessToken != nil { + tc.assertAccessToken(t, refreshedToken.AccessToken) + } - res, err := testRefresh(t, token, ts.URL) - require.NoError(t, err) - assert.Equal(t, http.StatusOK, res.StatusCode) + t.Run("the tokens should be different", func(t *testing.T) { + if strat.d != "jwt" { + t.Skip() + } - t.Run("duplicate code exchange fails", func(t *testing.T) { - token, err := oauthConfig.Exchange(oauth2.NoContext, code) - require.Error(t, err) - require.Nil(t, token) - }) + body, err := djwt.DecodeSegment(strings.Split(token.AccessToken, ".")[1]) + require.NoError(t, err) + + origPayload := map[string]interface{}{} + require.NoError(t, json.Unmarshal(body, &origPayload)) + + body, err = djwt.DecodeSegment(strings.Split(refreshedToken.AccessToken, ".")[1]) + require.NoError(t, err) + + refreshedPayload := map[string]interface{}{} + require.NoError(t, json.Unmarshal(body, &refreshedPayload)) + + assert.NotEqual(t, refreshedPayload["exp"], origPayload["exp"]) + assert.NotEqual(t, refreshedPayload["iat"], origPayload["iat"]) + assert.NotEqual(t, refreshedPayload["nbf"], origPayload["nbf"]) + assert.NotEqual(t, refreshedPayload["jti"], origPayload["jti"]) + assert.Equal(t, refreshedPayload["client_id"], origPayload["client_id"]) + }) + + require.NotEqual(t, token.AccessToken, refreshedToken.AccessToken) + + t.Run("old token should no longer be usable", func(t *testing.T) { + req, err := http.NewRequest("GET", ts.URL+"/userinfo", nil) + require.NoError(t, err) + req.Header.Add("Authorization", "bearer "+token.AccessToken) + res, err := http.DefaultClient.Do(req) + require.NoError(t, err) + assert.EqualValues(t, http.StatusUnauthorized, res.StatusCode) + }) + + t.Run("refreshing old token should no longer work", func(t *testing.T) { + res, err := testRefresh(t, token, ts.URL) + require.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, res.StatusCode) + }) + + t.Run("refreshing new refresh token should work", func(t *testing.T) { + res, err := testRefresh(t, &refreshedToken, ts.URL) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + }) + + t.Run("duplicate code exchange fails", func(t *testing.T) { + token, err := oauthConfig.Exchange(oauth2.NoContext, code) + require.Error(t, err) + require.Nil(t, token) + }) - code = "" + code = "" + }) + } }) } } func testRefresh(t *testing.T, token *oauth2.Token, u string) (*http.Response, error) { - oauthClientConfig := &clientcredentials.Config{ ClientID: "app-client", ClientSecret: "secret", diff --git a/oauth2/oauth2_client_credentials_test.go b/oauth2/oauth2_client_credentials_test.go index f84b690854b..b69ba3e78ff 100644 --- a/oauth2/oauth2_client_credentials_test.go +++ b/oauth2/oauth2_client_credentials_test.go @@ -22,14 +22,18 @@ package oauth2_test import ( "context" + "encoding/json" "net/http/httptest" + "strings" "testing" "time" + "github.com/dgrijalva/jwt-go" "github.com/gorilla/sessions" "github.com/julienschmidt/httprouter" "github.com/ory/fosite" "github.com/ory/fosite/compose" + "github.com/ory/fosite/handler/oauth2" "github.com/ory/herodot" hc "github.com/ory/hydra/client" "github.com/ory/hydra/jwk" @@ -42,57 +46,94 @@ import ( ) func TestClientCredentials(t *testing.T) { - router := httprouter.New() - l := logrus.New() - l.Level = logrus.DebugLevel - store := NewFositeMemoryStore(hc.NewMemoryManager(hasher), time.Second) + for _, tc := range []struct { + d string + s oauth2.CoreStrategy + assertAccessToken func(*testing.T, string) + }{ + { + d: "opaque", + s: oauth2OpqaueStrategy, + }, + { + d: "jwt", + s: oauth2JWTStrategy, + assertAccessToken: func(t *testing.T, token string) { + body, err := jwt.DecodeSegment(strings.Split(token, ".")[1]) + require.NoError(t, err) - jm := &jwk.MemoryManager{Keys: map[string]*jose.JSONWebKeySet{}} - keys, err := (&jwk.RS256Generator{}).Generate("", "sig") - require.NoError(t, err) - require.NoError(t, jm.AddKeySet(OpenIDConnectKeyName, keys)) - jwtStrategy, err := jwk.NewRS256JWTStrategy(jm, OpenIDConnectKeyName) + data := map[string]interface{}{} + require.NoError(t, json.Unmarshal(body, &data)) - ts := httptest.NewServer(router) - handler := &Handler{ - OAuth2: compose.Compose( - fc, - store, - oauth2Strategy, - nil, - compose.OAuth2ClientCredentialsGrantFactory, - compose.OAuth2TokenIntrospectionFactory, - ), - //Consent: consentStrategy, - CookieStore: sessions.NewCookieStore([]byte("foo-secret")), - ForcedHTTP: true, - ScopeStrategy: fosite.HierarchicScopeStrategy, - IDTokenLifespan: time.Minute, - H: herodot.NewJSONWriter(l), - L: l, - IssuerURL: ts.URL, - OpenIDJWTStrategy: jwtStrategy, - } + assert.EqualValues(t, "app-client", data["client_id"]) + assert.EqualValues(t, "app-client", data["sub"]) + assert.NotEmpty(t, data["iss"]) + assert.NotEmpty(t, data["jti"]) + assert.NotEmpty(t, data["exp"]) + assert.NotEmpty(t, data["iat"]) + assert.NotEmpty(t, data["nbf"]) + assert.EqualValues(t, data["nbf"], data["iat"]) + assert.EqualValues(t, []interface{}{"foobar"}, data["scp"]) + }, + }, + } { + t.Run("tc="+tc.d, func(t *testing.T) { + router := httprouter.New() + l := logrus.New() + l.Level = logrus.DebugLevel + store := NewFositeMemoryStore(hc.NewMemoryManager(hasher), time.Second) - handler.SetRoutes(router) + jm := &jwk.MemoryManager{Keys: map[string]*jose.JSONWebKeySet{}} + keys, err := (&jwk.RS256Generator{}).Generate("", "sig") + require.NoError(t, err) + require.NoError(t, jm.AddKeySet(OpenIDConnectKeyName, keys)) + jwtStrategy, err := jwk.NewRS256JWTStrategy(jm, OpenIDConnectKeyName) - require.NoError(t, store.CreateClient(&hc.Client{ - ClientID: "app-client", - Secret: "secret", - RedirectURIs: []string{ts.URL + "/callback"}, - ResponseTypes: []string{"token"}, - GrantTypes: []string{"client_credentials"}, - Scope: "foobar", - })) + ts := httptest.NewServer(router) + handler := &Handler{ + OAuth2: compose.Compose( + fc, + store, + tc.s, + nil, + compose.OAuth2ClientCredentialsGrantFactory, + compose.OAuth2TokenIntrospectionFactory, + ), + //Consent: consentStrategy, + CookieStore: sessions.NewCookieStore([]byte("foo-secret")), + ForcedHTTP: true, + ScopeStrategy: fosite.HierarchicScopeStrategy, + IDTokenLifespan: time.Minute, + H: herodot.NewJSONWriter(l), + L: l, + IssuerURL: ts.URL, + OpenIDJWTStrategy: jwtStrategy, + } - oauthClientConfig := &clientcredentials.Config{ - ClientID: "app-client", - ClientSecret: "secret", - TokenURL: ts.URL + "/oauth2/token", - Scopes: []string{"foobar"}, - } + handler.SetRoutes(router) + + require.NoError(t, store.CreateClient(&hc.Client{ + ClientID: "app-client", + Secret: "secret", + RedirectURIs: []string{ts.URL + "/callback"}, + ResponseTypes: []string{"token"}, + GrantTypes: []string{"client_credentials"}, + Scope: "foobar", + })) - tok, err := oauthClientConfig.Token(context.Background()) - require.NoError(t, err) - assert.NotEmpty(t, tok.AccessToken) + oauthClientConfig := &clientcredentials.Config{ + ClientID: "app-client", + ClientSecret: "secret", + TokenURL: ts.URL + "/oauth2/token", + Scopes: []string{"foobar"}, + } + + tok, err := oauthClientConfig.Token(context.Background()) + require.NoError(t, err) + assert.NotEmpty(t, tok.AccessToken) + if tc.assertAccessToken != nil { + tc.assertAccessToken(t, tok.AccessToken) + } + }) + } } diff --git a/oauth2/oauth2_helper_test.go b/oauth2/oauth2_helper_test.go index 3900721e0fb..42c8ba76cbb 100644 --- a/oauth2/oauth2_helper_test.go +++ b/oauth2/oauth2_helper_test.go @@ -31,10 +31,14 @@ import ( ) var hasher = &fosite.BCrypt{} -var oauth2Strategy = &compose.CommonStrategy{ +var oauth2OpqaueStrategy = &compose.CommonStrategy{ CoreStrategy: compose.NewOAuth2HMACStrategy(fc, []byte("some super secret secret secret secret")), OpenIDConnectTokenStrategy: compose.NewOpenIDConnectStrategy(pkg.MustINSECURELOWENTROPYRSAKEYFORTEST()), } +var oauth2JWTStrategy = &compose.CommonStrategy{ + CoreStrategy: compose.NewOAuth2JWTStrategy(pkg.MustINSECURELOWENTROPYRSAKEYFORTEST(), compose.NewOAuth2HMACStrategy(fc, []byte("some super secret secret secret secret"))), + OpenIDConnectTokenStrategy: compose.NewOpenIDConnectStrategy(pkg.MustINSECURELOWENTROPYRSAKEYFORTEST()), +} var fc = &compose.Config{ AccessTokenLifespan: time.Second, diff --git a/oauth2/session.go b/oauth2/session.go index 8681ee3b508..d3ef5b2654b 100644 --- a/oauth2/session.go +++ b/oauth2/session.go @@ -21,6 +21,8 @@ package oauth2 import ( + "time" + "github.com/mohae/deepcopy" "github.com/ory/fosite" "github.com/ory/fosite/handler/openid" @@ -32,9 +34,9 @@ type Session struct { *openid.DefaultSession `json:"idToken"` Audience []string Extra map[string]interface{} `json:"extra"` - JTI string - KID string - ClientID string + //JTI string + KID string + ClientID string } func NewSession(subject string) *Session { @@ -53,14 +55,18 @@ func (s *Session) GetJWTClaims() *jwt.JWTClaims { claims := &jwt.JWTClaims{ Subject: s.Subject, Audience: s.Audience, - JTI: s.JTI, - IssuedAt: s.DefaultSession.Claims.IssuedAt, Issuer: s.DefaultSession.Claims.Issuer, - NotBefore: s.DefaultSession.Claims.IssuedAt, - ExpiresAt: s.GetExpiresAt(fosite.AccessToken), Extra: s.Extra, + ExpiresAt: s.GetExpiresAt(fosite.AccessToken), + IssuedAt: time.Now(), + NotBefore: time.Now(), + // The JTI MUST NOT BE FIXED or refreshing tokens will yield the SAME token + // JTI: s.JTI, // These are set by the DefaultJWTStrategy // Scope: s.Scope, + // Setting these here will cause the token to have the same iat/nbf values always + // IssuedAt: s.DefaultSession.Claims.IssuedAt, + // NotBefore: s.DefaultSession.Claims.IssuedAt, } if claims.Extra == nil {