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

Honor request's scope value #25

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

Conversation

chuanma
Copy link

@chuanma chuanma commented Apr 28, 2013

In the OAuth2 spec on granting an access token, "the authorization server MAY fully or partially ignore the scope requested by the client based on the authorization server policy or the resource owner's instructions".

So OAuth2 spec doesn't have an opinion on how scope is being handled. But oauth2-php currently has an opinion: fully ignores the request's scope value.

As a result, there is at least one use case that's impossible:

  1. a client with a 'client_credentials' grant type has both 'read' and 'write' scope.
  2. the client wants to get an access token that can ONLY 'read' data.

With this PR, the client can ask for an access token of 'read' scope by passing in 'scope=read' in the request.

…fied in the request instead of the stored default scope
@bartvw
Copy link

bartvw commented Oct 2, 2013

What's keeping this PR from being merged?

It is currently impossible for a client to request a token with a scope that is narrower than the supported scope.

@Spomky Spomky mentioned this pull request Oct 21, 2013
@Spomky
Copy link

Spomky commented Oct 21, 2013

OK I have understood your PR.
I have been working on the policy functionality and it seems to fix your problem.

@pyrech
Copy link

pyrech commented Jul 14, 2014

I have created a related PR (#64) which does the same thing with according tests ;)

@Spomky
Copy link

Spomky commented Nov 3, 2014

This PR might be closed as #64 has been merged.
@chuanma Can you confirm that the requested scope is correctly honored?

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

Successfully merging this pull request may close these issues.

5 participants