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

Accept response_mode=form_post on the Supervisor callback #2238

Open
graindcafe opened this issue Feb 28, 2025 · 4 comments
Open

Accept response_mode=form_post on the Supervisor callback #2238

graindcafe opened this issue Feb 28, 2025 · 4 comments

Comments

@graindcafe
Copy link

graindcafe commented Feb 28, 2025

Is your feature request related to a problem? Please describe.

Microsoft ADFS 2016+ requires to use response_mode=form_post to send what is called custom claims or extra claims. Especially the claim "email" is not sent using response_mode=query.

We can configure pinniped to request the ADFS for a response_mode=form_post but it cannot handle it afterward.

Describe the solution you'd like

The solution I suggest is to modify the callback handler so that it accepts both GET request and POST request.
Then modify the methods accessing the code and state parameters to handle the case where it has been sent in the request body, if the verb was POST.

Describe alternatives you've considered

None

Are you considering submitting a PR for this feature?
Yes

  • How will this project improvement be tested?
    Currently : receiving a POST request on the callback failed with 405 method not allowed
    After: receiving a POST request with the parameters code and state in the request body will be successfully handled.

  • How does this change the current architecture?
    No impact on the architecture.

  • How will this change be backwards compatible?
    The callback handler will still answer 405 for verbs other than POST or GET.
    If the verb is POST and the code or state cannot be found, it will fail in the same way as a GET request without these parameters.

  • How will this feature be documented?
    The documentation should be updated to clarify the fact that receiving a response_mode=form_post is now possible on the supervisor.

Additional context

My tests were on pinniped 0.25 but reading the code there is no reason this behaviour has changed in between.

@graindcafe graindcafe changed the title Accept response_mode=form_data on the Supervisor callback Accept response_mode=form_post on the Supervisor callback Feb 28, 2025
@cfryanr
Copy link
Member

cfryanr commented Feb 28, 2025

Thanks for creating this issue @graindcafe.

You are correct in your analysis that the Supervisor will allow you to configure response_mode as one of the additionalAuthorizeParameters but does not yet support this on the callback handler.

For future reference to implementors, here is the relevant spec https://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html.

Do you have any links to ADFS documentation describing the requirement to use response_mode=form_post to send custom claims or extra claims? That would be helpful.

@cfryanr
Copy link
Member

cfryanr commented Feb 28, 2025

Thanks for sharing those docs @graindcafe. According to those docs, for a confidential client (like the Supervisor) you must have "KB4019472 or later security update installed on your AD FS servers". Just to confirm, is that is your situation?

@graindcafe
Copy link
Author

Yes

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

No branches or pull requests

2 participants