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

Adding RememberMeToken allowance #19

Closed
wants to merge 4 commits into from

Conversation

PonteIneptique
Copy link
Contributor

This addition doesn't break the service and allows users of this lib to user more various AuthToken without having to hack into the code. I am using it with SimpleUser for example. Both tokens have same API for what oauth2-php use.

This addition doesn't break the service and allows users of this lib to user more various AuthToken without having to hack into the code
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) when pulling 36f2305 on PonteIneptique:master into 498649e on authbucket:master.

@hswong3i
Copy link
Member

hswong3i commented Nov 5, 2014

May you add a test case for it, too?

@PonteIneptique
Copy link
Contributor Author

Yes I'll do

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.51%) when pulling 9195d52 on PonteIneptique:master into 498649e on authbucket:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.51%) when pulling d1b92a1 on PonteIneptique:master into 498649e on authbucket:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.51%) when pulling 7e99934 on PonteIneptique:master into 498649e on authbucket:master.

@PonteIneptique
Copy link
Contributor Author

Ok, I have added two different tests and upgraded one :

  • In general, I found out $this->assertTrue($client->getResponse()->isRedirect()); is not enough for testing authorize, you should add $this->assertTrue($client->getResponse()->getTargetUrl() != "http://localhost/oauth2/login"); as I did for testGoodCodeFormSubmit()
  • testNoRememberMeToken test with client regeneration
  • testRememberMeToken test with client regeneration but copied/pasted Cookie from previous client

@PonteIneptique
Copy link
Contributor Author

I think we should keep it separated from the other PR on the mount situation, because it's a minor change. Unlike the mount.

@hswong3i
Copy link
Member

Progress merge into 3bcb08d with cleanup, released as v2.4.0, thank you very much ;-)

@hswong3i hswong3i closed this Nov 18, 2014
@PonteIneptique
Copy link
Contributor Author

You are very welcome @hswong3i
Though, I am a very bit sad to not be shown in the contributors :/ It might be stupid but for my employer it's important :)

@hswong3i
Copy link
Member

Oooh sorry that since I also did some more cleanup as refer to overall coding style...
I will learn about how to include the original author information for ongoing development m(_ _)m

@hswong3i hswong3i mentioned this pull request Nov 18, 2014
@PonteIneptique
Copy link
Contributor Author

It's ok :) Have a great day

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

3 participants