From f9d2454b0be730c68666cc00c6186f24be77893b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 21 Jul 2023 11:22:23 -0400 Subject: [PATCH 01/15] feat: drop reading other 'user' permission Members of the platform can no longer read or list other users. Resources that have "created_by" or "initiated_by" still retain user context, but only include username and avatar url. Attempting to read a user found via those means will result in a 404. --- coderd/coderdtest/authorize.go | 2 +- coderd/database/dbauthz/dbauthz.go | 8 ++++---- coderd/database/dbauthz/dbauthz_test.go | 2 +- coderd/database/modelmethods.go | 6 +++--- coderd/rbac/object.go | 5 +++++ coderd/rbac/regosql/compile_test.go | 2 +- coderd/rbac/regosql/configs.go | 4 ++-- coderd/rbac/roles.go | 12 ++++++++++-- coderd/rbac/roles_test.go | 2 +- 9 files changed, 28 insertions(+), 15 deletions(-) diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index 5e3918e7e6f02..889e8f1ee485a 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -116,7 +116,7 @@ func (RBACAsserter) convertObjects(t *testing.T, objs ...interface{}) []rbac.Obj case codersdk.TemplateVersion: robj = rbac.ResourceTemplate.InOrg(obj.OrganizationID) case codersdk.User: - robj = rbac.ResourceUser.WithID(obj.ID) + robj = rbac.ResourceUserObject(obj.ID) case codersdk.Workspace: robj = rbac.ResourceWorkspace.WithID(obj.ID).InOrg(obj.OrganizationID).WithOwner(obj.OwnerID.String()) default: diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 891b322bc213d..8f06a0f02bf86 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1108,7 +1108,7 @@ func (q *querier) GetProvisionerLogsAfterID(ctx context.Context, arg database.Ge } func (q *querier) GetQuotaAllowanceForUser(ctx context.Context, userID uuid.UUID) (int64, error) { - err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(userID)) + err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUserObject(userID)) if err != nil { return -1, err } @@ -1116,7 +1116,7 @@ func (q *querier) GetQuotaAllowanceForUser(ctx context.Context, userID uuid.UUID } func (q *querier) GetQuotaConsumedForUser(ctx context.Context, userID uuid.UUID) (int64, error) { - err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(userID)) + err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUserObject(userID)) if err != nil { return -1, err } @@ -1367,7 +1367,7 @@ func (q *querier) GetUsers(ctx context.Context, arg database.GetUsersParams) ([] // itself. func (q *querier) GetUsersByIDs(ctx context.Context, ids []uuid.UUID) ([]database.User, error) { for _, uid := range ids { - if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(uid)); err != nil { + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUserObject(uid)); err != nil { return nil, err } } @@ -1876,7 +1876,7 @@ func (q *querier) InsertUserGroupsByName(ctx context.Context, arg database.Inser // TODO: Should this be in system.go? func (q *querier) InsertUserLink(ctx context.Context, arg database.InsertUserLinkParams) (database.UserLink, error) { - if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceUser.WithID(arg.UserID)); err != nil { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceUserObject(arg.UserID)); err != nil { return database.UserLink{}, err } return q.db.InsertUserLink(ctx, arg) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 6a4f09260b7ab..9532e5f2abd4e 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -521,7 +521,7 @@ func (s *MethodTestSuite) TestOrganization() { ma := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{OrganizationID: oa.ID}) mb := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{OrganizationID: ob.ID}) check.Args([]uuid.UUID{ma.UserID, mb.UserID}). - Asserts(rbac.ResourceUser.WithID(ma.UserID), rbac.ActionRead, rbac.ResourceUser.WithID(mb.UserID), rbac.ActionRead) + Asserts(rbac.ResourceUserObject(ma.UserID), rbac.ActionRead, rbac.ResourceUserObject(mb.UserID), rbac.ActionRead) })) s.Run("GetOrganizationMemberByUserID", s.Subtest(func(db database.Store, check *expects) { mem := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{}) diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 536cc4c1178fb..d07df7a02312f 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -201,7 +201,7 @@ func (m GetOrganizationIDsByMemberIDsRow) RBACObject() rbac.Object { // TODO: This feels incorrect as we are really returning a list of orgmembers. // This return type should be refactored to return a list of orgmembers, not this // special type. - return rbac.ResourceUser.WithID(m.UserID) + return rbac.ResourceUserObject(m.UserID) } func (o Organization) RBACObject() rbac.Object { @@ -233,7 +233,7 @@ func (f File) RBACObject() rbac.Object { // If you are trying to get the RBAC object for the UserData, use // u.UserDataRBACObject() instead. func (u User) RBACObject() rbac.Object { - return rbac.ResourceUser.WithID(u.ID) + return rbac.ResourceUserObject(u.ID) } func (u User) UserDataRBACObject() rbac.Object { @@ -241,7 +241,7 @@ func (u User) UserDataRBACObject() rbac.Object { } func (u GetUsersRow) RBACObject() rbac.Object { - return rbac.ResourceUser.WithID(u.ID) + return rbac.ResourceUserObject(u.ID) } func (u GitSSHKey) RBACObject() rbac.Object { diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go index b14583351290f..39f57c7fcc6da 100644 --- a/coderd/rbac/object.go +++ b/coderd/rbac/object.go @@ -195,6 +195,11 @@ var ( } ) +// ResourceUserObject is a helper function to create a user object for authz checks. +func ResourceUserObject(userID uuid.UUID) Object { + return ResourceUser.WithID(userID).WithOwner(userID.String()) +} + // Object is used to create objects for authz checks when you have none in // hand to run the check on. // An example is if you want to list all workspaces, you can create a Object diff --git a/coderd/rbac/regosql/compile_test.go b/coderd/rbac/regosql/compile_test.go index 1997b279d6808..5673b8621c2c7 100644 --- a/coderd/rbac/regosql/compile_test.go +++ b/coderd/rbac/regosql/compile_test.go @@ -259,7 +259,7 @@ neq(input.object.owner, ""); }, VariableConverter: regosql.UserConverter(), ExpectedSQL: p( - p("'10d03e62-7703-4df5-a358-4f76577d4e2f' = ''") + " AND " + p("'' != ''") + " AND " + p("'' = ''"), + p("'10d03e62-7703-4df5-a358-4f76577d4e2f' = id :: text") + " AND " + p("id :: text != ''") + " AND " + p("'' = ''"), ), }, } diff --git a/coderd/rbac/regosql/configs.go b/coderd/rbac/regosql/configs.go index 9f27809bf017c..b683a11af3123 100644 --- a/coderd/rbac/regosql/configs.go +++ b/coderd/rbac/regosql/configs.go @@ -42,8 +42,8 @@ func UserConverter() *sqltypes.VariableConverter { // Users are never owned by an organization, so always return the empty string // for the org owner. sqltypes.StringVarMatcher("''", []string{"input", "object", "org_owner"}), - // Users never have an owner, and are only owned site wide. - sqltypes.StringVarMatcher("''", []string{"input", "object", "owner"}), + // Users are always owned by themselves. + sqltypes.StringVarMatcher("id :: text", []string{"input", "object", "owner"}), ) matcher.RegisterMatcher( // No ACLs on the user type diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index ee3805b716402..59b414cb88223 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -151,8 +151,14 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // All users can see the provisioner daemons. ResourceProvisionerDaemon.Type: {ActionRead}, }), - Org: map[string][]Permission{}, - User: allPermsExcept(ResourceWorkspaceLocked), + Org: map[string][]Permission{}, + User: append(allPermsExcept(ResourceWorkspaceLocked, ResourceUser), + Permissions(map[string][]Action{ + // Users cannot do create/update/delete on themselves, but they + // can read their own details. + ResourceUser.Type: {ActionRead}, + })..., + ), }.withCachedRegoValue() auditorRole := Role{ @@ -163,6 +169,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // are not in. ResourceTemplate.Type: {ActionRead}, ResourceAuditLog.Type: {ActionRead}, + ResourceUser.Type: {ActionRead}, }), Org: map[string][]Permission{}, User: []Permission{}, @@ -180,6 +187,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceProvisionerDaemon.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, // Needs to read all organizations since ResourceOrganization.Type: {ActionRead}, + ResourceUser.Type: {ActionRead}, }), Org: map[string][]Permission{}, User: []Permission{}, diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 4c8b90bdfdb67..011eee4a25260 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -106,7 +106,7 @@ func TestRolePermissions(t *testing.T) { { Name: "MyUser", Actions: []rbac.Action{rbac.ActionRead}, - Resource: rbac.ResourceUser.WithID(currentUser), + Resource: rbac.ResourceUserObject(currentUser), AuthorizeMap: map[bool][]authSubject{ true: {owner, memberMe, orgMemberMe, orgAdmin, otherOrgMember, otherOrgAdmin, templateAdmin, userAdmin}, false: {}, From 4545d1a6ee770168504b84ff6430e7df7ea8038c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 25 Jul 2023 10:22:31 -0400 Subject: [PATCH 02/15] Hide /users page for regular users --- coderd/rbac/roles.go | 2 -- site/src/components/Navbar/Navbar.tsx | 2 ++ .../src/components/Navbar/NavbarView.test.tsx | 8 ++++++++ site/src/components/Navbar/NavbarView.tsx | 19 +++++++++++++------ 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 68fee710f0af2..8c04f4aca7910 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -145,8 +145,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Name: member, DisplayName: "", Site: Permissions(map[string][]Action{ - // All users can read all other users and know they exist. - ResourceUser.Type: {ActionRead}, ResourceRoleAssignment.Type: {ActionRead}, // All users can see the provisioner daemons. ResourceProvisionerDaemon.Type: {ActionRead}, diff --git a/site/src/components/Navbar/Navbar.tsx b/site/src/components/Navbar/Navbar.tsx index 88042b5bceb8d..d8cd46b8f21f7 100644 --- a/site/src/components/Navbar/Navbar.tsx +++ b/site/src/components/Navbar/Navbar.tsx @@ -16,6 +16,7 @@ export const Navbar: FC = () => { const canViewAuditLog = featureVisibility["audit_log"] && Boolean(permissions.viewAuditLog) const canViewDeployment = Boolean(permissions.viewDeploymentValues) + const canViewAllUsers = Boolean(permissions.readAllUsers) const onSignOut = () => authSend("SIGN_OUT") const proxyContextValue = useProxy() const dashboard = useDashboard() @@ -29,6 +30,7 @@ export const Navbar: FC = () => { onSignOut={onSignOut} canViewAuditLog={canViewAuditLog} canViewDeployment={canViewDeployment} + canViewAllUsers={canViewAllUsers} proxyContextValue={ dashboard.experiments.includes("moons") ? proxyContextValue : undefined } diff --git a/site/src/components/Navbar/NavbarView.test.tsx b/site/src/components/Navbar/NavbarView.test.tsx index 55f5dd35901c3..307670708cc72 100644 --- a/site/src/components/Navbar/NavbarView.test.tsx +++ b/site/src/components/Navbar/NavbarView.test.tsx @@ -48,6 +48,7 @@ describe("NavbarView", () => { onSignOut={noop} canViewAuditLog canViewDeployment + canViewAllUsers />, ) const workspacesLink = await screen.findByText(navLanguage.workspaces) @@ -62,6 +63,7 @@ describe("NavbarView", () => { onSignOut={noop} canViewAuditLog canViewDeployment + canViewAllUsers />, ) const templatesLink = await screen.findByText(navLanguage.templates) @@ -76,6 +78,7 @@ describe("NavbarView", () => { onSignOut={noop} canViewAuditLog canViewDeployment + canViewAllUsers />, ) const userLink = await screen.findByText(navLanguage.users) @@ -98,6 +101,7 @@ describe("NavbarView", () => { onSignOut={noop} canViewAuditLog canViewDeployment + canViewAllUsers />, ) @@ -115,6 +119,7 @@ describe("NavbarView", () => { onSignOut={noop} canViewAuditLog canViewDeployment + canViewAllUsers />, ) const auditLink = await screen.findByText(navLanguage.audit) @@ -129,6 +134,7 @@ describe("NavbarView", () => { onSignOut={noop} canViewAuditLog={false} canViewDeployment + canViewAllUsers />, ) const auditLink = screen.queryByText(navLanguage.audit) @@ -143,6 +149,7 @@ describe("NavbarView", () => { onSignOut={noop} canViewAuditLog canViewDeployment + canViewAllUsers />, ) const auditLink = await screen.findByText(navLanguage.deployment) @@ -159,6 +166,7 @@ describe("NavbarView", () => { onSignOut={noop} canViewAuditLog={false} canViewDeployment={false} + canViewAllUsers />, ) const auditLink = screen.queryByText(navLanguage.deployment) diff --git a/site/src/components/Navbar/NavbarView.tsx b/site/src/components/Navbar/NavbarView.tsx index 32d6d491de008..e7d9b4a9069d4 100644 --- a/site/src/components/Navbar/NavbarView.tsx +++ b/site/src/components/Navbar/NavbarView.tsx @@ -36,6 +36,7 @@ export interface NavbarViewProps { onSignOut: () => void canViewAuditLog: boolean canViewDeployment: boolean + canViewAllUsers: boolean proxyContextValue?: ProxyContextValue } @@ -52,8 +53,9 @@ const NavItems: React.FC< className?: string canViewAuditLog: boolean canViewDeployment: boolean + canViewAllUsers: boolean }> -> = ({ className, canViewAuditLog, canViewDeployment }) => { +> = ({ className, canViewAuditLog, canViewDeployment, canViewAllUsers }) => { const styles = useStyles() const location = useLocation() @@ -75,11 +77,13 @@ const NavItems: React.FC< {Language.templates} - - - {Language.users} - - + {canViewAllUsers && ( + + + {Language.users} + + + )} {canViewAuditLog && ( @@ -105,6 +109,7 @@ export const NavbarView: FC = ({ onSignOut, canViewAuditLog, canViewDeployment, + canViewAllUsers, proxyContextValue, }) => { const styles = useStyles() @@ -142,6 +147,7 @@ export const NavbarView: FC = ({ @@ -158,6 +164,7 @@ export const NavbarView: FC = ({ className={styles.desktopNavItems} canViewAuditLog={canViewAuditLog} canViewDeployment={canViewDeployment} + canViewAllUsers={canViewAllUsers} /> Date: Tue, 25 Jul 2023 11:29:58 -0400 Subject: [PATCH 03/15] make groups a privledged endpoint --- coderd/apidoc/docs.go | 55 ++++++++++++++ coderd/apidoc/swagger.json | 51 +++++++++++++ coderd/database/dbauthz/dbauthz.go | 8 +- coderd/rbac/roles.go | 18 ++--- coderd/users.go | 83 ++++++++++----------- codersdk/templates.go | 7 ++ docs/api/enterprise.md | 116 +++++++++++++++++++++++++++++ docs/api/schemas.md | 49 ++++++++++++ enterprise/coderd/coderd.go | 1 + enterprise/coderd/templates.go | 84 ++++++++++++++++++--- site/src/api/typesGenerated.ts | 6 ++ 11 files changed, 412 insertions(+), 66 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index f6cff3d136c00..de2e7ec7cc514 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -2109,6 +2109,44 @@ const docTemplate = `{ } } }, + "/templates/{template}/acl/available": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Enterprise" + ], + "summary": "Get template available acl users/groups", + "operationId": "get-template-available-acl-users-groups", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Template ID", + "name": "template", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.ACLAvailable" + } + } + } + } + } + }, "/templates/{template}/daus": { "get": { "security": [ @@ -6619,6 +6657,23 @@ const docTemplate = `{ } } }, + "codersdk.ACLAvailable": { + "type": "object", + "properties": { + "groups": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.Group" + } + }, + "users": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.MinimalUser" + } + } + } + }, "codersdk.APIKey": { "type": "object", "required": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index d3bc648e764ec..bff927a1ce624 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -1841,6 +1841,40 @@ } } }, + "/templates/{template}/acl/available": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Enterprise"], + "summary": "Get template available acl users/groups", + "operationId": "get-template-available-acl-users-groups", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Template ID", + "name": "template", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.ACLAvailable" + } + } + } + } + } + }, "/templates/{template}/daus": { "get": { "security": [ @@ -5879,6 +5913,23 @@ } } }, + "codersdk.ACLAvailable": { + "type": "object", + "properties": { + "groups": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.Group" + } + }, + "users": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.MinimalUser" + } + } + } + }, "codersdk.APIKey": { "type": "object", "required": [ diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 2e28da389a736..22254549218eb 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2614,24 +2614,24 @@ func (q *querier) GetAuthorizedTemplates(ctx context.Context, arg database.GetTe } func (q *querier) GetTemplateGroupRoles(ctx context.Context, id uuid.UUID) ([]database.TemplateGroup, error) { - // An actor is authorized to read template group roles if they are authorized to read the template. + // An actor is authorized to read template group roles if they are authorized to update the template. template, err := q.db.GetTemplateByID(ctx, id) if err != nil { return nil, err } - if err := q.authorizeContext(ctx, rbac.ActionRead, template); err != nil { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil { return nil, err } return q.db.GetTemplateGroupRoles(ctx, id) } func (q *querier) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]database.TemplateUser, error) { - // An actor is authorized to query template user roles if they are authorized to read the template. + // An actor is authorized to query template user roles if they are authorized to update the template. template, err := q.db.GetTemplateByID(ctx, id) if err != nil { return nil, err } - if err := q.authorizeContext(ctx, rbac.ActionRead, template); err != nil { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil { return nil, err } return q.db.GetTemplateUserRoles(ctx, id) diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 8c04f4aca7910..5624c55f414a0 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -168,6 +168,9 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceTemplate.Type: {ActionRead}, ResourceAuditLog.Type: {ActionRead}, ResourceUser.Type: {ActionRead}, + ResourceGroup.Type: {ActionRead}, + // Org roles are not really used yet, so grant the perm at the site level. + ResourceOrganizationMember.Type: {ActionRead}, }), Org: map[string][]Permission{}, User: []Permission{}, @@ -186,6 +189,9 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // Needs to read all organizations since ResourceOrganization.Type: {ActionRead}, ResourceUser.Type: {ActionRead}, + ResourceGroup.Type: {ActionRead}, + // Org roles are not really used yet, so grant the perm at the site level. + ResourceOrganizationMember.Type: {ActionRead}, }), Org: map[string][]Permission{}, User: []Permission{}, @@ -200,6 +206,9 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // Full perms to manage org members ResourceOrganizationMember.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, ResourceGroup.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, + + // Org roles are not really used yet, so grant the perm at the site level. + ResourceOrganizationMember.Type: {ActionRead}, }), Org: map[string][]Permission{}, User: []Permission{}, @@ -255,11 +264,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) { Site: []Permission{}, Org: map[string][]Permission{ organizationID: { - { - // All org members can read the other members in their org. - ResourceType: ResourceOrganizationMember.Type, - Action: ActionRead, - }, { // All org members can read the organization ResourceType: ResourceOrganization.Type, @@ -270,10 +274,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceType: ResourceOrgRoleAssignment.Type, Action: ActionRead, }, - { - ResourceType: ResourceGroup.Type, - Action: ActionRead, - }, }, }, User: []Permission{}, diff --git a/coderd/users.go b/coderd/users.go index cbd2880e52e88..5296c1134357c 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -182,6 +182,40 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) { // @Success 200 {object} codersdk.GetUsersResponse // @Router /users [get] func (api *API) users(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + users, userCount, ok := api.GetUsers(rw, r) + if !ok { + return + } + + userIDs := make([]uuid.UUID, 0, len(users)) + for _, user := range users { + userIDs = append(userIDs, user.ID) + } + organizationIDsByMemberIDsRows, err := api.Database.GetOrganizationIDsByMemberIDs(ctx, userIDs) + if xerrors.Is(err, sql.ErrNoRows) { + err = nil + } + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching user's organizations.", + Detail: err.Error(), + }) + return + } + organizationIDsByUserID := map[uuid.UUID][]uuid.UUID{} + for _, organizationIDsByMemberIDsRow := range organizationIDsByMemberIDsRows { + organizationIDsByUserID[organizationIDsByMemberIDsRow.UserID] = organizationIDsByMemberIDsRow.OrganizationIDs + } + + render.Status(r, http.StatusOK) + render.JSON(rw, r, codersdk.GetUsersResponse{ + Users: convertUsers(users, organizationIDsByUserID), + Count: int(userCount), + }) +} + +func (api *API) GetUsers(rw http.ResponseWriter, r *http.Request) ([]database.User, int64, bool) { ctx := r.Context() query := r.URL.Query().Get("q") params, errs := searchquery.Users(query) @@ -190,12 +224,12 @@ func (api *API) users(rw http.ResponseWriter, r *http.Request) { Message: "Invalid user search query.", Validations: errs, }) - return + return nil, -1, false } paginationParams, ok := parsePagination(rw, r) if !ok { - return + return nil, -1, false } userRows, err := api.Database.GetUsers(ctx, database.GetUsersParams{ @@ -213,52 +247,17 @@ func (api *API) users(rw http.ResponseWriter, r *http.Request) { Message: "Internal error fetching users.", Detail: err.Error(), }) - return + return nil, -1, false } + // GetUsers does not return ErrNoRows because it uses a window function to get the count. // So we need to check if the userRows is empty and return an empty array if so. if len(userRows) == 0 { - httpapi.Write(ctx, rw, http.StatusOK, codersdk.GetUsersResponse{ - Users: []codersdk.User{}, - Count: 0, - }) - return - } - - users, err := AuthorizeFilter(api.HTTPAuth, r, rbac.ActionRead, database.ConvertUserRows(userRows)) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching users.", - Detail: err.Error(), - }) - return - } - - userIDs := make([]uuid.UUID, 0, len(users)) - for _, user := range users { - userIDs = append(userIDs, user.ID) - } - organizationIDsByMemberIDsRows, err := api.Database.GetOrganizationIDsByMemberIDs(ctx, userIDs) - if xerrors.Is(err, sql.ErrNoRows) { - err = nil - } - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching user's organizations.", - Detail: err.Error(), - }) - return - } - organizationIDsByUserID := map[uuid.UUID][]uuid.UUID{} - for _, organizationIDsByMemberIDsRow := range organizationIDsByMemberIDsRows { - organizationIDsByUserID[organizationIDsByMemberIDsRow.UserID] = organizationIDsByMemberIDsRow.OrganizationIDs + return []database.User{}, -1, true } - render.Status(r, http.StatusOK) - render.JSON(rw, r, codersdk.GetUsersResponse{ - Users: convertUsers(users, organizationIDsByUserID), - Count: int(userRows[0].Count), - }) + users := database.ConvertUserRows(userRows) + return users, userRows[0].Count, true } // Creates a new user. diff --git a/codersdk/templates.go b/codersdk/templates.go index 91feb51b59eba..37fe6b5710971 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -167,6 +167,13 @@ type UpdateTemplateACL struct { GroupPerms map[string]TemplateRole `json:"group_perms,omitempty" example:">:admin,8bd26b20-f3e8-48be-a903-46bb920cf671:use"` } +// ACLAvailable is a list of users and groups that can be added to a template +// ACL. +type ACLAvailable struct { + Users []MinimalUser `json:"users"` + Groups []Group `json:"groups"` +} + type UpdateTemplateMeta struct { Name string `json:"name,omitempty" validate:"omitempty,template_name"` DisplayName string `json:"display_name,omitempty" validate:"omitempty,template_display_name"` diff --git a/docs/api/enterprise.md b/docs/api/enterprise.md index da03774b433e7..7ede077af73a5 100644 --- a/docs/api/enterprise.md +++ b/docs/api/enterprise.md @@ -1142,6 +1142,122 @@ curl -X PATCH http://coder-server:8080/api/v2/templates/{template}/acl \ To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Get template available acl users/groups + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/templates/{template}/acl/available \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /templates/{template}/acl/available` + +### Parameters + +| Name | In | Type | Required | Description | +| ---------- | ---- | ------------ | -------- | ----------- | +| `template` | path | string(uuid) | true | Template ID | + +### Example responses + +> 200 Response + +```json +[ + { + "groups": [ + { + "avatar_url": "string", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "members": [ + { + "avatar_url": "http://example.com", + "created_at": "2019-08-24T14:15:22Z", + "email": "user@example.com", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", + "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], + "roles": [ + { + "display_name": "string", + "name": "string" + } + ], + "status": "active", + "username": "string" + } + ], + "name": "string", + "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", + "quota_allowance": 0 + } + ], + "users": [ + { + "avatar_url": "http://example.com", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "username": "string" + } + ] + } +] +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ----------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of [codersdk.ACLAvailable](schemas.md#codersdkaclavailable) | + +

Response Schema

+ +Status Code **200** + +| Name | Type | Required | Restrictions | Description | +| ---------------------- | ---------------------------------------------------- | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» groups` | array | false | | | +| `»» avatar_url` | string | false | | | +| `»» id` | string(uuid) | false | | | +| `»» members` | array | false | | | +| `»»» avatar_url` | string(uri) | false | | | +| `»»» created_at` | string(date-time) | true | | | +| `»»» email` | string(email) | true | | | +| `»»» id` | string(uuid) | true | | | +| `»»» last_seen_at` | string(date-time) | false | | | +| `»»» login_type` | [codersdk.LoginType](schemas.md#codersdklogintype) | false | | | +| `»»» organization_ids` | array | false | | | +| `»»» roles` | array | false | | | +| `»»»» display_name` | string | false | | | +| `»»»» name` | string | false | | | +| `»»» status` | [codersdk.UserStatus](schemas.md#codersdkuserstatus) | false | | | +| `»»» username` | string | true | | | +| `»» name` | string | false | | | +| `»» organization_id` | string(uuid) | false | | | +| `»» quota_allowance` | integer | false | | | +| `» users` | array | false | | | +| `»» avatar_url` | string(uri) | false | | | +| `»» id` | string(uuid) | true | | | +| `»» username` | string | true | | | + +#### Enumerated Values + +| Property | Value | +| ------------ | ----------- | +| `login_type` | `password` | +| `login_type` | `github` | +| `login_type` | `oidc` | +| `login_type` | `token` | +| `login_type` | `none` | +| `status` | `active` | +| `status` | `suspended` | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Get user quiet hours schedule ### Code samples diff --git a/docs/api/schemas.md b/docs/api/schemas.md index d01275456b639..8297bfd5b78a3 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -755,6 +755,55 @@ | ------------ | ------ | -------- | ------------ | ----------- | | `csp-report` | object | false | | | +## codersdk.ACLAvailable + +```json +{ + "groups": [ + { + "avatar_url": "string", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "members": [ + { + "avatar_url": "http://example.com", + "created_at": "2019-08-24T14:15:22Z", + "email": "user@example.com", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", + "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], + "roles": [ + { + "display_name": "string", + "name": "string" + } + ], + "status": "active", + "username": "string" + } + ], + "name": "string", + "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", + "quota_allowance": 0 + } + ], + "users": [ + { + "avatar_url": "http://example.com", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "username": "string" + } + ] +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| -------- | ----------------------------------------------------- | -------- | ------------ | ----------- | +| `groups` | array of [codersdk.Group](#codersdkgroup) | false | | | +| `users` | array of [codersdk.MinimalUser](#codersdkminimaluser) | false | | | + ## codersdk.APIKey ```json diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 1d22e668c6e84..e50fe1213ec8c 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -202,6 +202,7 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { apiKeyMiddleware, httpmw.ExtractTemplateParam(api.Database), ) + r.Get("/available", api.templateAvailablePermissions) r.Get("/", api.templateACL) r.Patch("/", api.patchTemplateACL) }) diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go index dec2c1c9a6f14..4e7e66554d52a 100644 --- a/enterprise/coderd/templates.go +++ b/enterprise/coderd/templates.go @@ -9,15 +9,69 @@ import ( "github.com/google/uuid" "golang.org/x/xerrors" - "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" ) +// @Summary Get template available acl users/groups +// @ID get-template-available-acl-users-groups +// @Security CoderSessionToken +// @Produce json +// @Tags Enterprise +// @Param template path string true "Template ID" format(uuid) +// @Success 200 {array} codersdk.ACLAvailable +// @Router /templates/{template}/acl/available [get] +func (api *API) templateAvailablePermissions(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + template = httpmw.TemplateParam(r) + ) + + // Requires update permission on the template to list all avail users/groups + // for assignment. + if !api.Authorize(r, rbac.ActionUpdate, template) { + httpapi.ResourceNotFound(rw) + return + } + + // We have to use the system restricted context here because the caller + // might not have permission to read all users. + // nolint:gocritic + users, _, ok := api.AGPL.GetUsers(rw, r.WithContext(dbauthz.AsSystemRestricted(ctx))) + if !ok { + return + } + + // Perm check is the template update check. + // nolint:gocritic + groups, err := api.Database.GetGroupsByOrganizationID(dbauthz.AsSystemRestricted(ctx), template.OrganizationID) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + sdkGroups := make([]codersdk.Group, 0, len(groups)) + for _, group := range groups { + sdkGroups = append(sdkGroups, codersdk.Group{ + ID: group.ID, + Name: group.Name, + OrganizationID: group.OrganizationID, + AvatarURL: group.AvatarURL, + QuotaAllowance: int(group.QuotaAllowance), + }) + } + + httpapi.Write(ctx, rw, http.StatusOK, codersdk.ACLAvailable{ + Users: convertMinimalUser(users), + Groups: sdkGroups, + }) +} + // @Summary Get template ACLs // @ID get-template-acls // @Security CoderSessionToken @@ -44,15 +98,6 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) { return } - dbGroups, err = coderd.AuthorizeFilter(api.AGPL.HTTPAuth, r, rbac.ActionRead, dbGroups) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching users.", - Detail: err.Error(), - }) - return - } - userIDs := make([]uuid.UUID, 0, len(users)) for _, user := range users { userIDs = append(userIDs, user.ID) @@ -73,7 +118,12 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) { for _, group := range dbGroups { var members []database.User - members, err = api.Database.GetGroupMembers(ctx, group.ID) + // This is a bit of a hack. The caller might not have permission to do this, + // but they can read the acl list if the function got this far. So we let + // them read the group members. + // We should probably at least return more truncated user data here. + // nolint:gocritic + members, err = api.Database.GetGroupMembers(dbauthz.AsSystemRestricted(ctx), group.ID) if err != nil { httpapi.InternalServerError(rw, err) return @@ -224,6 +274,18 @@ func validateTemplateACLPerms(ctx context.Context, db database.Store, perms map[ return validErrs } +func convertMinimalUser(users []database.User) []codersdk.MinimalUser { + minimalUsers := make([]codersdk.MinimalUser, 0, len(users)) + for _, user := range users { + minimalUsers = append(minimalUsers, codersdk.MinimalUser{ + ID: user.ID, + Username: user.Username, + AvatarURL: user.AvatarURL.String, + }) + } + return minimalUsers +} + func convertTemplateUsers(tus []database.TemplateUser, orgIDsByUserIDs map[uuid.UUID][]uuid.UUID) []codersdk.TemplateUser { users := make([]codersdk.TemplateUser, 0, len(tus)) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index d5f60f4b19ed9..d0d0175bc6e5e 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1,5 +1,11 @@ // Code generated by 'make site/src/api/typesGenerated.ts'. DO NOT EDIT. +// From codersdk/templates.go +export interface ACLAvailable { + readonly users: MinimalUser[] + readonly groups: Group[] +} + // From codersdk/apikey.go export interface APIKey { readonly id: string From 8e86149884c01b9b6090f63f2f2161f1b9fd9418 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 25 Jul 2023 12:32:25 -0400 Subject: [PATCH 04/15] Permissions page for template perms --- coderd/apidoc/docs.go | 2 +- coderd/apidoc/swagger.json | 2 +- codersdk/templates.go | 2 +- docs/api/enterprise.md | 15 ++++++++--- docs/api/schemas.md | 20 ++++++++++++--- enterprise/coderd/templates.go | 21 ++++++++++------ site/src/api/api.ts | 9 +++++++ site/src/api/typesGenerated.ts | 2 +- .../UserOrGroupAutocomplete.tsx | 5 +++- .../TemplatePermissionsPage.tsx | 1 + .../TemplatePermissionsPageView.tsx | 6 +++++ site/src/utils/groups.ts | 4 +++ .../template/searchUsersAndGroupsXService.ts | 25 +++++++++++++------ 13 files changed, 87 insertions(+), 27 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index de2e7ec7cc514..b33d7a4146499 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -6669,7 +6669,7 @@ const docTemplate = `{ "users": { "type": "array", "items": { - "$ref": "#/definitions/codersdk.MinimalUser" + "$ref": "#/definitions/codersdk.User" } } } diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index bff927a1ce624..07f534aab6989 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -5925,7 +5925,7 @@ "users": { "type": "array", "items": { - "$ref": "#/definitions/codersdk.MinimalUser" + "$ref": "#/definitions/codersdk.User" } } } diff --git a/codersdk/templates.go b/codersdk/templates.go index 37fe6b5710971..ad78f3529681e 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -170,7 +170,7 @@ type UpdateTemplateACL struct { // ACLAvailable is a list of users and groups that can be added to a template // ACL. type ACLAvailable struct { - Users []MinimalUser `json:"users"` + Users []User `json:"users"` Groups []Group `json:"groups"` } diff --git a/docs/api/enterprise.md b/docs/api/enterprise.md index 7ede077af73a5..0eb076461a3be 100644 --- a/docs/api/enterprise.md +++ b/docs/api/enterprise.md @@ -1199,7 +1199,19 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/acl/available \ "users": [ { "avatar_url": "http://example.com", + "created_at": "2019-08-24T14:15:22Z", + "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", + "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], + "roles": [ + { + "display_name": "string", + "name": "string" + } + ], + "status": "active", "username": "string" } ] @@ -1240,9 +1252,6 @@ Status Code **200** | `»» organization_id` | string(uuid) | false | | | | `»» quota_allowance` | integer | false | | | | `» users` | array | false | | | -| `»» avatar_url` | string(uri) | false | | | -| `»» id` | string(uuid) | true | | | -| `»» username` | string | true | | | #### Enumerated Values diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 8297bfd5b78a3..1ebfab2e82b2d 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -790,7 +790,19 @@ "users": [ { "avatar_url": "http://example.com", + "created_at": "2019-08-24T14:15:22Z", + "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", + "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], + "roles": [ + { + "display_name": "string", + "name": "string" + } + ], + "status": "active", "username": "string" } ] @@ -799,10 +811,10 @@ ### Properties -| Name | Type | Required | Restrictions | Description | -| -------- | ----------------------------------------------------- | -------- | ------------ | ----------- | -| `groups` | array of [codersdk.Group](#codersdkgroup) | false | | | -| `users` | array of [codersdk.MinimalUser](#codersdkminimaluser) | false | | | +| Name | Type | Required | Restrictions | Description | +| -------- | ----------------------------------------- | -------- | ------------ | ----------- | +| `groups` | array of [codersdk.Group](#codersdkgroup) | false | | | +| `users` | array of [codersdk.User](#codersdkuser) | false | | | ## codersdk.APIKey diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go index 4e7e66554d52a..0dad65c8bcf33 100644 --- a/enterprise/coderd/templates.go +++ b/enterprise/coderd/templates.go @@ -57,17 +57,22 @@ func (api *API) templateAvailablePermissions(rw http.ResponseWriter, r *http.Req sdkGroups := make([]codersdk.Group, 0, len(groups)) for _, group := range groups { - sdkGroups = append(sdkGroups, codersdk.Group{ - ID: group.ID, - Name: group.Name, - OrganizationID: group.OrganizationID, - AvatarURL: group.AvatarURL, - QuotaAllowance: int(group.QuotaAllowance), - }) + // nolint:gocritic + members, err := api.Database.GetGroupMembers(dbauthz.AsSystemRestricted(ctx), group.ID) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + sdkGroups = append(sdkGroups, convertGroup(group, members)) } httpapi.Write(ctx, rw, http.StatusOK, codersdk.ACLAvailable{ - Users: convertMinimalUser(users), + // No need to pass organization info here. + // TODO: @emyrk we should return a MinimalUser here instead of a full user. + // The FE requires the `email` field, so this cannot be done without + // a UI change. + Users: convertUsers(users, map[uuid.UUID][]uuid.UUID{}), Groups: sdkGroups, }) } diff --git a/site/src/api/api.ts b/site/src/api/api.ts index ba412e39e8764..eeaafbf935ed4 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -869,6 +869,15 @@ export const getDeploymentDAUs = async ( return response.data } +export const getTemplateACLAvailable = async ( + templateId: string, + options: TypesGen.UsersRequest, +): Promise => { + const url = getURLWithSearchParams(`/api/v2/templates/${templateId}/acl/available`, options) + const response = await axios.get(url.toString()) + return response.data +} + export const getTemplateACL = async ( templateId: string, ): Promise => { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index d0d0175bc6e5e..dcd0548e636b1 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2,7 +2,7 @@ // From codersdk/templates.go export interface ACLAvailable { - readonly users: MinimalUser[] + readonly users: User[] readonly groups: Group[] } diff --git a/site/src/components/UserOrGroupAutocomplete/UserOrGroupAutocomplete.tsx b/site/src/components/UserOrGroupAutocomplete/UserOrGroupAutocomplete.tsx index f163a881270ec..5d4344506c56a 100644 --- a/site/src/components/UserOrGroupAutocomplete/UserOrGroupAutocomplete.tsx +++ b/site/src/components/UserOrGroupAutocomplete/UserOrGroupAutocomplete.tsx @@ -21,12 +21,13 @@ export type UserOrGroupAutocompleteProps = { value: UserOrGroupAutocompleteValue onChange: (value: UserOrGroupAutocompleteValue) => void organizationId: string + templateID?: string exclude: UserOrGroupAutocompleteValue[] } export const UserOrGroupAutocomplete: React.FC< UserOrGroupAutocompleteProps -> = ({ value, onChange, organizationId, exclude }) => { +> = ({ value, onChange, organizationId, templateID, exclude }) => { const styles = useStyles() const [isAutocompleteOpen, setIsAutocompleteOpen] = useState(false) const [searchState, sendSearch] = useMachine(searchUsersAndGroupsMachine, { @@ -34,6 +35,7 @@ export const UserOrGroupAutocomplete: React.FC< userResults: [], groupResults: [], organizationId, + templateID, }, }) const { userResults, groupResults } = searchState.context @@ -73,6 +75,7 @@ export const UserOrGroupAutocomplete: React.FC< } renderOption={(props, option) => { const isOptionGroup = isGroup(option) + console.log("option", option) return ( diff --git a/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPage.tsx b/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPage.tsx index 017a1ec1671ad..7e29b4f14713b 100644 --- a/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPage.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPage.tsx @@ -64,6 +64,7 @@ export const TemplatePermissionsPage: FC< { diff --git a/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx b/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx index 8fb42d92330ca..c42b78cba26f3 100644 --- a/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx @@ -34,6 +34,7 @@ import { PageHeader, PageHeaderTitle } from "components/PageHeader/PageHeader" type AddTemplateUserOrGroupProps = { organizationId: string + templateID: string isLoading: boolean templateACL: TemplateACL | undefined onSubmit: ( @@ -47,6 +48,7 @@ const AddTemplateUserOrGroup: React.FC = ({ isLoading, onSubmit, organizationId, + templateID, templateACL, }) => { const styles = useStyles() @@ -83,6 +85,7 @@ const AddTemplateUserOrGroup: React.FC = ({ { setSelectedOption(newValue) @@ -151,6 +154,7 @@ const RoleSelect: FC = (props) => { export interface TemplatePermissionsPageViewProps { templateACL: TemplateACL | undefined + templateID: string organizationId: string canUpdatePermissions: boolean // User @@ -177,6 +181,7 @@ export const TemplatePermissionsPageView: FC< templateACL, canUpdatePermissions, organizationId, + templateID, // User onAddUser, isAddingUser, @@ -207,6 +212,7 @@ export const TemplatePermissionsPageView: FC< diff --git a/site/src/utils/groups.ts b/site/src/utils/groups.ts index 2324a4a27fd60..d14d390fa33ed 100644 --- a/site/src/utils/groups.ts +++ b/site/src/utils/groups.ts @@ -15,6 +15,10 @@ export const getGroupSubtitle = (group: Group): string => { return `All users` } + if(!group.members) { + return `0 members` + } + if (group.members.length === 1) { return `1 member` } diff --git a/site/src/xServices/template/searchUsersAndGroupsXService.ts b/site/src/xServices/template/searchUsersAndGroupsXService.ts index e1c529713bc66..a80d2525f92b6 100644 --- a/site/src/xServices/template/searchUsersAndGroupsXService.ts +++ b/site/src/xServices/template/searchUsersAndGroupsXService.ts @@ -1,4 +1,4 @@ -import { getGroups, getUsers } from "api/api" +import { getGroups, getTemplateACLAvailable, getUsers } from "api/api" import { Group, User } from "api/typesGenerated" import { queryToFilter } from "utils/filters" import { everyOneGroup } from "utils/groups" @@ -15,6 +15,7 @@ export const searchUsersAndGroupsMachine = createMachine( schema: { context: {} as { organizationId: string + templateID?: string userResults: User[] groupResults: Group[] }, @@ -56,16 +57,26 @@ export const searchUsersAndGroupsMachine = createMachine( }, { services: { - search: async ({ organizationId }, { query }) => { - const [userRes, groups] = await Promise.all([ - getUsers(queryToFilter(query)), - getGroups(organizationId), - ]) + search: async ({ organizationId, templateID }, { query }) => { + let users, groups + if(templateID && templateID !== "") { + const res = await getTemplateACLAvailable(templateID, queryToFilter(query)) + users = res.users + groups = res.groups + } else { + const [userRes, groupsRes] = await Promise.all([ + getUsers(queryToFilter(query)), + getGroups(organizationId), + ]) + + users = userRes.users + groups = groupsRes + } // The Everyone groups is not returned by the API so we have to add it // manually return { - users: userRes.users, + users: users, groups: [everyOneGroup(organizationId), ...groups], } }, From 82ceab8f35c47d6b2d63a0980b5e91abec892d7c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 25 Jul 2023 12:37:54 -0400 Subject: [PATCH 05/15] remove console log --- .../UserOrGroupAutocomplete/UserOrGroupAutocomplete.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/site/src/components/UserOrGroupAutocomplete/UserOrGroupAutocomplete.tsx b/site/src/components/UserOrGroupAutocomplete/UserOrGroupAutocomplete.tsx index 5d4344506c56a..5a1e2f08aa45e 100644 --- a/site/src/components/UserOrGroupAutocomplete/UserOrGroupAutocomplete.tsx +++ b/site/src/components/UserOrGroupAutocomplete/UserOrGroupAutocomplete.tsx @@ -75,7 +75,6 @@ export const UserOrGroupAutocomplete: React.FC< } renderOption={(props, option) => { const isOptionGroup = isGroup(option) - console.log("option", option) return ( From ddd147dd5ecce3cd2855f6847c9932bb15e7d77b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 25 Jul 2023 12:53:25 -0400 Subject: [PATCH 06/15] Fix perms unit tests --- coderd/database/modelmethods.go | 3 ++- coderd/rbac/roles.go | 12 +++++++----- coderd/rbac/roles_test.go | 16 ++++++++-------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index d07df7a02312f..3daaec35da595 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -194,7 +194,8 @@ func (w Workspace) LockedRBAC() rbac.Object { func (m OrganizationMember) RBACObject() rbac.Object { return rbac.ResourceOrganizationMember. WithID(m.UserID). - InOrg(m.OrganizationID) + InOrg(m.OrganizationID). + WithOwner(m.UserID.String()) } func (m GetOrganizationIDsByMemberIDsRow) RBACObject() rbac.Object { diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index 5624c55f414a0..93aeaca017592 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -150,7 +150,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceProvisionerDaemon.Type: {ActionRead}, }), Org: map[string][]Permission{}, - User: append(allPermsExcept(ResourceWorkspaceLocked, ResourceUser), + User: append(allPermsExcept(ResourceWorkspaceLocked, ResourceUser, ResourceOrganizationMember), Permissions(map[string][]Action{ // Users cannot do create/update/delete on themselves, but they // can read their own details. @@ -206,9 +206,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // Full perms to manage org members ResourceOrganizationMember.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, ResourceGroup.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete}, - - // Org roles are not really used yet, so grant the perm at the site level. - ResourceOrganizationMember.Type: {ActionRead}, }), Org: map[string][]Permission{}, User: []Permission{}, @@ -276,7 +273,12 @@ func ReloadBuiltinRoles(opts *RoleOptions) { }, }, }, - User: []Permission{}, + User: []Permission{ + { + ResourceType: ResourceOrganizationMember.Type, + Action: ActionRead, + }, + }, } }, } diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index 011eee4a25260..4ecb53e1832d2 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -108,8 +108,8 @@ func TestRolePermissions(t *testing.T) { Actions: []rbac.Action{rbac.ActionRead}, Resource: rbac.ResourceUserObject(currentUser), AuthorizeMap: map[bool][]authSubject{ - true: {owner, memberMe, orgMemberMe, orgAdmin, otherOrgMember, otherOrgAdmin, templateAdmin, userAdmin}, - false: {}, + true: {orgMemberMe, owner, memberMe, templateAdmin, userAdmin}, + false: {otherOrgMember, otherOrgAdmin, orgAdmin}, }, }, { @@ -281,7 +281,7 @@ func TestRolePermissions(t *testing.T) { { Name: "ManageOrgMember", Actions: []rbac.Action{rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete}, - Resource: rbac.ResourceOrganizationMember.WithID(currentUser).InOrg(orgID), + Resource: rbac.ResourceOrganizationMember.WithID(currentUser).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ true: {owner, orgAdmin, userAdmin}, false: {orgMemberMe, memberMe, otherOrgAdmin, otherOrgMember, templateAdmin}, @@ -290,10 +290,10 @@ func TestRolePermissions(t *testing.T) { { Name: "ReadOrgMember", Actions: []rbac.Action{rbac.ActionRead}, - Resource: rbac.ResourceOrganizationMember.WithID(currentUser).InOrg(orgID), + Resource: rbac.ResourceOrganizationMember.WithID(currentUser).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]authSubject{ - true: {owner, orgAdmin, orgMemberMe, userAdmin}, - false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin}, + true: {owner, orgAdmin, userAdmin, orgMemberMe, templateAdmin}, + false: {memberMe, otherOrgAdmin, otherOrgMember}, }, }, { @@ -314,8 +314,8 @@ func TestRolePermissions(t *testing.T) { Actions: []rbac.Action{rbac.ActionRead}, Resource: rbac.ResourceGroup.WithID(groupID).InOrg(orgID), AuthorizeMap: map[bool][]authSubject{ - true: {owner, orgAdmin, userAdmin, orgMemberMe}, - false: {memberMe, otherOrgAdmin, otherOrgMember, templateAdmin}, + true: {owner, orgAdmin, userAdmin, templateAdmin}, + false: {memberMe, otherOrgAdmin, orgMemberMe, otherOrgMember}, }, }, { From 658e2e28c54e6759c165bb7a35aa4a16e4b92e0b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 25 Jul 2023 13:06:11 -0400 Subject: [PATCH 07/15] Fixing unit tests --- enterprise/coderd/templates.go | 5 ++++- enterprise/coderd/templates_test.go | 11 +++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go index 0dad65c8bcf33..f4959b41634e3 100644 --- a/enterprise/coderd/templates.go +++ b/enterprise/coderd/templates.go @@ -19,7 +19,7 @@ import ( ) // @Summary Get template available acl users/groups -// @ID get-template-available-acl-users-groups +// @ID get-template-available-acl-usersgroups // @Security CoderSessionToken // @Produce json // @Tags Enterprise @@ -246,6 +246,9 @@ func (api *API) patchTemplateACL(rw http.ResponseWriter, r *http.Request) { // nolint TODO fix stupid flag. func validateTemplateACLPerms(ctx context.Context, db database.Store, perms map[string]codersdk.TemplateRole, field string, isUser bool) []codersdk.ValidationError { + // Validate requires full read access to users and groups + // nolint:gocritic + ctx = dbauthz.AsSystemRestricted(ctx) var validErrs []codersdk.ValidationError for k, v := range perms { if err := validateTemplateRole(v); err != nil { diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 9b55d0a7e5a96..49faac1ed569a 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -897,10 +897,13 @@ func TestUpdateTemplateACL(t *testing.T) { acl, err := client2.TemplateACL(ctx, template.ID) require.NoError(t, err) - require.Contains(t, acl.Users, codersdk.TemplateUser{ - User: user3, - Role: codersdk.TemplateRoleUse, - }) + found := false + for _, u := range acl.Users { + if u.ID == user3.ID { + found = true + } + } + require.True(t, found, "user not found in acl") }) t.Run("allUsersGroup", func(t *testing.T) { From 9007b8eb0c71b6cb3a6aa5f4670a71f28b2e8c14 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 25 Jul 2023 13:10:40 -0400 Subject: [PATCH 08/15] make gen --- coderd/apidoc/docs.go | 2 +- coderd/apidoc/swagger.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index b33d7a4146499..08cdee1c2eb25 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -2123,7 +2123,7 @@ const docTemplate = `{ "Enterprise" ], "summary": "Get template available acl users/groups", - "operationId": "get-template-available-acl-users-groups", + "operationId": "get-template-available-acl-usersgroups", "parameters": [ { "type": "string", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 07f534aab6989..e23a42398424f 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -1851,7 +1851,7 @@ "produces": ["application/json"], "tags": ["Enterprise"], "summary": "Get template available acl users/groups", - "operationId": "get-template-available-acl-users-groups", + "operationId": "get-template-available-acl-usersgroups", "parameters": [ { "type": "string", From 72b08624e5f7f7eb77a148a486a8bfceb85e7f9b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 25 Jul 2023 13:14:42 -0400 Subject: [PATCH 09/15] Fix pagination users test --- coderd/users_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/users_test.go b/coderd/users_test.go index 504b29a52fcee..11c8d3e44f85a 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -1431,7 +1431,7 @@ func TestGetUsersPagination(t *testing.T) { require.Equal(t, res.Count, 2) // if offset is higher than the count postgres returns an empty array - // and not an ErrNoRows error. This also means the count must be 0. + // and not an ErrNoRows error. res, err = client.Users(ctx, codersdk.UsersRequest{ Pagination: codersdk.Pagination{ Offset: 3, @@ -1439,7 +1439,7 @@ func TestGetUsersPagination(t *testing.T) { }) require.NoError(t, err) require.Len(t, res.Users, 0) - require.Equal(t, res.Count, 0) + require.Equal(t, res.Count, -1) } func TestPostTokens(t *testing.T) { From f1ed8b39408ed59b94c231f126723d499244f7e1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 25 Jul 2023 13:20:30 -0400 Subject: [PATCH 10/15] Make fmt --- coderd/database/dbauthz/dbauthz_test.go | 4 ++-- codersdk/templates.go | 4 ++-- site/src/api/api.ts | 5 ++++- site/src/utils/groups.ts | 2 +- .../src/xServices/template/searchUsersAndGroupsXService.ts | 7 +++++-- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 8d6aac9d9e4ec..e13c64778f831 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -698,11 +698,11 @@ func (s *MethodTestSuite) TestTemplate() { })) s.Run("GetTemplateGroupRoles", s.Subtest(func(db database.Store, check *expects) { t1 := dbgen.Template(s.T(), db, database.Template{}) - check.Args(t1.ID).Asserts(t1, rbac.ActionRead) + check.Args(t1.ID).Asserts(t1, rbac.ActionUpdate) })) s.Run("GetTemplateUserRoles", s.Subtest(func(db database.Store, check *expects) { t1 := dbgen.Template(s.T(), db, database.Template{}) - check.Args(t1.ID).Asserts(t1, rbac.ActionRead) + check.Args(t1.ID).Asserts(t1, rbac.ActionUpdate) })) s.Run("GetTemplateVersionByID", s.Subtest(func(db database.Store, check *expects) { t1 := dbgen.Template(s.T(), db, database.Template{}) diff --git a/codersdk/templates.go b/codersdk/templates.go index ad78f3529681e..7acc6552fb5fa 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -170,8 +170,8 @@ type UpdateTemplateACL struct { // ACLAvailable is a list of users and groups that can be added to a template // ACL. type ACLAvailable struct { - Users []User `json:"users"` - Groups []Group `json:"groups"` + Users []User `json:"users"` + Groups []Group `json:"groups"` } type UpdateTemplateMeta struct { diff --git a/site/src/api/api.ts b/site/src/api/api.ts index eeaafbf935ed4..73f4a916f30e2 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -873,7 +873,10 @@ export const getTemplateACLAvailable = async ( templateId: string, options: TypesGen.UsersRequest, ): Promise => { - const url = getURLWithSearchParams(`/api/v2/templates/${templateId}/acl/available`, options) + const url = getURLWithSearchParams( + `/api/v2/templates/${templateId}/acl/available`, + options, + ) const response = await axios.get(url.toString()) return response.data } diff --git a/site/src/utils/groups.ts b/site/src/utils/groups.ts index d14d390fa33ed..f96a9f3545cd6 100644 --- a/site/src/utils/groups.ts +++ b/site/src/utils/groups.ts @@ -15,7 +15,7 @@ export const getGroupSubtitle = (group: Group): string => { return `All users` } - if(!group.members) { + if (!group.members) { return `0 members` } diff --git a/site/src/xServices/template/searchUsersAndGroupsXService.ts b/site/src/xServices/template/searchUsersAndGroupsXService.ts index a80d2525f92b6..2b962dce96295 100644 --- a/site/src/xServices/template/searchUsersAndGroupsXService.ts +++ b/site/src/xServices/template/searchUsersAndGroupsXService.ts @@ -59,8 +59,11 @@ export const searchUsersAndGroupsMachine = createMachine( services: { search: async ({ organizationId, templateID }, { query }) => { let users, groups - if(templateID && templateID !== "") { - const res = await getTemplateACLAvailable(templateID, queryToFilter(query)) + if (templateID && templateID !== "") { + const res = await getTemplateACLAvailable( + templateID, + queryToFilter(query), + ) users = res.users groups = res.groups } else { From 04d9166f56686ec2df93efba442a510286133d01 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 25 Jul 2023 13:26:50 -0400 Subject: [PATCH 11/15] Update unit test with perm changes --- coderd/authorize_test.go | 4 ++-- enterprise/coderd/groups_test.go | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/coderd/authorize_test.go b/coderd/authorize_test.go index 7c325cb7ffa47..55214d10473a7 100644 --- a/coderd/authorize_test.go +++ b/coderd/authorize_test.go @@ -103,7 +103,7 @@ func TestCheckPermissions(t *testing.T) { Client: orgAdminClient, UserID: orgAdminUser.ID, Check: map[string]bool{ - readAllUsers: true, + readAllUsers: false, readMyself: true, readOwnWorkspaces: true, readOrgWorkspaces: true, @@ -115,7 +115,7 @@ func TestCheckPermissions(t *testing.T) { Client: memberClient, UserID: memberUser.ID, Check: map[string]bool{ - readAllUsers: true, + readAllUsers: false, readMyself: true, readOwnWorkspaces: true, readOrgWorkspaces: false, diff --git a/enterprise/coderd/groups_test.go b/enterprise/coderd/groups_test.go index 3b1bd553f9752..322ee17c36cd4 100644 --- a/enterprise/coderd/groups_test.go +++ b/enterprise/coderd/groups_test.go @@ -474,9 +474,8 @@ func TestGroup(t *testing.T) { }) require.NoError(t, err) - ggroup, err := client1.Group(ctx, group.ID) - require.NoError(t, err) - require.Equal(t, group, ggroup) + _, err = client1.Group(ctx, group.ID) + require.Error(t, err, "regular users cannot read groups") }) t.Run("FilterDeletedUsers", func(t *testing.T) { From e0af28e6e0fd9106f603b13f6a52df3dbf1774c4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 25 Jul 2023 14:17:46 -0400 Subject: [PATCH 12/15] remove unused function --- enterprise/coderd/templates.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go index f4959b41634e3..5ba96712aaa89 100644 --- a/enterprise/coderd/templates.go +++ b/enterprise/coderd/templates.go @@ -282,18 +282,6 @@ func validateTemplateACLPerms(ctx context.Context, db database.Store, perms map[ return validErrs } -func convertMinimalUser(users []database.User) []codersdk.MinimalUser { - minimalUsers := make([]codersdk.MinimalUser, 0, len(users)) - for _, user := range users { - minimalUsers = append(minimalUsers, codersdk.MinimalUser{ - ID: user.ID, - Username: user.Username, - AvatarURL: user.AvatarURL.String, - }) - } - return minimalUsers -} - func convertTemplateUsers(tus []database.TemplateUser, orgIDsByUserIDs map[uuid.UUID][]uuid.UUID) []codersdk.TemplateUser { users := make([]codersdk.TemplateUser, 0, len(tus)) From 515edf1e73ee36ffe30893c6bb25ed8c5cdc95d9 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 25 Jul 2023 14:45:35 -0400 Subject: [PATCH 13/15] count 0 vs -1 --- coderd/users.go | 2 +- coderd/users_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/users.go b/coderd/users.go index 5296c1134357c..abe8d4a930b2a 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -253,7 +253,7 @@ func (api *API) GetUsers(rw http.ResponseWriter, r *http.Request) ([]database.Us // GetUsers does not return ErrNoRows because it uses a window function to get the count. // So we need to check if the userRows is empty and return an empty array if so. if len(userRows) == 0 { - return []database.User{}, -1, true + return []database.User{}, 0, true } users := database.ConvertUserRows(userRows) diff --git a/coderd/users_test.go b/coderd/users_test.go index 11c8d3e44f85a..0228f62f92359 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -1439,7 +1439,7 @@ func TestGetUsersPagination(t *testing.T) { }) require.NoError(t, err) require.Len(t, res.Users, 0) - require.Equal(t, res.Count, -1) + require.Equal(t, res.Count, 0) } func TestPostTokens(t *testing.T) { From 39912a28a7a9435c96fa379e1a426ca7c95bc17a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 25 Jul 2023 15:41:42 -0400 Subject: [PATCH 14/15] Unit test to test acl available --- codersdk/templates.go | 14 ++++++++++++++ enterprise/coderd/templates_test.go | 11 +++++++++++ 2 files changed, 25 insertions(+) diff --git a/codersdk/templates.go b/codersdk/templates.go index 7acc6552fb5fa..2022c876db360 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -258,6 +258,20 @@ func (c *Client) UpdateTemplateACL(ctx context.Context, templateID uuid.UUID, re return nil } +// TemplateACLAvailable returns available users + groups that can be assigned template perms +func (c *Client) TemplateACLAvailable(ctx context.Context, templateID uuid.UUID) (ACLAvailable, error) { + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/templates/%s/acl/available", templateID), nil) + if err != nil { + return ACLAvailable{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return ACLAvailable{}, ReadBodyAsError(res) + } + var acl ACLAvailable + return acl, json.NewDecoder(res.Body).Decode(&acl) +} + func (c *Client) TemplateACL(ctx context.Context, templateID uuid.UUID) (TemplateACL, error) { res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/templates/%s/acl", templateID), nil) if err != nil { diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 49faac1ed569a..9647046129923 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -885,6 +885,17 @@ func TestUpdateTemplateACL(t *testing.T) { err := client.UpdateTemplateACL(ctx, template.ID, req) require.NoError(t, err) + // Should be able to see user 3 + available, err := client2.TemplateACL(ctx, template.ID) + require.NoError(t, err) + userFound := false + for _, avail := range available.Users { + if avail.ID == user3.ID { + userFound = true + } + } + require.True(t, userFound, "user not found in acl available") + req = codersdk.UpdateTemplateACL{ UserPerms: map[string]codersdk.TemplateRole{ user3.ID.String(): codersdk.TemplateRoleUse, From 4463012b6d91e2bc46e066b4206e9eeb84d04668 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 26 Jul 2023 10:10:02 -0400 Subject: [PATCH 15/15] Use correct sdk method --- enterprise/coderd/templates_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 9647046129923..68e91a3ff0975 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -886,7 +886,7 @@ func TestUpdateTemplateACL(t *testing.T) { require.NoError(t, err) // Should be able to see user 3 - available, err := client2.TemplateACL(ctx, template.ID) + available, err := client2.TemplateACLAvailable(ctx, template.ID) require.NoError(t, err) userFound := false for _, avail := range available.Users {