6372710 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Support for passing through URL query parameters from /oauth2/start to the ID provider's login URL.

You must explicitly configure oauth2-proxy (alpha config only) with which parameters are allowed to pass through, and optionally provide an allow-list of valid values and/or regular expressions for each one. Note that this mechanism subsumes the functionality of the "prompt", "approval_prompt" and "acr_values" legacy configuration options, which must be converted to the equivalent YAML when running in alpha config mode.

commit63727103db4a8bb22872e8e82cb5fe38018ffbad authorIan Roberts <i.roberts@dcs.shef.ac.uk> dateWed 16 Feb 2022, 16:18 (5 years ago) parentb547fe0

21 files changed+500−91

compare with master

unifiedsplit

docs/docs/configuration/alpha_config.md+94−3

@@ -264,6 +264,85 @@ make up the header value

264264| `jwtKeyFile` | _string_ | JWTKeyFile is a path to the private key file in PEM format used to sign the JWT |

265265| `pubjwkURL` | _string_ | PubJWKURL is the JWK pubkey access endpoint |

266266

267+### LoginURLParameter

268+

269+(**Appears on:** [Provider](#provider))

270+

271+LoginURLParameter is the configuration for a single query parameter that

272+can be passed through from the `/oauth2/start` endpoint to the IdP login

273+URL. The "default" option specifies the default value or values (if any)

274+that will be passed to the IdP for this parameter, and "allow" is a list

275+of options for ways in which this parameter can be set or overridden via

276+the query string to `/oauth2/start`.

277+If _only_ a default is specified and no "allow" then the parameter is

278+effectively fixed - the default value will always be used and anything

279+passed to the start URL will be ignored. If _only_ "allow" is specified

280+but no default then the parameter will only be passed on to the IdP if

281+the caller provides it, and no value will be sent otherwise.

282+

283+Examples:

284+

285+A parameter whose value is fixed

286+

287+```

288+name: organization

289+default:

290+- myorg

291+```

292+

293+A parameter that is not passed by default, but may be set to one of a

294+fixed set of values

295+

296+```

297+name: prompt

298+allow:

299+- value: login

300+- value: consent

301+- value: select_account

302+```

303+

304+A parameter that is passed by default but may be overridden by one of

305+a fixed set of values

306+

307+```

308+name: prompt

309+default: ["login"]

310+allow:

311+- value: consent

312+- value: select_account

313+```

314+

315+A parameter that may be overridden, but only by values that match a

316+regular expression. For example to restrict `login_hint` to email

317+addresses in your organization's domain:

318+

319+```

320+name: login_hint

321+allow:

322+- pattern: '^[^@]*@example\.com$'

323+# this allows at most one "@" sign, and requires "example.com" domain.

324+```

325+

326+Note that the YAML rules around exactly which characters are allowed

327+and/or require escaping in different types of string literals are

328+convoluted. For regular expressions the single quoted form is simplest

329+as backslash is not considered to be an escape character. Alternatively

330+use the "chomped block" format `|-`:

331+

332+```

333+- pattern: |-

334+ ^[^@]*@example\.com$

335+```

336+

337+The hyphen is important, a `|` block would have a trailing newline

338+character.

339+

340+| Field | Type | Description |

341+| ----- | ---- | ----------- |

342+| `name` | _string_ | Name specifies the name of the query parameter. |

343+| `default` | _[]string_ | _(Optional)_ Default specifies a default value or values that will be passed to the IdP if not overridden. |

344+| `allow` | _[[]URLParameterRule](#urlparameterrule)_ | _(Optional)_ Allow specifies rules about how the default (if any) may be overridden via the query string to `/oauth2/start`. Only values that match one or more of the allow rules will be forwarded to the IdP. |

345+

267346### OIDCOptions

268347

269348(**Appears on:** [Provider](#provider))

@@ -309,15 +388,13 @@ Provider holds all configuration for a single provider

309388| `name` | _string_ | Name is the providers display name if set, it will be shown to the users in the login page. |

310389| `caFiles` | _[]string_ | CAFiles is a list of paths to CA certificates that should be used when connecting to the provider. If not specified, the default Go trust sources are used instead |

311390| `loginURL` | _string_ | LoginURL is the authentication endpoint |

391+| `loginURLParameters` | _[[]LoginURLParameter](#loginurlparameter)_ | LoginURLParameters defines the parameters that can be passed from the start URL to the IdP login URL |

312392| `redeemURL` | _string_ | RedeemURL is the token redemption endpoint |

313393| `profileURL` | _string_ | ProfileURL is the profile access endpoint |

314394| `resource` | _string_ | ProtectedResource is the resource that is protected (Azure AD and ADFS only) |

315395| `validateURL` | _string_ | ValidateURL is the access token validation endpoint |

316396| `scope` | _string_ | Scope is the OAuth scope specification |

317−| `prompt` | _string_ | Prompt is OIDC prompt |

318−| `approvalPrompt` | _string_ | ApprovalPrompt is the OAuth approval_prompt default is set to 'force' |

319397| `allowedGroups` | _[]string_ | AllowedGroups is a list of restrict logins to members of this group |

320−| `acrValues` | _string_ | AcrValues is a string of acr values |

321398

322399### ProviderType

323400#### (`string` alias)

@@ -377,6 +454,20 @@ as well as an optional minimal TLS version that is acceptable.

377454| `Cert` | _[SecretSource](#secretsource)_ | Cert is the TLS certificate data to use. Typically this will come from a file. |

378455| `MinVersion` | _string_ | MinVersion is the minimal TLS version that is acceptable. E.g. Set to "TLS1.3" to select TLS version 1.3 |

379456

457+### URLParameterRule

458+

459+(**Appears on:** [LoginURLParameter](#loginurlparameter))

460+

461+URLParameterRule represents a rule by which query parameters

462+passed to the `/oauth2/start` endpoint are checked to determine whether

463+they are valid overrides for the given parameter passed to the IdP's

464+login URL. Either Value or Pattern should be supplied, not both.

465+

466+| Field | Type | Description |

467+| ----- | ---- | ----------- |

468+| `value` | _string_ | A Value rule matches just this specific value |

469+| `pattern` | _string_ | A Pattern rule gives a regular expression that must be matched by some substring of the value. The expression is _not_ automatically anchored to the start and end of the value, if you _want_ to restrict the whole parameter value you must anchor it yourself with `^` and `$`. |

470+

380471### Upstream

381472

382473(**Appears on:** [UpstreamConfig](#upstreamconfig))

main_test.go+7−2

@@ -68,7 +68,6 @@ providers:

6868 ID: google=oauth2-proxy

6969 clientSecret: b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK

7070 clientID: oauth2-proxy

71− approvalPrompt: force

7271 azureConfig:

7372 tenant: common

7473 oidcConfig:

@@ -78,6 +77,10 @@ providers:

7877 insecureSkipNonce: true

7978 audienceClaims: [aud]

8079 extraAudiences: []

80+ loginURLParameters:

81+ - name: approval_prompt

82+ default:

83+ - force

8184`

8285

8386 const testCoreConfig = `

@@ -154,7 +157,9 @@ redirect_url="http://localhost:4180/oauth2/callback"

154157 ExtraAudiences: []string{},

155158 InsecureSkipNonce: true,

156159 },

157− ApprovalPrompt: "force",

160+ LoginURLParameters: []options.LoginURLParameter{

161+ {Name: "approval_prompt", Default: []string{"force"}},

162+ },

158163 },

159164 }

160165 return opts

oauthproxy.go+12−1

@@ -611,6 +611,7 @@ func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) {

611611 if p.SkipProviderButton {

612612 p.OAuthStart(rw, req)

613613 } else {

614+ // TODO - should we pass on /oauth2/sign_in query params to /oauth2/start?

614615 p.SignInPage(rw, req, http.StatusOK)

615616 }

616617 }

@@ -671,6 +672,12 @@ func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) {

671672

672673// OAuthStart starts the OAuth2 authentication flow

673674func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) {

675+ // start the flow permitting login URL query parameters to be overridden from the request URL

676+ p.doOAuthStart(rw, req, req.URL.Query())

677+}

678+

679+func (p *OAuthProxy) doOAuthStart(rw http.ResponseWriter, req *http.Request, overrides url.Values) {

680+ extraParams := p.provider.Data().LoginURLParams(overrides)

674681 prepareNoCache(rw)

675682

676683 csrf, err := cookies.NewCSRF(p.CookieOptions)

@@ -692,6 +699,7 @@ func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) {

692699 callbackRedirect,

693700 encodeState(csrf.HashOAuthState(), appRedirect),

694701 csrf.HashOIDCNonce(),

702+ extraParams,

695703 )

696704

697705 if _, err := csrf.SetCookie(rw, req); err != nil {

@@ -871,7 +879,10 @@ func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) {

871879

872880 logger.Printf("No valid authentication in request. Initiating login.")

873881 if p.SkipProviderButton {

874− p.OAuthStart(rw, req)

882+ // start OAuth flow, but only with the default login URL params - do not

883+ // consider this request's query params as potential overrides, since

884+ // the user did not explicitly start the login flow

885+ p.doOAuthStart(rw, req, nil)

875886 } else {

876887 p.SignInPage(rw, req, http.StatusForbidden)

877888 }

pkg/apis/options/legacy_options.go+18−3

@@ -632,10 +632,7 @@ func (l *LegacyProvider) convert() (Providers, error) {

632632 ProtectedResource: l.ProtectedResource,

633633 ValidateURL: l.ValidateURL,

634634 Scope: l.Scope,

635− Prompt: l.Prompt,

636− ApprovalPrompt: l.ApprovalPrompt,

637635 AllowedGroups: l.AllowedGroups,

638− AcrValues: l.AcrValues,

639636 }

640637

641638 // This part is out of the switch section for all providers that support OIDC

@@ -708,6 +705,24 @@ func (l *LegacyProvider) convert() (Providers, error) {

708705 provider.ID = l.ProviderType + "=" + l.ClientID

709706 }

710707

708+ // handle AcrValues, Prompt and ApprovalPrompt

709+ var urlParams []LoginURLParameter

710+ if l.AcrValues != "" {

711+ urlParams = append(urlParams, LoginURLParameter{Name: "acr_values", Default: []string{l.AcrValues}})

712+ }

713+ switch {

714+ case l.Prompt != "":

715+ urlParams = append(urlParams, LoginURLParameter{Name: "prompt", Default: []string{l.Prompt}})

716+ case l.ApprovalPrompt != "":

717+ urlParams = append(urlParams, LoginURLParameter{Name: "approval_prompt", Default: []string{l.ApprovalPrompt}})

718+ default:

719+ // match legacy behaviour by default - if neither prompt nor approval_prompt

720+ // specified, use approval_prompt=force

721+ urlParams = append(urlParams, LoginURLParameter{Name: "approval_prompt", Default: []string{"force"}})

722+ }

723+

724+ provider.LoginURLParameters = urlParams

725+

711726 providers = append(providers, provider)

712727

713728 return providers, nil

pkg/apis/options/legacy_options_test.go+34−5

@@ -118,6 +118,9 @@ var _ = Describe("Legacy Options", func() {

118118 opts.Providers[0].OIDCConfig.InsecureSkipNonce = true

119119 opts.Providers[0].OIDCConfig.AudienceClaims = []string{"aud"}

120120 opts.Providers[0].OIDCConfig.ExtraAudiences = []string{}

121+ opts.Providers[0].LoginURLParameters = []LoginURLParameter{

122+ {Name: "approval_prompt", Default: []string{"force"}},

123+ }

121124

122125 converted, err := legacyOpts.ToOptions()

123126 Expect(err).ToNot(HaveOccurred())

@@ -891,21 +894,41 @@ var _ = Describe("Legacy Options", func() {

891894 // Non defaults for these options

892895 clientID := "abcd"

893896

897+ defaultURLParams := []LoginURLParameter{

898+ {Name: "approval_prompt", Default: []string{"force"}},

899+ }

900+

894901 defaultProvider := Provider{

902+ ID: "google=" + clientID,

903+ ClientID: clientID,

904+ Type: "google",

905+ LoginURLParameters: defaultURLParams,

906+ }

907+ defaultLegacyProvider := LegacyProvider{

908+ ClientID: clientID,

909+ ProviderType: "google",

910+ }

911+

912+ defaultProviderWithPrompt := Provider{

895913 ID: "google=" + clientID,

896914 ClientID: clientID,

897915 Type: "google",

916+ LoginURLParameters: []LoginURLParameter{

917+ {Name: "prompt", Default: []string{"switch_user"}},

918+ },

898919 }

899− defaultLegacyProvider := LegacyProvider{

920+ defaultLegacyProviderWithPrompt := LegacyProvider{

900921 ClientID: clientID,

901922 ProviderType: "google",

923+ Prompt: "switch_user",

902924 }

903925

904926 displayNameProvider := Provider{

905− ID: "displayName",

906− Name: "displayName",

907− ClientID: clientID,

908− Type: "google",

927+ ID: "displayName",

928+ Name: "displayName",

929+ ClientID: clientID,

930+ Type: "google",

931+ LoginURLParameters: defaultURLParams,

909932 }

910933

911934 displayNameLegacyProvider := LegacyProvider{

@@ -923,6 +946,7 @@ var _ = Describe("Legacy Options", func() {

923946 ServiceAccountJSON: "test.json",

924947 Groups: []string{"1", "2"},

925948 },

949+ LoginURLParameters: defaultURLParams,

926950 }

927951

928952 internalConfigLegacyProvider := LegacyProvider{

@@ -950,6 +974,11 @@ var _ = Describe("Legacy Options", func() {

950974 expectedProviders: Providers{defaultProvider},

951975 errMsg: "",

952976 }),

977+ Entry("with prompt setting", &convertProvidersTableInput{

978+ legacyProvider: defaultLegacyProviderWithPrompt,

979+ expectedProviders: Providers{defaultProviderWithPrompt},

980+ errMsg: "",

981+ }),

953982 Entry("with provider display name", &convertProvidersTableInput{

954983 legacyProvider: displayNameLegacyProvider,

955984 expectedProviders: Providers{displayNameProvider},

pkg/apis/options/login_url_parameters.gonew file+101

@@ -0,0 +1,101 @@

1+package options

3+// LoginURLParameter is the configuration for a single query parameter that

4+// can be passed through from the `/oauth2/start` endpoint to the IdP login

5+// URL. The "default" option specifies the default value or values (if any)

6+// that will be passed to the IdP for this parameter, and "allow" is a list

7+// of options for ways in which this parameter can be set or overridden via

8+// the query string to `/oauth2/start`.

9+// If _only_ a default is specified and no "allow" then the parameter is

10+// effectively fixed - the default value will always be used and anything

11+// passed to the start URL will be ignored. If _only_ "allow" is specified

12+// but no default then the parameter will only be passed on to the IdP if

13+// the caller provides it, and no value will be sent otherwise.

14+//

15+// Examples:

16+//

17+// A parameter whose value is fixed

18+//

19+// ```

20+// name: organization

21+// default:

22+// - myorg

23+// ```

24+//

25+// A parameter that is not passed by default, but may be set to one of a

26+// fixed set of values

27+//

28+// ```

29+// name: prompt

30+// allow:

31+// - value: login

32+// - value: consent

33+// - value: select_account

34+// ```

35+//

36+// A parameter that is passed by default but may be overridden by one of

37+// a fixed set of values

38+//

39+// ```

40+// name: prompt

41+// default: ["login"]

42+// allow:

43+// - value: consent

44+// - value: select_account

45+// ```

46+//

47+// A parameter that may be overridden, but only by values that match a

48+// regular expression. For example to restrict `login_hint` to email

49+// addresses in your organization's domain:

50+//

51+// ```

52+// name: login_hint

53+// allow:

54+// - pattern: '^[^@]*@example\.com$'

55+// # this allows at most one "@" sign, and requires "example.com" domain.

56+// ```

57+//

58+// Note that the YAML rules around exactly which characters are allowed

59+// and/or require escaping in different types of string literals are

60+// convoluted. For regular expressions the single quoted form is simplest

61+// as backslash is not considered to be an escape character. Alternatively

62+// use the "chomped block" format `|-`:

63+//

64+// ```

65+// - pattern: |-

66+// ^[^@]*@example\.com$

67+// ```

68+//

69+// The hyphen is important, a `|` block would have a trailing newline

70+// character.

71+type LoginURLParameter struct {

72+ // Name specifies the name of the query parameter.

73+ Name string `json:"name"`

74+

75+ // Default specifies a default value or values that will be

76+ // passed to the IdP if not overridden.

77+ //+optional

78+ Default []string `json:"default,omitempty"`

79+

80+ // Allow specifies rules about how the default (if any) may be

81+ // overridden via the query string to `/oauth2/start`. Only

82+ // values that match one or more of the allow rules will be

83+ // forwarded to the IdP.

84+ //+optional

85+ Allow []URLParameterRule `json:"allow,omitempty"`

86+}

87+

88+// URLParameterRule represents a rule by which query parameters

89+// passed to the `/oauth2/start` endpoint are checked to determine whether

90+// they are valid overrides for the given parameter passed to the IdP's

91+// login URL. Either Value or Pattern should be supplied, not both.

92+type URLParameterRule struct {

93+ // A Value rule matches just this specific value

94+ Value *string `json:"value,omitempty"`

95+

96+ // A Pattern rule gives a regular expression that must be matched by

97+ // some substring of the value. The expression is _not_ automatically

98+ // anchored to the start and end of the value, if you _want_ to restrict

99+ // the whole parameter value you must anchor it yourself with `^` and `$`.

100+ Pattern *string `json:"pattern,omitempty"`

101+}

pkg/apis/options/providers.go+3−11

@@ -62,6 +62,8 @@ type Provider struct {

6262

6363 // LoginURL is the authentication endpoint

6464 LoginURL string `json:"loginURL,omitempty"`

65+ // LoginURLParameters defines the parameters that can be passed from the start URL to the IdP login URL

66+ LoginURLParameters []LoginURLParameter `json:"loginURLParameters,omitempty"`

6567 // RedeemURL is the token redemption endpoint

6668 RedeemURL string `json:"redeemURL,omitempty"`

6769 // ProfileURL is the profile access endpoint

@@ -72,16 +74,8 @@ type Provider struct {

7274 ValidateURL string `json:"validateURL,omitempty"`

7375 // Scope is the OAuth scope specification

7476 Scope string `json:"scope,omitempty"`

75− // Prompt is OIDC prompt

76− Prompt string `json:"prompt,omitempty"`

77− // ApprovalPrompt is the OAuth approval_prompt

78− // default is set to 'force'

79− ApprovalPrompt string `json:"approvalPrompt,omitempty"`

8077 // AllowedGroups is a list of restrict logins to members of this group

8178 AllowedGroups []string `json:"allowedGroups,omitempty"`

82−

83− // AcrValues is a string of acr values

84− AcrValues string `json:"acrValues,omitempty"`

8579}

8680

8781// ProviderType is used to enumerate the different provider type options

@@ -243,9 +237,7 @@ type LoginGovOptions struct {

243237func providerDefaults() Providers {

244238 providers := Providers{

245239 {

246− Type: "google",

247− Prompt: "", // Change to "login" when ApprovalPrompt officially deprecated

248− ApprovalPrompt: "force",

240+ Type: "google",

249241 AzureConfig: AzureOptions{

250242 Tenant: "common",

251243 },

providers/adfs.go+1−2

@@ -61,8 +61,7 @@ func NewADFSProvider(p *ProviderData, opts options.ADFSOptions) *ADFSProvider {

6161

6262// GetLoginURL Override to double encode the state parameter. If not query params are lost

6363// More info here: https://docs.microsoft.com/en-us/powerapps/maker/portals/configure/configure-saml2-settings

64−func (p *ADFSProvider) GetLoginURL(redirectURI, state, nonce string) string {

65− extraParams := url.Values{}

64+func (p *ADFSProvider) GetLoginURL(redirectURI, state, nonce string, extraParams url.Values) string {

6665 if !p.SkipNonce {

6766 extraParams.Add("nonce", nonce)

6867 }

providers/adfs_test.go+2−2

@@ -168,7 +168,7 @@ var _ = Describe("ADFS Provider Tests", func() {

168168 Scope: "",

169169 }, options.ADFSOptions{SkipScope: true})

170170

171− result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "")

171+ result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "", url.Values{})

172172 Expect(result).NotTo(ContainSubstring("scope="))

173173 })

174174 })

@@ -189,7 +189,7 @@ var _ = Describe("ADFS Provider Tests", func() {

189189 }, options.ADFSOptions{})

190190

191191 Expect(p.Data().Scope).To(Equal(in.expectedScope))

192− result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "")

192+ result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "", url.Values{})

193193 Expect(result).To(ContainSubstring("scope=" + url.QueryEscape(in.expectedScope)))

194194 },

195195 Entry("should add slash", scopeTableInput{

providers/azure.go+1−2

@@ -103,8 +103,7 @@ func overrideTenantURL(current, defaultURL *url.URL, tenant, path string) {

103103 }

104104}

105105

106−func (p *AzureProvider) GetLoginURL(redirectURI, state, _ string) string {

107− extraParams := url.Values{}

106+func (p *AzureProvider) GetLoginURL(redirectURI, state, _ string, extraParams url.Values) string {

108107 if p.ProtectedResource != nil && p.ProtectedResource.String() != "" {

109108 extraParams.Add("resource", p.ProtectedResource.String())

110109 }

providers/azure_test.go+1−1

@@ -348,7 +348,7 @@ func TestAzureProviderRedeem(t *testing.T) {

348348func TestAzureProviderProtectedResourceConfigured(t *testing.T) {

349349 p := testAzureProvider("", options.AzureOptions{})

350350 p.ProtectedResource, _ = url.Parse("http://my.resource.test")

351− result := p.GetLoginURL("https://my.test.app/oauth", "", "")

351+ result := p.GetLoginURL("https://my.test.app/oauth", "", "", url.Values{})

352352 assert.Contains(t, result, "resource="+url.QueryEscape("http://my.resource.test"))

353353}

354354

providers/logingov.go+2−3

@@ -270,9 +270,8 @@ func (p *LoginGovProvider) Redeem(ctx context.Context, _, code string) (*session

270270}

271271

272272// GetLoginURL overrides GetLoginURL to add login.gov parameters

273−func (p *LoginGovProvider) GetLoginURL(redirectURI, state, _ string) string {

274− extraParams := url.Values{}

275− if p.AcrValues == "" {

273+func (p *LoginGovProvider) GetLoginURL(redirectURI, state, _ string, extraParams url.Values) string {

274+ if len(extraParams["acr_values"]) == 0 {

276275 acr := "http://idmanagement.gov/ns/assurance/loa/1"

277276 extraParams.Add("acr_values", acr)

278277 }

providers/logingov_test.go+1−1

@@ -337,7 +337,7 @@ func TestLoginGovProviderBadNonce(t *testing.T) {

337337

338338func TestLoginGovProviderGetLoginURL(t *testing.T) {

339339 p, _, _ := newLoginGovProvider()

340− result := p.GetLoginURL("http://redirect/", "", "")

340+ result := p.GetLoginURL("http://redirect/", "", "", url.Values{})

341341 assert.Contains(t, result, "acr_values="+url.QueryEscape("http://idmanagement.gov/ns/assurance/loa/1"))

342342 assert.Contains(t, result, "nonce=fakenonce")

343343}

providers/oidc.go+1−2

@@ -34,8 +34,7 @@ func NewOIDCProvider(p *ProviderData, opts options.OIDCOptions) *OIDCProvider {

3434var _ Provider = (*OIDCProvider)(nil)

3535

3636// GetLoginURL makes the LoginURL with optional nonce support

37−func (p *OIDCProvider) GetLoginURL(redirectURI, state, nonce string) string {

38− extraParams := url.Values{}

37+func (p *OIDCProvider) GetLoginURL(redirectURI, state, nonce string, extraParams url.Values) string {

3938 if !p.SkipNonce {

4039 extraParams.Add("nonce", nonce)

4140 }

providers/oidc_test.go+2−2

@@ -96,11 +96,11 @@ func TestOIDCProviderGetLoginURL(t *testing.T) {

9696 nonce := base64.RawURLEncoding.EncodeToString(n)

9797

9898 // SkipNonce defaults to true

99− skipNonce := provider.GetLoginURL("http://redirect/", "", nonce)

99+ skipNonce := provider.GetLoginURL("http://redirect/", "", nonce, url.Values{})

100100 assert.NotContains(t, skipNonce, "nonce")

101101

102102 provider.SkipNonce = false

103− withNonce := provider.GetLoginURL("http://redirect/", "", nonce)

103+ withNonce := provider.GetLoginURL("http://redirect/", "", nonce, url.Values{})

104104 assert.Contains(t, withNonce, fmt.Sprintf("nonce=%s", nonce))

105105}

106106

providers/provider_data.go+101−9

@@ -7,6 +7,7 @@ import (

77 "io/ioutil"

88 "net/http"

99 "net/url"

10+ "regexp"

1011 "strings"

1112

1213 "github.com/coreos/go-oidc/v3/oidc"

@@ -32,15 +33,10 @@ type ProviderData struct {

3233 ProfileURL *url.URL

3334 ProtectedResource *url.URL

3435 ValidateURL *url.URL

35− // Auth request params & related, see

36− //https://openid.net/specs/openid-connect-basic-1_0.html#rfc.section.2.1.1.1

37− AcrValues string

38− ApprovalPrompt string // NOTE: Renamed to "prompt" in OAuth2

39− ClientID string

40− ClientSecret string

41− ClientSecretFile string

42− Scope string

43− Prompt string

36+ ClientID string

37+ ClientSecret string

38+ ClientSecretFile string

39+ Scope string

4440

4541 // Common OIDC options for any OIDC-based providers to consume

4642 AllowUnverifiedEmail bool

@@ -54,6 +50,8 @@ type ProviderData struct {

5450 AllowedGroups map[string]struct{}

5551

5652 getAuthorizationHeaderFunc func(string) http.Header

53+ loginURLParameterDefaults url.Values

54+ loginURLParameterOverrides map[string]*regexp.Regexp

5755}

5856

5957// Data returns the ProviderData

@@ -73,6 +71,100 @@ func (p *ProviderData) GetClientSecret() (clientSecret string, err error) {

7371 return string(fileClientSecret), nil

7472}

7573

74+// LoginURLParams returns the parameter values that should be passed to the IdP

75+// login URL. This is the default set of parameters configured for this provider,

76+// optionally overridden by the given overrides (typically from the URL of the

77+// /oauth2/start request) according to the configured rules for this provider.

78+func (p *ProviderData) LoginURLParams(overrides url.Values) url.Values {

79+ // the returned url.Values may be modified later in the request handling process

80+ // so shallow clone the default map

81+ params := url.Values{}

82+ for k, v := range p.loginURLParameterDefaults {

83+ params[k] = v

84+ }

85+ if len(overrides) > 0 {

86+ for param, re := range p.loginURLParameterOverrides {

87+ if reqValues, ok := overrides[param]; ok {

88+ actualValues := make([]string, 0, len(reqValues))

89+ for _, val := range reqValues {

90+ if re.MatchString(val) {

91+ actualValues = append(actualValues, val)

92+ }

93+ }

94+ if len(actualValues) > 0 {

95+ params.Del(param)

96+ params[param] = actualValues

97+ }

98+ }

99+ }

100+ }

101+ return params

102+}

103+

104+// Compile the given set of LoginURLParameter options into the internal defaults

105+// and regular expressions used to validate any overrides.

106+func (p *ProviderData) compileLoginParams(paramConfig []options.LoginURLParameter) []error {

107+ var errs []error

108+ p.loginURLParameterDefaults = url.Values{}

109+ p.loginURLParameterOverrides = make(map[string]*regexp.Regexp)

110+

111+ for _, param := range paramConfig {

112+ if p.seenParameter(param.Name) {

113+ errs = append(errs, fmt.Errorf("parameter %s provided more than once in loginURLParameters", param.Name))

114+ } else {

115+ // record default if parameter declares one

116+ if len(param.Default) > 0 {

117+ p.loginURLParameterDefaults[param.Name] = param.Default

118+ }

119+ // record allow rules if any

120+ if len(param.Allow) > 0 {

121+ errs = p.convertAllowRules(errs, param)

122+ }

123+ }

124+ }

125+ return errs

126+}

127+

128+// Converts the list of allow rules for the given parameter into a regexp

129+// and store it for use at runtime when validating overrides of that parameter.

130+func (p *ProviderData) convertAllowRules(errs []error, param options.LoginURLParameter) []error {

131+ var allowREs []string

132+ for idx, rule := range param.Allow {

133+ if (rule.Value == nil) == (rule.Pattern == nil) {

134+ errs = append(errs, fmt.Errorf("rule %d in LoginURLParameter %s must have exactly one of value or pattern", idx, param.Name))

135+ } else {

136+ allowREs = append(allowREs, regexpForRule(rule))

137+ }

138+ }

139+ if re, err := regexp.Compile(strings.Join(allowREs, "|")); err != nil {

140+ errs = append(errs, err)

141+ } else {

142+ p.loginURLParameterOverrides[param.Name] = re

143+ }

144+ return errs

145+}

146+

147+// Check whether we have already processed a configuration for the given parameter name

148+func (p *ProviderData) seenParameter(name string) bool {

149+ _, seenDefault := p.loginURLParameterDefaults[name]

150+ _, seenOverride := p.loginURLParameterOverrides[name]

151+ return seenDefault || seenOverride

152+}

153+

154+// Generate a validating regular expression pattern for a given URLParameterRule.

155+// If the rule is for a fixed value then returns a regexp that matches exactly

156+// that value, if the rule is itself a regexp just use that as-is.

157+func regexpForRule(rule options.URLParameterRule) string {

158+ if rule.Value != nil {

159+ // convert literal value into an equivalent regexp,

160+ // anchored at start and end

161+ return "^" + regexp.QuoteMeta(*rule.Value) + "$"

162+ }

163+ // just use the pattern as-is, but wrap in a non-capture group

164+ // to avoid any possibility of confusing the outer disjunction.

165+ return "(?:" + *rule.Pattern + ")"

166+}

167+

76168// setAllowedGroups organizes a group list into the AllowedGroups map

77169// to be consumed by Authorize implementations

78170func (p *ProviderData) setAllowedGroups(groups []string) {

providers/provider_data_test.go+114

@@ -8,10 +8,14 @@ import (

88 "encoding/json"

99 "errors"

1010 "fmt"

11+ "net/url"

1112 "strings"

1213 "testing"

1314 "time"

1415

16+ "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"

17+ "github.com/stretchr/testify/assert"

18+

1519 "github.com/coreos/go-oidc/v3/oidc"

1620 "github.com/golang-jwt/jwt"

1721 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"

@@ -498,3 +502,113 @@ func TestProviderData_checkNonce(t *testing.T) {

498502 })

499503 }

500504}

505+

506+func TestProviderData_loginURLParameters(t *testing.T) {

507+

508+ testCases := []struct {

509+ name string

510+ overrides url.Values

511+ has url.Values

512+ notHas []string

513+ }{

514+ {

515+ name: "no overrides",

516+ overrides: url.Values{},

517+ has: url.Values{

518+ "fixed": {"fixed-value"},

519+ "enum_with_default": {"default-value"},

520+ "free_with_default": {"default-value"},

521+ },

522+ notHas: []string{"enum_no_default", "free_no_default"},

523+ },

524+ {

525+ name: "attempt to override fixed value",

526+ overrides: url.Values{"fixed": {"another-value"}},

527+ has: url.Values{

528+ "fixed": {"fixed-value"},

529+ "enum_with_default": {"default-value"},

530+ "free_with_default": {"default-value"},

531+ },

532+ notHas: []string{"enum_no_default", "free_no_default"},

533+ },

534+ {

535+ name: "set one allowed and one forbidden enum",

536+ overrides: url.Values{

537+ "enum_no_default": {"allowed1", "not-allowed"},

538+ },

539+ has: url.Values{

540+ "fixed": {"fixed-value"},

541+ "enum_with_default": {"default-value"},

542+ "free_with_default": {"default-value"},

543+ "enum_no_default": {"allowed1"},

544+ },

545+ notHas: []string{"free_no_default"},

546+ },

547+ {

548+ name: "replace default value",

549+ overrides: url.Values{"free_with_default": {"something-else"}},

550+ has: url.Values{

551+ "fixed": {"fixed-value"},

552+ "enum_with_default": {"default-value"},

553+ "free_with_default": {"something-else"},

554+ },

555+ notHas: []string{"enum_no_default", "free_no_default"},

556+ },

557+ {

558+ name: "set free text value",

559+ overrides: url.Values{"free_no_default": {"some-value"}},

560+ has: url.Values{

561+ "fixed": {"fixed-value"},

562+ "enum_with_default": {"default-value"},

563+ "free_with_default": {"default-value"},

564+ "free_no_default": {"some-value"},

565+ },

566+ notHas: []string{"enum_no_default"},

567+ },

568+ {

569+ name: "attempt to set unapproved parameter",

570+ overrides: url.Values{"malicious_value": {"evil"}},

571+ has: url.Values{

572+ "fixed": {"fixed-value"},

573+ "enum_with_default": {"default-value"},

574+ "free_with_default": {"default-value"},

575+ },

576+ notHas: []string{"enum_no_default", "free_no_default"},

577+ },

578+ }

579+

580+ // fixed list of two allowed values

581+ allowed1 := "allowed1"

582+ allowed2 := "allowed2"

583+ allowEnum := []options.URLParameterRule{

584+ {Value: &allowed1},

585+ {Value: &allowed2},

586+ }

587+ // regex that will allow anything

588+ anything := "^.*$"

589+ allowAnything := []options.URLParameterRule{

590+ {Pattern: &anything},

591+ }

592+ for _, tc := range testCases {

593+ t.Run(tc.name, func(t *testing.T) {

594+ // set up LoginURLParameters for testing

595+ data := ProviderData{}

596+ data.compileLoginParams([]options.LoginURLParameter{

597+ {Name: "fixed", Default: []string{"fixed-value"}},

598+ {Name: "enum_with_default", Default: []string{"default-value"}, Allow: allowEnum},

599+ {Name: "enum_no_default", Allow: allowEnum},

600+ {Name: "free_with_default", Default: []string{"default-value"}, Allow: allowAnything},

601+ {Name: "free_no_default", Allow: allowAnything},

602+ })

603+

604+ redirectParams := data.LoginURLParams(tc.overrides)

605+ for _, k := range tc.notHas {

606+ assert.NotContains(t, redirectParams, k)

607+ }

608+ for k, vs := range tc.has {

609+ actualVals := redirectParams[k]

610+ assert.ElementsMatch(t, vs, actualVals)

611+ }

612+ })

613+ }

614+}

providers/provider_default.go+1−2

@@ -33,8 +33,7 @@ var (

3333)

3434

3535// GetLoginURL with typical oauth parameters

36−func (p *ProviderData) GetLoginURL(redirectURI, state, _ string) string {

37− extraParams := url.Values{}

36+func (p *ProviderData) GetLoginURL(redirectURI, state, _ string, extraParams url.Values) string {

3837 loginURL := makeLoginURL(p, redirectURI, state, extraParams)

3938 return loginURL.String()

4039}

providers/provider_default_test.go−28

@@ -2,7 +2,6 @@ package providers

33import (

44 "context"

5− "net/url"

65 "testing"

76 "time"

@@ -30,33 +29,6 @@ func TestRefresh(t *testing.T) {

3029 assert.Equal(t, ErrNotImplemented, err)

3130}

3231

33−func TestAcrValuesNotConfigured(t *testing.T) {

34− p := &ProviderData{

35− LoginURL: &url.URL{

36− Scheme: "http",

37− Host: "my.test.idp",

38− Path: "/oauth/authorize",

39− },

40− }

41−

42− result := p.GetLoginURL("https://my.test.app/oauth", "", "")

43− assert.NotContains(t, result, "acr_values")

44−}

45−

46−func TestAcrValuesConfigured(t *testing.T) {

47− p := &ProviderData{

48− LoginURL: &url.URL{

49− Scheme: "http",

50− Host: "my.test.idp",

51− Path: "/oauth/authorize",

52− },

53− AcrValues: "testValue",

54− }

55−

56− result := p.GetLoginURL("https://my.test.app/oauth", "", "")

57− assert.Contains(t, result, "acr_values=testValue")

58−}

59−

6032func TestProviderDataEnrichSession(t *testing.T) {

6133 g := NewWithT(t)

6234 p := &ProviderData{}

providers/providers.go+4−4

@@ -14,7 +14,7 @@ import (

1414// Provider represents an upstream identity provider implementation

1515type Provider interface {

1616 Data() *ProviderData

17− GetLoginURL(redirectURI, finalRedirect string, nonce string) string

17+ GetLoginURL(redirectURI, finalRedirect string, nonce string, extraParams url.Values) string

1818 Redeem(ctx context.Context, redirectURI, code string) (*sessions.SessionState, error)

1919 // Deprecated: Migrate to EnrichSession

2020 GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error)

@@ -70,9 +70,6 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,

7070 ClientID: providerConfig.ClientID,

7171 ClientSecret: providerConfig.ClientSecret,

7272 ClientSecretFile: providerConfig.ClientSecretFile,

73− Prompt: providerConfig.Prompt,

74− ApprovalPrompt: providerConfig.ApprovalPrompt,

75− AcrValues: providerConfig.AcrValues,

7673 }

7774

7875 needsVerifier, err := providerRequiresOIDCProviderVerifier(providerConfig.Type)

@@ -122,6 +119,9 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,

122119 errs = append(errs, fmt.Errorf("could not parse %s URL: %v", name, err))

123120 }

124121 }

122+ // handle LoginURLParameters

123+ errs = append(errs, p.compileLoginParams(providerConfig.LoginURLParameters)...)

124+

125125 if len(errs) > 0 {

126126 return nil, k8serrors.NewAggregate(errs)

127127 }

providers/util.go−8

@@ -38,14 +38,6 @@ func makeLoginURL(p *ProviderData, redirectURI, state string, extraParams url.Va

3838 a := *p.LoginURL

3939 params, _ := url.ParseQuery(a.RawQuery)

4040 params.Set("redirect_uri", redirectURI)

41− if p.AcrValues != "" {

42− params.Add("acr_values", p.AcrValues)

43− }

44− if p.Prompt != "" {

45− params.Set("prompt", p.Prompt)

46− } else { // Legacy variant of the prompt param:

47− params.Set("approval_prompt", p.ApprovalPrompt)

48− }

4941 params.Add("scope", p.Scope)

5042 params.Set("client_id", p.ClientID)

5143 params.Set("response_type", "code")