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

Add support for sending acr_values #331

Closed
Glowsome opened this issue Aug 27, 2021 · 8 comments · Fixed by #388 or #389
Closed

Add support for sending acr_values #331

Glowsome opened this issue Aug 27, 2021 · 8 comments · Fixed by #388 or #389
Labels
enhancement Issues & PRs related to new features.
Milestone

Comments

@Glowsome
Copy link
Contributor

Is your feature request related to a problem? Please describe.
The current implementation does not support asking for a specific authentication-contract by means of sending the parameter 'acr_values' as described in https://openid.net/specs/openid-connect-core-1_0.html - section 5.5.1.1.

Describe the solution you'd like
It would be very wishfull to be able to (when needed) specify the authentication contract available on an IDP via the acr_values parameter.

Describe alternatives you've considered
There is no alternative , as IDP's can have multiple authentication contracts available.
When no acr_values -parameter is sent with the authentication request an IDP will execute the default authentication contract, so there is no option to explicitly select one.

Additional context
In my situation my IDP has multiple authentication contracts :

  • a default username/password contract => urn:domainname.nl:idp:contract:password (default)
  • a multifactor contract => urn:domainname.nl:idp:contract:password:multifactor
    Via other OpenID implementations i can ( from the application side ) explicitly ask for my multifactor contract by sending the contract-name ( urn) in the acr_values - parameter.
    If i omit this (optional) parameter as stated above the default contract will be executed.
@Glowsome Glowsome added the enhancement Issues & PRs related to new features. label Aug 27, 2021
@Glowsome
Copy link
Contributor Author

Glowsome commented Dec 9, 2021

Due to the fact that no progress was seen as to assignment or otherwise, i have forked the repo and have been working on a own implementation.
When finished i will create a pull-request for the work i have done.
Current progress:

  • implemented additional option in settings-page.
  • added routine to check if the field is populated.
  • added code to honor the additional parameter in the URL when the option is not empty.
  • added verification to the content of the id_token to check if the correct authentication context was honored

@timnolte
Copy link
Collaborator

timnolte commented Dec 9, 2021

@Glowsome sounds good PRs are always welcome. Things have gotten busy the past many months and I've been more recently trying to get the project in better shape for other developers to contribute and spin up a local development environment easier. I've also been working on PHP & WordPress version compatibility testing.

@Glowsome
Copy link
Contributor Author

Glowsome commented Dec 9, 2021

@Glowsome sounds good PRs are always welcome. Things have gotten busy the past many months and I've been more recently trying to get the project in better shape for other developers to contribute and spin up a local development environment easier. I've also been working on PHP & WordPress version compatibility testing.

For your info, i have the fork running on a testsite with :

  1. current WP version 5.8.2
  2. A Microfocus AccessManager v.4.5.3 IDP with OIDC/Oauth2 capabillties
  3. A specifically defined authentication contract in the options matching the defined contract on the IDP for authentication request from WP

Latest additions to code are as stated above the implemenatation of verification if (when defined) the acr_values returned in the ID_token are the same as the value defined in the options of the plugin.

This is (next to the fact it is defined in the specs) to avoid url manipulation where one might be able to escape imposed authentication methods.

@Glowsome
Copy link
Contributor Author

Glowsome commented Dec 11, 2021

@timnolte i have issued a PR #353 hope i did it corectly :)

@Glowsome
Copy link
Contributor Author

Glowsome commented Jan 14, 2022

@timnolte do beware that i bumped the version of the addon in the PR to above release to avoid it being overwritten locally.
If this an issue in the PR then please let me know.

@timnolte
Copy link
Collaborator

@Glowsome hmm, generally I don't want people bumping the version, though I need to make sure that I bump the version to a future alpha after a release to assist with development testing. Can you revert that version bump and I will do so and then you can rebase with the latest. I have some dependency and local development changes to merge in as well, including some fixes for translations.

@Glowsome
Copy link
Contributor Author

@timnolte i will revise the PR to not bump the version as requested.

@Glowsome
Copy link
Contributor Author

@timnolte i will revise the PR to not bump the version as requested.

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues & PRs related to new features.
Projects
None yet
2 participants