Skip to content

Commit

Permalink
feat: allow to disable claim mirroring (#3563)
Browse files Browse the repository at this point in the history 
This PR introduces another config option called `oauth2:mirror_top_level_claims` which may be used to disable the mirroring of custom claims into the `ext` claim of the jwt.
This new config option is an opt-in. If unused the behavior remains as-is to ensure backwards compatibility.

Example:

```yaml
oauth2:
  allowed_top_level_claims:
    - test_claim
  mirror_top_level_claims: false # -> this will prevent test_claim to be mirrored within ext
```

Closes #3348
  • Loading branch information
@dastein1
dastein1 authored Aug 11, 2023
1 parent 71d1853 commit c72a316
Show file tree
Hide file tree
Showing 20 changed files with 71 additions and 31 deletions.
5 changes: 5 additions & 0 deletions driver/config/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ const (
KeyExposeOAuth2Debug = "oauth2.expose_internal_errors"
KeyExcludeNotBeforeClaim = "oauth2.exclude_not_before_claim"
KeyAllowedTopLevelClaims = "oauth2.allowed_top_level_claims"
KeyMirrorTopLevelClaims = "oauth2.mirror_top_level_claims"
KeyOAuth2GrantJWTIDOptional = "oauth2.grant.jwt.jti_optional"
KeyOAuth2GrantJWTIssuedDateOptional = "oauth2.grant.jwt.iat_optional"
KeyOAuth2GrantJWTMaxDuration = "oauth2.grant.jwt.max_ttl"
Expand Down Expand Up @@ -203,6 +204,10 @@ func (p *DefaultProvider) AllowedTopLevelClaims(ctx context.Context) []string {
return stringslice.Unique(p.getProvider(ctx).Strings(KeyAllowedTopLevelClaims))
}

func (p *DefaultProvider) MirrorTopLevelClaims(ctx context.Context) bool {
return p.getProvider(ctx).BoolF(KeyMirrorTopLevelClaims, true)
}

func (p *DefaultProvider) SubjectTypesSupported(ctx context.Context, additionalSources ...AccessTokenStrategySource) []string {
types := stringslice.Filter(
p.getProvider(ctx).StringsF(KeySubjectTypesSupported, []string{"public"}),
Expand Down
3 changes: 2 additions & 1 deletion ..._request_if_strategy_passes-should_call_refresh_token_hook_if_configured-hook=legacy.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"client_id": "app-client",
"consent_challenge": "",
"exclude_not_before_claim": false,
"allowed_top_level_claims": []
"allowed_top_level_claims": [],
"mirror_top_level_claims": true
},
"requester": {
"client_id": "app-client",
Expand Down
3 changes: 2 additions & 1 deletion ...ass_request_if_strategy_passes-should_call_refresh_token_hook_if_configured-hook=new.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
"client_id": "app-client",
"consent_challenge": "",
"exclude_not_before_claim": false,
"allowed_top_level_claims": []
"allowed_top_level_claims": [],
"mirror_top_level_claims": true
},
"request": {
"client_id": "app-client",
Expand Down
3 changes: 2 additions & 1 deletion ..._age_is_less_than_auth_time-should_call_refresh_token_hook_if_configured-hook=legacy.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"client_id": "app-client",
"consent_challenge": "",
"exclude_not_before_claim": false,
"allowed_top_level_claims": []
"allowed_top_level_claims": [],
"mirror_top_level_claims": true
},
"requester": {
"client_id": "app-client",
Expand Down
3 changes: 2 additions & 1 deletion ...max_age_is_less_than_auth_time-should_call_refresh_token_hook_if_configured-hook=new.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
"client_id": "app-client",
"consent_challenge": "",
"exclude_not_before_claim": false,
"allowed_top_level_claims": []
"allowed_top_level_claims": [],
"mirror_top_level_claims": true
},
"request": {
"client_id": "app-client",
Expand Down
3 changes: 2 additions & 1 deletion ...thentication_time_is_recent-should_call_refresh_token_hook_if_configured-hook=legacy.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"client_id": "app-client",
"consent_challenge": "",
"exclude_not_before_claim": false,
"allowed_top_level_claims": []
"allowed_top_level_claims": [],
"mirror_top_level_claims": true
},
"requester": {
"client_id": "app-client",
Expand Down
3 changes: 2 additions & 1 deletion ..._authentication_time_is_recent-should_call_refresh_token_hook_if_configured-hook=new.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
"client_id": "app-client",
"consent_challenge": "",
"exclude_not_before_claim": false,
"allowed_top_level_claims": []
"allowed_top_level_claims": [],
"mirror_top_level_claims": true
},
"request": {
"client_id": "app-client",
Expand Down
3 changes: 2 additions & 1 deletion ..._request_if_strategy_passes-should_call_refresh_token_hook_if_configured-hook=legacy.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"client_id": "app-client",
"consent_challenge": "",
"exclude_not_before_claim": false,
"allowed_top_level_claims": []
"allowed_top_level_claims": [],
"mirror_top_level_claims": true
},
"requester": {
"client_id": "app-client",
Expand Down
3 changes: 2 additions & 1 deletion ...ass_request_if_strategy_passes-should_call_refresh_token_hook_if_configured-hook=new.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
"client_id": "app-client",
"consent_challenge": "",
"exclude_not_before_claim": false,
"allowed_top_level_claims": []
"allowed_top_level_claims": [],
"mirror_top_level_claims": true
},
"request": {
"client_id": "app-client",
Expand Down
3 changes: 2 additions & 1 deletion ..._age_is_less_than_auth_time-should_call_refresh_token_hook_if_configured-hook=legacy.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"client_id": "app-client",
"consent_challenge": "",
"exclude_not_before_claim": false,
"allowed_top_level_claims": []
"allowed_top_level_claims": [],
"mirror_top_level_claims": true
},
"requester": {
"client_id": "app-client",
Expand Down
3 changes: 2 additions & 1 deletion ...max_age_is_less_than_auth_time-should_call_refresh_token_hook_if_configured-hook=new.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
"client_id": "app-client",
"consent_challenge": "",
"exclude_not_before_claim": false,
"allowed_top_level_claims": []
"allowed_top_level_claims": [],
"mirror_top_level_claims": true
},
"request": {
"client_id": "app-client",
Expand Down
3 changes: 2 additions & 1 deletion ...thentication_time_is_recent-should_call_refresh_token_hook_if_configured-hook=legacy.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"client_id": "app-client",
"consent_challenge": "",
"exclude_not_before_claim": false,
"allowed_top_level_claims": []
"allowed_top_level_claims": [],
"mirror_top_level_claims": true
},
"requester": {
"client_id": "app-client",
Expand Down
3 changes: 2 additions & 1 deletion ..._authentication_time_is_recent-should_call_refresh_token_hook_if_configured-hook=new.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
"client_id": "app-client",
"consent_challenge": "",
"exclude_not_before_claim": false,
"allowed_top_level_claims": []
"allowed_top_level_claims": [],
"mirror_top_level_claims": true
},
"request": {
"client_id": "app-client",
Expand Down
3 changes: 2 additions & 1 deletion oauth2/.snapshots/TestUnmarshalSession-v1.11.8.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,6 @@
"market",
"zone",
"login_session_id"
]
],
"mirror_top_level_claims": false
}
3 changes: 2 additions & 1 deletion oauth2/.snapshots/TestUnmarshalSession-v1.11.9.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,6 @@
"market",
"zone",
"login_session_id"
]
],
"mirror_top_level_claims": false
}
9 changes: 5 additions & 4 deletions oauth2/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ type oidcUserInfo struct {
// default: errorOAuth2
func (h *Handler) getOidcUserInfo(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
session := NewSessionWithCustomClaims("", h.c.AllowedTopLevelClaims(ctx))
session := NewSessionWithCustomClaims(ctx, h.c, "")
tokenType, ar, err := h.r.OAuth2Provider().IntrospectToken(ctx, fosite.AccessTokenFromRequest(r), fosite.AccessToken, session)
if err != nil {
rfcerr := fosite.ErrorToRFC6749Error(err)
Expand Down Expand Up @@ -776,8 +776,8 @@ type introspectOAuth2Token struct {
// 200: introspectedOAuth2Token
// default: errorOAuth2
func (h *Handler) introspectOAuth2Token(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
session := NewSessionWithCustomClaims("", h.c.AllowedTopLevelClaims(r.Context()))
ctx := r.Context()
session := NewSessionWithCustomClaims(ctx, h.c, "")

if r.Method != "POST" {
err := errorsx.WithStack(fosite.ErrInvalidRequest.WithHintf("HTTP method is \"%s\", expected \"POST\".", r.Method))
Expand Down Expand Up @@ -946,8 +946,8 @@ type oAuth2TokenExchange struct {
// 200: oAuth2TokenExchange
// default: errorOAuth2
func (h *Handler) oauth2TokenExchange(w http.ResponseWriter, r *http.Request) {
session := NewSessionWithCustomClaims("", h.c.AllowedTopLevelClaims(r.Context()))
ctx := r.Context()
session := NewSessionWithCustomClaims(ctx, h.c, "")

accessRequest, err := h.r.OAuth2Provider().NewAccessRequest(ctx, r, session)
if err != nil {
Expand Down Expand Up @@ -1135,6 +1135,7 @@ func (h *Handler) oAuth2Authorize(w http.ResponseWriter, r *http.Request, _ http
ConsentChallenge: session.ID,
ExcludeNotBeforeClaim: h.c.ExcludeNotBeforeClaim(ctx),
AllowedTopLevelClaims: h.c.AllowedTopLevelClaims(ctx),
MirrorTopLevelClaims: h.c.MirrorTopLevelClaims(ctx),
Flow: flow,
})
if err != nil {
Expand Down Expand Up @@ -1237,7 +1238,7 @@ func (h *Handler) logOrAudit(err error, r *http.Request) {
// default: errorOAuth2
func (h *Handler) createVerifiableCredential(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
session := NewSessionWithCustomClaims("", h.c.AllowedTopLevelClaims(ctx))
session := NewSessionWithCustomClaims(ctx, h.c, "")
accessToken := fosite.AccessTokenFromRequest(r)
tokenType, _, err := h.r.OAuth2Provider().IntrospectToken(ctx, accessToken, fosite.AccessToken, session)

Expand Down
17 changes: 14 additions & 3 deletions oauth2/session.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package oauth2

import (
"context"
"encoding/json"
"time"

Expand All @@ -16,8 +17,10 @@ import (
"github.com/ory/fosite"
"github.com/ory/fosite/handler/openid"
"github.com/ory/fosite/token/jwt"
"github.com/ory/hydra/v2/driver/config"
"github.com/ory/hydra/v2/flow"

"github.com/ory/x/logrusx"
"github.com/ory/x/stringslice"
)

Expand All @@ -30,15 +33,20 @@ type Session struct {
ConsentChallenge string `json:"consent_challenge"`
ExcludeNotBeforeClaim bool `json:"exclude_not_before_claim"`
AllowedTopLevelClaims []string `json:"allowed_top_level_claims"`
MirrorTopLevelClaims bool `json:"mirror_top_level_claims"`

Flow *flow.Flow `json:"-"`
}

func NewSession(subject string) *Session {
return NewSessionWithCustomClaims(subject, nil)
ctx := context.Background()
provider := config.MustNew(ctx, logrusx.New("", ""))
return NewSessionWithCustomClaims(ctx, provider, subject)
}

func NewSessionWithCustomClaims(subject string, allowedTopLevelClaims []string) *Session {
func NewSessionWithCustomClaims(ctx context.Context, p *config.DefaultProvider, subject string) *Session {
allowedTopLevelClaims := p.AllowedTopLevelClaims(ctx)
mirrorTopLevelClaims := p.MirrorTopLevelClaims(ctx)
return &Session{
DefaultSession: &openid.DefaultSession{
Claims: new(jwt.IDTokenClaims),
Expand All @@ -47,6 +55,7 @@ func NewSessionWithCustomClaims(subject string, allowedTopLevelClaims []string)
},
Extra: map[string]interface{}{},
AllowedTopLevelClaims: allowedTopLevelClaims,
MirrorTopLevelClaims: mirrorTopLevelClaims,
}
}

Expand All @@ -70,7 +79,9 @@ func (s *Session) GetJWTClaims() jwt.JWTClaimsContainer {
}

//for every other claim that was already reserved and for mirroring, add original extra under "ext"
topLevelExtraWithMirrorExt["ext"] = s.Extra
if s.MirrorTopLevelClaims {
topLevelExtraWithMirrorExt["ext"] = s.Extra
}

claims := &jwt.JWTClaims{
Subject: s.Subject,
Expand Down
21 changes: 12 additions & 9 deletions oauth2/session_custom_claims_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import (
"github.com/stretchr/testify/require"
)

func createSessionWithCustomClaims(extra map[string]interface{}, allowedTopLevelClaims []string) oauth2.Session {
func createSessionWithCustomClaims(ctx context.Context, p *config.DefaultProvider, extra map[string]interface{}) oauth2.Session {
allowedTopLevelClaims := p.AllowedTopLevelClaims(ctx)
mirrorTopLevelClaims := p.MirrorTopLevelClaims(ctx)
session := &oauth2.Session{
DefaultSession: &openid.DefaultSession{
Claims: &jwt.IDTokenClaims{
Expand All @@ -30,6 +32,7 @@ func createSessionWithCustomClaims(extra map[string]interface{}, allowedTopLevel
},
Extra: extra,
AllowedTopLevelClaims: allowedTopLevelClaims,
MirrorTopLevelClaims: mirrorTopLevelClaims,
}
return *session
}
Expand All @@ -41,7 +44,7 @@ func TestCustomClaimsInSession(t *testing.T) {
t.Run("no_custom_claims", func(t *testing.T) {
c.MustSet(ctx, config.KeyAllowedTopLevelClaims, []string{})

session := createSessionWithCustomClaims(nil, c.AllowedTopLevelClaims(ctx))
session := createSessionWithCustomClaims(ctx, c, nil)
claims := session.GetJWTClaims().ToMapClaims()

assert.EqualValues(t, "alice", claims["sub"])
Expand All @@ -56,7 +59,7 @@ func TestCustomClaimsInSession(t *testing.T) {
c.MustSet(ctx, config.KeyAllowedTopLevelClaims, []string{"foo"})
extra := map[string]interface{}{"foo": "bar"}

session := createSessionWithCustomClaims(extra, c.AllowedTopLevelClaims(ctx))
session := createSessionWithCustomClaims(ctx, c, extra)

claims := session.GetJWTClaims().ToMapClaims()

Expand All @@ -80,7 +83,7 @@ func TestCustomClaimsInSession(t *testing.T) {
c.MustSet(ctx, config.KeyAllowedTopLevelClaims, []string{"foo", "iss", "sub"})
extra := map[string]interface{}{"foo": "bar", "iss": "hydra.remote", "sub": "another-alice"}

session := createSessionWithCustomClaims(extra, c.AllowedTopLevelClaims(ctx))
session := createSessionWithCustomClaims(ctx, c, extra)

claims := session.GetJWTClaims().ToMapClaims()

Expand Down Expand Up @@ -111,7 +114,7 @@ func TestCustomClaimsInSession(t *testing.T) {
c.MustSet(ctx, config.KeyAllowedTopLevelClaims, []string{})
extra := map[string]interface{}{"foo": "bar", "iss": "hydra.remote", "sub": "another-alice"}

session := createSessionWithCustomClaims(extra, c.AllowedTopLevelClaims(ctx))
session := createSessionWithCustomClaims(ctx, c, extra)

claims := session.GetJWTClaims().ToMapClaims()

Expand Down Expand Up @@ -140,7 +143,7 @@ func TestCustomClaimsInSession(t *testing.T) {
c.MustSet(ctx, config.KeyAllowedTopLevelClaims, []string{"foo", "baz", "bar", "iss"})
extra := map[string]interface{}{"foo": "foo_value", "sub": "another-alice"}

session := createSessionWithCustomClaims(extra, c.AllowedTopLevelClaims(ctx))
session := createSessionWithCustomClaims(ctx, c, extra)

claims := session.GetJWTClaims().ToMapClaims()

Expand Down Expand Up @@ -168,7 +171,7 @@ func TestCustomClaimsInSession(t *testing.T) {
c.MustSet(ctx, config.KeyAllowedTopLevelClaims, []string{"foo", "sub"})
extra := map[string]interface{}{"foo": "foo_value", "bar": "bar_value", "baz": "baz_value", "sub": "another-alice"}

session := createSessionWithCustomClaims(extra, c.AllowedTopLevelClaims(ctx))
session := createSessionWithCustomClaims(ctx, c, extra)

claims := session.GetJWTClaims().ToMapClaims()

Expand Down Expand Up @@ -198,7 +201,7 @@ func TestCustomClaimsInSession(t *testing.T) {
c.MustSet(ctx, config.KeyAllowedTopLevelClaims, []string{"foo", "bar"})
extra := map[string]interface{}{"foo": "foo_value", "baz": "baz_value", "sub": "another-alice"}

session := createSessionWithCustomClaims(extra, c.AllowedTopLevelClaims(ctx))
session := createSessionWithCustomClaims(ctx, c, extra)

claims := session.GetJWTClaims().ToMapClaims()

Expand Down Expand Up @@ -228,7 +231,7 @@ func TestCustomClaimsInSession(t *testing.T) {
c.MustSet(ctx, config.KeyAllowedTopLevelClaims, []string{"iss", "sub"})
extra := map[string]interface{}{"iss": "hydra.remote", "sub": "another-alice"}

session := createSessionWithCustomClaims(extra, c.AllowedTopLevelClaims(ctx))
session := createSessionWithCustomClaims(ctx, c, extra)

claims := session.GetJWTClaims().ToMapClaims()

Expand Down
6 changes: 6 additions & 0 deletions spec/config.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,12 @@
},
"examples": [["username", "email", "user_uuid"]]
},
"mirror_top_level_claims": {
"type": "boolean",
"description": "Set to false if you don't want to mirror custom claims under 'ext'",
"default": true,
"examples": [false]
},
"hashers": {
"type": "object",
"additionalProperties": false,
Expand Down
2 changes: 1 addition & 1 deletion x/oauth2cors/cors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func Middleware(
return false
}

session := oauth2.NewSessionWithCustomClaims("", reg.Config().AllowedTopLevelClaims(ctx))
session := oauth2.NewSessionWithCustomClaims(ctx, reg.Config(), "")
_, ar, err := reg.OAuth2Provider().IntrospectToken(ctx, token, fosite.AccessToken, session)
if err != nil {
return false
Expand Down

0 comments on commit c72a316

Please sign in to comment.