Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Feathers auth client does not treat non-401 failures as failures #1787

Closed
jnardone opened this issue Jan 17, 2020 · 4 comments · Fixed by #2892
Closed

Feathers auth client does not treat non-401 failures as failures #1787

jnardone opened this issue Jan 17, 2020 · 4 comments · Fixed by #2892

Comments

@jnardone
Copy link
Contributor

If you have an existing JWT, when an initial reAuthenticate call happens, if the referenced entity does not exist any more (e.g. deleted from users) then the feathers client does not handle this as a failure and no response is delivered to the caller.

We hit this in some tests, but it's easy enough if you either remove the test user from the users collection or supply a JWT with an invalid user ID/sub reference.

I can make this succeed if I change jwt.ts to:

-    const result = await entityService.get(id, omit(params, 'provider'));
+    try {
+      const result = await entityService.get(id, omit(params, 'provider'));
+    } catch(err) {
+      throw new NotAuthenticated(`Could not find entity`);
+    }

but I'm not advocating this as a specific approach. The client is clearly not looking for a 404 to come back, but that's what gets delivered back to the feathers authentication client.

Node 12.14
Feathers 4.4.3 (server and client)

@jnardone jnardone changed the title Missing entity on JWT auth isn't caught by client Feathers auth client does not treat non-401 failures as failures Feb 4, 2020
@jnardone
Copy link
Contributor Author

jnardone commented Feb 4, 2020

What i've found is the feathers auth client will not ever respond to an authentication request if the error that's returned is not a 401 (say, a 404). We had to put a server-side hook in to make sure that any non-401 errors from authentication are changed to 401 while this bug exists.

@daffl
Copy link
Member

daffl commented Feb 5, 2020

Do you have an example to reproduce? I just tried throwing a different kind of error in the feathers-chat and the frontend catches it as expected.

@jnardone
Copy link
Contributor Author

jnardone commented Feb 5, 2020

The way we see it happen in practice is when we try to authenticate a jwt with a sub/userId reference to a user that no longer exists in the users service. Generally we only hit this while testing.

I'll try to work up a smaller, self-contained example.

@daffl
Copy link
Member

daffl commented Nov 24, 2022

I believe #2892 should fix this issue as well.

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

Successfully merging a pull request may close this issue.

2 participants