Skip to content
This repository was archived by the owner on Aug 29, 2018. It is now read-only.

Fixes #27 Strategy succeeds even if user is not found #28

Conversation

AlexanderArvidsson
Copy link

Does this have any side-effects?
OAuth2 authentication package does this already.

@AlexanderArvidsson
Copy link
Author

The tests expect the function to not return an error, anyone care to explain why this was the chosen functionality?

@daffl
Copy link
Member

daffl commented Jul 22, 2017

Some of the tests are failing. I thought there was a reason why it is doing it that way, but @ekryski might know.

@ekryski
Copy link
Member

ekryski commented Jul 24, 2017

@daffl Yes because sometimes you want to just create a JWT without anything in the payload (an anonymous JWT) or you may want to encode everything you need to make authorization decisions into the payload of a JWT that has a shorter TTL (for more scalable apps).

Thanks for the PR @Metamist but I don't think we can merge this. However, I did leave a comment on the issue with a few options for you. #27 (comment)

@ekryski ekryski closed this Jul 24, 2017
@daffl
Copy link
Member

daffl commented Jul 24, 2017

The problem is that this will return a valid JWT on any error, including database or syntax errors. I'm pretty sure that is the same problem @zusamann is having in his application (https://github.com/zusamann/feathers-socket-auth-issue) as well.

@daffl
Copy link
Member

daffl commented Jul 24, 2017

I would think it should only do this on a NotFound error.

@ekryski
Copy link
Member

ekryski commented Jul 24, 2017

@daffl Agreed. Which is related to #14. @Metamist if you wanted to update the PR to accommodate that and add a test then we can re-open this PR.

@subodhpareek18
Copy link

subodhpareek18 commented Jul 25, 2017

Actually @daffl I don't think I'm having a slightly different error. The custom auth service I've written first finds the user with that username, and only then proceeds, so I'm sure it's not a case of user not being found. Secondly I don't think there are any syntax errors either, the service pretty much works for all cases correctly (correct username/pass, incorrect username/ incorrect pass).

The token seems to be completely valid when returned. And works perfectly fine with the rest client. Only in the websocket client once authenticate call has been made, the rest of the calls fail due to the error Auth token not found.

@subodhpareek18
Copy link

Though in the repository I've shared there is no custom auth service in the latest commits as I was trying to simplify the logic and see if the error still persists, which it does.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants