Skip to content

Commit 452c72d

Browse files
EmyrkEmyrk
committed
Authorize per endpoint
1 parent 62ec87e commit 452c72d

File tree

4 files changed

+44
-11
lines changed

4 files changed

+44
-11
lines changed

‎coderd/authorize.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ package coderd
33
import (
44
"net/http"
55

6+
"cdr.dev/slog"
7+
"golang.org/x/xerrors"
8+
69
"github.com/coder/coder/coderd/httpapi"
710
"github.com/coder/coder/coderd/httpmw"
811
"github.com/coder/coder/coderd/rbac"
@@ -15,6 +18,24 @@ func (api *api) Authorize(rw http.ResponseWriter, r *http.Request, action rbac.A
1518
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
1619
Message: err.Error(),
1720
})
21+
22+
// Log the errors for debugging
23+
internalError := new(rbac.UnauthorizedError)
24+
logger := api.Logger
25+
if xerrors.As(err, internalError) {
26+
logger = api.Logger.With(slog.F("internal", internalError.Internal()))
27+
}
28+
// Log information for debugging. This will be very helpful
29+
// in the early days
30+
logger.Warn(r.Context(), "unauthorized",
31+
slog.F("roles", roles.Roles),
32+
slog.F("user_id", roles.ID),
33+
slog.F("username", roles.Username),
34+
slog.F("route", r.URL.Path),
35+
slog.F("action", action),
36+
slog.F("object", object),
37+
)
38+
1839
return false
1940
}
2041
return true

‎coderd/coderd.go

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,6 @@ func New(options *Options) (http.Handler, func()) {
8383
// TODO: @emyrk we should just move this into 'ExtractAPIKey'.
8484
authRolesMiddleware := httpmw.ExtractUserRoles(options.Database)
8585

86-
authorize := func(f http.HandlerFunc, actions rbac.Action) http.HandlerFunc {
87-
return httpmw.Authorize(api.Logger, api.Authorizer, actions)(f).ServeHTTP
88-
}
89-
9086
r := chi.NewRouter()
9187

9288
r.Use(
@@ -158,10 +154,7 @@ func New(options *Options) (http.Handler, func()) {
158154
})
159155
})
160156
r.Route("/members", func(r chi.Router) {
161-
r.Route("/roles", func(r chi.Router) {
162-
r.Use(httpmw.WithRBACObject(rbac.ResourceUserRole))
163-
r.Get("/", authorize(api.assignableOrgRoles, rbac.ActionRead))
164-
})
157+
r.Get("/roles", api.assignableOrgRoles)
165158
r.Route("/{user}", func(r chi.Router) {
166159
r.Use(
167160
httpmw.ExtractUserParam(options.Database),
@@ -232,16 +225,15 @@ func New(options *Options) (http.Handler, func()) {
232225
r.Get("/", api.users)
233226
// These routes query information about site wide roles.
234227
r.Route("/roles", func(r chi.Router) {
235-
r.Use(httpmw.WithRBACObject(rbac.ResourceUserRole))
236-
r.Get("/", authorize(api.assignableSiteRoles, rbac.ActionRead))
228+
r.Get("/", api.assignableSiteRoles)
237229
})
238230
r.Route("/{user}", func(r chi.Router) {
239231
r.Use(httpmw.ExtractUserParam(options.Database))
240232
r.Get("/", api.userByName)
241233
r.Put("/profile", api.putUserProfile)
242234
r.Put("/suspend", api.putUserSuspend)
243235
r.Route("/password", func(r chi.Router) {
244-
r.Put("/", authorize(api.putUserPassword, rbac.ActionUpdate))
236+
r.Put("/", api.putUserPassword)
245237
})
246238
r.Get("/organizations", api.organizationsByUser)
247239
r.Post("/organizations", api.postOrganizationsByUser)

‎coderd/roles.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
func (*api) assignableSiteRoles(rw http.ResponseWriter, _ *http.Request) {
1515
// TODO: @emyrk in the future, allow granular subsets of roles to be returned based on the
1616
// role of the user.
17+
1718
roles := rbac.SiteRoles()
1819
httpapi.Write(rw, http.StatusOK, convertRoles(roles))
1920
}

‎coderd/users.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,11 @@ func (api *api) users(rw http.ResponseWriter, r *http.Request) {
109109
statusFilter = r.URL.Query().Get("status")
110110
)
111111

112+
// Reading all users across the site
113+
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUser) {
114+
return
115+
}
116+
112117
paginationParams, ok := parsePagination(rw, r)
113118
if !ok {
114119
return
@@ -175,6 +180,8 @@ func (api *api) postUser(rw http.ResponseWriter, r *http.Request) {
175180
return
176181
}
177182

183+
// TODO: @emyrk Authorize the organization create if the createUser will do that.
184+
178185
_, err := api.Database.GetUserByEmailOrUsername(r.Context(), database.GetUserByEmailOrUsernameParams{
179186
Username: createUser.Username,
180187
Email: createUser.Email,
@@ -253,6 +260,10 @@ func (api *api) userByName(rw http.ResponseWriter, r *http.Request) {
253260
func (api *api) putUserProfile(rw http.ResponseWriter, r *http.Request) {
254261
user := httpmw.UserParam(r)
255262

263+
if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceUser.WithID(user.ID.String())) {
264+
return
265+
}
266+
256267
var params codersdk.UpdateUserProfileRequest
257268
if !httpapi.Read(rw, r, &params) {
258269
return
@@ -318,6 +329,10 @@ func (api *api) putUserProfile(rw http.ResponseWriter, r *http.Request) {
318329
func (api *api) putUserSuspend(rw http.ResponseWriter, r *http.Request) {
319330
user := httpmw.UserParam(r)
320331

332+
if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceUser.WithID(user.ID.String())) {
333+
return
334+
}
335+
321336
suspendedUser, err := api.Database.UpdateUserStatus(r.Context(), database.UpdateUserStatusParams{
322337
ID: user.ID,
323338
Status: database.UserStatusSuspended,
@@ -351,6 +366,10 @@ func (api *api) putUserPassword(rw http.ResponseWriter, r *http.Request) {
351366
return
352367
}
353368

369+
if !api.Authorize(rw, r, rbac.ActionUpdate, rbac.ResourceUser.WithID(user.ID.String())) {
370+
return
371+
}
372+
354373
hashedPassword, err := userpassword.Hash(params.Password)
355374
if err != nil {
356375
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{

0 commit comments

Comments
 (0)