diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f6065b4..74c272eb 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,7 @@ - [#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) +- [#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 652ada9e..d5e58312 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -235,10 +235,17 @@ func parseProviderInfo(o *options.Options, msgs []string) []string { // Make the OIDC options available to all providers that support it p.AllowUnverifiedEmail = o.InsecureOIDCAllowUnverifiedEmail - p.EmailClaim = o.UserIDClaim + 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) @@ -276,9 +283,6 @@ func parseProviderInfo(o *options.Options, msgs []string) []string { p.SetTeam(o.BitbucketTeam) p.SetRepository(o.BitbucketRepository) case *providers.OIDCProvider: - p.AllowUnverifiedEmail = o.InsecureOIDCAllowUnverifiedEmail - p.EmailClaim = o.UserIDClaim - p.GroupsClaim = o.OIDCGroupsClaim if p.Verifier == nil { msgs = append(msgs, "oidc provider requires an oidc issuer URL") } diff --git a/providers/oidc.go b/providers/oidc.go index f90348d6..d7d34700 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -42,7 +42,7 @@ 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 failure: %v", err) + return nil, fmt.Errorf("token exchange failed: %v", err) } return p.createSession(ctx, token, false) diff --git a/providers/provider_data.go b/providers/provider_data.go index 09eadd25..ae434515 100644 --- a/providers/provider_data.go +++ b/providers/provider_data.go @@ -15,6 +15,11 @@ import ( "golang.org/x/oauth2" ) +const ( + OIDCEmailClaim = "email" + OIDCGroupsClaim = "groups" +) + // ProviderData contains information required to configure all implementations // of OAuth2 providers type ProviderData struct { @@ -154,7 +159,7 @@ func (p *ProviderData) buildSessionFromClaims(idToken *oidc.IDToken) (*sessions. // `email_verified` must be present and explicitly set to `false` to be // considered unverified. - verifyEmail := (p.EmailClaim == emailClaim) && !p.AllowUnverifiedEmail + 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) } diff --git a/providers/provider_default.go b/providers/provider_default.go index 69dddb06..012a538c 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -13,8 +13,6 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" ) -const emailClaim = "email" - var ( // ErrNotImplemented is returned when a provider did not override a default // implementation method that doesn't have sensible defaults