From 27de382c1e514ee208c5541fcf0194e6a055b573 Mon Sep 17 00:00:00 2001 From: hackerman <3372410+aeneasr@users.noreply.github.com> Date: Wed, 10 Jul 2024 15:19:30 +0200 Subject: [PATCH] revert: splitting the HMAC SHA strategy (#813) (#815) This reverts commit 576230ace1a2a488d9f4d4af4272df0caae9eec9. --- compose/compose_strategy.go | 6 +-- handler/oauth2/strategy_hmacsha.go | 60 +++++++++++++-------- handler/oauth2/strategy_hmacsha_prefixed.go | 60 --------------------- handler/oauth2/strategy_hmacsha_test.go | 10 ++-- handler/openid/flow_hybrid_test.go | 8 ++- handler/pkce/handler_test.go | 8 ++- integration/helper_setup_test.go | 14 +++-- 7 files changed, 57 insertions(+), 109 deletions(-) delete mode 100644 handler/oauth2/strategy_hmacsha_prefixed.go diff --git a/compose/compose_strategy.go b/compose/compose_strategy.go index f2ecbafa..267e3295 100644 --- a/compose/compose_strategy.go +++ b/compose/compose_strategy.go @@ -31,10 +31,8 @@ type HMACSHAStrategyConfigurator interface { func NewOAuth2HMACStrategy(config HMACSHAStrategyConfigurator) *oauth2.HMACSHAStrategy { return &oauth2.HMACSHAStrategy{ - BaseHMACSHAStrategy: &oauth2.BaseHMACSHAStrategy{ - Enigma: &hmac.HMACStrategy{Config: config}, - Config: config, - }, + Enigma: &hmac.HMACStrategy{Config: config}, + Config: config, } } diff --git a/handler/oauth2/strategy_hmacsha.go b/handler/oauth2/strategy_hmacsha.go index 293407fa..71193459 100644 --- a/handler/oauth2/strategy_hmacsha.go +++ b/handler/oauth2/strategy_hmacsha.go @@ -5,6 +5,8 @@ package oauth2 import ( "context" + "fmt" + "strings" "time" "github.com/ory/x/errorsx" @@ -13,39 +15,55 @@ import ( enigma "github.com/ory/fosite/token/hmac" ) -var _ CoreStrategy = (*BaseHMACSHAStrategy)(nil) - -type BaseHMACSHAStrategy struct { +type HMACSHAStrategy struct { Enigma *enigma.HMACStrategy Config interface { fosite.AccessTokenLifespanProvider fosite.RefreshTokenLifespanProvider fosite.AuthorizeCodeLifespanProvider } + prefix *string } -func (h *BaseHMACSHAStrategy) AccessTokenSignature(_ context.Context, token string) string { +func (h *HMACSHAStrategy) AccessTokenSignature(ctx context.Context, token string) string { return h.Enigma.Signature(token) } - -func (h *BaseHMACSHAStrategy) RefreshTokenSignature(_ context.Context, token string) string { +func (h *HMACSHAStrategy) RefreshTokenSignature(ctx context.Context, token string) string { return h.Enigma.Signature(token) } - -func (h *BaseHMACSHAStrategy) AuthorizeCodeSignature(_ context.Context, token string) string { +func (h *HMACSHAStrategy) AuthorizeCodeSignature(ctx context.Context, token string) string { return h.Enigma.Signature(token) } -func (h *BaseHMACSHAStrategy) GenerateAccessToken(ctx context.Context, _ fosite.Requester) (token string, signature string, err error) { +func (h *HMACSHAStrategy) getPrefix(part string) string { + if h.prefix == nil { + prefix := "ory_%s_" + h.prefix = &prefix + } else if len(*h.prefix) == 0 { + return "" + } + + return fmt.Sprintf(*h.prefix, part) +} + +func (h *HMACSHAStrategy) trimPrefix(token, part string) string { + return strings.TrimPrefix(token, h.getPrefix(part)) +} + +func (h *HMACSHAStrategy) setPrefix(token, part string) string { + return h.getPrefix(part) + token +} + +func (h *HMACSHAStrategy) GenerateAccessToken(ctx context.Context, _ fosite.Requester) (token string, signature string, err error) { token, sig, err := h.Enigma.Generate(ctx) if err != nil { return "", "", err } - return token, sig, nil + return h.setPrefix(token, "at"), sig, nil } -func (h *BaseHMACSHAStrategy) ValidateAccessToken(ctx context.Context, r fosite.Requester, token string) (err error) { +func (h *HMACSHAStrategy) ValidateAccessToken(ctx context.Context, r fosite.Requester, token string) (err error) { var exp = r.GetSession().GetExpiresAt(fosite.AccessToken) if exp.IsZero() && r.GetRequestedAt().Add(h.Config.GetAccessTokenLifespan(ctx)).Before(time.Now().UTC()) { return errorsx.WithStack(fosite.ErrTokenExpired.WithHintf("Access token expired at '%s'.", r.GetRequestedAt().Add(h.Config.GetAccessTokenLifespan(ctx)))) @@ -55,42 +73,42 @@ func (h *BaseHMACSHAStrategy) ValidateAccessToken(ctx context.Context, r fosite. return errorsx.WithStack(fosite.ErrTokenExpired.WithHintf("Access token expired at '%s'.", exp)) } - return h.Enigma.Validate(ctx, token) + return h.Enigma.Validate(ctx, h.trimPrefix(token, "at")) } -func (h *BaseHMACSHAStrategy) GenerateRefreshToken(ctx context.Context, _ fosite.Requester) (token string, signature string, err error) { +func (h *HMACSHAStrategy) GenerateRefreshToken(ctx context.Context, _ fosite.Requester) (token string, signature string, err error) { token, sig, err := h.Enigma.Generate(ctx) if err != nil { return "", "", err } - return token, sig, nil + return h.setPrefix(token, "rt"), sig, nil } -func (h *BaseHMACSHAStrategy) ValidateRefreshToken(ctx context.Context, r fosite.Requester, token string) (err error) { +func (h *HMACSHAStrategy) ValidateRefreshToken(ctx context.Context, r fosite.Requester, token string) (err error) { var exp = r.GetSession().GetExpiresAt(fosite.RefreshToken) if exp.IsZero() { // Unlimited lifetime - return h.Enigma.Validate(ctx, token) + return h.Enigma.Validate(ctx, h.trimPrefix(token, "rt")) } if !exp.IsZero() && exp.Before(time.Now().UTC()) { return errorsx.WithStack(fosite.ErrTokenExpired.WithHintf("Refresh token expired at '%s'.", exp)) } - return h.Enigma.Validate(ctx, token) + return h.Enigma.Validate(ctx, h.trimPrefix(token, "rt")) } -func (h *BaseHMACSHAStrategy) GenerateAuthorizeCode(ctx context.Context, _ fosite.Requester) (token string, signature string, err error) { +func (h *HMACSHAStrategy) GenerateAuthorizeCode(ctx context.Context, _ fosite.Requester) (token string, signature string, err error) { token, sig, err := h.Enigma.Generate(ctx) if err != nil { return "", "", err } - return token, sig, nil + return h.setPrefix(token, "ac"), sig, nil } -func (h *BaseHMACSHAStrategy) ValidateAuthorizeCode(ctx context.Context, r fosite.Requester, token string) (err error) { +func (h *HMACSHAStrategy) ValidateAuthorizeCode(ctx context.Context, r fosite.Requester, token string) (err error) { var exp = r.GetSession().GetExpiresAt(fosite.AuthorizeCode) if exp.IsZero() && r.GetRequestedAt().Add(h.Config.GetAuthorizeCodeLifespan(ctx)).Before(time.Now().UTC()) { return errorsx.WithStack(fosite.ErrTokenExpired.WithHintf("Authorize code expired at '%s'.", r.GetRequestedAt().Add(h.Config.GetAuthorizeCodeLifespan(ctx)))) @@ -100,5 +118,5 @@ func (h *BaseHMACSHAStrategy) ValidateAuthorizeCode(ctx context.Context, r fosit return errorsx.WithStack(fosite.ErrTokenExpired.WithHintf("Authorize code expired at '%s'.", exp)) } - return h.Enigma.Validate(ctx, token) + return h.Enigma.Validate(ctx, h.trimPrefix(token, "ac")) } diff --git a/handler/oauth2/strategy_hmacsha_prefixed.go b/handler/oauth2/strategy_hmacsha_prefixed.go deleted file mode 100644 index 8849deb8..00000000 --- a/handler/oauth2/strategy_hmacsha_prefixed.go +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright © 2024 Ory Corp -// SPDX-License-Identifier: Apache-2.0 - -package oauth2 - -import ( - "context" - "fmt" - "strings" - - "github.com/ory/fosite" -) - -var _ CoreStrategy = (*HMACSHAStrategy)(nil) - -type HMACSHAStrategy struct { - *BaseHMACSHAStrategy -} - -func (h *HMACSHAStrategy) getPrefix(part string) string { - return fmt.Sprintf("ory_%s_", part) -} - -func (h *HMACSHAStrategy) trimPrefix(token, part string) string { - return strings.TrimPrefix(token, h.getPrefix(part)) -} - -func (h *HMACSHAStrategy) setPrefix(token, part string) string { - if token == "" { - return "" - } - return h.getPrefix(part) + token -} - -func (h *HMACSHAStrategy) GenerateAccessToken(ctx context.Context, r fosite.Requester) (token string, signature string, err error) { - token, sig, err := h.BaseHMACSHAStrategy.GenerateAccessToken(ctx, r) - return h.setPrefix(token, "at"), sig, err -} - -func (h *HMACSHAStrategy) ValidateAccessToken(ctx context.Context, r fosite.Requester, token string) (err error) { - return h.BaseHMACSHAStrategy.ValidateAccessToken(ctx, r, h.trimPrefix(token, "at")) -} - -func (h *HMACSHAStrategy) GenerateRefreshToken(ctx context.Context, r fosite.Requester) (token string, signature string, err error) { - token, sig, err := h.BaseHMACSHAStrategy.GenerateRefreshToken(ctx, r) - return h.setPrefix(token, "rt"), sig, err -} - -func (h *HMACSHAStrategy) ValidateRefreshToken(ctx context.Context, r fosite.Requester, token string) (err error) { - return h.BaseHMACSHAStrategy.ValidateRefreshToken(ctx, r, h.trimPrefix(token, "rt")) -} - -func (h *HMACSHAStrategy) GenerateAuthorizeCode(ctx context.Context, r fosite.Requester) (token string, signature string, err error) { - token, sig, err := h.BaseHMACSHAStrategy.GenerateAuthorizeCode(ctx, r) - return h.setPrefix(token, "ac"), sig, err -} - -func (h *HMACSHAStrategy) ValidateAuthorizeCode(ctx context.Context, r fosite.Requester, token string) (err error) { - return h.BaseHMACSHAStrategy.ValidateAuthorizeCode(ctx, r, h.trimPrefix(token, "ac")) -} diff --git a/handler/oauth2/strategy_hmacsha_test.go b/handler/oauth2/strategy_hmacsha_test.go index 9096bdba..9b780d28 100644 --- a/handler/oauth2/strategy_hmacsha_test.go +++ b/handler/oauth2/strategy_hmacsha_test.go @@ -17,12 +17,10 @@ import ( ) var hmacshaStrategy = HMACSHAStrategy{ - BaseHMACSHAStrategy: &BaseHMACSHAStrategy{ - Enigma: &hmac.HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobar")}}, - Config: &fosite.Config{ - AccessTokenLifespan: time.Hour * 24, - AuthorizeCodeLifespan: time.Hour * 24, - }, + Enigma: &hmac.HMACStrategy{Config: &fosite.Config{GlobalSecret: []byte("foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobar")}}, + Config: &fosite.Config{ + AccessTokenLifespan: time.Hour * 24, + AuthorizeCodeLifespan: time.Hour * 24, }, } diff --git a/handler/openid/flow_hybrid_test.go b/handler/openid/flow_hybrid_test.go index 96803133..3412675a 100644 --- a/handler/openid/flow_hybrid_test.go +++ b/handler/openid/flow_hybrid_test.go @@ -27,11 +27,9 @@ import ( ) var hmacStrategy = &oauth2.HMACSHAStrategy{ - BaseHMACSHAStrategy: &oauth2.BaseHMACSHAStrategy{ - Enigma: &hmac.HMACStrategy{ - Config: &fosite.Config{ - GlobalSecret: []byte("some-super-cool-secret-that-nobody-knows-nobody-knows"), - }, + Enigma: &hmac.HMACStrategy{ + Config: &fosite.Config{ + GlobalSecret: []byte("some-super-cool-secret-that-nobody-knows-nobody-knows"), }, }, } diff --git a/handler/pkce/handler_test.go b/handler/pkce/handler_test.go index 393f0bf1..bdd50650 100644 --- a/handler/pkce/handler_test.go +++ b/handler/pkce/handler_test.go @@ -38,11 +38,9 @@ func (m *mockCodeStrategy) ValidateAuthorizeCode(ctx context.Context, requester func TestPKCEHandleAuthorizeEndpointRequest(t *testing.T) { var config fosite.Config h := &Handler{ - Storage: storage.NewMemoryStore(), - AuthorizeCodeStrategy: &oauth2.HMACSHAStrategy{ - BaseHMACSHAStrategy: new(oauth2.BaseHMACSHAStrategy), - }, - Config: &config, + Storage: storage.NewMemoryStore(), + AuthorizeCodeStrategy: new(oauth2.HMACSHAStrategy), + Config: &config, } w := fosite.NewAuthorizeResponse() r := fosite.NewAuthorizeRequest() diff --git a/integration/helper_setup_test.go b/integration/helper_setup_test.go index 1381608e..62a99d4f 100644 --- a/integration/helper_setup_test.go +++ b/integration/helper_setup_test.go @@ -173,17 +173,15 @@ func newJWTBearerAppClient(ts *httptest.Server) *clients.JWTBearer { } var hmacStrategy = &oauth2.HMACSHAStrategy{ - BaseHMACSHAStrategy: &oauth2.BaseHMACSHAStrategy{ - Enigma: &hmac.HMACStrategy{ - Config: &fosite.Config{ - GlobalSecret: []byte("some-super-cool-secret-that-nobody-knows"), - }, - }, + Enigma: &hmac.HMACStrategy{ Config: &fosite.Config{ - AccessTokenLifespan: accessTokenLifespan, - AuthorizeCodeLifespan: authCodeLifespan, + GlobalSecret: []byte("some-super-cool-secret-that-nobody-knows"), }, }, + Config: &fosite.Config{ + AccessTokenLifespan: accessTokenLifespan, + AuthorizeCodeLifespan: authCodeLifespan, + }, } var defaultRSAKey = gen.MustRSAKey()