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

Giving the ScopeRepository a reference to the AuthCode during AuthCode grant. #672

Closed
Shkeats opened this issue Oct 17, 2016 · 10 comments
Closed
Milestone

Comments

@Shkeats
Copy link

Shkeats commented Oct 17, 2016

It would be useful if the ScopeRepositoryInterface finalizeScopes() method took a nullable param of $authCodeIdentifier so that we can use contextual information stored on the authCode object to make decisions about which scopes a user can get. The AuthCode Grant could pass in it $authCodePayload->auth_code_id to this.

In my case this would be helpful as I need to persist some information about the state of the users session at the auth server when the auth code is issued and use this to determine if they get a certain scope when their token is issued. Obviously as the auth_code->access_token exchange is server to server the user's session is no longer present at the point the scopes are determined. The AuthCode object seems a natural place to persist this.

Let me know your thoughts or if I'm missing an obvious solution to my problem.

@alexbilbie
Copy link
Contributor

I believe the finalizeScopes does what you require?

@Shkeats
Copy link
Author

Shkeats commented Oct 17, 2016

@alexbilbie as far as I can tell there's currently no way to get a reference to the authcode inside finalize scopes?

@alexbilbie
Copy link
Contributor

finalizeScopes tells you the user ID, the scopes that were requested, the client and the grant type - what additional information do you need to make a decision?

@Shkeats
Copy link
Author

Shkeats commented Oct 17, 2016

@alexbilbie It would be useful to have the AuthCode's id to access information persisted alongside the AuthCode when the user was present.

In my case, these are details about the state of the user's session at the point in time the AuthCode was made. This factors into the decision about which scopes can be requested.

Currently what I'm doing is using the user_id, client_id and scopes to grab the newest auth code that matches those criteria from the table, but this isn't always guaranteed to be the same one.

@alexbilbie
Copy link
Contributor

Okay; this will have to wait until V6 as it will require an interface change 👍

@alexbilbie alexbilbie added this to the 6.0.0 milestone Oct 17, 2016
@Shkeats
Copy link
Author

Shkeats commented Oct 28, 2016

On a related note I think it would also be helpful to give the access token repository a reference to the auth code so that we can persist the relationship between the two without needing to create a custom Grant type.

@Richard87
Copy link

@alexbilbie Sorry for bringing this upp, 4! years later...

I have a similar problem, I need to know the security level the user authenticated with (username/password, username/password+otp or high-security external oidc-provider).

This information is available when authorizing the user, and stored in the AuthCode, but it's not available when I produce the AccessToken or the IdToken. I don't care if the security level is stored in the jwt, or in the entity/database, but I need the information when authorizing a request using the provided access-token (or IdToken)

@Richard87
Copy link

Richard87 commented Apr 7, 2020

Is this something you would accept a pull-request to fix?

Edit

I guess the easiset way would be to add the AuthCode to AccessTokenRepositoryInterface::getNewToken() and to AbstractGrant::issueAccessToken()

@Sephster
Copy link
Member

Sephster commented Apr 7, 2020

@Richard87 I would definitely accept a PR for this issue as it should be straight forward. As @Shkeats suggested, we just need a nullable reference to the auth code ID.

This is an interface change though so wouldn't be released until the next major version and would need to be created on the v9 branch. Cheers

@Sephster
Copy link
Member

Closing as merged in #1112

LukDrews added a commit to LukDrews/oauth2-server that referenced this issue Nov 18, 2021
* change errorType to invalid_grant on invalid refresh_token

* Abstract CryptKey public methods to the CryptKeyInterface

* Fix CS

* Update changelog

* Remove type hinting

* Add inherit docblocks to CryptKey

* Fix capitalisation of inheritdoc

* Update changelog

* Change HTTP status code to 400

* Update travis to also run on version 9 branch

* Return invalid_grant error when the authorization code is revoked

* Added test

* Updating PR number

* Update changelog

* RefreshTokenGrant calls finalizeScopes method

* Add a hint when the user authentication fails. Fixes issue thephpleague#1097

* Update the changelog with a placeholder

* Update pull request number

* Remove hint if user credentials are incorrect

* Update changelog

* introduce an AuthorizationRequestInterface for the AuthorizationRequest

* make sure we cover the setState method of the interface as well

* test complete authorization request with multiple request uris

* Update change log

* refactor by extracting the creation of the authorization request to a dedicated function in AbstractAuthorizeGrant

* Update changelog

* Allow auth code ID to be passed to finalizeScopes. Fixes thephpleague#672

* Update changelog

* Readded changes for v9 to changelog

* Add redirect URI to client for RefreshToken test

* Apply StyleCI fixes

Co-authored-by: omdmhd <om.mo1375@gmail.com>
Co-authored-by: ErickSkrauch <erickskrauch@yandex.ru>
Co-authored-by: Andrew Millington <andrew@noexceptions.io>
Co-authored-by: Arie Timmerman <arietimmerman@gmail.com>
Co-authored-by: Andrew Millington <andrew.millington@ed.ac.uk>
Co-authored-by: Paul Jay <paul.jay@tagpay.fr>
Co-authored-by: Patrick Rodacker <patrick@rodacker.de>
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