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 for Pushed Authorization Requests (PAR) #269

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

d-javu
Copy link

@d-javu d-javu commented Jun 14, 2021

Implement OAuth 2.0 Pushed Authorization Requests is defined in
(draft-ietf-oauth-par-08)

PAR lets the client push the authorization request to the IP
ahead of end-user involvement

OAuth 2.0 Pushed Authorization Requests is defined in
(draft-ietf-oauth-par-08)

PAR lets the client push the authorization request to the IP
ahead of end-user involvement
@azmeuk
Copy link
Collaborator

azmeuk commented Oct 19, 2022

For the record, now this is RFC9126.

@d-javu
Copy link
Author

d-javu commented Oct 19, 2022

I know, but it works for me, and no one seemed to be interested in this, so I just let it be.
Do you think anyone would care about this enough to merge this feature if I were to update it to apply to the current repository?
Assuming no one has already implemented it differently in the mean time, I haven't followed this project closely.

@azmeuk
Copy link
Collaborator

azmeuk commented Oct 19, 2022

Do you think anyone would care about this enough to merge this feature if I were to update it to apply to the current repository?

I know I could not review this at the moment because I have no test setup supporting PAR to check the PR. Maybe @DeepDiver1975 could?

@DeepDiver1975
Copy link
Collaborator

Maybe @DeepDiver1975 could?

I am lacking a test env as well ....

I merged other prs which "looked" good but did cause regression afterwards - so I am not that positive at the moment ...

@azmeuk
Copy link
Collaborator

azmeuk commented Oct 19, 2022

Maybe the solution to this would be to set up an integration test environment, so patches could be merged at the condition they implement tests.

@DeepDiver1975
Copy link
Collaborator

Maybe a more generic extension point could help devs to integrate such functionalities?

In this line https://github.com/jumbojett/OpenID-Connect-PHP/pull/269/files#diff-153547f1c0203c10f6f31847c1601d01a841d4b6dc6a8797c9f98feeb0e84bf8R678
something like this:

$this->beforeRequestAuthorize(&$auth_params);

The default impl will do nothing and any developer could subclass the client and add logic as needed - like in this PR.

Or we introduce event dispatcher so that people don't need to subclass .....

@DeepDiver1975
Copy link
Collaborator

Maybe the solution to this would be to set up an integration test environment, so patches could be merged at the condition they implement tests.

In an ideal world I absolutely agree. e.g. spinning up a key-cloak docker image as part of the github workflow should work.

let's see if somebody finds the time .... (would love to do it .....)

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

Successfully merging this pull request may close these issues.

3 participants