Skip to content

Commit d083a7c

Browse files
EmyrkEmyrk
committed
Some cleanup
1 parent 54bc054 commit d083a7c

File tree

5 files changed

+17
-14
lines changed

5 files changed

+17
-14
lines changed

‎coderd/coderd.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ func New(options *Options) (http.Handler, func()) {
7979
Github: options.GithubOAuth2Config,
8080
})
8181

82+
// TODO: @emyrk we should just move this into 'ExtractAPIKey'.
8283
authRolesMiddleware := httpmw.ExtractUserRoles(options.Database)
8384

8485
authorize := func(f http.HandlerFunc, actions rbac.Action) http.HandlerFunc {
@@ -142,7 +143,7 @@ func New(options *Options) (http.Handler, func()) {
142143
r.Route("/members", func(r chi.Router) {
143144
r.Route("/roles", func(r chi.Router) {
144145
r.Use(httpmw.RBACObject(rbac.ResourceUserRole))
145-
r.Get("/", authorize(api.assignableOrgRoles, rbac.ActionCreate))
146+
r.Get("/", authorize(api.assignableOrgRoles, rbac.ActionRead))
146147
})
147148
r.Route("/{user}", func(r chi.Router) {
148149
r.Use(
@@ -215,7 +216,7 @@ func New(options *Options) (http.Handler, func()) {
215216
// These routes query information about site wide roles.
216217
r.Route("/roles", func(r chi.Router) {
217218
r.Use(httpmw.RBACObject(rbac.ResourceUserRole))
218-
r.Get("/", authorize(api.assignableSiteRoles, rbac.ActionCreate))
219+
r.Get("/", authorize(api.assignableSiteRoles, rbac.ActionRead))
219220
})
220221
r.Route("/{user}", func(r chi.Router) {
221222
r.Use(httpmw.ExtractUserParam(options.Database))

‎coderd/httpmw/authorize.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,21 @@ type AuthObject struct {
2020
Object rbac.Object
2121
}
2222

23-
// Authorize allows for static object & action authorize checking. If the object is a static object, this is an easy way
24-
// to enforce the route.
23+
// Authorize will enforce if the user roles can complete the action on the AuthObject.
24+
// The organization and owner are found using the ExtractOrganization and
25+
// ExtractUser middleware if present.
2526
func Authorize(logger slog.Logger, auth *rbac.RegoAuthorizer, action rbac.Action) func(http.Handler) http.Handler {
2627
return func(next http.Handler) http.Handler {
2728
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
2829
roles := UserRoles(r)
2930
args := GetAuthObject(r)
3031

3132
object := args.Object
33+
if object.Type == "" {
34+
panic("developer error: auth object has no type")
35+
}
3236

37+
// First extract the object's owner and organization if present.
3338
unknownOrg := r.Context().Value(organizationParamContextKey{})
3439
if organization, castOK := unknownOrg.(database.Organization); unknownOrg != nil {
3540
if !castOK {
@@ -46,13 +51,14 @@ func Authorize(logger slog.Logger, auth *rbac.RegoAuthorizer, action rbac.Action
4651
object = object.WithOwner(owner.ID.String())
4752
}
4853

49-
// Error on the first action that fails
5054
err := auth.AuthorizeByRoleName(r.Context(), roles.ID.String(), roles.Roles, action, object)
5155
if err != nil {
5256
internalError := new(rbac.UnauthorizedError)
5357
if xerrors.As(err, internalError) {
5458
logger = logger.With(slog.F("internal", internalError.Internal()))
5559
}
60+
// Log information for debugging. This will be very helpful
61+
// in the early days if we over secure endpoints.
5662
logger.Warn(r.Context(), "unauthorized",
5763
slog.F("roles", roles.Roles),
5864
slog.F("user_id", roles.ID),
@@ -77,11 +83,13 @@ type authObjectKey struct{}
7783
func GetAuthObject(r *http.Request) AuthObject {
7884
obj, ok := r.Context().Value(authObjectKey{}).(AuthObject)
7985
if !ok {
80-
return AuthObject{}
86+
panic("developer error: auth object middleware not provided")
8187
}
8288
return obj
8389
}
8490

91+
// RBACObject sets the object for 'Authorize()' for all routes handled
92+
// by this middleware. The important field to set is 'Type'
8593
func RBACObject(object rbac.Object) func(http.Handler) http.Handler {
8694
return func(next http.Handler) http.Handler {
8795
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {

‎coderd/httpmw/organizationparam.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ func ExtractOrganizationParam(db database.Store) func(http.Handler) http.Handler
7777

7878
ctx := context.WithValue(r.Context(), organizationParamContextKey{}, organization)
7979
ctx = context.WithValue(ctx, organizationMemberParamContextKey{}, organizationMember)
80-
8180
next.ServeHTTP(rw, r.WithContext(ctx))
8281
})
8382
}

‎coderd/rbac/authz.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ func (a RegoAuthorizer) AuthorizeByRoleName(ctx context.Context, subjectID strin
5050
}
5151
roles = append(roles, r)
5252
}
53-
5453
return a.Authorize(ctx, subjectID, roles, action, object)
5554
}
5655

‎coderd/roles_test.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,11 @@ import (
55
"net/http"
66
"testing"
77

8+
"github.com/coder/coder/coderd/coderdtest"
89
"github.com/coder/coder/coderd/rbac"
9-
10-
"github.com/google/uuid"
11-
1210
"github.com/coder/coder/codersdk"
13-
11+
"github.com/google/uuid"
1412
"github.com/stretchr/testify/require"
15-
16-
"github.com/coder/coder/coderd/coderdtest"
1713
)
1814

1915
func TestListRoles(t *testing.T) {

0 commit comments

Comments
 (0)