Fix 401 -> 403

Emyrk · Emyrk · commit 186eb5f4ed44 · 2022-05-16T14:24:08.000-05:00
Fix comments
diff --git a/coderd/authorize.go b/coderd/authorize.go
@@ -15,7 +15,7 @@ func (api *api) Authorize(rw http.ResponseWriter, r *http.Request, action rbac.A
 	roles := httpmw.UserRoles(r)
 	err := api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, action, object)
 	if err != nil {
-		httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
+		httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
 			Message: err.Error(),
 		})
 
diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go
@@ -62,6 +62,12 @@ type Error struct {
 	Detail string `json:"detail" validate:"required"`
 }
 
+func Forbidden(rw http.ResponseWriter) {
+	Write(rw, http.StatusForbidden, Response{
+		Message: "forbidden",
+	})
+}
+
 // Write outputs a standardized format to an HTTP response body.
 func Write(rw http.ResponseWriter, status int, response interface{}) {
 	buf := &bytes.Buffer{}
diff --git a/coderd/organizations.go b/coderd/organizations.go
@@ -428,7 +428,7 @@ func (api *api) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
 	}
 
 	if workspace.OrganizationID != organization.ID {
-		httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
+		httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
 			Message: fmt.Sprintf("workspace is not owned by organization %q", organization.Name),
 		})
 		return
@@ -493,7 +493,7 @@ func (api *api) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
 	}
 
 	if organization.ID != template.OrganizationID {
-		httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
+		httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
 			Message: fmt.Sprintf("template is not in organization %q", organization.Name),
 		})
 		return
@@ -503,7 +503,7 @@ func (api *api) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
 		UserID:         apiKey.UserID,
 	})
 	if errors.Is(err, sql.ErrNoRows) {
-		httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
+		httpapi.Write(rw, http.StatusForbidden, httpapi.Response{
 			Message: "you aren't allowed to access templates in that organization",
 		})
 		return
diff --git a/coderd/rbac/error.go b/coderd/rbac/error.go
@@ -6,7 +6,7 @@ const (
 	// errUnauthorized is the error message that should be returned to
 	// clients when an action is forbidden. It is intentionally vague to prevent
 	// disclosing information that a client should not have access to.
-	errUnauthorized = "unauthorized"
+	errUnauthorized = "forbidden"
 )
 
 // UnauthorizedError is the error type for authorization errors
diff --git a/coderd/rbac/object.go b/coderd/rbac/object.go
@@ -25,7 +25,7 @@ var (
 		Type: "file",
 	}
 
-	// ResourceOrganization CRUD. Always has an org owner.
+	// ResourceOrganization CRUD. Has an org owner on all but 'create'.
 	//	create/delete = make or delete organizations
 	// 	read = view org information (Can add user owner for read)
 	//	update = ??
@@ -56,8 +56,8 @@ var (
 	// ResourceUser is the user in the 'users' table.
 	// ResourceUser never has any owners or in an org, as it's site wide.
 	// 	create/delete = make or delete a new user.
-	// 	read = view all user's settings
-	// 	update = update all user field & settings
+	// 	read = view all 'user' table data
+	// 	update = update all 'user' table data
 	ResourceUser = Object{
 		Type: "user",
 	}
diff --git a/coderd/roles.go b/coderd/roles.go
@@ -38,10 +38,16 @@ func (api *api) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
 }
 
 func (api *api) checkPermissions(rw http.ResponseWriter, r *http.Request) {
-	roles := httpmw.UserRoles(r)
 	user := httpmw.UserParam(r)
 
-	if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUserData.WithOwner(user.ID.String())) {
+	if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUser.WithOwner(user.ID.String())) {
+		return
+	}
+
+	// use the roles of the user specified, not the person making the request.
+	roles, err := api.Database.GetAllUserRoles(r.Context(), user.ID)
+	if err != nil {
+		httpapi.Forbidden(rw)
 		return
 	}
 
diff --git a/coderd/users.go b/coderd/users.go
@@ -538,15 +538,11 @@ func (api *api) organizationByUserAndName(rw http.ResponseWriter, r *http.Reques
 	if errors.Is(err, sql.ErrNoRows) {
 		// Return unauthorized rather than a 404 to not leak if the organization
 		// exists.
-		httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-			Message: "unauthorized",
-		})
+		httpapi.Forbidden(rw)
 		return
 	}
 	if err != nil {
-		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("get organization by name: %s", err),
-		})
+		httpapi.Forbidden(rw)
 		return
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@ const (`
`6`	`6`	`// errUnauthorized is the error message that should be returned to`
`7`	`7`	`// clients when an action is forbidden. It is intentionally vague to prevent`
`8`	`8`	`// disclosing information that a client should not have access to.`
`9`		`- errUnauthorized = "unauthorized"`
	`9`	`+ errUnauthorized = "forbidden"`
`10`	`10`	`)`
`11`	`11`
`12`	`12`	`// UnauthorizedError is the error type for authorization errors`