-
Notifications
You must be signed in to change notification settings - Fork 988
feat: Check permissions endpoint #1389
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
Changes from 1 commit
fb7adf5
e584a16
8530cdf
85dc6a7
d74eeb4
79c0886
899eccc
9f3e8e4
210d2c1
9a66d88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
Allows FE to query backend for permission capabilities. Batch requests supported
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,49 @@ func (*api) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) { | |
httpapi.Write(rw, http.StatusOK, convertRoles(roles)) | ||
} | ||
|
||
func (api *api) checkPermissions(rw http.ResponseWriter, r *http.Request) { | ||
roles := httpmw.UserRoles(r) | ||
user := httpmw.UserParam(r) | ||
if user.ID != roles.ID { | ||
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ | ||
// TODO: @Emyrk in the future we could have an rbac check here. | ||
// If the user can masquerade/impersonate as the user passed in, | ||
// we could allow this or something like that. | ||
Message: "only allowed to check permissions on yourself", | ||
}) | ||
return | ||
} | ||
|
||
var params codersdk.UserPermissionCheckRequest | ||
if !httpapi.Read(rw, r, ¶ms) { | ||
return | ||
} | ||
|
||
response := make(codersdk.UserPermissionCheckResponse) | ||
for k, v := range params.Checks { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems odd to echo user-specified keys back in the response. Do we require this endpoint to check for N permissions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. It's a batch request, so the map key is to return the right There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand, but what is the use-case that attempting to fetch the workspace and checking the status code wouldn't catch? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is primarily for deciding which UI options to show. Currently the admin This is probably not going to be exactly how it's done, but something like this. The manage dropdown has 4 different options, you can do a permission check for each one and conditionally show the ui elements the user can access. If any return true, you show the
In the past, the UI used the user's roles, but that means the UI has to know what an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this standard amongst other products with RBAC? Something feels fishy to me which isn't enough to prevent this, but maybe is sign to look for validation with other products. GitHub appears to display UI options based off role, but I know that diverges from our abstraction a bit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I like most on not being attached to the role itself but the permission is in the future, if we want, we can create custom roles without having to update the front end at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Kubernetes has SubjectAccessReview and SelfSubjectAccessReview API endpoints, for example: https://kubernetes.io/docs/reference/access-authn-authz/authorization/#checking-api-access There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @spikecurtis good example that is exactly what we are doing here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kylecarbs What do you think now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still feel it's odd to echo the key-value pairs, but I don't think it should block progress! Seems fine to me. |
||
if v.Object.ResourceType == "" { | ||
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ | ||
Message: "'resource_type' must be defined", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is resource type required? It has a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is. @Emyrk is that possible to have this reflected in the TS types? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, fixed.Ah |
||
}) | ||
return | ||
} | ||
|
||
if v.Object.OwnerID == "me" { | ||
v.Object.OwnerID = roles.ID.String() | ||
} | ||
err := api.Authorizer.AuthorizeByRoleName(r.Context(), roles.ID.String(), roles.Roles, v.Action, | ||
rbac.Object{ | ||
ResourceID: v.Object.ResourceID, | ||
Owner: v.Object.OwnerID, | ||
OrgID: v.Object.OrganizationID, | ||
Type: v.Object.ResourceType, | ||
}) | ||
response[k] = err == nil | ||
} | ||
|
||
httpapi.Write(rw, http.StatusOK, response) | ||
} | ||
|
||
func convertRole(role rbac.Role) codersdk.Role { | ||
return codersdk.Role{ | ||
DisplayName: role.DisplayName, | ||
|
Uh oh!
There was an error while loading. Please reload this page.