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

Discord permissions system example #145

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,8 @@ cedar-rust-hello-world/target/*

# Don't check build files for tinytodo
tinytodo/Cargo.lock
tinytodo/target/*
tinytodo/target/*

# Don't check build files for cedar-discord
cedar-example-use-cases/discord/Cargo.lock
cedar-example-use-cases/discord/target/*
6 changes: 6 additions & 0 deletions cedar-example-use-cases/discord/ALLOW/oflatt_manage_role.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"principal": "User::\"oflatt-general\"",
"action": "Action::\"ManageRole\"",
"resource": "Role::\"everyone\"",
"context": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"principal": "User::\"oflatt-announcements\"",
"action": "Action::\"SendMessage\"",
"resource": "Channel::\"announcements\"",
"context": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"principal": "User::\"oflatt-general\"",
"action": "Action::\"SendMessage\"",
"resource": "Channel::\"general\"",
"context": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"principal": "User::\"yihong-general\"",
"action": "Action::\"SendMessage\"",
"resource": "Channel::\"general\"",
"context": {}
}
6 changes: 6 additions & 0 deletions cedar-example-use-cases/discord/DENY/yihong_kick_oliver.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"principal": "User::\"yihong-general\"",
"action": "Action::\"KickMember\"",
"resource": "Channel::\"general\"",
"context": {}
}
6 changes: 6 additions & 0 deletions cedar-example-use-cases/discord/DENY/yihong_manage_role.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"principal": "User::\"yihong\"",
"action": "Action::\"ManageRole\"",
"resource": "Role::\"everyone\"",
"context": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"principal": "User::\"yihong-announcements\"",
"action": "Action::\"SendMessage\"",
"resource": "Channel::\"announcements\"",
"context": {}
}
61 changes: 61 additions & 0 deletions cedar-example-use-cases/discord/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Cedar Discord Example

This repository contains a model of the [discord permissions system](https://support.discord.com/hc/en-us/articles/206029707-Setting-Up-Permissions-FAQ).

In brief, the discord permission system has a notion of "Roles",
which are sets of permissions.
Users can have multiple Roles.
Roles can allow or disallow permissions, such as sending messages.

Adding more complexity, Discord allows users to configure
the permissions associated with a role for a particular channel.
Not only does discord store the permissions associated with a role,
but also the permissions associated with that role per channel (ChannelRole).


In this example, we implement this functionality by using
Cedar's parent system to build a DAG.
We can then user Cedar's `in` construct to check if the permission
is reachable from a given user.
Note that it's currently unclear if this is the best way to use
Cedar for discord's permissions model. Another approach is to generate
many Cedar policies, one per role and permission pair.

Here is the DAG representing roles:

```
Allow::"SendMessage" Allow::"KickMember"
Role::"everyone" Role::"owner"
▲ ▲ ▲
│ └──────────────────────┐ │
│ │ │
Roles::"yihong" Roles::"oflatt"
```

The "owner" role is special, and don't need to be connected
to each of the `Allow` permissions.

In addition, we build a dag representing channel-specific permissions.
In particular, in the `announcements` channel we forbid everyone from sending messages.
As usual, the "owner" role overrides this in the cedar policy.

```
Disallow::"SendMessage"
ChannelRole::"everyone-announcements"
▲ ▲
│ └─────────────────────────┐
│ │
ChannelRoles::"yihong-announcements" ChannelRoles::"oflatt-announcements"
```

Each `User` has a `Roles` reference.
Each `User` and `Channel` combination has a `ChannelRoles` reference.
The cedar policy can then first check channel roles, which override normal roles.
It can then check normal roles when channel roles are not set.

216 changes: 216 additions & 0 deletions cedar-example-use-cases/discord/entities.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
[
{
"uid": {
"type": "Channel",
"id": "announcements"
},
"attrs": {},
"parents": []
},
{
"uid": {
"type": "Channel",
"id": "general"
},
"attrs": {},
"parents": []
},
{
"uid": {
"type": "Allow",
"id": "SendMessage"
},
"attrs": {},
"parents": []
},
{
"uid": {
"type": "Deny",
"id": "SendMessage"
},
"attrs": {},
"parents": []
},
{
"uid": {
"type": "Role",
"id": "everyone"
},
"attrs": {},
"parents": [
{
"type": "Allow",
"id": "SendMessage"
}
]
},
{
"uid": {
"type": "ChannelRole",
"id": "everyone-announcements"
},
"attrs": {},
"parents": [
{
"type": "Deny",
"id": "SendMessage"
}
]
},
{
"uid": {
"type": "Roles",
"id": "yihong"
},
"attrs": {},
"parents": [
{
"type": "Role",
"id": "everyone"
}
]
},
{
"uid": {
"type": "ChannelRoles",
"id": "yihong-announcements-roles"
},
"attrs": {},
"parents": [
{
"type": "ChannelRole",
"id": "everyone-announcements"
}
]
},
{
"uid": {
"type": "ChannelRoles",
"id": "yihong-general-roles"
},
"attrs": {},
"parents": []
},
{
"uid": {
"type": "User",
"id": "yihong-announcements"
},
"attrs": {
"roles": {
"type": "Roles",
"id": "yihong"
},
"channel_roles": {
"type": "ChannelRoles",
"id": "yihong-announcements-roles"
},
"channel": {
"type": "Channel",
"id": "announcements"
}
},
"parents": []
},
{
"uid": {
"type": "User",
"id": "yihong-general"
},
"attrs": {
"roles": {
"type": "Roles",
"id": "yihong"
},
"channel_roles": {
"type": "ChannelRoles",
"id": "yihong-general-roles"
},
"channel": {
"type": "Channel",
"id": "general"
}
},
"parents": []
},
{
"uid": {
"type": "Roles",
"id": "oflatt"
},
"attrs": {},
"parents": [
{
"type": "Role",
"id": "everyone"
},
{
"type": "Role",
"id": "owner"
}
]
},
{
"uid": {
"type": "ChannelRoles",
"id": "oflatt-announcements-roles"
},
"attrs": {},
"parents": [
{
"type": "ChannelRole",
"id": "everyone-announcements"
}
]
},
{
"uid": {
"type": "ChannelRoles",
"id": "oflatt-general-roles"
},
"attrs": {},
"parents": []
},
{
"uid": {
"type": "User",
"id": "oflatt-announcements"
},
"attrs": {
"roles": {
"type": "Roles",
"id": "oflatt"
},
"channel_roles": {
"type": "ChannelRoles",
"id": "oflatt-announcements-roles"
},
"channel": {
"type": "Channel",
"id": "announcements"
}
},
"parents": []
},
{
"uid": {
"type": "User",
"id": "oflatt-general"
},
"attrs": {
"roles": {
"type": "Roles",
"id": "oflatt"
},
"channel_roles": {
"type": "ChannelRoles",
"id": "oflatt-general-roles"
},
"channel": {
"type": "Channel",
"id": "general"
}
},
"parents": []
}
]
32 changes: 32 additions & 0 deletions cedar-example-use-cases/discord/policies.cedar
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
permit (
oflatt marked this conversation as resolved.
Show resolved Hide resolved
principal,
action == Action::"SendMessage",
resource
)
when
{
// the user's channel matches the resource
(principal.channel == resource) &&
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really the case that Discord makes users specific to a channel? Or is this a modeling artifact?

If the latter, can we model this more simply without the entity hierarchy and channel-specific users as follows. Each user has an attribute channelRoles : Set<{channel : Channel, role : String}>. Then, to check if the user has “SendMessage” role, we write:

permit (
  principal, 
  action == Action::”SendMessage”,
  resource 
 when {
   principal.channelRoles.contains({channel : resource, role : “SendMessage” } )
 }

Would this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you've pointed out the problem: users should not be channel-specific.

Unfortunately, your cedar rule won't work because it drops the principal.channel_roles in Allow::"SendMessage" line. What this rule needs is a map from channels to ChannelRoles so that we can perform this lookup. (see the comment in the User entity)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the problem is that the permissions need to be transitively inherited. In that case, you'd have to keep the transitive closure of the reachable roles / channels in this attribute, which is clunky and defeats the point of using Cedar.

// the channel-specific roles this user has allow it
(principal.channel_roles in Allow::"SendMessage")
};

permit (
principal,
action == Action::"SendMessage",
resource
)
when
{
// the channel of the user matches the resource
(principal.channel == resource) &&
// the channel roles don't mention this permission
(!(principal.channel_roles in Allow::"SendMessage" ||
principal.channel_roles in Deny::"SendMessage")) &&
// the global roles give the user permission
(principal.roles in Allow::"SendMessage")
};

// Allow any action by the owner of the server
permit (principal, action, resource)
when { principal.roles in Role::"owner" };
Loading
Loading