Meteor-Community-Packages / meteor-roles

Authorization package for Meteor, compatible with built-in accounts packages
http://meteor-community-packages.github.io/meteor-roles/
MIT License
921 stars 166 forks source link

Add ability to disable the roles subscription #256

Open dobesv opened 6 years ago

dobesv commented 6 years ago

Currently every user who visits the site creates a subscription to the roles collection. However, this is a waste since we never update that collection, thus it would be better to just embed the list of roles into the source code. I wish we could disable this subscription globally, or make it use a "virtual collection" of some sort that doesn't have any server overhead. Any idea how this could be done?

mitar commented 6 years ago

Yea. I also dislike this. Maybe we could break this in 2.0 branch and remove this automatic subscription. I am not sure what was design decision here why this is automatic.

What you could do at least is call Roles.subscription.stop() for now.

SimonSimCity commented 6 years ago

@mitar I need this automatic subscription since the administrators of my platform have a view where they at any time can change a role or add them, which should immediately be reflected in the view of the user, having this role - without the need of reloading or even rebuilding the system.

mitar commented 6 years ago

But you can subscribe yourself for that view, no?

SimonSimCity commented 6 years ago

@mitar As of now this is true, but I'm also planning on refactoring the way the roles are connected to the user. The current design makes it likely for us to run into the limitation of the size of a document.

We have a project-management software, where the user has permissions per project he's working on. Since we leave the user with his permissions to the project also after the project is done and archived, and some users are working on a new project daily where they each time get up to 30 permissions, this will hit us in the long run.

But maybe another collection, where only these connections are hold, would already answer it.

mitar commented 6 years ago

As of now this is true, but I'm also planning on refactoring the way the roles are connected to the user.

I am not sure how is this related to the question of automatic subscription to all roles. You can always subscribe to all roles yourself.

We have a project-management software, where the user has permissions per project he's working on.

Yes, I do not believe this package is suitable for per-object permissions. In my view, per-object permissions belong to object documents. Not to user document. I see this package to be suitable for per-collection permissions. So a permission to edit all blog posts, for example. This package is also useful when you have partitions of those collections, because you can make groups. So if you have multiple blog posts belonging to website A, and other blog posts belonging to website B.

But if you have per blog post permissions, put those permissions into blog post documents themselves.

mitar commented 6 years ago

The current design makes it likely for us to run into the limitation of the size of a document.

You will reach 16 MB of permissions? Even if you have 1 KB of permissions per project, and you have one project per day, this would still take 44 years to reach.

SimonSimCity commented 6 years ago

Sorry for spoiling this ... I think I misunderstood something about this subscription.

Excepted for the getAllRoles() method of the shared code I don't see any reason not to remove it. The other methods using it are manipulating methods, which should only be used within meteor-methods.

I guess by dropping this the developers, using allow/deny instead of meteor-methods, have to update their code respectively if using one of the manipulating methods without publishing the roles themselves.

RobertLowe commented 6 years ago
screen shot 2018-05-10 at 4 10 25 pm

25% of our fetched docs for what is an enum with 4 roles...

SimonSimCity commented 4 years ago

Could be very well suited to pick up for the next major version #295