feat: Implement list roles & enforce authorize examples #1273
Next
Next commit
feat: First draft of adding authorize to an http endpoint
- Loading branch information
commit 30e203196965ebcfc0b9bfd640c326fc76870ee7
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| package httpmw | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "net/http" | ||
|
|
||
| "github.com/google/uuid" | ||
|
|
||
| "golang.org/x/xerrors" | ||
|
|
||
| "cdr.dev/slog" | ||
| "github.com/coder/coder/coderd/database" | ||
| "github.com/coder/coder/coderd/httpapi" | ||
| "github.com/coder/coder/coderd/rbac" | ||
| ) | ||
|
|
||
| type AuthObject struct { | ||
| // WithUser sets the owner of the object to the value returned by the func | ||
| WithUser func(r *http.Request) uuid.UUID | ||
|
|
||
| // InOrg sets the org owner of the object to the value returned by the func | ||
| InOrg func(r *http.Request) uuid.UUID | ||
|
|
||
| // WithOwner sets the object id to the value returned by the func | ||
| WithOwner func(r *http.Request) uuid.UUID | ||
|
|
||
| // Object is that base static object the above functions can modify. | ||
| Object rbac.Object | ||
| //// Actions are the various actions the middleware will check can be done on the object. | ||
| //Actions []rbac.Action | ||
| } | ||
|
|
||
| func WithOwner(owner func(r *http.Request) database.User) 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.WithOwner = func(r *http.Request) uuid.UUID { | ||
| return owner(r).ID | ||
| } | ||
|
|
||
| ctx := context.WithValue(r.Context(), authObjectKey{}, ao) | ||
| next.ServeHTTP(rw, r.WithContext(ctx)) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func InOrg(org func(r *http.Request) database.Organization) 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.InOrg = func(r *http.Request) uuid.UUID { | ||
| return org(r).ID | ||
| } | ||
|
|
||
| ctx := context.WithValue(r.Context(), authObjectKey{}, ao) | ||
| next.ServeHTTP(rw, r.WithContext(ctx)) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Authorize allows for static object & action authorize checking. If the object is a static object, this is an easy way | ||
| // to enforce the route. | ||
| func Authorize(logger slog.Logger, auth *rbac.RegoAuthorizer, actions ...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 := args.Object | ||
| if args.InOrg != nil { | ||
| object.InOrg(args.InOrg(r)) | ||
| } | ||
| if args.WithUser != nil { | ||
| object.WithOwner(args.InOrg(r).String()) | ||
| } | ||
| if args.WithOwner != nil { | ||
| object.WithID(args.InOrg(r).String()) | ||
| } | ||
|
|
||
| // Error on the first action that fails | ||
| for _, act := range actions { | ||
| err := auth.AuthorizeByRoleName(r.Context(), roles.ID.String(), roles.Roles, act, object) | ||
| if err != nil { | ||
| var internalError *rbac.UnauthorizedError | ||
| if xerrors.As(err, internalError) { | ||
| logger = logger.With(slog.F("internal", internalError.Internal())) | ||
| } | ||
| logger.Warn(r.Context(), "unauthorized", | ||
| slog.F("roles", roles.Roles), | ||
| slog.F("user_id", roles.ID), | ||
| slog.F("username", roles.Username), | ||
| slog.F("route", r.URL.Path), | ||
| slog.F("action", act), | ||
| slog.F("object", object), | ||
| ) | ||
| httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ | ||
| Message: err.Error(), | ||
| }) | ||
| return | ||
| } | ||
| } | ||
| next.ServeHTTP(rw, r) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| type authObjectKey struct{} | ||
|
|
||
| // APIKey returns the API key from the ExtractAPIKey handler. | ||
| func GetAuthObject(r *http.Request) AuthObject { | ||
Emyrk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| obj, ok := r.Context().Value(authObjectKey{}).(AuthObject) | ||
| if !ok { | ||
| return AuthObject{} | ||
| } | ||
| return obj | ||
| } | ||
|
|
||
| func Object(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) | ||
| next.ServeHTTP(rw, r.WithContext(ctx)) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // User roles are the 'subject' field of Authorize() | ||
| type userRolesKey struct{} | ||
|
|
||
| // APIKey returns the API key from the ExtractAPIKey handler. | ||
Emyrk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| func UserRoles(r *http.Request) database.GetAllUserRolesRow { | ||
| apiKey, ok := r.Context().Value(userRolesKey{}).(database.GetAllUserRolesRow) | ||
| if !ok { | ||
| panic("developer error: user roles middleware not provided") | ||
| } | ||
| return apiKey | ||
| } | ||
|
|
||
| // ExtractUserRoles requires authentication using a valid API key. | ||
| func ExtractUserRoles(db database.Store) func(http.Handler) http.Handler { | ||
| return func(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { | ||
| apiKey := APIKey(r) | ||
| role, err := db.GetAllUserRoles(r.Context(), apiKey.UserID) | ||
| if err != nil { | ||
| httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{ | ||
| Message: fmt.Sprintf("roles not found", AuthCookie), | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| ctx := context.WithValue(r.Context(), userRolesKey{}, role) | ||
| next.ServeHTTP(rw, r.WithContext(ctx)) | ||
| }) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be exposed via options? It doesn't seem to be used outside of here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking of making it an interface that is "denyall" or something to trigger in unit tests. But for now, we don't, so I'll drop it from
OptionsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah wait yes. @kylecarbs this will be needed if you decide to do the rbac
Authorize()check manually inside the function, instead of the middleware. So this is required.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could go on
apiinstead, that way tests couldn't mistakenly pass it viaOptions.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now nothing else is on the api only like that.
coder/coderd/coderd.go
Lines 333 to 338 in db04d67
I don't think it'd be bad to pass one in via options 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my bad, I thought there was precedent for this 🤦
If you feel like it's fine I'll defer, but I generally think it's hasty to expose a value unless it needs to be used elsewhere. It's really easy to expose a parameter, but much harder to hide one.