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

Add JWTAuthenticationResponse #177

Merged
merged 2 commits into from
Jun 6, 2016
Merged

Add JWTAuthenticationResponse #177

merged 2 commits into from
Jun 6, 2016

Conversation

chalasr
Copy link
Collaborator

@chalasr chalasr commented May 20, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations yes
Fixed tickets n/a
Tests pass? yes

This introduces a generic Response class for successful/failed authentication.
BTW remove duplicated failure Responses with pretty much the same data (3times).

Let me know what do you think.

Steps:

  • Add JWTAuthenticationFailureResponse & JWTAuthenticationSuccessResponses
  • Add tests
  • Update documentation

CS Fixes

[WIP] Use the JWTResponse in authentication handlers

Add authentication failure/success Response classes

Fix tests
@chalasr
Copy link
Collaborator Author

chalasr commented May 30, 2016

ping @slashfan

@slashfan
Copy link
Contributor

OK for me, I was just waiting for the last checkbox to be checked

@chalasr
Copy link
Collaborator Author

chalasr commented May 31, 2016

I was waiting your opinion about the feature before writing the doc :) I ping you when it's done

@chalasr
Copy link
Collaborator Author

chalasr commented Jun 3, 2016

@slashfan Doc slightly updated, ready to review. No changed typehint and no difference in events that developers listen on, no BC break.

*/
public function setMessage($message)
{
$this->message = $message;
Copy link

Choose a reason for hiding this comment

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

Is it expected that the JSON body keep the initial message when the message is modified ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch @GromNaN

Scrutinizer Fixes

Fix segmentation fault (infinite loop)

BTW fix deprecation warning if funct test config

Fixed setData logic in success+failure responses

Add a JWTAuthenticationFailureTest::testSetMessage
@slashfan
Copy link
Contributor

slashfan commented Jun 6, 2016

👍

@chalasr chalasr merged commit 474a4d8 into lexik:2.0 Jun 6, 2016
chalasr added a commit to chalasr/LexikJWTAuthenticationBundle that referenced this pull request Jun 22, 2016
| Q             | A    |
|---------------|------|
| Bug fix?      | no  |
| New feature?  | yes |
| BC breaks?    | no  |
| Deprecations | yes |
| Fixed tickets | n/a |
| Tests pass?   | yes  |

This introduces a generic Response class for successful/failed authentication.
BTW remove duplicated failure Responses with pretty much the same data (3times).

Let me know what do you think.

Steps:

- [x] Add JWTAuthenticationFailureResponse & JWTAuthenticationSuccessResponses
- [x] Add tests
- [x] Update documentation
@chalasr chalasr deleted the jwt_response branch September 17, 2016 10:48
@lexik lexik locked and limited conversation to collaborators Sep 17, 2016
@lexik lexik unlocked this conversation Sep 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants