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

Block the request to issue new tokens if an invalid provider is given. #50

Merged
merged 3 commits into from
Sep 1, 2018

Conversation

Alymosul
Copy link
Contributor

This PR adds an early protection in the AddCustomProvider middleware by checking that the provider given in the request is registered in the auth configuration, if not it will throw an exception to block the request also if the request is missing the provider param it will block it.

…quest is missing the provider param when requesting to issue new tokens.
Copy link
Owner

@sfelix-martins sfelix-martins left a comment

Choose a reason for hiding this comment

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

@Alymosul Firstly, thanks for your contribution. Just check the review comments and update to able the merge.

}
}

return false;
Copy link
Owner

Choose a reason for hiding this comment

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

@Alymosul Has an error here. If you pass a provider that not found on config('auth.guards') the OAuthServerException should be thrown. But returning false the provider don't will be invalid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah! I missed that, i'll fix it.

tests/Unit/AddCustomProviderTest.php Show resolved Hide resolved
tests/Unit/AddCustomProviderTest.php Show resolved Hide resolved
@Alymosul Alymosul changed the base branch from 2.0 to 3.0 September 1, 2018 14:44
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.

2 participants