Support the configuration of additional auth request parameters on OIDC connector

roddyherries commented 2 years ago

Preflight Checklist

[X] I agree to follow the Code of Conduct that this project adheres to.
[X] I have searched the issue tracker for an issue that matches the one I want to file, without success.

Problem Description

Some IdPs support login features that are accessible via additional parameters on an authorization request. The current OIDC connector implementation provides no mechanism to configure additional authorization request parameters and hence the IdP specific features are out of reach when using DEX.

For example, when authorizing with Auth0 a client can pass an organization parameter to tell Auth0 which organization the user must authenticate via:

HTTP/1.1 302 Found
  Location: https://server.example.com/authorize?
    response_type=code
    &scope=openid profile email
    &client_id=1234
    &state=abcde
    &redirect_uri=https://client.example.org/callback
    &organization=my_org

Proposed Solution

Extend the existing OIDC configuration with new map element additionalAuthRequestParams.

connectors:
- type: oidc
  id: oidc
  name: OIDC
  config:
    --- existing config 
    additionalAuthRequestParams:
      paramKey1: value1
      paramKey2: value2

Enhance LoginURL to add these parameters to the auth request:

if len(c.additionalAuthRequestParams) > 0 {
    for k, v := range c.additionalAuthRequestParams {
    opts = append(opts, oauth2.SetAuthURLParam(k, v))
    }
}

Alternatives Considered

No response

Additional Information

No response

nabokihms commented 2 years ago

It looks like an excellent addition to the OIDC connector! Thanks for the idea.

The only concern about implementation is that this feature requires splitting command line parameters into two lists: params maintained by dex and additional params.

Dex should validate that provided additional auth request params do not interfere with the first list.
It should be covered by tests that Dex itself uses only parameters from the first list.

roddyherries commented 2 years ago

Hi @nabokihms - yes, agreed on that additional check for splatting standard OIDC params. I've implemented this on a fork for our own internal use for now - I'll raise a PR if that helps?

nabokihms commented 2 years ago

Yeah, sure. I think it is worth seeing the code and reviewing it.

roddyherries commented 2 years ago

Hi @nabokihms - could you clarify something please ...

The only concern about implementation is that this feature requires splitting command line parameters into two lists: params maintained by dex and additional params.

By "maintained by dex" do you mean only the existing auth params managed in oidcConnector.LoginURL, i.e hd, acr_values and prompt or some other list?

roddyherries commented 2 years ago

Hi @nabokihms, I've raised a draft PR for this change https://github.com/dexidp/dex/pull/2546.

roddyherries commented 2 years ago

Hi @nabokihms , I've opened a new PR for this issue: https://github.com/dexidp/dex/pull/2631

a-nych commented 1 week ago

@roddyherries I've created a refreshed PR of this issue here: https://github.com/dexidp/dex/pull/3831.

I've made sure that your original work is still there and that there is a mention of you as an author. Hope you don't mind!

dexidp / dex