1
0
mirror of https://github.com/oauth2-proxy/oauth2-proxy.git synced 2025-01-24 05:26:55 +02:00

Merge pull request #1418 from ianroberts/start-url-parameters

Pass URL parameters from /oauth2/start through to IdP login URL
This commit is contained in:
Joel Speed 2022-02-19 16:22:45 +00:00 committed by GitHub
commit 7dc984e664
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
22 changed files with 502 additions and 92 deletions

View File

@ -8,6 +8,7 @@
## Changes since v7.2.1
- [#1418](https://github.com/oauth2-proxy/oauth2-proxy/pull/1418) Support for passing arbitrary query parameters through from `/oauth2/start` to the identity provider's login URL. Configuration settings control which parameters are passed by default and precisely which values can be overridden per-request (@ianroberts)
- [#1559](https://github.com/oauth2-proxy/oauth2-proxy/pull/1559) Introduce ProviderVerifier to clean up OIDC discovery code (@JoelSpeed)
- [#1561](https://github.com/oauth2-proxy/oauth2-proxy/pull/1561) Add ppc64le support (@mgiessing)
- [#1563](https://github.com/oauth2-proxy/oauth2-proxy/pull/1563) Ensure claim extractor does not attempt profile call when URL is empty (@JoelSpeed)

View File

@ -264,6 +264,85 @@ make up the header value
| `jwtKeyFile` | _string_ | JWTKeyFile is a path to the private key file in PEM format used to sign the JWT |
| `pubjwkURL` | _string_ | PubJWKURL is the JWK pubkey access endpoint |
### LoginURLParameter
(**Appears on:** [Provider](#provider))
LoginURLParameter is the configuration for a single query parameter that
can be passed through from the `/oauth2/start` endpoint to the IdP login
URL. The "default" option specifies the default value or values (if any)
that will be passed to the IdP for this parameter, and "allow" is a list
of options for ways in which this parameter can be set or overridden via
the query string to `/oauth2/start`.
If _only_ a default is specified and no "allow" then the parameter is
effectively fixed - the default value will always be used and anything
passed to the start URL will be ignored. If _only_ "allow" is specified
but no default then the parameter will only be passed on to the IdP if
the caller provides it, and no value will be sent otherwise.
Examples:
A parameter whose value is fixed
```
name: organization
default:
- myorg
```
A parameter that is not passed by default, but may be set to one of a
fixed set of values
```
name: prompt
allow:
- value: login
- value: consent
- value: select_account
```
A parameter that is passed by default but may be overridden by one of
a fixed set of values
```
name: prompt
default: ["login"]
allow:
- value: consent
- value: select_account
```
A parameter that may be overridden, but only by values that match a
regular expression. For example to restrict `login_hint` to email
addresses in your organization's domain:
```
name: login_hint
allow:
- pattern: '^[^@]*@example\.com$'
# this allows at most one "@" sign, and requires "example.com" domain.
```
Note that the YAML rules around exactly which characters are allowed
and/or require escaping in different types of string literals are
convoluted. For regular expressions the single quoted form is simplest
as backslash is not considered to be an escape character. Alternatively
use the "chomped block" format `|-`:
```
- pattern: |-
^[^@]*@example\.com$
```
The hyphen is important, a `|` block would have a trailing newline
character.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `name` | _string_ | Name specifies the name of the query parameter. |
| `default` | _[]string_ | _(Optional)_ Default specifies a default value or values that will be<br/>passed to the IdP if not overridden. |
| `allow` | _[[]URLParameterRule](#urlparameterrule)_ | _(Optional)_ Allow specifies rules about how the default (if any) may be<br/>overridden via the query string to `/oauth2/start`. Only<br/>values that match one or more of the allow rules will be<br/>forwarded to the IdP. |
### OIDCOptions
(**Appears on:** [Provider](#provider))
@ -309,15 +388,13 @@ Provider holds all configuration for a single provider
| `name` | _string_ | Name is the providers display name<br/>if set, it will be shown to the users in the login page. |
| `caFiles` | _[]string_ | CAFiles is a list of paths to CA certificates that should be used when connecting to the provider.<br/>If not specified, the default Go trust sources are used instead |
| `loginURL` | _string_ | LoginURL is the authentication endpoint |
| `loginURLParameters` | _[[]LoginURLParameter](#loginurlparameter)_ | LoginURLParameters defines the parameters that can be passed from the start URL to the IdP login URL |
| `redeemURL` | _string_ | RedeemURL is the token redemption endpoint |
| `profileURL` | _string_ | ProfileURL is the profile access endpoint |
| `resource` | _string_ | ProtectedResource is the resource that is protected (Azure AD and ADFS only) |
| `validateURL` | _string_ | ValidateURL is the access token validation endpoint |
| `scope` | _string_ | Scope is the OAuth scope specification |
| `prompt` | _string_ | Prompt is OIDC prompt |
| `approvalPrompt` | _string_ | ApprovalPrompt is the OAuth approval_prompt<br/>default is set to 'force' |
| `allowedGroups` | _[]string_ | AllowedGroups is a list of restrict logins to members of this group |
| `acrValues` | _string_ | AcrValues is a string of acr values |
### ProviderType
#### (`string` alias)
@ -377,6 +454,20 @@ as well as an optional minimal TLS version that is acceptable.
| `Cert` | _[SecretSource](#secretsource)_ | Cert is the TLS certificate data to use.<br/>Typically this will come from a file. |
| `MinVersion` | _string_ | MinVersion is the minimal TLS version that is acceptable.<br/>E.g. Set to "TLS1.3" to select TLS version 1.3 |
### URLParameterRule
(**Appears on:** [LoginURLParameter](#loginurlparameter))
URLParameterRule represents a rule by which query parameters
passed to the `/oauth2/start` endpoint are checked to determine whether
they are valid overrides for the given parameter passed to the IdP's
login URL. Either Value or Pattern should be supplied, not both.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `value` | _string_ | A Value rule matches just this specific value |
| `pattern` | _string_ | A Pattern rule gives a regular expression that must be matched by<br/>some substring of the value. The expression is _not_ automatically<br/>anchored to the start and end of the value, if you _want_ to restrict<br/>the whole parameter value you must anchor it yourself with `^` and `$`. |
### Upstream
(**Appears on:** [UpstreamConfig](#upstreamconfig))

View File

@ -68,7 +68,6 @@ providers:
ID: google=oauth2-proxy
clientSecret: b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK
clientID: oauth2-proxy
approvalPrompt: force
azureConfig:
tenant: common
oidcConfig:
@ -78,6 +77,10 @@ providers:
insecureSkipNonce: true
audienceClaims: [aud]
extraAudiences: []
loginURLParameters:
- name: approval_prompt
default:
- force
`
const testCoreConfig = `
@ -154,7 +157,9 @@ redirect_url="http://localhost:4180/oauth2/callback"
ExtraAudiences: []string{},
InsecureSkipNonce: true,
},
ApprovalPrompt: "force",
LoginURLParameters: []options.LoginURLParameter{
{Name: "approval_prompt", Default: []string{"force"}},
},
},
}
return opts

View File

@ -611,6 +611,7 @@ func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) {
if p.SkipProviderButton {
p.OAuthStart(rw, req)
} else {
// TODO - should we pass on /oauth2/sign_in query params to /oauth2/start?
p.SignInPage(rw, req, http.StatusOK)
}
}
@ -671,6 +672,12 @@ func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) {
// OAuthStart starts the OAuth2 authentication flow
func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) {
// start the flow permitting login URL query parameters to be overridden from the request URL
p.doOAuthStart(rw, req, req.URL.Query())
}
func (p *OAuthProxy) doOAuthStart(rw http.ResponseWriter, req *http.Request, overrides url.Values) {
extraParams := p.provider.Data().LoginURLParams(overrides)
prepareNoCache(rw)
csrf, err := cookies.NewCSRF(p.CookieOptions)
@ -692,6 +699,7 @@ func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) {
callbackRedirect,
encodeState(csrf.HashOAuthState(), appRedirect),
csrf.HashOIDCNonce(),
extraParams,
)
if _, err := csrf.SetCookie(rw, req); err != nil {
@ -871,7 +879,10 @@ func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) {
logger.Printf("No valid authentication in request. Initiating login.")
if p.SkipProviderButton {
p.OAuthStart(rw, req)
// start OAuth flow, but only with the default login URL params - do not
// consider this request's query params as potential overrides, since
// the user did not explicitly start the login flow
p.doOAuthStart(rw, req, nil)
} else {
p.SignInPage(rw, req, http.StatusForbidden)
}

View File

@ -632,10 +632,7 @@ func (l *LegacyProvider) convert() (Providers, error) {
ProtectedResource: l.ProtectedResource,
ValidateURL: l.ValidateURL,
Scope: l.Scope,
Prompt: l.Prompt,
ApprovalPrompt: l.ApprovalPrompt,
AllowedGroups: l.AllowedGroups,
AcrValues: l.AcrValues,
}
// This part is out of the switch section for all providers that support OIDC
@ -708,6 +705,24 @@ func (l *LegacyProvider) convert() (Providers, error) {
provider.ID = l.ProviderType + "=" + l.ClientID
}
// handle AcrValues, Prompt and ApprovalPrompt
var urlParams []LoginURLParameter
if l.AcrValues != "" {
urlParams = append(urlParams, LoginURLParameter{Name: "acr_values", Default: []string{l.AcrValues}})
}
switch {
case l.Prompt != "":
urlParams = append(urlParams, LoginURLParameter{Name: "prompt", Default: []string{l.Prompt}})
case l.ApprovalPrompt != "":
urlParams = append(urlParams, LoginURLParameter{Name: "approval_prompt", Default: []string{l.ApprovalPrompt}})
default:
// match legacy behaviour by default - if neither prompt nor approval_prompt
// specified, use approval_prompt=force
urlParams = append(urlParams, LoginURLParameter{Name: "approval_prompt", Default: []string{"force"}})
}
provider.LoginURLParameters = urlParams
providers = append(providers, provider)
return providers, nil

View File

@ -118,6 +118,9 @@ var _ = Describe("Legacy Options", func() {
opts.Providers[0].OIDCConfig.InsecureSkipNonce = true
opts.Providers[0].OIDCConfig.AudienceClaims = []string{"aud"}
opts.Providers[0].OIDCConfig.ExtraAudiences = []string{}
opts.Providers[0].LoginURLParameters = []LoginURLParameter{
{Name: "approval_prompt", Default: []string{"force"}},
}
converted, err := legacyOpts.ToOptions()
Expect(err).ToNot(HaveOccurred())
@ -891,21 +894,41 @@ var _ = Describe("Legacy Options", func() {
// Non defaults for these options
clientID := "abcd"
defaultURLParams := []LoginURLParameter{
{Name: "approval_prompt", Default: []string{"force"}},
}
defaultProvider := Provider{
ID: "google=" + clientID,
ClientID: clientID,
Type: "google",
ID: "google=" + clientID,
ClientID: clientID,
Type: "google",
LoginURLParameters: defaultURLParams,
}
defaultLegacyProvider := LegacyProvider{
ClientID: clientID,
ProviderType: "google",
}
displayNameProvider := Provider{
ID: "displayName",
Name: "displayName",
defaultProviderWithPrompt := Provider{
ID: "google=" + clientID,
ClientID: clientID,
Type: "google",
LoginURLParameters: []LoginURLParameter{
{Name: "prompt", Default: []string{"switch_user"}},
},
}
defaultLegacyProviderWithPrompt := LegacyProvider{
ClientID: clientID,
ProviderType: "google",
Prompt: "switch_user",
}
displayNameProvider := Provider{
ID: "displayName",
Name: "displayName",
ClientID: clientID,
Type: "google",
LoginURLParameters: defaultURLParams,
}
displayNameLegacyProvider := LegacyProvider{
@ -923,6 +946,7 @@ var _ = Describe("Legacy Options", func() {
ServiceAccountJSON: "test.json",
Groups: []string{"1", "2"},
},
LoginURLParameters: defaultURLParams,
}
internalConfigLegacyProvider := LegacyProvider{
@ -950,6 +974,11 @@ var _ = Describe("Legacy Options", func() {
expectedProviders: Providers{defaultProvider},
errMsg: "",
}),
Entry("with prompt setting", &convertProvidersTableInput{
legacyProvider: defaultLegacyProviderWithPrompt,
expectedProviders: Providers{defaultProviderWithPrompt},
errMsg: "",
}),
Entry("with provider display name", &convertProvidersTableInput{
legacyProvider: displayNameLegacyProvider,
expectedProviders: Providers{displayNameProvider},

View File

@ -0,0 +1,101 @@
package options
// LoginURLParameter is the configuration for a single query parameter that
// can be passed through from the `/oauth2/start` endpoint to the IdP login
// URL. The "default" option specifies the default value or values (if any)
// that will be passed to the IdP for this parameter, and "allow" is a list
// of options for ways in which this parameter can be set or overridden via
// the query string to `/oauth2/start`.
// If _only_ a default is specified and no "allow" then the parameter is
// effectively fixed - the default value will always be used and anything
// passed to the start URL will be ignored. If _only_ "allow" is specified
// but no default then the parameter will only be passed on to the IdP if
// the caller provides it, and no value will be sent otherwise.
//
// Examples:
//
// A parameter whose value is fixed
//
// ```
// name: organization
// default:
// - myorg
// ```
//
// A parameter that is not passed by default, but may be set to one of a
// fixed set of values
//
// ```
// name: prompt
// allow:
// - value: login
// - value: consent
// - value: select_account
// ```
//
// A parameter that is passed by default but may be overridden by one of
// a fixed set of values
//
// ```
// name: prompt
// default: ["login"]
// allow:
// - value: consent
// - value: select_account
// ```
//
// A parameter that may be overridden, but only by values that match a
// regular expression. For example to restrict `login_hint` to email
// addresses in your organization's domain:
//
// ```
// name: login_hint
// allow:
// - pattern: '^[^@]*@example\.com$'
// # this allows at most one "@" sign, and requires "example.com" domain.
// ```
//
// Note that the YAML rules around exactly which characters are allowed
// and/or require escaping in different types of string literals are
// convoluted. For regular expressions the single quoted form is simplest
// as backslash is not considered to be an escape character. Alternatively
// use the "chomped block" format `|-`:
//
// ```
// - pattern: |-
// ^[^@]*@example\.com$
// ```
//
// The hyphen is important, a `|` block would have a trailing newline
// character.
type LoginURLParameter struct {
// Name specifies the name of the query parameter.
Name string `json:"name"`
// Default specifies a default value or values that will be
// passed to the IdP if not overridden.
//+optional
Default []string `json:"default,omitempty"`
// 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.
//+optional
Allow []URLParameterRule `json:"allow,omitempty"`
}
// URLParameterRule represents a rule by which query parameters
// passed to the `/oauth2/start` endpoint are checked to determine whether
// they are valid overrides for the given parameter passed to the IdP's
// login URL. Either Value or Pattern should be supplied, not both.
type URLParameterRule struct {
// A Value rule matches just this specific value
Value *string `json:"value,omitempty"`
// 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 `$`.
Pattern *string `json:"pattern,omitempty"`
}

View File

@ -62,6 +62,8 @@ type Provider struct {
// LoginURL is the authentication endpoint
LoginURL string `json:"loginURL,omitempty"`
// LoginURLParameters defines the parameters that can be passed from the start URL to the IdP login URL
LoginURLParameters []LoginURLParameter `json:"loginURLParameters,omitempty"`
// RedeemURL is the token redemption endpoint
RedeemURL string `json:"redeemURL,omitempty"`
// ProfileURL is the profile access endpoint
@ -72,16 +74,8 @@ type Provider struct {
ValidateURL string `json:"validateURL,omitempty"`
// Scope is the OAuth scope specification
Scope string `json:"scope,omitempty"`
// Prompt is OIDC prompt
Prompt string `json:"prompt,omitempty"`
// ApprovalPrompt is the OAuth approval_prompt
// default is set to 'force'
ApprovalPrompt string `json:"approvalPrompt,omitempty"`
// AllowedGroups is a list of restrict logins to members of this group
AllowedGroups []string `json:"allowedGroups,omitempty"`
// AcrValues is a string of acr values
AcrValues string `json:"acrValues,omitempty"`
}
// ProviderType is used to enumerate the different provider type options
@ -243,9 +237,7 @@ type LoginGovOptions struct {
func providerDefaults() Providers {
providers := Providers{
{
Type: "google",
Prompt: "", // Change to "login" when ApprovalPrompt officially deprecated
ApprovalPrompt: "force",
Type: "google",
AzureConfig: AzureOptions{
Tenant: "common",
},

View File

@ -61,8 +61,7 @@ func NewADFSProvider(p *ProviderData, opts options.ADFSOptions) *ADFSProvider {
// GetLoginURL Override to double encode the state parameter. If not query params are lost
// More info here: https://docs.microsoft.com/en-us/powerapps/maker/portals/configure/configure-saml2-settings
func (p *ADFSProvider) GetLoginURL(redirectURI, state, nonce string) string {
extraParams := url.Values{}
func (p *ADFSProvider) GetLoginURL(redirectURI, state, nonce string, extraParams url.Values) string {
if !p.SkipNonce {
extraParams.Add("nonce", nonce)
}

View File

@ -168,7 +168,7 @@ var _ = Describe("ADFS Provider Tests", func() {
Scope: "",
}, options.ADFSOptions{SkipScope: true})
result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "")
result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "", url.Values{})
Expect(result).NotTo(ContainSubstring("scope="))
})
})
@ -189,7 +189,7 @@ var _ = Describe("ADFS Provider Tests", func() {
}, options.ADFSOptions{})
Expect(p.Data().Scope).To(Equal(in.expectedScope))
result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "")
result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "", url.Values{})
Expect(result).To(ContainSubstring("scope=" + url.QueryEscape(in.expectedScope)))
},
Entry("should add slash", scopeTableInput{

View File

@ -103,8 +103,7 @@ func overrideTenantURL(current, defaultURL *url.URL, tenant, path string) {
}
}
func (p *AzureProvider) GetLoginURL(redirectURI, state, _ string) string {
extraParams := url.Values{}
func (p *AzureProvider) GetLoginURL(redirectURI, state, _ string, extraParams url.Values) string {
if p.ProtectedResource != nil && p.ProtectedResource.String() != "" {
extraParams.Add("resource", p.ProtectedResource.String())
}

View File

@ -348,7 +348,7 @@ func TestAzureProviderRedeem(t *testing.T) {
func TestAzureProviderProtectedResourceConfigured(t *testing.T) {
p := testAzureProvider("", options.AzureOptions{})
p.ProtectedResource, _ = url.Parse("http://my.resource.test")
result := p.GetLoginURL("https://my.test.app/oauth", "", "")
result := p.GetLoginURL("https://my.test.app/oauth", "", "", url.Values{})
assert.Contains(t, result, "resource="+url.QueryEscape("http://my.resource.test"))
}

View File

@ -270,9 +270,8 @@ func (p *LoginGovProvider) Redeem(ctx context.Context, _, code string) (*session
}
// GetLoginURL overrides GetLoginURL to add login.gov parameters
func (p *LoginGovProvider) GetLoginURL(redirectURI, state, _ string) string {
extraParams := url.Values{}
if p.AcrValues == "" {
func (p *LoginGovProvider) GetLoginURL(redirectURI, state, _ string, extraParams url.Values) string {
if len(extraParams["acr_values"]) == 0 {
acr := "http://idmanagement.gov/ns/assurance/loa/1"
extraParams.Add("acr_values", acr)
}

View File

@ -337,7 +337,7 @@ func TestLoginGovProviderBadNonce(t *testing.T) {
func TestLoginGovProviderGetLoginURL(t *testing.T) {
p, _, _ := newLoginGovProvider()
result := p.GetLoginURL("http://redirect/", "", "")
result := p.GetLoginURL("http://redirect/", "", "", url.Values{})
assert.Contains(t, result, "acr_values="+url.QueryEscape("http://idmanagement.gov/ns/assurance/loa/1"))
assert.Contains(t, result, "nonce=fakenonce")
}

View File

@ -34,8 +34,7 @@ func NewOIDCProvider(p *ProviderData, opts options.OIDCOptions) *OIDCProvider {
var _ Provider = (*OIDCProvider)(nil)
// GetLoginURL makes the LoginURL with optional nonce support
func (p *OIDCProvider) GetLoginURL(redirectURI, state, nonce string) string {
extraParams := url.Values{}
func (p *OIDCProvider) GetLoginURL(redirectURI, state, nonce string, extraParams url.Values) string {
if !p.SkipNonce {
extraParams.Add("nonce", nonce)
}

View File

@ -96,11 +96,11 @@ func TestOIDCProviderGetLoginURL(t *testing.T) {
nonce := base64.RawURLEncoding.EncodeToString(n)
// SkipNonce defaults to true
skipNonce := provider.GetLoginURL("http://redirect/", "", nonce)
skipNonce := provider.GetLoginURL("http://redirect/", "", nonce, url.Values{})
assert.NotContains(t, skipNonce, "nonce")
provider.SkipNonce = false
withNonce := provider.GetLoginURL("http://redirect/", "", nonce)
withNonce := provider.GetLoginURL("http://redirect/", "", nonce, url.Values{})
assert.Contains(t, withNonce, fmt.Sprintf("nonce=%s", nonce))
}

View File

@ -7,6 +7,7 @@ import (
"io/ioutil"
"net/http"
"net/url"
"regexp"
"strings"
"github.com/coreos/go-oidc/v3/oidc"
@ -32,15 +33,10 @@ type ProviderData struct {
ProfileURL *url.URL
ProtectedResource *url.URL
ValidateURL *url.URL
// Auth request params & related, see
//https://openid.net/specs/openid-connect-basic-1_0.html#rfc.section.2.1.1.1
AcrValues string
ApprovalPrompt string // NOTE: Renamed to "prompt" in OAuth2
ClientID string
ClientSecret string
ClientSecretFile string
Scope string
Prompt string
ClientID string
ClientSecret string
ClientSecretFile string
Scope string
// Common OIDC options for any OIDC-based providers to consume
AllowUnverifiedEmail bool
@ -54,6 +50,8 @@ type ProviderData struct {
AllowedGroups map[string]struct{}
getAuthorizationHeaderFunc func(string) http.Header
loginURLParameterDefaults url.Values
loginURLParameterOverrides map[string]*regexp.Regexp
}
// Data returns the ProviderData
@ -73,6 +71,100 @@ func (p *ProviderData) GetClientSecret() (clientSecret string, err error) {
return string(fileClientSecret), nil
}
// LoginURLParams returns the parameter values that should be passed to the IdP
// login URL. This is the default set of parameters configured for this provider,
// optionally overridden by the given overrides (typically from the URL of the
// /oauth2/start request) according to the configured rules for this provider.
func (p *ProviderData) LoginURLParams(overrides url.Values) url.Values {
// the returned url.Values may be modified later in the request handling process
// so shallow clone the default map
params := url.Values{}
for k, v := range p.loginURLParameterDefaults {
params[k] = v
}
if len(overrides) > 0 {
for param, re := range p.loginURLParameterOverrides {
if reqValues, ok := overrides[param]; ok {
actualValues := make([]string, 0, len(reqValues))
for _, val := range reqValues {
if re.MatchString(val) {
actualValues = append(actualValues, val)
}
}
if len(actualValues) > 0 {
params.Del(param)
params[param] = actualValues
}
}
}
}
return params
}
// Compile the given set of LoginURLParameter options into the internal defaults
// and regular expressions used to validate any overrides.
func (p *ProviderData) compileLoginParams(paramConfig []options.LoginURLParameter) []error {
var errs []error
p.loginURLParameterDefaults = url.Values{}
p.loginURLParameterOverrides = make(map[string]*regexp.Regexp)
for _, param := range paramConfig {
if p.seenParameter(param.Name) {
errs = append(errs, fmt.Errorf("parameter %s provided more than once in loginURLParameters", param.Name))
} else {
// record default if parameter declares one
if len(param.Default) > 0 {
p.loginURLParameterDefaults[param.Name] = param.Default
}
// record allow rules if any
if len(param.Allow) > 0 {
errs = p.convertAllowRules(errs, param)
}
}
}
return errs
}
// Converts the list of allow rules for the given parameter into a regexp
// and store it for use at runtime when validating overrides of that parameter.
func (p *ProviderData) convertAllowRules(errs []error, param options.LoginURLParameter) []error {
var allowREs []string
for idx, rule := range param.Allow {
if (rule.Value == nil) == (rule.Pattern == nil) {
errs = append(errs, fmt.Errorf("rule %d in LoginURLParameter %s must have exactly one of value or pattern", idx, param.Name))
} else {
allowREs = append(allowREs, regexpForRule(rule))
}
}
if re, err := regexp.Compile(strings.Join(allowREs, "|")); err != nil {
errs = append(errs, err)
} else {
p.loginURLParameterOverrides[param.Name] = re
}
return errs
}
// Check whether we have already processed a configuration for the given parameter name
func (p *ProviderData) seenParameter(name string) bool {
_, seenDefault := p.loginURLParameterDefaults[name]
_, seenOverride := p.loginURLParameterOverrides[name]
return seenDefault || seenOverride
}
// Generate a validating regular expression pattern for a given URLParameterRule.
// If the rule is for a fixed value then returns a regexp that matches exactly
// that value, if the rule is itself a regexp just use that as-is.
func regexpForRule(rule options.URLParameterRule) string {
if rule.Value != nil {
// convert literal value into an equivalent regexp,
// anchored at start and end
return "^" + regexp.QuoteMeta(*rule.Value) + "$"
}
// just use the pattern as-is, but wrap in a non-capture group
// to avoid any possibility of confusing the outer disjunction.
return "(?:" + *rule.Pattern + ")"
}
// setAllowedGroups organizes a group list into the AllowedGroups map
// to be consumed by Authorize implementations
func (p *ProviderData) setAllowedGroups(groups []string) {

View File

@ -8,10 +8,14 @@ import (
"encoding/json"
"errors"
"fmt"
"net/url"
"strings"
"testing"
"time"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
"github.com/stretchr/testify/assert"
"github.com/coreos/go-oidc/v3/oidc"
"github.com/golang-jwt/jwt"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
@ -498,3 +502,113 @@ func TestProviderData_checkNonce(t *testing.T) {
})
}
}
func TestProviderData_loginURLParameters(t *testing.T) {
testCases := []struct {
name string
overrides url.Values
has url.Values
notHas []string
}{
{
name: "no overrides",
overrides: url.Values{},
has: url.Values{
"fixed": {"fixed-value"},
"enum_with_default": {"default-value"},
"free_with_default": {"default-value"},
},
notHas: []string{"enum_no_default", "free_no_default"},
},
{
name: "attempt to override fixed value",
overrides: url.Values{"fixed": {"another-value"}},
has: url.Values{
"fixed": {"fixed-value"},
"enum_with_default": {"default-value"},
"free_with_default": {"default-value"},
},
notHas: []string{"enum_no_default", "free_no_default"},
},
{
name: "set one allowed and one forbidden enum",
overrides: url.Values{
"enum_no_default": {"allowed1", "not-allowed"},
},
has: url.Values{
"fixed": {"fixed-value"},
"enum_with_default": {"default-value"},
"free_with_default": {"default-value"},
"enum_no_default": {"allowed1"},
},
notHas: []string{"free_no_default"},
},
{
name: "replace default value",
overrides: url.Values{"free_with_default": {"something-else"}},
has: url.Values{
"fixed": {"fixed-value"},
"enum_with_default": {"default-value"},
"free_with_default": {"something-else"},
},
notHas: []string{"enum_no_default", "free_no_default"},
},
{
name: "set free text value",
overrides: url.Values{"free_no_default": {"some-value"}},
has: url.Values{
"fixed": {"fixed-value"},
"enum_with_default": {"default-value"},
"free_with_default": {"default-value"},
"free_no_default": {"some-value"},
},
notHas: []string{"enum_no_default"},
},
{
name: "attempt to set unapproved parameter",
overrides: url.Values{"malicious_value": {"evil"}},
has: url.Values{
"fixed": {"fixed-value"},
"enum_with_default": {"default-value"},
"free_with_default": {"default-value"},
},
notHas: []string{"enum_no_default", "free_no_default"},
},
}
// fixed list of two allowed values
allowed1 := "allowed1"
allowed2 := "allowed2"
allowEnum := []options.URLParameterRule{
{Value: &allowed1},
{Value: &allowed2},
}
// regex that will allow anything
anything := "^.*$"
allowAnything := []options.URLParameterRule{
{Pattern: &anything},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
// set up LoginURLParameters for testing
data := ProviderData{}
data.compileLoginParams([]options.LoginURLParameter{
{Name: "fixed", Default: []string{"fixed-value"}},
{Name: "enum_with_default", Default: []string{"default-value"}, Allow: allowEnum},
{Name: "enum_no_default", Allow: allowEnum},
{Name: "free_with_default", Default: []string{"default-value"}, Allow: allowAnything},
{Name: "free_no_default", Allow: allowAnything},
})
redirectParams := data.LoginURLParams(tc.overrides)
for _, k := range tc.notHas {
assert.NotContains(t, redirectParams, k)
}
for k, vs := range tc.has {
actualVals := redirectParams[k]
assert.ElementsMatch(t, vs, actualVals)
}
})
}
}

View File

@ -33,8 +33,7 @@ var (
)
// GetLoginURL with typical oauth parameters
func (p *ProviderData) GetLoginURL(redirectURI, state, _ string) string {
extraParams := url.Values{}
func (p *ProviderData) GetLoginURL(redirectURI, state, _ string, extraParams url.Values) string {
loginURL := makeLoginURL(p, redirectURI, state, extraParams)
return loginURL.String()
}

View File

@ -2,7 +2,6 @@ package providers
import (
"context"
"net/url"
"testing"
"time"
@ -30,33 +29,6 @@ func TestRefresh(t *testing.T) {
assert.Equal(t, ErrNotImplemented, err)
}
func TestAcrValuesNotConfigured(t *testing.T) {
p := &ProviderData{
LoginURL: &url.URL{
Scheme: "http",
Host: "my.test.idp",
Path: "/oauth/authorize",
},
}
result := p.GetLoginURL("https://my.test.app/oauth", "", "")
assert.NotContains(t, result, "acr_values")
}
func TestAcrValuesConfigured(t *testing.T) {
p := &ProviderData{
LoginURL: &url.URL{
Scheme: "http",
Host: "my.test.idp",
Path: "/oauth/authorize",
},
AcrValues: "testValue",
}
result := p.GetLoginURL("https://my.test.app/oauth", "", "")
assert.Contains(t, result, "acr_values=testValue")
}
func TestProviderDataEnrichSession(t *testing.T) {
g := NewWithT(t)
p := &ProviderData{}

View File

@ -14,7 +14,7 @@ import (
// Provider represents an upstream identity provider implementation
type Provider interface {
Data() *ProviderData
GetLoginURL(redirectURI, finalRedirect string, nonce string) string
GetLoginURL(redirectURI, finalRedirect string, nonce string, extraParams url.Values) string
Redeem(ctx context.Context, redirectURI, code string) (*sessions.SessionState, error)
// Deprecated: Migrate to EnrichSession
GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error)
@ -70,9 +70,6 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,
ClientID: providerConfig.ClientID,
ClientSecret: providerConfig.ClientSecret,
ClientSecretFile: providerConfig.ClientSecretFile,
Prompt: providerConfig.Prompt,
ApprovalPrompt: providerConfig.ApprovalPrompt,
AcrValues: providerConfig.AcrValues,
}
needsVerifier, err := providerRequiresOIDCProviderVerifier(providerConfig.Type)
@ -122,6 +119,9 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,
errs = append(errs, fmt.Errorf("could not parse %s URL: %v", name, err))
}
}
// handle LoginURLParameters
errs = append(errs, p.compileLoginParams(providerConfig.LoginURLParameters)...)
if len(errs) > 0 {
return nil, k8serrors.NewAggregate(errs)
}

View File

@ -38,14 +38,6 @@ func makeLoginURL(p *ProviderData, redirectURI, state string, extraParams url.Va
a := *p.LoginURL
params, _ := url.ParseQuery(a.RawQuery)
params.Set("redirect_uri", redirectURI)
if p.AcrValues != "" {
params.Add("acr_values", p.AcrValues)
}
if p.Prompt != "" {
params.Set("prompt", p.Prompt)
} else { // Legacy variant of the prompt param:
params.Set("approval_prompt", p.ApprovalPrompt)
}
params.Add("scope", p.Scope)
params.Set("client_id", p.ClientID)
params.Set("response_type", "code")