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

Introduce AuthorizationRequestInterface #1110

Merged

Conversation

lordrhodos
Copy link

This PR introduces a new AuthorizationRequestInterface which is implemented by the current AuthorizationRequest. This way implementors are provided with a more flexible way to override certain parts of the authorization grant types than with the concrete class alone.

In my case I was looking into a way to use a decorated authorization request to be able to add the nonce attribute used of an OpenID Connect (OICD) implementation. I understand that this library is only supporting the OAuth2 specs and nothing related to OIDC (see #962) which is totally fine with me.

This PR raises the flexibility of the library without doing any harm 🙂

@lordrhodos
Copy link
Author

related to #1003

@Sephster
Copy link
Member

Looks good to me @lordrhodos although this is a breaking change so will need to go in version 9.

@Sephster Sephster changed the base branch from master to 9.0.0-WIP April 17, 2020 00:04
@Sephster Sephster merged commit 5d58959 into thephpleague:9.0.0-WIP Apr 17, 2020
@lordrhodos lordrhodos deleted the authorization-request-interface branch April 17, 2020 04:26
@lordrhodos
Copy link
Author

Thank you @Sephster 👍

@iNviNho
Copy link

iNviNho commented Apr 17, 2020

Looks good :)

@shineability
Copy link

@lordrhodos Would you be willing to share how you fixed the nonce issue using the decorator? I solved it in a very hacky way by saving the nonce on the User when authenticating and then retrieve the value from the User again when generating the ID token in the response. I could use a more elegant solution :)

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.

4 participants