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

You're not allowed to get on 'users' #52

Closed
lukashass opened this issue Jan 18, 2022 · 3 comments · Fixed by #74
Closed

You're not allowed to get on 'users' #52

lukashass opened this issue Jan 18, 2022 · 3 comments · Fixed by #74
Labels
documentation Improvements or additions to documentation

Comments

@lukashass
Copy link
Contributor

When configuring feathers-casl I ran into a problem when authenticating with @feathersjs/authentication:

You're not allowed to get on 'users'

The responsible code that calls get with provider set should be:
https://github.com/feathersjs/feathers/blob/d14f57ed8316c89ffde85c9acb17ecd790e454c5/packages/authentication/src/jwt.ts#L112

Steps to reproduce

lukashass/feathers-chat-ts@a0e2604

Possible fix

My current approach to fix this would be to explicitly add the ability with a hook on users.get, just like https://feathers-casl.netlify.app/getting-started.html#using-casl-with-the-rest-express-js-transport.

Maybe there is even an easy fix that could be applied in feathers-casl?

System configuration

Module versions:

This occurred while using dove but the linked reproduction is:

  • feathers: ^4.5.11
  • feathers-casl: ^0.7.1

NodeJS version: v16.13.1

@fratzinger
Copy link
Owner

Thanks for the exemplary issue and for the repro link! Highly appreciated!
Is this rest or websocket? For rest, we have this note: https://feathers-casl.netlify.app/getting-started.html#using-casl-with-the-rest-express-js-transport
I think this could even be optimized like the following:

// Add this to set abilities, if a user exists
  context => {
    if (context.params.ability) { return context; } // for socket.io connections
    const { user } = context.params
    if (user) context.params.ability = defineAbilitiesFor(user)
    return context
  }

Explanation: @feathersjs/express does not seem to map the result from authentication onto the context.params object, like @feathers/socket.io does. Maybe that should be fixed in @feathersjs/express. Don't even know where the magic happens in the source code of @feathersjs/authentication.

@lukashass
Copy link
Contributor Author

Is this rest or websocket?

I'm using a websocket, but repurposed the hook from the REST docs 😉.

That optimization looks good, haven't thought of that.

@fratzinger
Copy link
Owner

The note about rest could be more present, of course. Let me know, if it works.
If you got an idea how that note could be better highlighted, please let me know. Maybe in the Gotchas section.

Greets from Rostock.

@fratzinger fratzinger added the documentation Improvements or additions to documentation label Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants