feathersjs-ecosystem / feathers-permissions

Simple role and service method permissions for Feathers
MIT License
184 stars 29 forks source link

Unexpected behavior on checkPermissions with error: false #71

Closed jd1378 closed 4 years ago

jd1378 commented 4 years ago

Expected behavior

What I expect when using it is to not throw even when user is not authenticated (provider is undefined) Instead just set context.params.permitted to false

Actual behavior

throws 403 with You do not have the correct permissions (invalid permission entity).

daffl commented 4 years ago

You still need to implement Anonymous authentication.

jd1378 commented 4 years ago

why should we need to do that ? Isn't it easier to not throw any error as it should not ? what are the advantages of implementing anonymous authentication?

jd1378 commented 4 years ago

It doesn't seem right to me to just implement anonymous authentication to achieve something so simple

daffl commented 4 years ago

Sorry you did say that provider was undefined. Can you provide a complete example with what you expect and what you get? Because the error you are getting will definitely not be thrown if params.provider is falsy (see https://github.com/feathersjs-ecosystem/feathers-permissions/blob/master/lib/index.js#L33).

jd1378 commented 4 years ago

Sorry, my bad, the provider was not undefined. this is what I'm trying to do:

find: [
      iff(
        isProvider('external'),
        checkPermissions({
          roles: ['admin'],
          error: false,
        }),
        iff(
          (context) => !context.params.permitted,
          (context) => {
            set(context, 'params.query', {
              public: true,
            });
            return context;
          }
        )
      ),
    ],

the code above does throw the said error on find, but I expect it not when error is false

jd1378 commented 4 years ago

instead I expect to have context.params.permitted set to false and it doesn't feel right to have anonymous authentication implemented just for that

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Apologies if the issue could not be resolved. FeathersJS ecosystem modules are community maintained so there may be a chance that there isn't anybody available to address the issue at the moment. For other ways to get help see here.