OIDC Provider Refactor

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)

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://<status_code>` 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 | |

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())

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 {

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,