Skip to content

New templates purposely with no permissions (RBAC) #7658

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

Closed
sharkymark opened this issue May 24, 2023 · 13 comments · Fixed by #7982
Closed

New templates purposely with no permissions (RBAC) #7658

sharkymark opened this issue May 24, 2023 · 13 comments · Fixed by #7982
Assignees
Milestone

Comments

@sharkymark
Copy link
Contributor

A customer reported a suggestion to allow new templates have no RBAC (groups).

Existing state is the Everyone group is attached to any new template.

This is a very good enterprise suggestion akin to zero trust then add permissions later.

@matifali
Copy link
Member

I am totally in support.
I invited a guest to my deployment and permitted them to view only one template
Later when I added a new template, the invited user, by default, could see and use that which I did not want. So I had to adjust the permissions manually.

@ammario
Copy link
Member

ammario commented May 24, 2023

We should definitely change our default behavior.

@ammario ammario added this to the ❓Sprint 1 milestone May 30, 2023
@ammario ammario added bug and removed feature labels May 31, 2023
@Emyrk
Copy link
Member

Emyrk commented Jun 1, 2023

One annoying issue with this is that AGPL does not have the group feature, so if we make the default to exclude "everyone", then only admins can see templates on AGPL deployments.

This is where the default code lives now for "everyone" can read:

coder/coderd/templates.go

Lines 290 to 292 in 5296e1a

GroupACL: database.TemplateACL{
organization.ID.String(): []rbac.Action{rbac.ActionRead},
},


I prefer not to add more args, but we could add an arg to the create to set the initial permissions. Then we can put the default behavior on the caller, and allow the FE/cli to control things.

@ammario
Copy link
Member

ammario commented Jun 9, 2023

@sreya @bpmct met this morning and discussed this issue.

We think that in any sizeable deployment both "everyone has access" and "everyone has no access" are useless modes and the creator would immediately change permissions following create. Thus, if a deployment has template RBAC enabled we should prompt them to configure permissions in the create flow.

@Emyrk
Copy link
Member

Emyrk commented Jun 9, 2023

@ammario just to confirm,

configure the permissions in the create flow

The create flow being the create template flow correct?

Screenshot from 2023-06-09 12-58-54

@Emyrk Emyrk self-assigned this Jun 9, 2023
@ammario
Copy link
Member

ammario commented Jun 9, 2023

Yeah that was the idea. Not sure how it should look visually.

@Emyrk
Copy link
Member

Emyrk commented Jun 10, 2023

I'll consult with @BrunoQuaresma on looks 👍

@BrunoQuaresma
Copy link
Collaborator

So, what we want is to have a "Permissions" section in the create flow where the user can add users or groups. Is that correct?

@Emyrk
Copy link
Member

Emyrk commented Jun 12, 2023

@BrunoQuaresma Yes, or we just have a checkbox for the "allow everyone", and say a message like, "if you create this without everyone access, no one can use it until you edit the permissions on the template page".

But ideally some way to add groups/users on this page probably 🤔

@BrunoQuaresma
Copy link
Collaborator

BrunoQuaresma commented Jun 12, 2023

I like the one you suggested most, it is easier and should solve the issue. We can get the one I mentioned if users ask for that.

@Emyrk
Copy link
Member

Emyrk commented Jun 12, 2023

@BrunoQuaresma I will make the backend api change to add a boolean field that disables usage by the "everyone group".

If we need to expand this later, we can add fields for adding specific users/groups

@BrunoQuaresma
Copy link
Collaborator

For the FE, if you want to do, you can just copy and paste the checkbox section we already have on that page and update the copy. Let me know if you need any help from me.

@Emyrk
Copy link
Member

Emyrk commented Jun 12, 2023

I'll do that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants