diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f6065b4..84f54dec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ## Important Notes +- [#936](https://github.com/oauth2-proxy/oauth2-proxy/pull/936) `--user-id-claim` option is deprecated and replaced by `--oidc-email-claim` - [#630](https://github.com/oauth2-proxy/oauth2-proxy/pull/630) Gitlab projects needs a Gitlab application with the extra `read_api` enabled - [#905](https://github.com/oauth2-proxy/oauth2-proxy/pull/905) Existing sessions from v6.0.0 or earlier are no longer valid. They will trigger a reauthentication. - [#826](https://github.com/oauth2-proxy/oauth2-proxy/pull/826) `skip-auth-strip-headers` now applies to all requests, not just those where authentication would be skipped. @@ -47,6 +48,8 @@ - [#630](https://github.com/oauth2-proxy/oauth2-proxy/pull/630) Add support for Gitlab project based authentication (@factorysh) - [#907](https://github.com/oauth2-proxy/oauth2-proxy/pull/907) Introduce alpha configuration option to enable testing of structured configuration (@JoelSpeed) - [#938](https://github.com/oauth2-proxy/oauth2-proxy/pull/938) Cleanup missed provider renaming refactor methods (@NickMeves) +- [#816](https://github.com/oauth2-proxy/oauth2-proxy/pull/816) (via [#936](https://github.com/oauth2-proxy/oauth2-proxy/pull/936)) Support non-list group claims (@loafoe) +- [#936](https://github.com/oauth2-proxy/oauth2-proxy/pull/936) Refactor OIDC Provider and support groups from Profile URL (@NickMeves) - [#925](https://github.com/oauth2-proxy/oauth2-proxy/pull/925) Fix basic auth legacy header conversion (@JoelSpeed) - [#916](https://github.com/oauth2-proxy/oauth2-proxy/pull/916) Add AlphaOptions struct to prepare for alpha config loading (@JoelSpeed) - [#923](https://github.com/oauth2-proxy/oauth2-proxy/pull/923) Support TLS 1.3 (@aajisaka) diff --git a/docs/docs/configuration/overview.md b/docs/docs/configuration/overview.md index 46e72d67..a91c6264 100644 --- a/docs/docs/configuration/overview.md +++ b/docs/docs/configuration/overview.md @@ -74,7 +74,8 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/ | `--insecure-oidc-skip-issuer-verification` | bool | allow the OIDC issuer URL to differ from the expected (currently required for Azure multi-tenant compatibility) | false | | `--oidc-issuer-url` | string | the OpenID Connect issuer URL, e.g. `"https://accounts.google.com"` | | | `--oidc-jwks-url` | string | OIDC JWKS URI for token verification; required if OIDC discovery is disabled | | -| `--oidc-groups-claim` | string | which claim contains the user groups | `"groups"` | +| `--oidc-email-claim` | string | which OIDC claim contains the user's email | `"email"` | +| `--oidc-groups-claim` | string | which OIDC claim contains the user groups | `"groups"` | | `--pass-access-token` | bool | pass OAuth access_token to upstream via X-Forwarded-Access-Token header. When used with `--set-xauthrequest` this adds the X-Auth-Request-Access-Token header to the response | false | | `--pass-authorization-header` | bool | pass OIDC IDToken to upstream via Authorization Bearer header | false | | `--pass-basic-auth` | bool | pass HTTP Basic Auth, X-Forwarded-User, X-Forwarded-Email and X-Forwarded-Preferred-Username information to upstream | true | @@ -128,7 +129,6 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/ | `--tls-cert-file` | string | path to certificate file | | | `--tls-key-file` | string | path to private key file | | | `--upstream` | string \| list | the http url(s) of the upstream endpoint, file:// paths for static files or `static://` for static response. Routing is based on the path | | -| `--user-id-claim` | string | which claim contains the user ID | \["email"\] | | `--allowed-group` | string \| list | restrict logins to members of this group (may be given multiple times) | | | `--validate-url` | string | Access token validation endpoint | | | `--version` | n/a | print version string | | diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index cf4e2414..46cdcedb 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -87,6 +87,7 @@ type Options struct { InsecureOIDCSkipIssuerVerification bool `flag:"insecure-oidc-skip-issuer-verification" cfg:"insecure_oidc_skip_issuer_verification"` SkipOIDCDiscovery bool `flag:"skip-oidc-discovery" cfg:"skip_oidc_discovery"` OIDCJwksURL string `flag:"oidc-jwks-url" cfg:"oidc_jwks_url"` + OIDCEmailClaim string `flag:"oidc-email-claim" cfg:"oidc_email_claim"` OIDCGroupsClaim string `flag:"oidc-groups-claim" cfg:"oidc_groups_claim"` LoginURL string `flag:"login-url" cfg:"login_url"` RedeemURL string `flag:"redeem-url" cfg:"redeem_url"` @@ -148,11 +149,12 @@ func NewOptions() *Options { SkipAuthPreflight: false, Prompt: "", // Change to "login" when ApprovalPrompt officially deprecated ApprovalPrompt: "force", - UserIDClaim: "email", InsecureOIDCAllowUnverifiedEmail: false, SkipOIDCDiscovery: false, Logging: loggingDefaults(), - OIDCGroupsClaim: "groups", + UserIDClaim: providers.OIDCEmailClaim, // Deprecated: Use OIDCEmailClaim + OIDCEmailClaim: providers.OIDCEmailClaim, + OIDCGroupsClaim: providers.OIDCGroupsClaim, } } @@ -226,7 +228,8 @@ func NewFlagSet() *pflag.FlagSet { flagSet.Bool("insecure-oidc-skip-issuer-verification", false, "Do not verify if issuer matches OIDC discovery URL") flagSet.Bool("skip-oidc-discovery", false, "Skip OIDC discovery and use manually supplied Endpoints") flagSet.String("oidc-jwks-url", "", "OpenID Connect JWKS URL (ie: https://www.googleapis.com/oauth2/v3/certs)") - flagSet.String("oidc-groups-claim", "groups", "which claim contains the user groups") + flagSet.String("oidc-groups-claim", providers.OIDCGroupsClaim, "which OIDC claim contains the user groups") + flagSet.String("oidc-email-claim", providers.OIDCEmailClaim, "which OIDC claim contains the user's email") flagSet.String("login-url", "", "Authentication endpoint") flagSet.String("redeem-url", "", "Token redemption endpoint") flagSet.String("profile-url", "", "Profile access endpoint") @@ -243,7 +246,7 @@ func NewFlagSet() *pflag.FlagSet { flagSet.String("pubjwk-url", "", "JWK pubkey access endpoint: required by login.gov") flagSet.Bool("gcp-healthchecks", false, "Enable GCP/GKE healthcheck endpoints") - flagSet.String("user-id-claim", "email", "which claim contains the user ID") + flagSet.String("user-id-claim", providers.OIDCEmailClaim, "(DEPRECATED for `oidc-email-claim`) which claim contains the user ID") flagSet.StringSlice("allowed-group", []string{}, "restrict logins to members of this group (may be given multiple times)") flagSet.AddFlagSet(cookieFlagSet()) diff --git a/pkg/validation/options.go b/pkg/validation/options.go index fc89000b..4fc0b0a4 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -233,9 +233,19 @@ func parseProviderInfo(o *options.Options, msgs []string) []string { p.ValidateURL, msgs = parseURL(o.ValidateURL, "validate", msgs) p.ProtectedResource, msgs = parseURL(o.ProtectedResource, "resource", msgs) - // Make the OIDC Verifier accessible to all providers that can support it + // Make the OIDC options available to all providers that support it + p.AllowUnverifiedEmail = o.InsecureOIDCAllowUnverifiedEmail + p.EmailClaim = o.OIDCEmailClaim + p.GroupsClaim = o.OIDCGroupsClaim p.Verifier = o.GetOIDCVerifier() + // TODO (@NickMeves) - Remove This + // Backwards Compatibility for Deprecated UserIDClaim option + if o.OIDCEmailClaim == providers.OIDCEmailClaim && + o.UserIDClaim != providers.OIDCEmailClaim { + p.EmailClaim = o.UserIDClaim + } + p.SetAllowedGroups(o.AllowedGroups) provider := providers.New(o.ProviderType, p) @@ -273,14 +283,10 @@ func parseProviderInfo(o *options.Options, msgs []string) []string { p.SetTeam(o.BitbucketTeam) p.SetRepository(o.BitbucketRepository) case *providers.OIDCProvider: - p.AllowUnverifiedEmail = o.InsecureOIDCAllowUnverifiedEmail - p.UserIDClaim = o.UserIDClaim - p.GroupsClaim = o.OIDCGroupsClaim if p.Verifier == nil { msgs = append(msgs, "oidc provider requires an oidc issuer URL") } case *providers.GitLabProvider: - p.AllowUnverifiedEmail = o.InsecureOIDCAllowUnverifiedEmail p.Groups = o.GitLabGroup err := p.AddProjects(o.GitlabProjects) if err != nil { diff --git a/providers/gitlab.go b/providers/gitlab.go index 246dd78c..c5922abb 100644 --- a/providers/gitlab.go +++ b/providers/gitlab.go @@ -20,8 +20,6 @@ type GitLabProvider struct { Groups []string Projects []*GitlabProject - - AllowUnverifiedEmail bool } // GitlabProject represents a Gitlab project constraint entity @@ -103,7 +101,7 @@ func (p *GitLabProvider) Redeem(ctx context.Context, redirectURL, code string) ( if err != nil { return nil, fmt.Errorf("token exchange: %v", err) } - s, err = p.createSessionState(ctx, token) + s, err = p.createSession(ctx, token) if err != nil { return nil, fmt.Errorf("unable to update session: %v", err) } @@ -162,7 +160,7 @@ func (p *GitLabProvider) redeemRefreshToken(ctx context.Context, s *sessions.Ses if err != nil { return fmt.Errorf("failed to get token: %v", err) } - newSession, err := p.createSessionState(ctx, token) + newSession, err := p.createSession(ctx, token) if err != nil { return fmt.Errorf("unable to update session: %v", err) } @@ -255,22 +253,21 @@ func (p *GitLabProvider) AddProjects(projects []string) error { return nil } -func (p *GitLabProvider) createSessionState(ctx context.Context, token *oauth2.Token) (*sessions.SessionState, error) { - rawIDToken, ok := token.Extra("id_token").(string) - if !ok { - return nil, fmt.Errorf("token response did not contain an id_token") - } - - // Parse and verify ID Token payload. - idToken, err := p.Verifier.Verify(ctx, rawIDToken) +func (p *GitLabProvider) createSession(ctx context.Context, token *oauth2.Token) (*sessions.SessionState, error) { + idToken, err := p.verifyIDToken(ctx, token) if err != nil { - return nil, fmt.Errorf("could not verify id_token: %v", err) + switch err { + case ErrMissingIDToken: + return nil, fmt.Errorf("token response did not contain an id_token") + default: + return nil, fmt.Errorf("could not verify id_token: %v", err) + } } created := time.Now() return &sessions.SessionState{ AccessToken: token.AccessToken, - IDToken: rawIDToken, + IDToken: getIDToken(token), RefreshToken: token.RefreshToken, CreatedAt: &created, ExpiresOn: &idToken.Expiry, diff --git a/providers/oidc.go b/providers/oidc.go index 704e3341..df133f4d 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -2,29 +2,20 @@ package providers import ( "context" - "encoding/json" + "errors" "fmt" "reflect" - "strings" "time" - oidc "github.com/coreos/go-oidc" - "golang.org/x/oauth2" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" + "golang.org/x/oauth2" ) -const emailClaim = "email" - // OIDCProvider represents an OIDC based Identity Provider type OIDCProvider struct { *ProviderData - - AllowUnverifiedEmail bool - UserIDClaim string - GroupsClaim string } // NewOIDCProvider initiates a new OIDCProvider @@ -36,10 +27,10 @@ func NewOIDCProvider(p *ProviderData) *OIDCProvider { var _ Provider = (*OIDCProvider)(nil) // Redeem exchanges the OAuth2 authentication token for an ID token -func (p *OIDCProvider) Redeem(ctx context.Context, redirectURL, code string) (s *sessions.SessionState, err error) { +func (p *OIDCProvider) Redeem(ctx context.Context, redirectURL, code string) (*sessions.SessionState, error) { clientSecret, err := p.GetClientSecret() if err != nil { - return + return nil, err } c := oauth2.Config{ @@ -52,23 +43,74 @@ func (p *OIDCProvider) Redeem(ctx context.Context, redirectURL, code string) (s } token, err := c.Exchange(ctx, code) if err != nil { - return nil, fmt.Errorf("token exchange: %v", err) + return nil, fmt.Errorf("token exchange failed: %v", err) } - // in the initial exchange the id token is mandatory - idToken, err := p.findVerifiedIDToken(ctx, token) + return p.createSession(ctx, token, false) +} + +// EnrichSession is called after Redeem to allow providers to enrich session fields +// such as User, Email, Groups with provider specific API calls. +func (p *OIDCProvider) EnrichSession(ctx context.Context, s *sessions.SessionState) error { + if p.ProfileURL.String() == "" { + if s.Email == "" { + return errors.New("id_token did not contain an email and profileURL is not defined") + } + return nil + } + + // Try to get missing emails or groups from a profileURL + if s.Email == "" || s.Groups == nil { + err := p.enrichFromProfileURL(ctx, s) + if err != nil { + logger.Errorf("Warning: Profile URL request failed: %v", err) + } + } + + // If a mandatory email wasn't set, error at this point. + if s.Email == "" { + return errors.New("neither the id_token nor the profileURL set an email") + } + return nil +} + +// enrichFromProfileURL enriches a session's Email & Groups via the JSON response of +// an OIDC profile URL +func (p *OIDCProvider) enrichFromProfileURL(ctx context.Context, s *sessions.SessionState) error { + respJSON, err := requests.New(p.ProfileURL.String()). + WithContext(ctx). + WithHeaders(makeOIDCHeader(s.AccessToken)). + Do(). + UnmarshalJSON() if err != nil { - return nil, fmt.Errorf("could not verify id_token: %v", err) - } else if idToken == nil { - return nil, fmt.Errorf("token response did not contain an id_token") + return err } - s, err = p.createSessionState(ctx, token, idToken) - if err != nil { - return nil, fmt.Errorf("unable to update session: %v", err) + email, err := respJSON.Get(p.EmailClaim).String() + if err == nil && s.Email == "" { + s.Email = email } - return + if len(s.Groups) > 0 { + return nil + } + for _, group := range coerceArray(respJSON, p.GroupsClaim) { + formatted, err := formatGroup(group) + if err != nil { + logger.Errorf("Warning: unable to format group of type %s with error %s", + reflect.TypeOf(group), err) + continue + } + s.Groups = append(s.Groups, formatted) + } + + return nil +} + +// ValidateSession checks that the session's IDToken is still valid +func (p *OIDCProvider) ValidateSession(ctx context.Context, s *sessions.SessionState) bool { + _, err := p.Verifier.Verify(ctx, s.IDToken) + return err == nil } // RefreshSessionIfNeeded checks if the session has expired and uses the @@ -83,14 +125,16 @@ func (p *OIDCProvider) RefreshSessionIfNeeded(ctx context.Context, s *sessions.S return false, fmt.Errorf("unable to redeem refresh token: %v", err) } - fmt.Printf("refreshed access token %s (expired on %s)\n", s, s.ExpiresOn) + logger.Printf("refreshed session: %s", s) return true, nil } -func (p *OIDCProvider) redeemRefreshToken(ctx context.Context, s *sessions.SessionState) (err error) { +// redeemRefreshToken uses a RefreshToken with the RedeemURL to refresh the +// Access Token and (probably) the ID Token. +func (p *OIDCProvider) redeemRefreshToken(ctx context.Context, s *sessions.SessionState) error { clientSecret, err := p.GetClientSecret() if err != nil { - return + return err } c := oauth2.Config{ @@ -109,19 +153,14 @@ func (p *OIDCProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sessi return fmt.Errorf("failed to get token: %v", err) } - // in the token refresh response the id_token is optional - idToken, err := p.findVerifiedIDToken(ctx, token) - if err != nil { - return fmt.Errorf("unable to extract id_token from response: %v", err) - } - - newSession, err := p.createSessionState(ctx, token, idToken) + newSession, err := p.createSession(ctx, token, true) if err != nil { return fmt.Errorf("unable create new session state from response: %v", err) } - // It's possible that if the refresh token isn't in the token response the session will not contain an id token - // if it doesn't it's probably better to retain the old one + // It's possible that if the refresh token isn't in the token response the + // session will not contain an id token. + // If it doesn't it's probably better to retain the old one if newSession.IDToken != "" { s.IDToken = newSession.IDToken s.Email = newSession.Email @@ -135,193 +174,62 @@ func (p *OIDCProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sessi s.CreatedAt = newSession.CreatedAt s.ExpiresOn = newSession.ExpiresOn - return -} - -func (p *OIDCProvider) findVerifiedIDToken(ctx context.Context, token *oauth2.Token) (*oidc.IDToken, error) { - - getIDToken := func() (string, bool) { - rawIDToken, _ := token.Extra("id_token").(string) - return rawIDToken, len(strings.TrimSpace(rawIDToken)) > 0 - } - - if rawIDToken, present := getIDToken(); present { - verifiedIDToken, err := p.Verifier.Verify(ctx, rawIDToken) - return verifiedIDToken, err - } - return nil, nil -} - -func (p *OIDCProvider) createSessionState(ctx context.Context, token *oauth2.Token, idToken *oidc.IDToken) (*sessions.SessionState, error) { - - var newSession *sessions.SessionState - - if idToken == nil { - newSession = &sessions.SessionState{} - } else { - var err error - newSession, err = p.createSessionStateInternal(ctx, idToken, token) - if err != nil { - return nil, err - } - } - - created := time.Now() - newSession.AccessToken = token.AccessToken - newSession.RefreshToken = token.RefreshToken - newSession.CreatedAt = &created - newSession.ExpiresOn = &token.Expiry - return newSession, nil + return nil } +// CreateSessionFromToken converts Bearer IDTokens into sessions func (p *OIDCProvider) CreateSessionFromToken(ctx context.Context, token string) (*sessions.SessionState, error) { idToken, err := p.Verifier.Verify(ctx, token) if err != nil { return nil, err } - newSession, err := p.createSessionStateInternal(ctx, idToken, nil) + ss, err := p.buildSessionFromClaims(idToken) if err != nil { return nil, err } - newSession.AccessToken = token - newSession.IDToken = token - newSession.RefreshToken = "" - newSession.ExpiresOn = &idToken.Expiry - - return newSession, nil -} - -func (p *OIDCProvider) createSessionStateInternal(ctx context.Context, idToken *oidc.IDToken, token *oauth2.Token) (*sessions.SessionState, error) { - - newSession := &sessions.SessionState{} - - if idToken == nil { - return newSession, nil + // Allow empty Email in Bearer case since we can't hit the ProfileURL + if ss.Email == "" { + ss.Email = ss.User } - claims, err := p.findClaimsFromIDToken(ctx, idToken, token) + ss.AccessToken = token + ss.IDToken = token + ss.RefreshToken = "" + ss.ExpiresOn = &idToken.Expiry + + return ss, nil +} + +// createSession takes an oauth2.Token and creates a SessionState from it. +// It alters behavior if called from Redeem vs Refresh +func (p *OIDCProvider) createSession(ctx context.Context, token *oauth2.Token, refresh bool) (*sessions.SessionState, error) { + idToken, err := p.verifyIDToken(ctx, token) if err != nil { - return nil, fmt.Errorf("couldn't extract claims from id_token (%v)", err) - } - - if token != nil { - newSession.IDToken = token.Extra("id_token").(string) - } - - newSession.Email = claims.UserID // TODO Rename SessionState.Email to .UserID in the near future - - newSession.User = claims.Subject - newSession.Groups = claims.Groups - newSession.PreferredUsername = claims.PreferredUsername - - verifyEmail := (p.UserIDClaim == emailClaim) && !p.AllowUnverifiedEmail - if verifyEmail && claims.Verified != nil && !*claims.Verified { - return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.UserID) - } - - return newSession, nil -} - -// ValidateSessionState checks that the session's IDToken is still valid -func (p *OIDCProvider) ValidateSession(ctx context.Context, s *sessions.SessionState) bool { - _, err := p.Verifier.Verify(ctx, s.IDToken) - return err == nil -} - -func (p *OIDCProvider) findClaimsFromIDToken(ctx context.Context, idToken *oidc.IDToken, token *oauth2.Token) (*OIDCClaims, error) { - claims := &OIDCClaims{} - - // Extract default claims. - if err := idToken.Claims(&claims); err != nil { - return nil, fmt.Errorf("failed to parse default id_token claims: %v", err) - } - // Extract custom claims. - if err := idToken.Claims(&claims.rawClaims); err != nil { - return nil, fmt.Errorf("failed to parse all id_token claims: %v", err) - } - - userID := claims.rawClaims[p.UserIDClaim] - if userID != nil { - claims.UserID = fmt.Sprint(userID) - } - - claims.Groups = p.extractGroupsFromRawClaims(claims.rawClaims) - - // userID claim was not present or was empty in the ID Token - if claims.UserID == "" { - // BearerToken case, allow empty UserID - // ProfileURL checks below won't work since we don't have an access token - if token == nil { - claims.UserID = claims.Subject - return claims, nil - } - - profileURL := p.ProfileURL.String() - if profileURL == "" || token.AccessToken == "" { - return nil, fmt.Errorf("id_token did not contain user ID claim (%q)", p.UserIDClaim) - } - - // If the userinfo endpoint profileURL is defined, then there is a chance the userinfo - // contents at the profileURL contains the email. - // Make a query to the userinfo endpoint, and attempt to locate the email from there. - respJSON, err := requests.New(profileURL). - WithContext(ctx). - WithHeaders(makeOIDCHeader(token.AccessToken)). - Do(). - UnmarshalJSON() - if err != nil { - return nil, err - } - - userID, err := respJSON.Get(p.UserIDClaim).String() - if err != nil { - return nil, fmt.Errorf("neither id_token nor userinfo endpoint contained user ID claim (%q)", p.UserIDClaim) - } - - claims.UserID = userID - } - - return claims, nil -} - -func (p *OIDCProvider) extractGroupsFromRawClaims(rawClaims map[string]interface{}) []string { - groups := []string{} - - rawGroups, ok := rawClaims[p.GroupsClaim].([]interface{}) - if rawGroups != nil && ok { - for _, rawGroup := range rawGroups { - formattedGroup, err := formatGroup(rawGroup) - if err != nil { - logger.Errorf("unable to format group of type %s with error %s", reflect.TypeOf(rawGroup), err) - continue + switch err { + case ErrMissingIDToken: + // IDToken is mandatory in Redeem but optional in Refresh + if !refresh { + return nil, errors.New("token response did not contain an id_token") } - groups = append(groups, formattedGroup) + default: + return nil, fmt.Errorf("could not verify id_token: %v", err) } } - return groups - -} - -func formatGroup(rawGroup interface{}) (string, error) { - group, ok := rawGroup.(string) - if !ok { - jsonGroup, err := json.Marshal(rawGroup) - if err != nil { - return "", err - } - group = string(jsonGroup) + ss, err := p.buildSessionFromClaims(idToken) + if err != nil { + return nil, err } - return group, nil -} -type OIDCClaims struct { - rawClaims map[string]interface{} - UserID string - Subject string `json:"sub"` - Verified *bool `json:"email_verified"` - PreferredUsername string `json:"preferred_username"` - Groups []string `json:"-"` + ss.AccessToken = token.AccessToken + ss.RefreshToken = token.RefreshToken + ss.IDToken = getIDToken(token) + + created := time.Now() + ss.CreatedAt = &created + ss.ExpiresOn = &token.Expiry + + return ss, nil } diff --git a/providers/oidc_test.go b/providers/oidc_test.go index 54df1b31..7ac98634 100644 --- a/providers/oidc_test.go +++ b/providers/oidc_test.go @@ -2,42 +2,18 @@ package providers import ( "context" - "crypto/rand" - "crypto/rsa" - "encoding/base64" "encoding/json" "errors" - "fmt" "net/http" "net/http/httptest" "net/url" - "strings" "testing" - "time" "github.com/coreos/go-oidc" - "github.com/dgrijalva/jwt-go" - "github.com/stretchr/testify/assert" - "golang.org/x/oauth2" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" + "github.com/stretchr/testify/assert" ) -const accessToken = "access_token" -const refreshToken = "refresh_token" -const clientID = "https://test.myapp.com" -const secret = "secret" - -type idTokenClaims struct { - Name string `json:"name,omitempty"` - Email string `json:"email,omitempty"` - Phone string `json:"phone_number,omitempty"` - Picture string `json:"picture,omitempty"` - Groups interface{} `json:"groups,omitempty"` - OtherGroups interface{} `json:"other_groups,omitempty"` - jwt.StandardClaims -} - type redeemTokenResponse struct { AccessToken string `json:"access_token"` RefreshToken string `json:"refresh_token"` @@ -46,88 +22,12 @@ type redeemTokenResponse struct { IDToken string `json:"id_token,omitempty"` } -var defaultIDToken idTokenClaims = idTokenClaims{ - "Jane Dobbs", - "janed@me.com", - "+4798765432", - "http://mugbook.com/janed/me.jpg", - []string{"test:a", "test:b"}, - []string{"test:c", "test:d"}, - jwt.StandardClaims{ - Audience: "https://test.myapp.com", - ExpiresAt: time.Now().Add(time.Duration(5) * time.Minute).Unix(), - Id: "id-some-id", - IssuedAt: time.Now().Unix(), - Issuer: "https://issuer.example.com", - NotBefore: 0, - Subject: "123456789", - }, -} - -var customGroupClaimIDToken idTokenClaims = idTokenClaims{ - "Jane Dobbs", - "janed@me.com", - "+4798765432", - "http://mugbook.com/janed/me.jpg", - []map[string]interface{}{ - { - "groupId": "Admin Group Id", - "roles": []string{"Admin"}, - }, - }, - []string{"test:c", "test:d"}, - jwt.StandardClaims{ - Audience: "https://test.myapp.com", - ExpiresAt: time.Now().Add(time.Duration(5) * time.Minute).Unix(), - Id: "id-some-id", - IssuedAt: time.Now().Unix(), - Issuer: "https://issuer.example.com", - NotBefore: 0, - Subject: "123456789", - }, -} - -var minimalIDToken idTokenClaims = idTokenClaims{ - "", - "", - "", - "", - []string{}, - []string{}, - jwt.StandardClaims{ - Audience: "https://test.myapp.com", - ExpiresAt: time.Now().Add(time.Duration(5) * time.Minute).Unix(), - Id: "id-some-id", - IssuedAt: time.Now().Unix(), - Issuer: "https://issuer.example.com", - NotBefore: 0, - Subject: "minimal", - }, -} - -type fakeKeySetStub struct{} - -func (fakeKeySetStub) VerifySignature(_ context.Context, jwt string) (payload []byte, err error) { - decodeString, err := base64.RawURLEncoding.DecodeString(strings.Split(jwt, ".")[1]) - if err != nil { - return nil, err - } - tokenClaims := &idTokenClaims{} - err = json.Unmarshal(decodeString, tokenClaims) - - if err != nil || tokenClaims.Id == "this-id-fails-validation" { - return nil, fmt.Errorf("the validation failed for subject [%v]", tokenClaims.Subject) - } - - return decodeString, err -} - func newOIDCProvider(serverURL *url.URL) *OIDCProvider { providerData := &ProviderData{ ProviderName: "oidc", - ClientID: clientID, - ClientSecret: secret, + ClientID: oidcClientID, + ClientSecret: oidcSecret, LoginURL: &url.URL{ Scheme: serverURL.Scheme, Host: serverURL.Host, @@ -144,18 +44,17 @@ func newOIDCProvider(serverURL *url.URL) *OIDCProvider { Scheme: serverURL.Scheme, Host: serverURL.Host, Path: "/api"}, - Scope: "openid profile offline_access", + Scope: "openid profile offline_access", + EmailClaim: "email", + GroupsClaim: "groups", Verifier: oidc.NewVerifier( - "https://issuer.example.com", - fakeKeySetStub{}, - &oidc.Config{ClientID: clientID}, + oidcIssuer, + mockJWKS{}, + &oidc.Config{ClientID: oidcClientID}, ), } - p := &OIDCProvider{ - ProviderData: providerData, - UserIDClaim: "email", - } + p := &OIDCProvider{ProviderData: providerData} return p } @@ -169,22 +68,7 @@ func newOIDCServer(body []byte) (*url.URL, *httptest.Server) { return u, s } -func newSignedTestIDToken(tokenClaims idTokenClaims) (string, error) { - key, _ := rsa.GenerateKey(rand.Reader, 2048) - standardClaims := jwt.NewWithClaims(jwt.SigningMethodRS256, tokenClaims) - return standardClaims.SignedString(key) -} - -func newOauth2Token() *oauth2.Token { - return &oauth2.Token{ - AccessToken: accessToken, - TokenType: "Bearer", - RefreshToken: refreshToken, - Expiry: time.Time{}.Add(time.Duration(5) * time.Second), - } -} - -func newTestSetup(body []byte) (*httptest.Server, *OIDCProvider) { +func newTestOIDCSetup(body []byte) (*httptest.Server, *OIDCProvider) { redeemURL, server := newOIDCServer(body) provider := newOIDCProvider(redeemURL) return server, provider @@ -201,7 +85,7 @@ func TestOIDCProviderRedeem(t *testing.T) { IDToken: idToken, }) - server, provider := newTestSetup(body) + server, provider := newTestOIDCSetup(body) defer server.Close() session, err := provider.Redeem(context.Background(), provider.RedeemURL.String(), "code1234") @@ -224,8 +108,8 @@ func TestOIDCProviderRedeem_custom_userid(t *testing.T) { IDToken: idToken, }) - server, provider := newTestSetup(body) - provider.UserIDClaim = "phone_number" + server, provider := newTestOIDCSetup(body) + provider.EmailClaim = "phone_number" defer server.Close() session, err := provider.Redeem(context.Background(), provider.RedeemURL.String(), "code1234") @@ -233,6 +117,333 @@ func TestOIDCProviderRedeem_custom_userid(t *testing.T) { assert.Equal(t, defaultIDToken.Phone, session.Email) } +func TestOIDCProvider_EnrichSession(t *testing.T) { + testCases := map[string]struct { + ExistingSession *sessions.SessionState + EmailClaim string + GroupsClaim string + ProfileJSON map[string]interface{} + ExpectedError error + ExpectedSession *sessions.SessionState + }{ + "Already Populated": { + ExistingSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: []string{"already", "populated"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + EmailClaim: "email", + GroupsClaim: "groups", + ProfileJSON: map[string]interface{}{ + "email": "new@thing.com", + "groups": []string{"new", "thing"}, + }, + ExpectedError: nil, + ExpectedSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: []string{"already", "populated"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + }, + "Missing Email": { + ExistingSession: &sessions.SessionState{ + User: "missing.email", + Groups: []string{"already", "populated"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + EmailClaim: "email", + GroupsClaim: "groups", + ProfileJSON: map[string]interface{}{ + "email": "found@email.com", + "groups": []string{"new", "thing"}, + }, + ExpectedError: nil, + ExpectedSession: &sessions.SessionState{ + User: "missing.email", + Email: "found@email.com", + Groups: []string{"already", "populated"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + }, + + "Missing Email Only in Profile URL": { + ExistingSession: &sessions.SessionState{ + User: "missing.email", + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + EmailClaim: "email", + GroupsClaim: "groups", + ProfileJSON: map[string]interface{}{ + "email": "found@email.com", + }, + ExpectedError: nil, + ExpectedSession: &sessions.SessionState{ + User: "missing.email", + Email: "found@email.com", + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + }, + "Missing Email with Custom Claim": { + ExistingSession: &sessions.SessionState{ + User: "missing.email", + Groups: []string{"already", "populated"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + EmailClaim: "weird", + GroupsClaim: "groups", + ProfileJSON: map[string]interface{}{ + "weird": "weird@claim.com", + "groups": []string{"new", "thing"}, + }, + ExpectedError: nil, + ExpectedSession: &sessions.SessionState{ + User: "missing.email", + Email: "weird@claim.com", + Groups: []string{"already", "populated"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + }, + "Missing Email not in Profile URL": { + ExistingSession: &sessions.SessionState{ + User: "missing.email", + Groups: []string{"already", "populated"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + EmailClaim: "email", + GroupsClaim: "groups", + ProfileJSON: map[string]interface{}{ + "groups": []string{"new", "thing"}, + }, + ExpectedError: errors.New("neither the id_token nor the profileURL set an email"), + ExpectedSession: &sessions.SessionState{ + User: "missing.email", + Groups: []string{"already", "populated"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + }, + "Missing Groups": { + ExistingSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: nil, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + EmailClaim: "email", + GroupsClaim: "groups", + ProfileJSON: map[string]interface{}{ + "email": "new@thing.com", + "groups": []string{"new", "thing"}, + }, + ExpectedError: nil, + ExpectedSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: []string{"new", "thing"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + }, + "Missing Groups with Complex Groups in Profile URL": { + ExistingSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: nil, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + EmailClaim: "email", + GroupsClaim: "groups", + ProfileJSON: map[string]interface{}{ + "email": "new@thing.com", + "groups": []map[string]interface{}{ + { + "groupId": "Admin Group Id", + "roles": []string{"Admin"}, + }, + }, + }, + ExpectedError: nil, + ExpectedSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: []string{"{\"groupId\":\"Admin Group Id\",\"roles\":[\"Admin\"]}"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + }, + "Missing Groups with Singleton Complex Group in Profile URL": { + ExistingSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: nil, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + EmailClaim: "email", + GroupsClaim: "groups", + ProfileJSON: map[string]interface{}{ + "email": "new@thing.com", + "groups": map[string]interface{}{ + "groupId": "Admin Group Id", + "roles": []string{"Admin"}, + }, + }, + ExpectedError: nil, + ExpectedSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: []string{"{\"groupId\":\"Admin Group Id\",\"roles\":[\"Admin\"]}"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + }, + "Empty Groups Claims": { + ExistingSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: []string{}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + EmailClaim: "email", + GroupsClaim: "groups", + ProfileJSON: map[string]interface{}{ + "email": "new@thing.com", + "groups": []string{"new", "thing"}, + }, + ExpectedError: nil, + ExpectedSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: []string{}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + }, + "Missing Groups with Custom Claim": { + ExistingSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: nil, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + EmailClaim: "email", + GroupsClaim: "roles", + ProfileJSON: map[string]interface{}{ + "email": "new@thing.com", + "roles": []string{"new", "thing", "roles"}, + }, + ExpectedError: nil, + ExpectedSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: []string{"new", "thing", "roles"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + }, + "Missing Groups String Profile URL Response": { + ExistingSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: nil, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + EmailClaim: "email", + GroupsClaim: "groups", + ProfileJSON: map[string]interface{}{ + "email": "new@thing.com", + "groups": "singleton", + }, + ExpectedError: nil, + ExpectedSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + Groups: []string{"singleton"}, + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + }, + "Missing Groups in both Claims and Profile URL": { + ExistingSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + EmailClaim: "email", + GroupsClaim: "groups", + ProfileJSON: map[string]interface{}{ + "email": "new@thing.com", + }, + ExpectedError: nil, + ExpectedSession: &sessions.SessionState{ + User: "already", + Email: "already@populated.com", + IDToken: idToken, + AccessToken: accessToken, + RefreshToken: refreshToken, + }, + }, + } + for testName, tc := range testCases { + t.Run(testName, func(t *testing.T) { + jsonResp, err := json.Marshal(tc.ProfileJSON) + assert.NoError(t, err) + + server, provider := newTestOIDCSetup(jsonResp) + provider.ProfileURL, err = url.Parse(server.URL) + assert.NoError(t, err) + + provider.EmailClaim = tc.EmailClaim + provider.GroupsClaim = tc.GroupsClaim + defer server.Close() + + err = provider.EnrichSession(context.Background(), tc.ExistingSession) + assert.Equal(t, tc.ExpectedError, err) + assert.Equal(t, *tc.ExpectedSession, *tc.ExistingSession) + }) + } +} + func TestOIDCProviderRefreshSessionIfNeededWithoutIdToken(t *testing.T) { idToken, _ := newSignedTestIDToken(defaultIDToken) @@ -243,7 +454,7 @@ func TestOIDCProviderRefreshSessionIfNeededWithoutIdToken(t *testing.T) { RefreshToken: refreshToken, }) - server, provider := newTestSetup(body) + server, provider := newTestOIDCSetup(body) defer server.Close() existingSession := &sessions.SessionState{ @@ -277,7 +488,7 @@ func TestOIDCProviderRefreshSessionIfNeededWithIdToken(t *testing.T) { IDToken: idToken, }) - server, provider := newTestSetup(body) + server, provider := newTestOIDCSetup(body) defer server.Close() existingSession := &sessions.SessionState{ @@ -300,48 +511,45 @@ func TestOIDCProviderRefreshSessionIfNeededWithIdToken(t *testing.T) { } func TestOIDCProviderCreateSessionFromToken(t *testing.T) { - const profileURLEmail = "janed@me.com" - testCases := map[string]struct { IDToken idTokenClaims GroupsClaim string ExpectedUser string ExpectedEmail string - ExpectedGroups interface{} + ExpectedGroups []string }{ "Default IDToken": { IDToken: defaultIDToken, GroupsClaim: "groups", - ExpectedUser: defaultIDToken.Subject, - ExpectedEmail: defaultIDToken.Email, + ExpectedUser: "123456789", + ExpectedEmail: "janed@me.com", ExpectedGroups: []string{"test:a", "test:b"}, }, "Minimal IDToken with no email claim": { IDToken: minimalIDToken, GroupsClaim: "groups", - ExpectedUser: minimalIDToken.Subject, - ExpectedEmail: minimalIDToken.Subject, - ExpectedGroups: []string{}, + ExpectedUser: "123456789", + ExpectedEmail: "123456789", + ExpectedGroups: nil, }, "Custom Groups Claim": { IDToken: defaultIDToken, - GroupsClaim: "other_groups", - ExpectedUser: defaultIDToken.Subject, - ExpectedEmail: defaultIDToken.Email, + GroupsClaim: "roles", + ExpectedUser: "123456789", + ExpectedEmail: "janed@me.com", ExpectedGroups: []string{"test:c", "test:d"}, }, - "Custom Groups Claim2": { - IDToken: customGroupClaimIDToken, + "Complex Groups Claim": { + IDToken: complexGroupsIDToken, GroupsClaim: "groups", - ExpectedUser: customGroupClaimIDToken.Subject, - ExpectedEmail: customGroupClaimIDToken.Email, + ExpectedUser: "123456789", + ExpectedEmail: "complex@claims.com", ExpectedGroups: []string{"{\"groupId\":\"Admin Group Id\",\"roles\":[\"Admin\"]}"}, }, } for testName, tc := range testCases { t.Run(testName, func(t *testing.T) { - jsonResp := []byte(fmt.Sprintf(`{"email":"%s"}`, profileURLEmail)) - server, provider := newTestSetup(jsonResp) + server, provider := newTestOIDCSetup([]byte(`{}`)) provider.GroupsClaim = tc.GroupsClaim defer server.Close() @@ -353,75 +561,10 @@ func TestOIDCProviderCreateSessionFromToken(t *testing.T) { assert.Equal(t, tc.ExpectedUser, ss.User) assert.Equal(t, tc.ExpectedEmail, ss.Email) + assert.Equal(t, tc.ExpectedGroups, ss.Groups) assert.Equal(t, rawIDToken, ss.IDToken) assert.Equal(t, rawIDToken, ss.AccessToken) - assert.Equal(t, tc.ExpectedGroups, ss.Groups) assert.Equal(t, "", ss.RefreshToken) }) } } - -func TestOIDCProvider_findVerifiedIdToken(t *testing.T) { - - server, provider := newTestSetup([]byte("")) - - defer server.Close() - - token := newOauth2Token() - signedIDToken, _ := newSignedTestIDToken(defaultIDToken) - tokenWithIDToken := token.WithExtra(map[string]interface{}{ - "id_token": signedIDToken, - }) - - verifiedIDToken, err := provider.findVerifiedIDToken(context.Background(), tokenWithIDToken) - assert.Equal(t, true, err == nil) - if verifiedIDToken == nil { - t.Fatal("verifiedIDToken is nil") - } - assert.Equal(t, defaultIDToken.Issuer, verifiedIDToken.Issuer) - assert.Equal(t, defaultIDToken.Subject, verifiedIDToken.Subject) - - // When the validation fails the response should be nil - defaultIDToken.Id = "this-id-fails-validation" - signedIDToken, _ = newSignedTestIDToken(defaultIDToken) - tokenWithIDToken = token.WithExtra(map[string]interface{}{ - "id_token": signedIDToken, - }) - - verifiedIDToken, err = provider.findVerifiedIDToken(context.Background(), tokenWithIDToken) - assert.Equal(t, errors.New("failed to verify signature: the validation failed for subject [123456789]"), err) - assert.Equal(t, true, verifiedIDToken == nil) - - // When there is no id token in the oauth token - verifiedIDToken, err = provider.findVerifiedIDToken(context.Background(), newOauth2Token()) - assert.Equal(t, nil, err) - assert.Equal(t, true, verifiedIDToken == nil) -} - -func Test_formatGroup(t *testing.T) { - testCases := map[string]struct { - RawGroup interface{} - ExpectedFormattedGroupValue string - }{ - "String Group": { - RawGroup: "group", - ExpectedFormattedGroupValue: "group", - }, - "Map Group": { - RawGroup: map[string]string{"id": "1", "name": "Test"}, - ExpectedFormattedGroupValue: "{\"id\":\"1\",\"name\":\"Test\"}", - }, - "List Group": { - RawGroup: []string{"First", "Second"}, - ExpectedFormattedGroupValue: "[\"First\",\"Second\"]", - }, - } - - for testName, tc := range testCases { - t.Run(testName, func(t *testing.T) { - formattedGroup, err := formatGroup(tc.RawGroup) - assert.Nil(t, err) - assert.Equal(t, tc.ExpectedFormattedGroupValue, formattedGroup) - }) - } -} diff --git a/providers/provider_data.go b/providers/provider_data.go index 330df7ca..a9a41232 100644 --- a/providers/provider_data.go +++ b/providers/provider_data.go @@ -1,12 +1,23 @@ package providers import ( + "context" "errors" + "fmt" "io/ioutil" "net/url" + "reflect" + "strings" "github.com/coreos/go-oidc" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" + "golang.org/x/oauth2" +) + +const ( + OIDCEmailClaim = "email" + OIDCGroupsClaim = "groups" ) // ProviderData contains information required to configure all implementations @@ -27,7 +38,12 @@ type ProviderData struct { ClientSecretFile string Scope string Prompt string - Verifier *oidc.IDTokenVerifier + + // Common OIDC options for any OIDC-based providers to consume + AllowUnverifiedEmail bool + EmailClaim string + GroupsClaim string + Verifier *oidc.IDTokenVerifier // Universal Group authorization data structure // any provider can set to consume @@ -94,3 +110,116 @@ func defaultURL(u *url.URL, d *url.URL) *url.URL { } return &url.URL{} } + +// **************************************************************************** +// These private OIDC helper methods are available to any providers that are +// OIDC compliant +// **************************************************************************** + +// OIDCClaims is a struct to unmarshal the OIDC claims from an ID Token payload +type OIDCClaims struct { + Subject string `json:"sub"` + Email string `json:"-"` + Groups []string `json:"-"` + Verified *bool `json:"email_verified"` + + raw map[string]interface{} +} + +func (p *ProviderData) verifyIDToken(ctx context.Context, token *oauth2.Token) (*oidc.IDToken, error) { + rawIDToken := getIDToken(token) + if strings.TrimSpace(rawIDToken) == "" { + return nil, ErrMissingIDToken + } + if p.Verifier == nil { + return nil, ErrMissingOIDCVerifier + } + return p.Verifier.Verify(ctx, rawIDToken) +} + +// buildSessionFromClaims uses IDToken claims to populate a fresh SessionState +// with non-Token related fields. +func (p *ProviderData) buildSessionFromClaims(idToken *oidc.IDToken) (*sessions.SessionState, error) { + ss := &sessions.SessionState{} + + if idToken == nil { + return ss, nil + } + + claims, err := p.getClaims(idToken) + if err != nil { + return nil, fmt.Errorf("couldn't extract claims from id_token (%v)", err) + } + + ss.User = claims.Subject + ss.Email = claims.Email + ss.Groups = claims.Groups + + // TODO (@NickMeves) Deprecate for dynamic claim to session mapping + if pref, ok := claims.raw["preferred_username"].(string); ok { + ss.PreferredUsername = pref + } + + // `email_verified` must be present and explicitly set to `false` to be + // considered unverified. + verifyEmail := (p.EmailClaim == OIDCEmailClaim) && !p.AllowUnverifiedEmail + if verifyEmail && claims.Verified != nil && !*claims.Verified { + return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email) + } + + return ss, nil +} + +// getClaims extracts IDToken claims into an OIDCClaims +func (p *ProviderData) getClaims(idToken *oidc.IDToken) (*OIDCClaims, error) { + claims := &OIDCClaims{} + + // Extract default claims. + if err := idToken.Claims(&claims); err != nil { + return nil, fmt.Errorf("failed to parse default id_token claims: %v", err) + } + // Extract custom claims. + if err := idToken.Claims(&claims.raw); err != nil { + return nil, fmt.Errorf("failed to parse all id_token claims: %v", err) + } + + email := claims.raw[p.EmailClaim] + if email != nil { + claims.Email = fmt.Sprint(email) + } + claims.Groups = p.extractGroups(claims.raw) + + return claims, nil +} + +// extractGroups extracts groups from a claim to a list in a type safe manner. +// If the claim isn't present, `nil` is returned. If the groups claim is +// present but empty, `[]string{}` is returned. +func (p *ProviderData) extractGroups(claims map[string]interface{}) []string { + rawClaim, ok := claims[p.GroupsClaim] + if !ok { + return nil + } + + // Handle traditional list-based groups as well as non-standard singleton + // based groups. Both variants support complex objects if needed. + var claimGroups []interface{} + switch raw := rawClaim.(type) { + case []interface{}: + claimGroups = raw + case interface{}: + claimGroups = []interface{}{raw} + } + + groups := []string{} + for _, rawGroup := range claimGroups { + formattedGroup, err := formatGroup(rawGroup) + if err != nil { + logger.Errorf("Warning: unable to format group of type %s with error %s", + reflect.TypeOf(rawGroup), err) + continue + } + groups = append(groups, formattedGroup) + } + return groups +} diff --git a/providers/provider_data_test.go b/providers/provider_data_test.go new file mode 100644 index 00000000..80f6ecab --- /dev/null +++ b/providers/provider_data_test.go @@ -0,0 +1,437 @@ +package providers + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "strings" + "testing" + "time" + + "github.com/coreos/go-oidc" + "github.com/dgrijalva/jwt-go" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" + . "github.com/onsi/gomega" + "golang.org/x/oauth2" +) + +const ( + idToken = "eyJfoobar123.eyJbaz987.IDToken" + accessToken = "eyJfoobar123.eyJbaz987.AccessToken" + refreshToken = "eyJfoobar123.eyJbaz987.RefreshToken" + + oidcIssuer = "https://issuer.example.com" + oidcClientID = "https://test.myapp.com" + oidcSecret = "SuperSecret123456789" + + failureTokenID = "this-id-fails-verification" +) + +var ( + verified = true + unverified = false + + standardClaims = jwt.StandardClaims{ + Audience: oidcClientID, + ExpiresAt: time.Now().Add(time.Duration(5) * time.Minute).Unix(), + Id: "id-some-id", + IssuedAt: time.Now().Unix(), + Issuer: oidcIssuer, + NotBefore: 0, + Subject: "123456789", + } + + defaultIDToken = idTokenClaims{ + Name: "Jane Dobbs", + Email: "janed@me.com", + Phone: "+4798765432", + Picture: "http://mugbook.com/janed/me.jpg", + Groups: []string{"test:a", "test:b"}, + Roles: []string{"test:c", "test:d"}, + Verified: &verified, + StandardClaims: standardClaims, + } + + complexGroupsIDToken = idTokenClaims{ + Name: "Complex Claim", + Email: "complex@claims.com", + Phone: "+5439871234", + Picture: "http://mugbook.com/complex/claims.jpg", + Groups: []map[string]interface{}{ + { + "groupId": "Admin Group Id", + "roles": []string{"Admin"}, + }, + }, + Roles: []string{"test:simple", "test:roles"}, + Verified: &verified, + StandardClaims: standardClaims, + } + + unverifiedIDToken = idTokenClaims{ + Name: "Mystery Man", + Email: "unverified@email.com", + Phone: "+4025205729", + Picture: "http://mugbook.com/unverified/email.jpg", + Groups: []string{"test:a", "test:b"}, + Roles: []string{"test:c", "test:d"}, + Verified: &unverified, + StandardClaims: standardClaims, + } + + minimalIDToken = idTokenClaims{ + StandardClaims: standardClaims, + } +) + +type idTokenClaims struct { + Name string `json:"preferred_username,omitempty"` + Email string `json:"email,omitempty"` + Phone string `json:"phone_number,omitempty"` + Picture string `json:"picture,omitempty"` + Groups interface{} `json:"groups,omitempty"` + Roles interface{} `json:"roles,omitempty"` + Verified *bool `json:"email_verified,omitempty"` + jwt.StandardClaims +} + +type mockJWKS struct{} + +func (mockJWKS) VerifySignature(_ context.Context, jwt string) ([]byte, error) { + decoded, err := base64.RawURLEncoding.DecodeString(strings.Split(jwt, ".")[1]) + if err != nil { + return nil, err + } + + tokenClaims := &idTokenClaims{} + err = json.Unmarshal(decoded, tokenClaims) + if err != nil || tokenClaims.Id == failureTokenID { + return nil, fmt.Errorf("the validation failed for subject [%v]", tokenClaims.Subject) + } + + return decoded, nil +} + +func newSignedTestIDToken(tokenClaims idTokenClaims) (string, error) { + key, _ := rsa.GenerateKey(rand.Reader, 2048) + standardClaims := jwt.NewWithClaims(jwt.SigningMethodRS256, tokenClaims) + return standardClaims.SignedString(key) +} + +func newTestOauth2Token() *oauth2.Token { + return &oauth2.Token{ + AccessToken: accessToken, + TokenType: "Bearer", + RefreshToken: refreshToken, + Expiry: time.Time{}.Add(time.Duration(5) * time.Second), + } +} + +func TestProviderData_verifyIDToken(t *testing.T) { + failureIDToken := defaultIDToken + failureIDToken.Id = failureTokenID + + testCases := map[string]struct { + IDToken *idTokenClaims + Verifier bool + ExpectIDToken bool + ExpectedError error + }{ + "Valid ID Token": { + IDToken: &defaultIDToken, + Verifier: true, + ExpectIDToken: true, + ExpectedError: nil, + }, + "Invalid ID Token": { + IDToken: &failureIDToken, + Verifier: true, + ExpectIDToken: false, + ExpectedError: errors.New("failed to verify signature: the validation failed for subject [123456789]"), + }, + "Missing ID Token": { + IDToken: nil, + Verifier: true, + ExpectIDToken: false, + ExpectedError: ErrMissingIDToken, + }, + "OIDC Verifier not Configured": { + IDToken: &defaultIDToken, + Verifier: false, + ExpectIDToken: false, + ExpectedError: ErrMissingOIDCVerifier, + }, + } + + for testName, tc := range testCases { + t.Run(testName, func(t *testing.T) { + g := NewWithT(t) + + token := newTestOauth2Token() + if tc.IDToken != nil { + idToken, err := newSignedTestIDToken(*tc.IDToken) + g.Expect(err).ToNot(HaveOccurred()) + token = token.WithExtra(map[string]interface{}{ + "id_token": idToken, + }) + } + + provider := &ProviderData{} + if tc.Verifier { + provider.Verifier = oidc.NewVerifier( + oidcIssuer, + mockJWKS{}, + &oidc.Config{ClientID: oidcClientID}, + ) + } + verified, err := provider.verifyIDToken(context.Background(), token) + if err != nil { + g.Expect(err).To(Equal(tc.ExpectedError)) + } + + if tc.ExpectIDToken { + g.Expect(verified).ToNot(BeNil()) + g.Expect(*verified).To(BeAssignableToTypeOf(oidc.IDToken{})) + } else { + g.Expect(verified).To(BeNil()) + } + }) + } +} + +func TestProviderData_buildSessionFromClaims(t *testing.T) { + testCases := map[string]struct { + IDToken idTokenClaims + AllowUnverified bool + EmailClaim string + GroupsClaim string + ExpectedError error + ExpectedSession *sessions.SessionState + }{ + "Standard": { + IDToken: defaultIDToken, + AllowUnverified: false, + EmailClaim: "email", + GroupsClaim: "groups", + ExpectedSession: &sessions.SessionState{ + User: "123456789", + Email: "janed@me.com", + Groups: []string{"test:a", "test:b"}, + PreferredUsername: "Jane Dobbs", + }, + }, + "Unverified Denied": { + IDToken: unverifiedIDToken, + AllowUnverified: false, + EmailClaim: "email", + GroupsClaim: "groups", + ExpectedError: errors.New("email in id_token (unverified@email.com) isn't verified"), + }, + "Unverified Allowed": { + IDToken: unverifiedIDToken, + AllowUnverified: true, + EmailClaim: "email", + GroupsClaim: "groups", + ExpectedSession: &sessions.SessionState{ + User: "123456789", + Email: "unverified@email.com", + Groups: []string{"test:a", "test:b"}, + PreferredUsername: "Mystery Man", + }, + }, + "Complex Groups": { + IDToken: complexGroupsIDToken, + AllowUnverified: true, + EmailClaim: "email", + GroupsClaim: "groups", + ExpectedSession: &sessions.SessionState{ + User: "123456789", + Email: "complex@claims.com", + Groups: []string{"{\"groupId\":\"Admin Group Id\",\"roles\":[\"Admin\"]}"}, + PreferredUsername: "Complex Claim", + }, + }, + "Email Claim Switched": { + IDToken: unverifiedIDToken, + AllowUnverified: true, + EmailClaim: "phone_number", + GroupsClaim: "groups", + ExpectedSession: &sessions.SessionState{ + User: "123456789", + Email: "+4025205729", + Groups: []string{"test:a", "test:b"}, + PreferredUsername: "Mystery Man", + }, + }, + "Email Claim Switched to Non String": { + IDToken: unverifiedIDToken, + AllowUnverified: true, + EmailClaim: "roles", + GroupsClaim: "groups", + ExpectedSession: &sessions.SessionState{ + User: "123456789", + Email: "[test:c test:d]", + Groups: []string{"test:a", "test:b"}, + PreferredUsername: "Mystery Man", + }, + }, + "Email Claim Non Existent": { + IDToken: unverifiedIDToken, + AllowUnverified: true, + EmailClaim: "aksjdfhjksadh", + GroupsClaim: "groups", + ExpectedSession: &sessions.SessionState{ + User: "123456789", + Email: "", + Groups: []string{"test:a", "test:b"}, + PreferredUsername: "Mystery Man", + }, + }, + "Groups Claim Switched": { + IDToken: defaultIDToken, + AllowUnverified: false, + EmailClaim: "email", + GroupsClaim: "roles", + ExpectedSession: &sessions.SessionState{ + User: "123456789", + Email: "janed@me.com", + Groups: []string{"test:c", "test:d"}, + PreferredUsername: "Jane Dobbs", + }, + }, + "Groups Claim Non Existent": { + IDToken: defaultIDToken, + AllowUnverified: false, + EmailClaim: "email", + GroupsClaim: "alskdjfsalkdjf", + ExpectedSession: &sessions.SessionState{ + User: "123456789", + Email: "janed@me.com", + Groups: nil, + PreferredUsername: "Jane Dobbs", + }, + }, + } + for testName, tc := range testCases { + t.Run(testName, func(t *testing.T) { + g := NewWithT(t) + + provider := &ProviderData{ + Verifier: oidc.NewVerifier( + oidcIssuer, + mockJWKS{}, + &oidc.Config{ClientID: oidcClientID}, + ), + } + provider.AllowUnverifiedEmail = tc.AllowUnverified + provider.EmailClaim = tc.EmailClaim + provider.GroupsClaim = tc.GroupsClaim + + rawIDToken, err := newSignedTestIDToken(tc.IDToken) + g.Expect(err).ToNot(HaveOccurred()) + + idToken, err := provider.Verifier.Verify(context.Background(), rawIDToken) + g.Expect(err).ToNot(HaveOccurred()) + + ss, err := provider.buildSessionFromClaims(idToken) + if err != nil { + g.Expect(err).To(Equal(tc.ExpectedError)) + } + if ss != nil { + g.Expect(ss).To(Equal(tc.ExpectedSession)) + } + }) + } +} + +func TestProviderData_extractGroups(t *testing.T) { + testCases := map[string]struct { + Claims map[string]interface{} + GroupsClaim string + ExpectedGroups []string + }{ + "Standard String Groups": { + Claims: map[string]interface{}{ + "email": "this@does.not.matter.com", + "groups": []interface{}{"three", "string", "groups"}, + }, + GroupsClaim: "groups", + ExpectedGroups: []string{"three", "string", "groups"}, + }, + "Different Claim Name": { + Claims: map[string]interface{}{ + "email": "this@does.not.matter.com", + "roles": []interface{}{"three", "string", "roles"}, + }, + GroupsClaim: "roles", + ExpectedGroups: []string{"three", "string", "roles"}, + }, + "Numeric Groups": { + Claims: map[string]interface{}{ + "email": "this@does.not.matter.com", + "groups": []interface{}{1, 2, 3}, + }, + GroupsClaim: "groups", + ExpectedGroups: []string{"1", "2", "3"}, + }, + "Complex Groups": { + Claims: map[string]interface{}{ + "email": "this@does.not.matter.com", + "groups": []interface{}{ + map[string]interface{}{ + "groupId": "Admin Group Id", + "roles": []string{"Admin"}, + }, + 12345, + "Just::A::String", + }, + }, + GroupsClaim: "groups", + ExpectedGroups: []string{ + "{\"groupId\":\"Admin Group Id\",\"roles\":[\"Admin\"]}", + "12345", + "Just::A::String", + }, + }, + "Missing Groups Claim Returns Nil": { + Claims: map[string]interface{}{ + "email": "this@does.not.matter.com", + }, + GroupsClaim: "groups", + ExpectedGroups: nil, + }, + "Non List Groups": { + Claims: map[string]interface{}{ + "email": "this@does.not.matter.com", + "groups": "singleton", + }, + GroupsClaim: "groups", + ExpectedGroups: []string{"singleton"}, + }, + } + for testName, tc := range testCases { + t.Run(testName, func(t *testing.T) { + g := NewWithT(t) + + provider := &ProviderData{ + Verifier: oidc.NewVerifier( + oidcIssuer, + mockJWKS{}, + &oidc.Config{ClientID: oidcClientID}, + ), + } + provider.GroupsClaim = tc.GroupsClaim + + groups := provider.extractGroups(tc.Claims) + if tc.ExpectedGroups != nil { + g.Expect(groups).To(Equal(tc.ExpectedGroups)) + } else { + g.Expect(groups).To(BeNil()) + } + }) + } +} diff --git a/providers/provider_default.go b/providers/provider_default.go index 012a538c..d3c6d113 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -22,6 +22,14 @@ var ( // code ErrMissingCode = errors.New("missing code") + // ErrMissingIDToken is returned when an oidc.Token does not contain the + // extra `id_token` field for an IDToken. + ErrMissingIDToken = errors.New("missing id_token") + + // ErrMissingOIDCVerifier is returned when a provider didn't set `Verifier` + // but an attempt to call `Verifier.Verify` was about to be made. + ErrMissingOIDCVerifier = errors.New("oidc verifier is not configured") + _ Provider = (*ProviderData)(nil) ) diff --git a/providers/util.go b/providers/util.go index b4b65ac6..e6fdc344 100644 --- a/providers/util.go +++ b/providers/util.go @@ -1,9 +1,13 @@ package providers import ( + "encoding/json" "fmt" "net/http" "net/url" + + "github.com/bitly/go-simplejson" + "golang.org/x/oauth2" ) const ( @@ -55,3 +59,42 @@ func makeLoginURL(p *ProviderData, redirectURI, state string, extraParams url.Va a.RawQuery = params.Encode() return a } + +// getIDToken extracts an IDToken stored in the `Extra` fields of an +// oauth2.Token +func getIDToken(token *oauth2.Token) string { + idToken, ok := token.Extra("id_token").(string) + if !ok { + return "" + } + return idToken +} + +// formatGroup coerces an OIDC groups claim into a string +// If it is non-string, marshal it into JSON. +func formatGroup(rawGroup interface{}) (string, error) { + if group, ok := rawGroup.(string); ok { + return group, nil + } + + jsonGroup, err := json.Marshal(rawGroup) + if err != nil { + return "", err + } + return string(jsonGroup), nil +} + +// coerceArray extracts a field from simplejson.Json that might be a +// singleton or a list and coerces it into a list. +func coerceArray(sj *simplejson.Json, key string) []interface{} { + array, err := sj.Get(key).Array() + if err == nil { + return array + } + + single := sj.Get(key).Interface() + if single == nil { + return nil + } + return []interface{}{single} +} diff --git a/providers/util_test.go b/providers/util_test.go index 798df6cb..e14ff061 100644 --- a/providers/util_test.go +++ b/providers/util_test.go @@ -5,9 +5,10 @@ import ( "testing" . "github.com/onsi/gomega" + "golang.org/x/oauth2" ) -func TestMakeAuhtorizationHeader(t *testing.T) { +func Test_makeAuthorizationHeader(t *testing.T) { testCases := []struct { name string prefix string @@ -64,3 +65,49 @@ func TestMakeAuhtorizationHeader(t *testing.T) { }) } } + +func Test_getIDToken(t *testing.T) { + const idToken = "eyJfoobar.eyJfoobar.12345asdf" + g := NewWithT(t) + + token := &oauth2.Token{} + g.Expect(getIDToken(token)).To(Equal("")) + + extraToken := token.WithExtra(map[string]interface{}{ + "id_token": idToken, + }) + g.Expect(getIDToken(extraToken)).To(Equal(idToken)) +} + +func Test_formatGroup(t *testing.T) { + testCases := map[string]struct { + rawGroup interface{} + expected string + }{ + "String Group": { + rawGroup: "group", + expected: "group", + }, + "Numeric Group": { + rawGroup: 123, + expected: "123", + }, + "Map Group": { + rawGroup: map[string]string{"id": "1", "name": "Test"}, + expected: "{\"id\":\"1\",\"name\":\"Test\"}", + }, + "List Group": { + rawGroup: []string{"First", "Second"}, + expected: "[\"First\",\"Second\"]", + }, + } + + for testName, tc := range testCases { + t.Run(testName, func(t *testing.T) { + g := NewWithT(t) + formattedGroup, err := formatGroup(tc.rawGroup) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(formattedGroup).To(Equal(tc.expected)) + }) + } +}