-
-
Notifications
You must be signed in to change notification settings - Fork 169
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
Reusability: Can we take roles into a separate package, or should I fork? #270
Comments
So the second point I do in my publish endpoint where I limit which permissions get published with a simple filter on the array. The issue is only that Meteor does not auto-merge those together if you have multiple publish endpoints publishing different sets of permissions. For third, I already do that with new schema where you have a document per assignment. So you can add stuff there. I add auditing information there, for example when and who assigned a permission. Personally I think that per-resource permissions belong to the document itself. And for hierarchical resources (like you trying to address with grouping), I have logic which knows the hierarchy. For example, in my case I have permissions on a post and then permissions on comments, and some permissions from a post tr to comments. Do you think groups of resources are enough or would some general relation between be better? Also in your example: folder -> file. But I do see why there could be a separate collection for all permissions. It definitely makes sense. I do not have an answer about collaboration. Myself, I do not have time at the moment. So I will leave to others to way in here. But one option would be to or make a switch in this package in which way one wants to record permissions, or to maybe try to keep APIs the same between packages. Also, a question is if storing permissions inside user document have any advantage at all? |
Not sure if this is helpful but while we call the base object a "user", the API doesn't really care except that we hard-coded So if you could update the target collection, you could conceivably use of the Roles package to control access to a document resource by storing the user IDs on the resource record itself. The usage would look something like this:
Would this cover your use case? |
Thanks for the response of you two, really appreciate that you take yourself time for this.
If we would extract all the permissions a user has into an extra collection, we could avoid this problem.
Good point there. If we go the way of saving the assigned roles in a separate collection (one document per role assigned) this looks tempting but might in fact be not the best place if we want to keep this flexible.
The reason to me is that I also will have quite an amount of people who will have global permission (how would you store this?) and some documents where the amount of users would exceed 100 users. This would again leave me with the same performance problems as I have of today. And you would have the hassle of adding a bunch of permissions to a new comment on creation. As I talk about resource groups, this would be very hard to handle it using the approach you suggested, alanning, because I'm not talking about only one type here. I'd have to define multiple target collections then ... Long story short: I'll create a PR where I extract the roles from the user-document into a separate collection. This should work by keeping the API the same. Later on I'll find a way for realizing the other improvements I suggested and think of ways to get them working, but the extraction of roles from the user is a major requirement to me as I have an N:N relation here where both of the N's wouldn't be low numbers in the long run. |
The more I think about it, I get on the idea to extend a collection by a set of methods provided by this package. This could cover the approach alanning was suggesting here. This would then allow users to create permissions per resource, saved on the resource. The field for scope then would most likely be used as a reference to the user (if it now is the user itself or his group). In this approach we'd have to drop that the system grants permission if the scope is not set. On the other side, our system of permissions is quite fine-grained, which makes it quite flexible but again performance-heavy if we'd take this approach, I feel ... I'd now start with creating a document per assignment in a new collection called |
I've just finished a PR (#276) which addresses the first two items on my list. Addressing the 3rd would be done by adding my own logic by wrapping The 5th can be achieved (as @mitar suggested) by adding some custom logic around This said, getting #276 would open up for me to solve all my concerns and still stick to this package 🎉 |
@alanning, @mitar New thought on #276 - and I want to have your feedback on this. First and foremost: What do you think about the changes so far? As I just realized, I've only one tight couple towards the To realize this I'm asking you for a name of this first. Some name which could be generalize a user or a group. Like Removing all the I don't need this myself, but I think it would be a big step towards a direction which makes this tool very useful and handy for many. What's your opinion on this? We could easily change it now ... |
Frankly, and sadly, I will not have time to look into this in foreseeable future. :-( |
Thanks for saying it out. @alanning how is it on your side? I'll soon start putting my version into production. After some rethinking of things back and force, I came to the conclusion that we don't need any additional changes to realize everything I initially asked in this ticket. I separated it into different PRs to keep it readable and keep the scope of each change low while still fitting one single scope. I've lived with a custom package to gain the benefits of 2.x, and I'd be open to keep this repository going by releasing my changes as a 3.x and providing support as far as I can. Feel free to respond if you feel you won't have time to review or help me driving the package into the direction you'd like to see it going. Also tell me if you think this goes off too far. In this case, please provide me with details and I'll see how far I can bend to get both ideas aligned. |
Hi @SimonSimCity. |
@alanning @mitar since both of you don't have much time at hand, would you help me moving this package over to https://github.com/Meteor-Community-Packages and give me access to publish a new version on Atmosphere so people using an older version would get a notification about the release? |
Hi @SimonSimCity, happy to help get the package set up in Meteor-Community-Packages. I would keep this repo open and point to that version in this repo's readme. It looks like you are not a member of the Meteor-Community-Packages group, would you be able to become one? Also, I'd highly recommend that the package we create in that repo be based on the v2 branch. This repo can live on as the v1 maintenance branch. |
Actually I am - maybe it's hidden for you as well 😊 I'd start off publishing the current v2 branch as a second major version followed by my changes as a v3. Publishing my version as v2 doesn't make sense since the v2 branch is stable and different enough to justify a major release. Please let me know if I can assist you in moving the repo. Also let me know how I can publish a new version on atmosphere. |
Hello, Thanks! |
Why not publish the new version under a new name? Like ostrio:flow-router-extra? |
@alanning @mitar it's now almost half a year waiting on your action to grant me permission to atmosphere ... If I don't get access by the end of this month, I'll see forking as the best option to continue. This repository can then focus on v1, maybe the unpublished v2 and I'll continue supporting a v3 which includes all the changes I've made. |
I would suggest that we move this project to https://github.com/Meteor-Community-Packages and then leave to @SimonSimCity to merge his changes into the project and continue with it, given ownership of the repository. I think his proposal is good and it seems both @alanning and me do not have really time to deal with this project. If @alanning agrees, then let's do that? @SimonSimCity would that be something you would agree to do? |
Great solution! |
@mitar yes! Nice to see this moving! Let me know if there's anything I can help you here. Would be nice to get it moving before the end of this month. |
I can also help with this a bit. I think we should:
Let's wait for @alanning to OK this plan (I am not an admin of this repository to be able to move it). Maybe you can make me one? |
@mitar @SimonSimCity Sounds good! |
I think this has been resolved. Let's move discussion to other issues. @SimonSimCity feel free to push your stuff to |
Done, I'll release a v3 soon. |
I don't know how much sense this will make to you, but I'll try my best explaining what I want in detail.
Personally, I find the package here pretty good to start with and I've used this package in version 2.0 for quite a while now. There are some things I want to address which would require quite some changes to this package, but I'm not sure if we should not collaborate instead of me just forking off and brewing my own soup.
I like the concept of having roles for everything, and this is also the part I would like to reuse in my package. Roles having nested roles, where each of them has a list of children containing the inherited roles. Even though I would not only include the immediate children, but also nested - but let's start from the beginning.
There are some limitations I hit which I need to do address in a different way.
I need to save the assignment of a role, which currently is stored in the property
roles
in the user document, in a separate collection to keep up with my performance requirements. Here's a list of reasons and features I need:To keep the first reason valid, I would also need to store the hierarchy of roles in the parent-roles, which would be the only change I see for the
roles
collection.So, how does this sound? I think it justifies a separate package, but - at the same time - I would like to cooperate with you on a common basis so users could easily choose one of our packages, even if they only share the roles and all the methods there.
If you say that the differences of our ideas are not compatible, I'd go and brew something on my own and keep it separate from yours.
The text was updated successfully, but these errors were encountered: