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

Role-based access to cloud functions #7093

Closed
3 tasks done
bahaa-kallas opened this issue Dec 23, 2020 · 7 comments
Closed
3 tasks done

Role-based access to cloud functions #7093

bahaa-kallas opened this issue Dec 23, 2020 · 7 comments
Labels
type:feature New feature or improvement of existing feature

Comments

@bahaa-kallas
Copy link

bahaa-kallas commented Dec 23, 2020

New Feature / Enhancement Checklist

Current Limitation

I just went through the documentation of the new feature "Cloud Functions Validators" while it's without any doubt a feature that we needed a long time ago (btw Thanks for making it happened) But I think the feature missis a "very very" common use case which is Role-based access to cloud functions

Feature / Enhancement Description

Adding another attribute to the options object called allowUsersWithRole which take an array contains the names of the Roles (And sure the underlying logic to make this work :D)

Example Use Case

Parse.Cloud.define("averageStars", async (request) => {
  const query = new Parse.Query("Review");
  query.equalTo("movie", request.params.movie);
  const results = await query.find();
  return sum / results.length;
},{
  fields : ['movie'],
  requireUser: true,
  allowUsersWithRole: ["Admin","Moderator"]
});

Alternatives / Workarounds

In the documentation you offered an alternative in one of the examples by using a field on the user object which serves exactly like user Role (while I don't agree with the approach to handle this use case but it solves the issue)

Parse.Cloud.define("averageStars", async (request) => {
  const query = new Parse.Query("Review");
  query.equalTo("movie", request.params.movie);
  const results = await query.find();
  let sum = 0;
  for (let i = 0; i < results.length; ++i) {
    sum += results[i].get("stars");
  }
  return sum / results.length;
},{
  fields : {
    movie : {
      required: true,
      type: String,
      options: val => {
        return val < 20;
      },
      error: "Movie must be less than 20 characters"
    }
  },
  requireUserKeys: {
    accType : {
      options: 'reviewer',
      error: 'Only reviewers can get average stars'
    }
  }
});
@dblythy
Copy link
Member

dblythy commented Dec 24, 2020

Hi @bahaa-kallas i’m really glad you like the validation feature and I like your suggestion! I am happy to work on this feature if need be. 😊

@dblythy
Copy link
Member

dblythy commented Feb 13, 2021

This can be closed, as on the master you can pass:

requireAnyUserRoles: request.user must match one of the specified roles.
requireAllUserRoles: request.user must match all of the specified roles.

@bussedesigngb
Copy link

bussedesigngb commented Jun 13, 2021

Okay I can see we can pass
requireAnyUserRoles and requireAnyUserRoles but they don't work.
I'm on parse version 4.5.0
I've tried to pass in a string array and nothing happened
I've tried passing in a function that returns a single string with role name or array with role names and nothing.
Does this have to work in conjunction with any additional parameters?

I'm trying to run this on a Parse.Cloud function.

Parse.Cloud.define("functionName", async req => {
    try {
        //Something here
    } catch (error) {
        throw error;
    }
}, {
    requireUser: true,
    requireAnyUserRoles: ['Admin']
})

@mtrezza
Copy link
Member

mtrezza commented Jun 13, 2021

I'm on parse version 4.5.0

It seems @dblythy was referring to the current master branch, not the latest release 4.5. You would need to point the parse server dependency to a commit on the master branch.

Mind that master contains a number of breaking changes, because we are still working on implementing more predictable release cycles. The list of changes can be found in the changelog which should be fairly complete.

@unicornist
Copy link

unicornist commented Jun 14, 2021

when this will be released? is there any specific plan?
Can we use the same name for two cloud functions with different access validation middleware?
Are there any benefits other than shorter code to use these options instead of manually checking for a role?

@unicornist
Copy link

I have a problem that I thought would be solved by this PR, but I don't think so, please enlighten me.
Whenever we want to use triggers to force some filtering for normal users (for example filter out soft-deleted objects onBeforeFind) we put that trigger for ALL users, and if we want to exclude this for some specific Role (e.g. disable the trigger for admins), on each trigger call (basically for each query) we have to query for the user's roles to check if he/she has the required role. If admin calls are 1% of the total query calls for that Class, we are running that extra Role checking query for 99% unnecessarily.
The only solution I found for this is to use an extra cloud function for admins to check the Role and run the query with a useMasterKey and exclude the trigger for req.master.
Generally, I think there should be a manageable validation layer and we have to be able to have redundant triggers or cloud functions for different roles.

@unicornist
Copy link

Just now I realized that the problem is not with how to validate the roles for the triggers or functions, the root problem is why we need to query the DB for checking Role?
Personally I also have problems with storing the session tokens in DB and checking with it all the time. I think if we use a JWT for 'user session' AND 'roles', many scaling problems will be resolved.
And then we can talk about fancy ways of validation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests

5 participants