diff --git a/CHANGELOG.md b/CHANGELOG.md index 788e82c2..61328bee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ## Changes since v7.15.3 +- [#3455](https://github.com/oauth2-proxy/oauth2-proxy/pull/3455) feat: allow configuring an explicit JWKS URL per extra JWT issuer via `--extra-jwt-issuer-jwks-url` (fixes AD FS-style issuers whose JWKS is not served at `/.well-known/jwks.json`) + # V7.15.3 ## Release Highlights diff --git a/docs/docs/configuration/overview.md b/docs/docs/configuration/overview.md index 965953fa..77908c0b 100644 --- a/docs/docs/configuration/overview.md +++ b/docs/docs/configuration/overview.md @@ -206,6 +206,7 @@ When `--reverse-proxy` is enabled, configure `--trusted-proxy-ip` to the IPs or | flag: `--email-domain`
toml: `email_domains` | string \| list | authenticate emails with the specified domain (may be given multiple times). Use `*` to authenticate any email | | | flag: `--encode-state`
toml: `encode_state` | bool | encode the state parameter as UrlEncodedBase64 | false | | flag: `--extra-jwt-issuers`
toml: `extra_jwt_issuers` | string | if `--skip-jwt-bearer-tokens` is set, a list of extra JWT `issuer=audience` (see a token's `iss`, `aud` fields) pairs (where the issuer URL has a `.well-known/openid-configuration` or a `.well-known/jwks.json`) | | +| flag: `--extra-jwt-issuer-jwks-url`
toml: `extra_jwt_issuer_jwks_urls` | string | for an extra JWT issuer whose JWKS is not served at `/.well-known/jwks.json` (e.g. AD FS, where the `iss` is `http://host/adfs/services/trust` but the keys live at `https://host/adfs/discovery/keys`), an explicit `issuer=jwksURL` pair to use instead of discovery (may be given multiple times) | | | flag: `--force-https`
toml: `force_https` | bool | enforce https redirect | `false` | | flag: `--force-json-errors`
toml: `force_json_errors` | bool | force JSON errors instead of HTTP error pages or redirects | `false` | | flag: `--htpasswd-file`
toml: `htpasswd_file` | string | additionally authenticate against a htpasswd file. Entries must be created with `htpasswd -B` for bcrypt encryption | | diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index ac2b13c8..ac0e785f 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -59,6 +59,7 @@ type Options struct { SkipJwtBearerTokens bool `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens"` BearerTokenLoginFallback bool `flag:"bearer-token-login-fallback" cfg:"bearer_token_login_fallback"` ExtraJwtIssuers []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers"` + ExtraJwtIssuerJWKsURLs []string `flag:"extra-jwt-issuer-jwks-url" cfg:"extra_jwt_issuer_jwks_urls"` SkipProviderButton bool `flag:"skip-provider-button" cfg:"skip_provider_button"` SSLInsecureSkipVerify bool `flag:"ssl-insecure-skip-verify" cfg:"ssl_insecure_skip_verify"` SkipAuthPreflight bool `flag:"skip-auth-preflight" cfg:"skip_auth_preflight"` @@ -137,6 +138,7 @@ func NewFlagSet() *pflag.FlagSet { flagSet.Bool("encode-state", false, "will encode oauth state with base64") flagSet.Bool("allow-query-semicolons", false, "allow the use of semicolons in query args") flagSet.StringSlice("extra-jwt-issuers", []string{}, "if skip-jwt-bearer-tokens is set, a list of extra JWT issuer=audience pairs (where the issuer URL has a .well-known/openid-configuration or a .well-known/jwks.json)") + flagSet.StringSlice("extra-jwt-issuer-jwks-url", []string{}, "for an extra JWT issuer whose JWKS is not served at /.well-known/jwks.json (e.g. AD FS), an explicit issuer=jwksURL pair to use instead of discovery (may be given multiple times)") flagSet.StringSlice("email-domain", []string{}, "authenticate emails with the specified domain (may be given multiple times). Use * to authenticate any email") flagSet.StringSlice("whitelist-domain", []string{}, "allowed domains for redirection after authentication. Prefix domain with a . or a *. to allow subdomains (eg .example.com, *.example.com)") diff --git a/pkg/validation/options.go b/pkg/validation/options.go index 13ce2e0b..14a76b41 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -56,7 +56,7 @@ func Validate(o *options.Options) error { // Configure extra issuers if len(o.ExtraJwtIssuers) > 0 { var jwtIssuers []jwtIssuer - jwtIssuers, msgs = parseJwtIssuers(o.ExtraJwtIssuers, msgs) + jwtIssuers, msgs = parseJwtIssuers(o.ExtraJwtIssuers, o.ExtraJwtIssuerJWKsURLs, msgs) for _, jwtIssuer := range jwtIssuers { verifier, err := newVerifierFromJwtIssuer( o.Providers[0].OIDCConfig.AudienceClaims, @@ -126,8 +126,10 @@ func parseSignatureKey(o *options.Options, msgs []string) []string { } // parseJwtIssuers takes in an array of strings in the form of issuer=audience -// and parses to an array of jwtIssuer structs. -func parseJwtIssuers(issuers []string, msgs []string) ([]jwtIssuer, []string) { +// and parses to an array of jwtIssuer structs. The optional jwksURLs array holds +// issuer=jwksURL pairs that override the discovered/derived JWKS URL per issuer. +func parseJwtIssuers(issuers []string, jwksURLs []string, msgs []string) ([]jwtIssuer, []string) { + jwksByIssuer, msgs := parseJwtIssuerJWKsURLs(jwksURLs, msgs) parsedIssuers := make([]jwtIssuer, 0, len(issuers)) for _, jwtVerifier := range issuers { components := strings.Split(jwtVerifier, "=") @@ -136,11 +138,27 @@ func parseJwtIssuers(issuers []string, msgs []string) ([]jwtIssuer, []string) { continue } uri, audience := components[0], strings.Join(components[1:], "=") - parsedIssuers = append(parsedIssuers, jwtIssuer{issuerURI: uri, audience: audience}) + parsedIssuers = append(parsedIssuers, jwtIssuer{issuerURI: uri, audience: audience, jwksURI: jwksByIssuer[uri]}) } return parsedIssuers, msgs } +// parseJwtIssuerJWKsURLs parses an array of strings in the form of issuer=jwksURL +// into a map keyed by issuer. The jwksURL value may itself contain "=", so only the +// first separator is significant. +func parseJwtIssuerJWKsURLs(jwksURLs []string, msgs []string) (map[string]string, []string) { + jwksByIssuer := make(map[string]string, len(jwksURLs)) + for _, spec := range jwksURLs { + issuer, jwksURL, found := strings.Cut(spec, "=") + if !found || issuer == "" || jwksURL == "" { + msgs = append(msgs, fmt.Sprintf("invalid jwt issuer jwks url spec: %s", spec)) + continue + } + jwksByIssuer[issuer] = jwksURL + } + return jwksByIssuer, msgs +} + // newVerifierFromJwtIssuer takes in issuer information in jwtIssuer info and returns // a verifier for that issuer. func newVerifierFromJwtIssuer(audienceClaims []string, extraAudiences []string, jwtIssuer jwtIssuer) (internaloidc.IDTokenVerifier, error) { @@ -151,6 +169,22 @@ func newVerifierFromJwtIssuer(audienceClaims []string, extraAudiences []string, IssuerURL: jwtIssuer.issuerURI, } + // If an explicit JWKS URL is configured for this issuer, use it directly and + // skip both discovery and the hardcoded /.well-known/jwks.json fallback. + // This is required for issuers (e.g. AD FS) whose JWKS lives at a different + // path/host/scheme than the issuer-derived default, while keeping the issuer + // match against the token's "iss" claim intact. + if jwtIssuer.jwksURI != "" { + pvOpts.JWKsURL = jwtIssuer.jwksURI + pvOpts.SkipDiscovery = true + + pv, err := internaloidc.NewProviderVerifier(context.TODO(), pvOpts) + if err != nil { + return nil, fmt.Errorf("could not construct provider verifier for JWT Issuer: %v", err) + } + return pv.Verifier(), nil + } + pv, err := internaloidc.NewProviderVerifier(context.TODO(), pvOpts) if err != nil { // If the discovery didn't work, try again without discovery @@ -170,6 +204,7 @@ func newVerifierFromJwtIssuer(audienceClaims []string, extraAudiences []string, type jwtIssuer struct { issuerURI string audience string + jwksURI string } func parseURL(toParse string, urltype string, msgs []string) (*url.URL, []string) { diff --git a/pkg/validation/options_test.go b/pkg/validation/options_test.go index 5ea748c1..41950e48 100644 --- a/pkg/validation/options_test.go +++ b/pkg/validation/options_test.go @@ -236,3 +236,128 @@ func TestProviderCAFilesError(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "unable to load provider CA file(s)") } + +func TestParseJwtIssuers(t *testing.T) { + testCases := []struct { + name string + issuers []string + jwksURLs []string + expected []jwtIssuer + msgs []string + }{ + { + name: "issuer and audience without jwks url", + issuers: []string{"https://issuer.example.com=client-id"}, + expected: []jwtIssuer{ + {issuerURI: "https://issuer.example.com", audience: "client-id"}, + }, + }, + { + name: "audience containing an equals sign", + issuers: []string{"https://issuer.example.com=aud=with=equals"}, + expected: []jwtIssuer{ + {issuerURI: "https://issuer.example.com", audience: "aud=with=equals"}, + }, + }, + { + name: "explicit jwks url overrides the derived url", + issuers: []string{"http://sts.example.com/adfs/services/trust=urn:microsoft:userinfo"}, + jwksURLs: []string{"http://sts.example.com/adfs/services/trust=https://sts.example.com/adfs/discovery/keys"}, + expected: []jwtIssuer{ + { + issuerURI: "http://sts.example.com/adfs/services/trust", + audience: "urn:microsoft:userinfo", + jwksURI: "https://sts.example.com/adfs/discovery/keys", + }, + }, + }, + { + name: "jwks url containing an equals sign", + issuers: []string{"https://issuer.example.com=client-id"}, + jwksURLs: []string{"https://issuer.example.com=https://issuer.example.com/keys?format=jwks"}, + expected: []jwtIssuer{ + { + issuerURI: "https://issuer.example.com", + audience: "client-id", + jwksURI: "https://issuer.example.com/keys?format=jwks", + }, + }, + }, + { + name: "jwks url for an unrelated issuer is ignored", + issuers: []string{"https://issuer.example.com=client-id"}, + jwksURLs: []string{"https://other.example.com=https://other.example.com/keys"}, + expected: []jwtIssuer{ + {issuerURI: "https://issuer.example.com", audience: "client-id"}, + }, + }, + { + name: "invalid issuer spec is reported", + issuers: []string{"https://issuer.example.com"}, + expected: []jwtIssuer{}, + msgs: []string{"invalid jwt verifier uri=audience spec: https://issuer.example.com"}, + }, + { + name: "invalid jwks url spec is reported", + issuers: []string{"https://issuer.example.com=client-id"}, + jwksURLs: []string{"https://issuer.example.com"}, + expected: []jwtIssuer{ + {issuerURI: "https://issuer.example.com", audience: "client-id"}, + }, + msgs: []string{"invalid jwt issuer jwks url spec: https://issuer.example.com"}, + }, + { + name: "empty jwks url value is reported", + issuers: []string{"https://issuer.example.com=client-id"}, + jwksURLs: []string{"https://issuer.example.com="}, + expected: []jwtIssuer{ + {issuerURI: "https://issuer.example.com", audience: "client-id"}, + }, + msgs: []string{"invalid jwt issuer jwks url spec: https://issuer.example.com="}, + }, + { + name: "empty issuer in jwks url spec is reported", + issuers: []string{"https://issuer.example.com=client-id"}, + jwksURLs: []string{"=https://issuer.example.com/keys"}, + expected: []jwtIssuer{ + {issuerURI: "https://issuer.example.com", audience: "client-id"}, + }, + msgs: []string{"invalid jwt issuer jwks url spec: =https://issuer.example.com/keys"}, + }, + { + name: "multiple issuers each with their own jwks url", + issuers: []string{"https://a.example.com=aud-a", "https://b.example.com=aud-b"}, + jwksURLs: []string{ + "https://a.example.com=https://a.example.com/keys", + "https://b.example.com=https://b.example.com/keys", + }, + expected: []jwtIssuer{ + {issuerURI: "https://a.example.com", audience: "aud-a", jwksURI: "https://a.example.com/keys"}, + {issuerURI: "https://b.example.com", audience: "aud-b", jwksURI: "https://b.example.com/keys"}, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + issuers, msgs := parseJwtIssuers(tc.issuers, tc.jwksURLs, nil) + assert.Equal(t, tc.expected, issuers) + assert.Equal(t, tc.msgs, msgs) + }) + } +} + +func TestNewVerifierFromJwtIssuerWithExplicitJWKsURL(t *testing.T) { + // An explicit JWKS URL must skip discovery entirely, so the verifier can be + // constructed without reaching the issuer (the JWKS is fetched lazily). + issuer := jwtIssuer{ + issuerURI: "http://sts.example.com/adfs/services/trust", + audience: "urn:microsoft:userinfo", + jwksURI: "https://sts.example.com/adfs/discovery/keys", + } + + verifier, err := newVerifierFromJwtIssuer(nil, nil, issuer) + + assert.NoError(t, err) + assert.NotNil(t, verifier) +}