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

update idpexception to account for empty strings #203

Closed
wants to merge 60 commits into from
Closed

update idpexception to account for empty strings #203

wants to merge 60 commits into from

Conversation

geofflancaster
Copy link
Contributor

Some providers (ex. Google) specify an error response object which is an empty string in cases where a 403/401 are returned. This will force the exception to check that the value isset and it is not an empty string which will help prevent the Unknown error case from occurring.

Sergio Vera and others added 20 commits December 23, 2014 16:16
Added support for Github Enterprise
Added Naver as Third-Party Provider
update google scopes and endpoint to remove deprecated values
New protected method is responsible for making the request to a
provided URL, allowing for multiple API endpoints to be used within the same
provider.

AbstractProvider::fetchUserDetails() remains, but now only builds the
user details URL and passes it to AbstractProvider::fetchProviderData().
Adds methods to allow fetching user email addresses from Github
Updated README.md to include Google Nest provider
User id being set for Google API
Fix UserTest.php namespace
Some providers (ex. Google) specify an error response object which is an empty string in cases where a 403/401 are returned. This will force the exception to check that the value isset and it is not an empty string which will help prevent the Unknown error case from occurring.
@ramsey
Copy link
Contributor

ramsey commented Feb 3, 2015

@geofflancaster Can you provide a test for this edge case? Thanks!

@ramsey
Copy link
Contributor

ramsey commented Feb 10, 2015

Ping, @geofflancaster

@geofflancaster
Copy link
Contributor Author

I'm here. Just swamped. I'll try to get to it tonight.

Thanks,
Geoff

On Tue, Feb 10, 2015 at 11:37 AM, Ben Ramsey notifications@github.com
wrote:

Ping, @geofflancaster https://github.com/geofflancaster


Reply to this email directly or view it on GitHub
#203 (comment)
.

@ramsey
Copy link
Contributor

ramsey commented Mar 10, 2015

@geofflancaster, I'm ready to merge this in. I'd just like a test for it, please. :-)

Added test cases for the empty string edge cases
@geofflancaster
Copy link
Contributor Author

Test cases added.

@ramsey
Copy link
Contributor

ramsey commented Mar 10, 2015

Looks like there might be conflicts. Can you merge master into your branch and see what breaks, please? :-)

zot24 and others added 26 commits March 10, 2015 15:34
Added Twitch to the additional providers, and alphabetized the list for more grokkability.
This will be moved to its own package
Problem:

Some providers are sending additional information when an error occurs,
which can't be accessed because property `$result` is protected.

Solution:

Data from this property can be accessed using `getResponseBody()` method.

Closes #185
As per recent Travis builds:

> You are using the deprecated option "dev". Dev packages are installed by default now.
Conflicts:
	README.md
	composer.json
	src/Provider/Github.php
	test/src/Provider/GithubTest.php
@geofflancaster
Copy link
Contributor Author

merge complete.

On Tue, Mar 10, 2015 at 3:25 PM, Ben Ramsey notifications@github.com
wrote:

Looks like there might be conflicts. Can you merge master into your branch
and see what break, please. :-)


Reply to this email directly or view it on GitHub
#203 (comment)
.

@ramsey
Copy link
Contributor

ramsey commented Mar 10, 2015

Thanks, @geofflancaster.

That merge looked quite a bit hairy, and I'm not sure why the files listed as being in conflict were in conflict (for example, the README), since you didn't touch them in your branch. So, I pulled down your branch, backed out the merge, and then merged just your changes, resolving the conflict in IDPExceptionTest.

As a result, the following new commits were created in thephpleague/oauth2-client master branch:

Thanks!

@ramsey ramsey closed this Mar 10, 2015
@geofflancaster
Copy link
Contributor Author

Agree. I thought that was strange too. Thanks for taking care of it.

On Tue, Mar 10, 2015 at 4:10 PM, Ben Ramsey notifications@github.com
wrote:

Thanks, @geofflancaster https://github.com/geofflancaster.

That merge looked quite a bit hairy, and I'm not sure why the files listed
as being in conflict were in conflict (for example, the README), since you
didn't touch them in your branch. So, I pulled down your branch, backed out
the merge, and then merged just your changes, resolving the conflict in
IDPExceptionTest.

As a result, the following new commits were created in
thephpleague/oauth2-client master branch:

Thanks!


Reply to this email directly or view it on GitHub
#203 (comment)
.

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