May 3, 2022 · May 3, 2022 · May 3, 2022 · May 3, 2022 · May 3, 2022 · May 3, 2022
diff --git a/coderd/httpmw/authorize.go b/coderd/httpmw/authorize.go
 "github.com/coder/coder/coderd/rbac"
 )

 // AuthObject wraps the rbac object type for middleware to customize this value
 // before being passed to Authorize().
 type AuthObject struct {
 // Object is that base static object the above functions can modify.
 Object rbac.Object
 }

 // Authorize will enforce if the user roles can complete the action on the AuthObject.
 // The organization and owner are found using the ExtractOrganization and
 // ExtractUser middleware if present.
 func Authorize(logger slog.Logger, auth *rbac.RegoAuthorizer, action rbac.Action) func(http.Handler) http.Handler {
 return func(next http.Handler) http.Handler {
 return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
 roles := UserRoles(r)
 args :=GetAuthObject(r)
 object :=authObject(r)

 object := args.Object
 if object.Type == "" {
 panic("developer error: auth object has no type")
 }
 type authObjectKey struct{}

 // APIKey returns the API key from the ExtractAPIKey handler.
 funcGetAuthObject(r *http.Request)AuthObject {
 obj, ok := r.Context().Value(authObjectKey{}).(AuthObject)
 funcauthObject(r *http.Request)rbac.Object {
 obj, ok := r.Context().Value(authObjectKey{}).(rbac.Object)
 if !ok {
 panic("developer error: auth object middleware not provided")
 }
 func WithRBACObject(object rbac.Object) func(http.Handler) http.Handler {
 return func(next http.Handler) http.Handler {
 return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
 ao := GetAuthObject(r)
 ao.Object = object

 ctx := context.WithValue(r.Context(), authObjectKey{}, ao)
 ctx := context.WithValue(r.Context(), authObjectKey{}, object)
 next.ServeHTTP(rw, r.WithContext(ctx))
 })
 }
diff --git a/coderd/roles_test.go b/coderd/roles_test.go
 Roles: []string{rbac.RoleOrgMember(admin.OrganizationID), rbac.RoleOrgAdmin(admin.OrganizationID)},
 },
 )
 require.NoError(t, err)
 require.NoError(t, err, "update org member roles")

 testCases := []struct {
 Name          string
Original file line number	Diff line number	Diff line change
Expand Up		@@ -13,23 +13,15 @@ import (
		"github.com/coder/coder/coderd/rbac"
		)

		// AuthObject wraps the rbac object type for middleware to customize this value
		// before being passed to Authorize().
		type AuthObject struct {
		// Object is that base static object the above functions can modify.
		Object rbac.Object
		}

		// Authorize will enforce if the user roles can complete the action on the AuthObject.
		// The organization and owner are found using the ExtractOrganization and
		// ExtractUser middleware if present.
		func Authorize(logger slog.Logger, auth *rbac.RegoAuthorizer, action rbac.Action) func(http.Handler) http.Handler {
Copy link Member kylecarbsMay 3, 2022 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. `Authorize` feels like action, not something that would return a handler. What do you think about renaming this to`EnforceRBAC`? Copy link MemberAuthor EmyrkMay 3, 2022 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I think`EnforceRBAC` is also weak though. I was thinking the package`httpmw` provides enough context, and it does do the action`Authorize()`. `Authorize` is the correct word for what is happening, as it's not authentication. I feel`EnforceRBAC` doesn't indicate the`object` and`action` are included. Another word that comes to mind is "Access". Idk,`EnforceAccess`,`EnforcePermissions`. Maybe`EnforceRBAC` isn't that bad, just felt odd to me at first. Copy link Member kylecarbsMay 3, 2022 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Fair enough. I'm primarily trying to display that the`RBAC` package is being leveraged when calling this handle.`Enforce` is a bit sketchy too. While it isauthorizing, I'm nervous that this will get conflated with authentication really easily. Copy link MemberAuthor EmyrkMay 3, 2022 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. yea this is classic authorization vs authentication. If you aren't familiar with it, it's easy to mix up. kylecarbs reacted with thumbs up emoji Copy link Member kylecarbsMay 3, 2022 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Agreed agreed
		return func(next http.Handler) http.Handler {
		return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
		roles := UserRoles(r)
		args :=GetAuthObject(r)
		object :=authObject(r)

		object := args.Object
		if object.Type == "" {
		panic("developer error: auth object has no type")
		}
Expand DownExpand Up		@@ -80,8 +72,8 @@ func Authorize(logger slog.Logger, auth *rbac.RegoAuthorizer, action rbac.Action
		type authObjectKey struct{}

		// APIKey returns the API key from the ExtractAPIKey handler.
		funcGetAuthObject(r *http.Request)AuthObject {
		obj, ok := r.Context().Value(authObjectKey{}).(AuthObject)
		funcauthObject(r *http.Request)rbac.Object {
		obj, ok := r.Context().Value(authObjectKey{}).(rbac.Object)
		if !ok {
		panic("developer error: auth object middleware not provided")
		}
Expand All		@@ -93,10 +85,7 @@ func GetAuthObject(r *http.Request) AuthObject {
		func WithRBACObject(object rbac.Object) func(http.Handler) http.Handler {
Copy link Member kylecarbsMay 3, 2022 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. It might be confusing that this is called`WithRBACObject`, but the values are`rbac.ResourceX`. It could be helpful to make these the same names!
		return func(next http.Handler) http.Handler {
		return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
		ao := GetAuthObject(r)
		ao.Object = object

		ctx := context.WithValue(r.Context(), authObjectKey{}, ao)
		ctx := context.WithValue(r.Context(), authObjectKey{}, object)
		next.ServeHTTP(rw, r.WithContext(ctx))
		})
		}
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -39,7 +39,7 @@ func TestListRoles(t *testing.T) {
		Roles: []string{rbac.RoleOrgMember(admin.OrganizationID), rbac.RoleOrgAdmin(admin.OrganizationID)},
		},
		)
		require.NoError(t, err)
		require.NoError(t, err, "update org member roles")

		testCases := []struct {
		Name string
Expand Down