Create roles and permissions #818

rwakida · 2015-09-16T06:31:04Z

Create RocketChat authorization package that handles role and permission based authorization

Leverages alanning:roles package to associate a user to a role. Uses
alanning:roles optional "group" parameter to limit the role's scope to
either the global level or room level. The global level is applicable
to users that can perform administrative functions. The room level is
applicable to users that can perform room specific administrative
functions (like a moderator).

A role can have zero or more permissions. Permissions and their
association to roles are defined by this package

Authorization checks are based on whether or not the user has a role or permission.

The roles, permissions, and their association are statically defined at
this time. Eventually, there should be an API to dynamically create a
role and associate it to static permission(s).

Old 'isAdmin' and '.admin is true' checks have been replaced with
corresponding hasPermission authorization checks. Additionally, code
that automatically assigned admin privileges are updated to assign
'admin' role instead.

channel/direct message/private group code checks authorization to edit
properties (e.g. title) and edit/delete messages (regardless of the
system level allow edit/delete settings).

user with 'admin' role are authorized to do anything
room creator is assigned 'moderator' role that can edit the room and
edit/delete messages
members can only edit/delete their own messages IF system wide
settings permit them to.

v19 migration will

add 'admin' role to users with admin:true property
add 'moderator' role scoped to room for room creators
add 'user' role to all users.

There are known issues unrelated to the changes made

If a user with edit/delete message room permissions logs out then a user without
edit/delete message room permissions logs in, then they will see
edit/delete icons. The server will deny execution
edit/delete icons are not reactive Thus if the system level allow
edit/delete message setting is toggled, the icons will not reflect it.
The server will deny execution.

based authorization Leverages alanning:roles package to associate a user to a role. Uses alanning:roles optional "group" parameter to limit the role's scope to either the global level or room level. The global level is applicable to users that can perform administrative functions. The room level is applicable to users that can perform room specific administrative functions (like a moderator). A role can have zero or more permissions. Permissions and their association to roles are defined by this package Authorization checks are based on whether or not the user has a role or permission. The roles, permissions, and their association are statically defined at this time. Eventually, there should be an API to dynamically create a role and associate it to static permission(s). Old 'isAdmin' and '.admin is true' checks have been replaced with corresponding hasPermission authorization checks. Additionally, code that automatically assigned admin privileges are updated to assign 'admin' role instead. channel/direct message/private group code checks authorization to edit properties (e.g. title) and edit/delete messages (regardless of the system level allow edit/delete settings). - user with 'admin' role are authorized to do anything - room creator is assigned 'moderator' role that can edit the room and edit/delete messages - members can only edit/delete their own messages IF system wide settings permit them to. v19 migration will - add 'admin' role to users with admin:true property - add 'moderator' role scoped to room for room creators - add 'user' role to all users. There are known issues unrelated to the changes made - If a user with edit/delete message room permissions logs out then a user without edit/delete message room permissions logs in, then they will see edit/delete icons. The server will deny execution - edit/delete icons are not reactive Thus if the system level allow edit/delete message setting is toggled, the icons will not reflect it. The server will deny execution.

vahid-sohrabloo · 2015-09-16T09:40:52Z

+1

sampaiodiego · 2015-09-16T11:54:04Z

wow.. looks a lot of work.. congrats and thank you.. let's try it out

marceloschmidt · 2015-09-16T12:28:41Z

I'm baffled at your work!! It looks amazing. I'll test it right away. Thank you very much, @rwakida

Sing-Li · 2015-09-16T12:44:00Z

WOW! 👍 Just keeping this in sync with base during development must have been an incredibly tedious task! THANK YOU!! @rwakida

sampaiodiego · 2015-09-16T13:49:07Z

server/methods/deleteMessage.coffee

@@ -18,7 +17,7 @@ Meteor.methods

 		deleteQuery = 
 			_id: message._id
-		deleteQuery['u._id'] = Meteor.userId() if user?.admin isnt true 
+		#deleteQuery['u._id'] = Meteor.userId() if user?.admin isnt true 


this was an additional check, since message obj is passed by the client, so message?.u?._id could be different from real message stored on db.

That was before moderators could delete channel messages. We need a different check now.

@marceloschmidt now is if user is admin or moderator instead if user is admin, but the line is still required to prevent normal users to delete messages from another users.

Actually, by looking at the code above, we can see that we are already checking for permission:

unless hasPermission or (deleteAllowed and deleteOwn) throw new Meteor.Error 'message-deleting-not-allowed', "[methods] deleteMessage -> Message deleting not allowed"

hasPermission is checking if user is either admin or channel moderator: RocketChat.authz.hasPermission(Meteor.userId(), 'delete-message', message.rid)

Here's the permission:
{ _id: 'delete-message', roles : ['admin', 'site-moderator', 'moderator']}

ok for unless hasPermission or (deleteAllowed and deleteOwn), the problem is deleteOwn = message?.u?._id is Meteor.userId() that is believing in the user passed to the method.

I get it, so the user could pass in { _id: original_message_id, u: { _id: hacker_user_id } } and they'd be deleting an arbitrary message. In this case, should we perform a find on rocketchat_message and validate that?

No, if user is a normal user, just pass user to delete query as the commented code
deleteQuery['u._id'] = Meteor.userId()

So, deleteQuery['u._id'] = Meteor.userId() unless hasPermission

AdrianoCahete · 2015-09-16T14:26:11Z

Just a question (i dind't see the code): Can moderator add/delete others moderators?

And about

If a user with edit/delete message room permissions logs out then a user without edit/delete message room permissions logs in, then they will see edit/delete icons

We can put permissons in classes (css), don't? If yes, we can make style hide/show that buttons according to correct permissions.

rodrigok · 2015-09-16T14:31:59Z

Reviewed, looks good except by the comments.

@AdrianoCahete, for now there is no way to add moderators, is possible but we need an interface for that.

About the icons for delete/edit, no, permissions was based on privileges, so admins can delete any message, users can delete only their messages, we only need to refresh the some data when permissions changes.

AdrianoCahete · 2015-09-16T15:03:55Z

@AdrianoCahete, for now there is no way to add moderators, is possible but we need an interface for that.

This question is just because, if any mod can add/delete any other mods, channel creator needs most privileges than "normal mods". Something like "founder" privilege.

But it's just a question for future. :)

rodrigok · 2015-09-16T16:20:56Z

@AdrianoCahete, I see, we need to define if channel creator can do something that moderators can't

Create roles and permissions

rwakida · 2015-09-16T20:41:20Z

@sampaiodiego @marceloschmidt @Sing-Li @rodrigok
Thanks! 😄. You all work so fast that it was hard to keep up.

rodrigok · 2015-09-16T20:48:20Z

@rwakida you too work fast, look my conflicts with your changes 😸

engelgabriel · 2015-09-17T01:09:12Z

Guys... I am speechless. Thanks @rwakida and congrats on the work done! And thanks @rodrigok for the painful merge work :)

For once I was not happy to be the one to merge the PR.. hahaha

engelgabriel · 2015-09-17T01:15:49Z

@rwakida what is your username at https://demo.rocket.chat ??

rwakida · 2015-09-17T02:39:22Z

haha, yeah, thanks @rodrigok for doing the merge. 😃

thanks @engelgabriel! My username is r.wakida. I usually work on RocketChat after work so I don't log on often. are you guys on a lot?

marceloschmidt · 2015-09-17T02:43:12Z

@rwakida we're online most of the time we are awake ;) we're always trying to help others willing to contribute, and there are quite a few that return just for hanging out and chatting with strangers. We found out that it makes a huge difference to have a live demo chat, with all the features, where people can actually see it works by chatting with anybody that's online.

graywolf336 · 2015-09-17T02:55:09Z

It's a fun environment to be part of in the demo. 👍 You should join us sometime!!

gmsecrieru · 2015-09-17T02:55:38Z

👍

geekgonecrazy · 2015-09-17T02:56:59Z

👍 great community :)

Sing-Li · 2015-09-17T03:11:27Z

@rwakida 👍 like five nines - 24 x 7 sorta hangout ... Mahalo !

rwakida · 2015-09-17T03:37:17Z

@Sing-Li nice.. u have connections to Hawaii? ☀️

Sing-Li · 2015-09-18T13:15:33Z

@rwakida ... let me see ... switched my email alias to westmakaha ions ago ... regular dreams of spam mushubi, huli huli chicken, and loco moco ... drifting into peaceful nap with Na Leo Pilimehana, Henry Kapono or Brudda Iz ... lining up f at To-Chau .... combing Kalalau for secret road to weed valley... picked guava at Guava Kai ... floating with horny honu galore at Poipu ... hanged out at the good o' Corner Kitchen ... stuffed my face at the Aloha tower Makino Chaya ... fighting for tables and $9 'kona' lobsters @ Fook Yuen... cruising the after hour wahine action at the Ala Moana tiki bar ....

Unfortunately, ohana ... no 😄 But I love the islands, its people, and the underlying culture - and wished I can spend A LOT more time there.

neurobe · 2015-11-08T07:20:19Z

Currently the person creating a Channel is assigned Role "moderator" but I can't see anywhere how that person or other people can identify that that person is the moderator (for that Channel). Personally I think that..

Admin's list of Users should identify those that are Admins and those that are Moderators (and the name of the Channel that they moderate would be helpful)
In the respective Channel, when the moderator posts, there should be a flag to remind them that they are the moderator (we will forget which rooms we moderate!). (not sure whether others should see this flag)

Secondly, for what I think is currently the un-used Role of "site-moderator" I think that the current method of assigning "admins" should be extended to assign "site-moderators". That is, Admins can assign "site-moderators". -> new method setSiteModeratorStatus

I think we then have a working multi-tier Role system.
At some point in time, the Role config settings established in rocketchat-authorization/server/startup.coffee can be made configurable in the settings panel, but I think there is no urgency for that.

neurobe · 2015-11-08T08:09:52Z

I also note that in client/startup/defaultRoomTypes.coffee, we have:
roles = ['admin', 'moderator', 'user']
whereas in packages/rocketchat-authorization/server/startup.coffee, under "permissions", we have
eg roles : ['admin', 'site-moderator', 'moderator']} # (+ 'user', a la alanning:roles)

marceloschmidt added the stat: in progress label Sep 16, 2015

rwakida mentioned this pull request Sep 16, 2015

Add roles and permissions schema #292

Closed

removed file referenced for testing

4f55dcf

sampaiodiego reviewed Sep 16, 2015
View reviewed changes

rodrigok added 5 commits September 16, 2015 16:53

Remove unused code

65c8e83

Verify is user can delete message getting the message from db before

e920f80

Remove unecessary code

e9fbad7

Move code to use originalMessage instead message

007390a

Remove empty line

8aab1ea

rodrigok added a commit that referenced this pull request Sep 16, 2015

Merge pull request #818 from RocketChat/feature/292-roles

a134a87

Create roles and permissions

rodrigok merged commit a134a87 into master Sep 16, 2015

marceloschmidt removed the stat: in progress label Sep 16, 2015

rodrigok deleted the feature/292-roles branch September 16, 2015 20:35

marceloschmidt mentioned this pull request Sep 16, 2015

Admins should be able to delete (and/or) edit other users' mesages #615

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create roles and permissions #818

Create roles and permissions #818

rwakida commented Sep 16, 2015

vahid-sohrabloo commented Sep 16, 2015

sampaiodiego commented Sep 16, 2015

marceloschmidt commented Sep 16, 2015

Sing-Li commented Sep 16, 2015

sampaiodiego Sep 16, 2015

marceloschmidt Sep 16, 2015

rodrigok Sep 16, 2015

marceloschmidt Sep 16, 2015

rodrigok Sep 16, 2015

marceloschmidt Sep 16, 2015

rodrigok Sep 16, 2015

marceloschmidt Sep 16, 2015

AdrianoCahete commented Sep 16, 2015

rodrigok commented Sep 16, 2015

AdrianoCahete commented Sep 16, 2015

rodrigok commented Sep 16, 2015

rwakida commented Sep 16, 2015

rodrigok commented Sep 16, 2015

engelgabriel commented Sep 17, 2015

engelgabriel commented Sep 17, 2015

rwakida commented Sep 17, 2015

marceloschmidt commented Sep 17, 2015

graywolf336 commented Sep 17, 2015

gmsecrieru commented Sep 17, 2015

geekgonecrazy commented Sep 17, 2015

Sing-Li commented Sep 17, 2015

rwakida commented Sep 17, 2015

Sing-Li commented Sep 18, 2015

neurobe commented Nov 8, 2015

neurobe commented Nov 8, 2015

Create roles and permissions #818

Create roles and permissions #818

Conversation

rwakida commented Sep 16, 2015

vahid-sohrabloo commented Sep 16, 2015

sampaiodiego commented Sep 16, 2015

marceloschmidt commented Sep 16, 2015

Sing-Li commented Sep 16, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdrianoCahete commented Sep 16, 2015

rodrigok commented Sep 16, 2015

AdrianoCahete commented Sep 16, 2015

rodrigok commented Sep 16, 2015

rwakida commented Sep 16, 2015

rodrigok commented Sep 16, 2015

engelgabriel commented Sep 17, 2015

engelgabriel commented Sep 17, 2015

rwakida commented Sep 17, 2015

marceloschmidt commented Sep 17, 2015

graywolf336 commented Sep 17, 2015

gmsecrieru commented Sep 17, 2015

geekgonecrazy commented Sep 17, 2015

Sing-Li commented Sep 17, 2015

rwakida commented Sep 17, 2015

Sing-Li commented Sep 18, 2015

neurobe commented Nov 8, 2015

neurobe commented Nov 8, 2015