Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: allow to disable claim mirroring #3563

Merged
merged 8 commits into from
Aug 11, 2023
Merged

feat: allow to disable claim mirroring #3563

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
691c01c
feat: allow to disable claim mirroring
davidsteinberger Jul 5, 2023
05c4b62
chore: update tests and snapshots
davidsteinberger Jul 5, 2023
067450d
chore: add missing snapshot
davidsteinberger Jul 5, 2023
5ce713f
chore: pass in config provider
davidsteinberger Aug 6, 2023
2e1c4aa
Merge branch 'master' into feat/allow-to-disable-claims-mirroring
dastein1 Aug 6, 2023
f26ccb1
chore: pass in config provider
davidsteinberger Aug 6, 2023
af77986
chore: add logger
davidsteinberger Aug 6, 2023
e1fa88f
Merge branch 'master' into feat/allow-to-disable-claims-mirroring
hperl Aug 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions driver/config/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,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 @@ -201,6 +202,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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
}
7 changes: 4 additions & 3 deletions oauth2/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,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("", h.c.AllowedTopLevelClaims(ctx), h.c.MirrorTopLevelClaims(ctx))
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 @@ -716,7 +716,7 @@ 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()))
session := NewSessionWithCustomClaims("", h.c.AllowedTopLevelClaims(r.Context()), h.c.MirrorTopLevelClaims(r.Context()))
ctx := r.Context()

if r.Method != "POST" {
Expand Down Expand Up @@ -886,7 +886,7 @@ type oAuth2TokenExchange struct {
// 200: oAuth2TokenExchange
// default: errorOAuth2
func (h *Handler) oauth2TokenExchange(w http.ResponseWriter, r *http.Request) {
session := NewSessionWithCustomClaims("", h.c.AllowedTopLevelClaims(r.Context()))
session := NewSessionWithCustomClaims("", h.c.AllowedTopLevelClaims(r.Context()), h.c.MirrorTopLevelClaims(r.Context()))
ctx := r.Context()

accessRequest, err := h.r.OAuth2Provider().NewAccessRequest(ctx, r, session)
Expand Down Expand Up @@ -1075,6 +1075,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
10 changes: 7 additions & 3 deletions oauth2/session.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,16 @@ 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)
return NewSessionWithCustomClaims(subject, nil, true)
}

func NewSessionWithCustomClaims(subject string, allowedTopLevelClaims []string) *Session {
func NewSessionWithCustomClaims(subject string, allowedTopLevelClaims []string, mirrorTopLevelClaims bool) *Session {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing each config option, we could pass the configuration provider here and read out the config values in the function body.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hperl. The methods on the config provider require the context. So I've changed the signature to accept ctx first followed by the config provider.

return &Session{
DefaultSession: &openid.DefaultSession{
Claims: new(jwt.IDTokenClaims),
Expand All @@ -47,6 +48,7 @@ func NewSessionWithCustomClaims(subject string, allowedTopLevelClaims []string)
},
Extra: map[string]interface{}{},
AllowedTopLevelClaims: allowedTopLevelClaims,
MirrorTopLevelClaims: mirrorTopLevelClaims,
}
}

Expand All @@ -70,7 +72,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
19 changes: 10 additions & 9 deletions oauth2/session_custom_claims_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"github.com/stretchr/testify/require"
)

func createSessionWithCustomClaims(extra map[string]interface{}, allowedTopLevelClaims []string) oauth2.Session {
func createSessionWithCustomClaims(extra map[string]interface{}, allowedTopLevelClaims []string, mirrorTopLevelClaims bool) oauth2.Session {
session := &oauth2.Session{
DefaultSession: &openid.DefaultSession{
Claims: &jwt.IDTokenClaims{
Expand All @@ -30,6 +30,7 @@ func createSessionWithCustomClaims(extra map[string]interface{}, allowedTopLevel
},
Extra: extra,
AllowedTopLevelClaims: allowedTopLevelClaims,
MirrorTopLevelClaims: mirrorTopLevelClaims,
}
return *session
}
Expand All @@ -41,7 +42,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(nil, c.AllowedTopLevelClaims(ctx), c.MirrorTopLevelClaims(ctx))
claims := session.GetJWTClaims().ToMapClaims()

assert.EqualValues(t, "alice", claims["sub"])
Expand All @@ -56,7 +57,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(extra, c.AllowedTopLevelClaims(ctx), c.MirrorTopLevelClaims(ctx))

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

Expand All @@ -80,7 +81,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(extra, c.AllowedTopLevelClaims(ctx), c.MirrorTopLevelClaims(ctx))

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

Expand Down Expand Up @@ -111,7 +112,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(extra, c.AllowedTopLevelClaims(ctx), c.MirrorTopLevelClaims(ctx))

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

Expand Down Expand Up @@ -140,7 +141,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(extra, c.AllowedTopLevelClaims(ctx), c.MirrorTopLevelClaims(ctx))

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

Expand Down Expand Up @@ -168,7 +169,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(extra, c.AllowedTopLevelClaims(ctx), c.MirrorTopLevelClaims(ctx))

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

Expand Down Expand Up @@ -198,7 +199,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(extra, c.AllowedTopLevelClaims(ctx), c.MirrorTopLevelClaims(ctx))

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

Expand Down Expand Up @@ -228,7 +229,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(extra, c.AllowedTopLevelClaims(ctx), c.MirrorTopLevelClaims(ctx))

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

Expand Down
6 changes: 6 additions & 0 deletions spec/config.json
View file
Open in desktop
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
Open in desktop
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("", reg.Config().AllowedTopLevelClaims(ctx), reg.Config().MirrorTopLevelClaims(ctx))
_, ar, err := reg.OAuth2Provider().IntrospectToken(ctx, token, fosite.AccessToken, session)
if err != nil {
return false
Expand Down