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.

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

feat: allow to disable claim mirroring #3563

Merged
merged 8 commits into from
Aug 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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