Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support the configuration of additional auth request parameters on OIDC connector #2504

Open
2 tasks done
roddyherries opened this issue May 5, 2022 · 7 comments
Open
2 tasks done

Comments

@roddyherries
Copy link

roddyherries commented May 5, 2022

Preflight Checklist

  • I agree to follow the Code of Conduct that this project adheres to.
  • 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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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

@roddyherries
Copy link
Author

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
Copy link
Author

Hi @nabokihms, I've raised a draft PR for this change #2546.

@roddyherries
Copy link
Author

Hi @nabokihms , I've opened a new PR for this issue: #2631

@a-nych
Copy link

a-nych commented Nov 7, 2024

@roddyherries I've created a refreshed PR of this issue here: #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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants