Rbac users endpoints

Skip workspace agent endpoints
coder · Emyrk · May 17, 2022 · May 11, 2022 · May 12, 2022 · May 12, 2022
commit be5b0b3e4aaebe5af099a552b2166cb9294080fe
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -133,7 +133,7 @@ func New(options *Options) (http.Handler, func()) {
 				httpmw.RateLimitPerMinute(12),
 				// TODO: @emyrk (rbac) Currently files are owned by the site?
 				//	Should files be org scoped? User scoped?
-				httpmw.WithRBACObject(rbac.ResourceTypeFile),
+				httpmw.WithRBACObject(rbac.ResourceFile),
 			)
 			r.Get("/{hash}", authorize(api.fileByHash, rbac.ActionRead))
 			r.Post("/", authorize(api.postFile, rbac.ActionCreate, rbac.ActionUpdate))
@@ -237,36 +237,55 @@ func New(options *Options) (http.Handler, func()) {
 					apiKeyMiddleware,
 					authRolesMiddleware,
 				)
-				r.Post("/", api.postUser)
-				r.Get("/", api.users)
+				r.Group(func(r chi.Router) {
+					// Site wide, all users
+					r.Use(httpmw.WithRBACObject(rbac.ResourceUser))
+					r.Post("/", authorize(api.postUser, rbac.ActionCreate))
+					r.Get("/", authorize(api.users, rbac.ActionRead))
+				})
 				// These routes query information about site wide roles.
 				r.Route("/roles", func(r chi.Router) {
 					r.Use(httpmw.WithRBACObject(rbac.ResourceUserRole))
 					r.Get("/", authorize(api.assignableSiteRoles, rbac.ActionRead))
 				})
 				r.Route("/{user}", func(r chi.Router) {
 					r.Use(httpmw.ExtractUserParam(options.Database))
-					r.Get("/", api.userByName)
-					r.Put("/profile", api.putUserProfile)
-					r.Put("/suspend", api.putUserSuspend)
-					r.Route("/password", func(r chi.Router) {
-						r.Use(httpmw.WithRBACObject(rbac.ResourceUserPasswordRole))
-						r.Put("/", authorize(api.putUserPassword, rbac.ActionUpdate))
+					r.Group(func(r chi.Router) {
+						r.Use(httpmw.WithRBACObject(rbac.ResourceUser))
+						r.Get("/", authorize(api.userByName, rbac.ActionRead))
+						r.Put("/profile", authorize(api.putUserProfile, rbac.ActionUpdate))
+						// suspension is deleting for a user
+						r.Put("/suspend", authorize(api.putUserSuspend, rbac.ActionDelete))
+						r.Route("/password", func(r chi.Router) {
+							r.Put("/", authorize(api.putUserPassword, rbac.ActionUpdate))
+						})
+						// This route technically also fetches the organization member struct, but only
+						// returns the roles.
+						r.Get("/roles", authorize(api.userRoles, rbac.ActionRead))
+
+						// This has 2 authorize calls. The second is explicitly called
+						// in the handler.
+						r.Put("/roles", authorize(api.putUserRoles, rbac.ActionUpdate))
+
+						// For now, just use the "user" role for their ssh keys.
+						// We can always split this out to it's own resource if we need to.
+						r.Get("/gitsshkey", authorize(api.gitSSHKey, rbac.ActionRead))
+						r.Put("/gitsshkey", authorize(api.regenerateGitSSHKey, rbac.ActionUpdate))
 					})
-					r.Get("/organizations", api.organizationsByUser)
-					r.Post("/organizations", api.postOrganizationsByUser)
-					// These roles apply to the site wide permissions.
-					r.Put("/roles", api.putUserRoles)
-					r.Get("/roles", api.userRoles)
 
-					r.Post("/keys", api.postAPIKey)
+					r.With(httpmw.WithRBACObject(rbac.ResourceAPIKey)).Post("/keys", authorize(api.postAPIKey, rbac.ActionCreate))
+
 					r.Route("/organizations", func(r chi.Router) {
+						// TODO: @emyrk This creates an organization, so why is it nested under {user}?
+						//	Shouldn't this be outside the {user} param subpath? Maybe in the organizations/
+						//	path?
 						r.Post("/", api.postOrganizationsByUser)
+
 						r.Get("/", api.organizationsByUser)
+
+						// TODO: @emyrk why is this nested under {user} when the user param is not used?
 						r.Get("/{organizationname}", api.organizationByUserAndName)
 					})
-					r.Get("/gitsshkey", api.gitSSHKey)
-					r.Put("/gitsshkey", api.regenerateGitSSHKey)
 					r.Get("/workspaces", api.workspacesByUser)
 				})
 			})

diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go
@@ -55,6 +55,22 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 		"POST:/api/v2/users/logout":     {NoAuthorize: true},
 		"GET:/api/v2/users/authmethods": {NoAuthorize: true},
 
+		// All workspaceagents endpoints do not use rbac
+		"POST:/api/v2/workspaceagents/aws-instance-identity":      {NoAuthorize: true},
+		"POST:/api/v2/workspaceagents/azure-instance-identity":    {NoAuthorize: true},
+		"POST:/api/v2/workspaceagents/google-instance-identity":   {NoAuthorize: true},
+		"GET:/api/v2/workspaceagents/me/gitsshkey":                {NoAuthorize: true},
+		"GET:/api/v2/workspaceagents/me/iceservers":               {NoAuthorize: true},
+		"GET:/api/v2/workspaceagents/me/listen":                   {NoAuthorize: true},
+		"GET:/api/v2/workspaceagents/me/metadata":                 {NoAuthorize: true},
+		"GET:/api/v2/workspaceagents/me/turn":                     {NoAuthorize: true},
+		"GET:/api/v2/workspaceagents/{workspaceagent}":            {NoAuthorize: true},
+		"GET:/api/v2/workspaceagents/{workspaceagent}/":           {NoAuthorize: true},
+		"GET:/api/v2/workspaceagents/{workspaceagent}/dial":       {NoAuthorize: true},
+		"GET:/api/v2/workspaceagents/{workspaceagent}/iceservers": {NoAuthorize: true},
+		"GET:/api/v2/workspaceagents/{workspaceagent}/pty":        {NoAuthorize: true},
+		"GET:/api/v2/workspaceagents/{workspaceagent}/turn":       {NoAuthorize: true},
+
 		// TODO: @emyrk these need to be fixed by adding authorize calls
 		"/api/v2/organizations/{organization}/provisionerdaemons": {NoAuthorize: true},
 		"GET:/api/v2/organizations/{organization}":                {AssertObject: rbac.ResourceOrganization.InOrg(admin.OrganizationID)},
@@ -71,8 +87,9 @@ func TestAuthorizeAllEndpoints(t *testing.T) {
 				routeAssertions = routeCheck{}
 			}
 
-			// Replace all url params with expected
+			// Replace all url params with known values
 			route = strings.ReplaceAll(route, "{organization}", admin.OrganizationID.String())
+			route = strings.ReplaceAll(route, "{user}", admin.UserID.String())
 
 			resp, err := client.Request(context.Background(), method, route, nil)
 			require.NoError(t, err, "do req")

diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
@@ -55,12 +55,13 @@ import (
 
 type Options struct {
 	AWSCertificates      awsidentity.Certificates
+	Authorizer           rbac.Authorizer
 	AzureCertificates    x509.VerifyOptions
 	GithubOAuth2Config   *coderd.GithubOAuth2Config
 	GoogleTokenValidator *idtoken.Validator
 	SSHKeygenAlgorithm   gitsshkey.Algorithm
 	APIRateLimit         int
-	Authorizer           rbac.Authorizer
+	LifecycleTicker      <-chan time.Time
 }
 
 // New constructs an in-memory coderd instance and returns

diff --git a/coderd/rbac/builtin.go b/coderd/rbac/builtin.go
@@ -64,6 +64,11 @@ var (
 			return Role{
 				Name:        member,
 				DisplayName: "Member",
+				Site: permissions(map[Object][]Action{
+					// All users can read all organizations.
+					// TODO: @emyrk is this ok?
+					ResourceOrganization: {ActionRead},
+				}),
 				User: permissions(map[Object][]Action{
 					ResourceWildcard: {WildcardSymbol},
 				}),
@@ -111,7 +116,18 @@ var (
 				Name:        roleName(orgMember, organizationID),
 				DisplayName: "Organization Member",
 				Org: map[string][]Permission{
-					organizationID: {},
+					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,
+							Action:       ActionRead,
+						},
+					},
 				},
 			}
 		},

diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go
@@ -21,13 +21,13 @@ var (
 		Type: "template",
 	}
 
-	ResourceTypeFile = Object{
+	ResourceFile = Object{
 		Type: "file",
 	}
 
-	// ResourceOrganization CRUD. Org owner
+	// ResourceOrganization CRUD. Always has an org owner.
 	//	create/delete = make or delete organizations
-	// 	read = view org information
+	// 	read = view org information (Can add user owner for read)
 	//	update = ??
 	ResourceOrganization = Object{
 		Type: "organization",
@@ -36,12 +36,38 @@ var (
 	// ResourceUserRole might be expanded later to allow more granular permissions
 	// to modifying roles. For now, this covers all possible roles, so having this permission
 	// allows granting/deleting **ALL** roles.
+	//	create  = Assign roles
+	//	update  = ??
+	//	read	= View available roles to assign
+	//	delete	= Remove role
 	ResourceUserRole = Object{
 		Type: "user_role",
 	}
 
-	ResourceUserPasswordRole = Object{
-		Type: "user_password",
+	// ResourceAPIKey is owned by a user.
+	//	create  = Create a new api key for user
+	//	update  = ??
+	//	read	= View api key
+	//	delete	= Delete api key
+	ResourceAPIKey = Object{
+		Type: "api_key",
+	}
+
+	// ResourceUser is the user in the 'users' table.
+	// 	create/delete = make or delete a new user.
+	// 	read = view all user's settings
+	// 	update = update all user field & settings
+	ResourceUser = Object{
+		Type: "user",
+	}
+
+	// ResourceOrganizationMember is a user's membership in an organization.
+	// Has ONLY an organization owner.
+	//	create/delete  = Create/delete member from org.
+	//	update  = Update organization member
+	//	read	= View member
+	ResourceOrganizationMember = Object{
+		Type: "organization_member",
 	}
 
 	// ResourceWildcard represents all resource types

diff --git a/coderd/users.go b/coderd/users.go
@@ -385,22 +385,51 @@ func (api *api) userRoles(rw http.ResponseWriter, r *http.Request) {
 
 func (api *api) putUserRoles(rw http.ResponseWriter, r *http.Request) {
 	// User is the user to modify
-	// TODO: Until rbac authorize is implemented, only be able to change your
-	//		own roles. This also means you can grant yourself whatever roles you want.
 	user := httpmw.UserParam(r)
-	apiKey := httpmw.APIKey(r)
-	if apiKey.UserID != user.ID {
-		httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-			Message: "modifying other users is not supported at this time",
-		})
-		return
-	}
+	roles := httpmw.UserRoles(r)
 
 	var params codersdk.UpdateRoles
 	if !httpapi.Read(rw, r, &params) {
 		return
 	}
 
+	has := make(map[string]struct{})
+	for _, exists := range roles.Roles {
+		has[exists] = struct{}{}
+	}
+
+	for _, roleName := range params.Roles {
+		// If the user already has the role, we don't need to check the permission.
+		if _, ok := has[roleName]; ok {
+			delete(has, roleName)
+			continue
+		}
+
+		// Assigning a role requires the create permission. The middleware checks if
+		// we can update this user, so the combination of the 2 permissions enables
+		// assigning new roles.
+		err := api.Authorizer.AuthorizeByRoleName(r.Context(), roles.ID.String(), roles.Roles,
+			rbac.ActionCreate, rbac.ResourceUserRole.WithID(roleName))
+		if err != nil {
+			httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
+				Message: "unauthorized",
+			})
+			return
+		}
+	}
+
+	// Any roles that were removed also need to be checked.
+	for roleName := range has {
+		err := api.Authorizer.AuthorizeByRoleName(r.Context(), roles.ID.String(), roles.Roles,
+			rbac.ActionDelete, rbac.ResourceUserRole.WithID(roleName))
+		if err != nil {
+			httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
+				Message: "unauthorized",
+			})
+			return
+		}
+	}
+
 	updatedUser, err := api.updateSiteUserRoles(r.Context(), database.UpdateUserRolesParams{
 		GrantedRoles: params.Roles,
 		ID:           user.ID,
@@ -445,6 +474,7 @@ func (api *api) updateSiteUserRoles(ctx context.Context, args database.UpdateUse
 // Returns organizations the parameterized user has access to.
 func (api *api) organizationsByUser(rw http.ResponseWriter, r *http.Request) {
 	user := httpmw.UserParam(r)
+	roles := httpmw.UserRoles(r)
 
 	organizations, err := api.Database.GetOrganizationsByUserID(r.Context(), user.ID)
 	if errors.Is(err, sql.ErrNoRows) {
@@ -460,14 +490,23 @@ func (api *api) organizationsByUser(rw http.ResponseWriter, r *http.Request) {
 
 	publicOrganizations := make([]codersdk.Organization, 0, len(organizations))
 	for _, organization := range organizations {
-		publicOrganizations = append(publicOrganizations, convertOrganization(organization))
+		err := api.Authorizer.AuthorizeByRoleName(r.Context(), roles.ID.String(), roles.Roles, rbac.ActionRead,
+			rbac.ResourceOrganization.
+				WithID(organization.ID.String()).
+				InOrg(organization.ID),
+		)
+		if err == nil {
+			// Only return orgs the user can read
+			publicOrganizations = append(publicOrganizations, convertOrganization(organization))
+		}
 	}
 
 	httpapi.Write(rw, http.StatusOK, publicOrganizations)
 }
 
 func (api *api) organizationByUserAndName(rw http.ResponseWriter, r *http.Request) {
-	user := httpmw.UserParam(r)
+	roles := httpmw.UserRoles(r)
+
 	organizationName := chi.URLParam(r, "organizationname")
 	organization, err := api.Database.GetOrganizationByName(r.Context(), organizationName)
 	if errors.Is(err, sql.ErrNoRows) {
@@ -482,19 +521,15 @@ func (api *api) organizationByUserAndName(rw http.ResponseWriter, r *http.Reques
 		})
 		return
 	}
-	_, err = api.Database.GetOrganizationMemberByUserID(r.Context(), database.GetOrganizationMemberByUserIDParams{
-		OrganizationID: organization.ID,
-		UserID:         user.ID,
-	})
-	if errors.Is(err, sql.ErrNoRows) {
-		httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-			Message: "you are not a member of that organization",
-		})
-		return
-	}
+
+	err = api.Authorizer.AuthorizeByRoleName(r.Context(), roles.ID.String(), roles.Roles, rbac.ActionRead,
+		rbac.ResourceOrganization.
+			InOrg(organization.ID).
+			WithID(organization.ID.String()),
+	)
 	if err != nil {
-		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("get organization member: %s", err),
+		httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
+			Message: err.Error(),
 		})
 		return
 	}