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 response message in case of invalid token #142

Merged
merged 1 commit into from
Mar 14, 2016
Merged

Add response message in case of invalid token #142

merged 1 commit into from
Mar 14, 2016

Conversation

chalasr
Copy link
Collaborator

@chalasr chalasr commented Mar 9, 2016

In response of issue #124 , in the JWTListener I replaced the Response by a JsonResponse (such as done by the AuthenticationFailureHandler) but using the exception's message.

The result is that when the token is found but invalid, the following response is returned :

{
  "code": 401,
  "message": "Invalid JWT Token"
}

Instead of an empty response as it's done currently.

If this makes sense, I can adapt the JWTListenerTest to check the behavior in this specific case (invalid token).

@chalasr chalasr changed the title Added JsonResponse with message in case of invalid token Add JsonResponse with message in case of invalid token Mar 9, 2016
@chalasr chalasr changed the title Add JsonResponse with message in case of invalid token Add response message in case of invalid token Mar 9, 2016
@slashfan
Copy link
Contributor

Hi @chalasr it's a good idea if it fixes the the bad token and empty response issue. Could you update the tests or add new ones for this features ?

@chalasr
Copy link
Collaborator Author

chalasr commented Mar 11, 2016

Hi @slashfan. No problem for the tests, I think verifiying only that the response is not empty. I'll add a check in the JWTListenerTest this week, please tell me if you have more insight or if it's not sufficient. I be back when it's done.

Use short array syntax + fix cs

Update JWTListenerTest to check response on invalid token found

Removed useless comment

Use Exception message as response message for assertion
@chalasr
Copy link
Collaborator Author

chalasr commented Mar 12, 2016

Hello @slashfan

I updated the tests (added a check to see if the response message corresponds to the exception message).
Have a good week end.

@slashfan
Copy link
Contributor

Thanks @chalasr

slashfan added a commit that referenced this pull request Mar 14, 2016
Added json response and message in case of invalid token
@slashfan slashfan merged commit bd9040f into lexik:master Mar 14, 2016
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