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

Implicit grant now handles scope like password grant, keys and tokens are now case-sensitive #187

Closed
wants to merge 3 commits into from

Conversation

josephspurrier
Copy link

http://tools.ietf.org/html/rfc6749#section-4.2.1

The scope should be pulled from $authParams['scope'](without an s). The scope should also be handled the same way as the other grants.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 2f49789 on josephspurrier:develop into 4480aa3 on thephpleague:develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d04c93d on josephspurrier:develop into 4480aa3 on thephpleague:develop.

@josephspurrier josephspurrier changed the title Implicit grant now handles scope like password grant Implicit grant now handles scope like password grant, keys and tokens are now case-sensitive Jul 12, 2014
@shadowhand
Copy link
Member

👍


for ($i = 0; $i < count($scopes); $i++) {
$scopes[$i] = trim($scopes[$i]);
if ($scopes[$i] === '') unset($scopes[$i]); // Remove any junk scopes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this entire foreach block can be replaced with:

$scopes = array_filter($scopes);

@alexbilbie alexbilbie closed this Nov 8, 2014
@shadowhand
Copy link
Member

@alexbilbie can you comment a little bit more on close status? Is this integrated with v4, or simply too old to be merged?

@alexbilbie
Copy link
Contributor

I've removed the implicit grant in v4.

I will consider a sibling package that adds support but I don't want it to be part of the main library because it isn't safe to use (in terms of the grant from the main spec, not just this implementation)

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.

None yet

4 participants