From b742bd89c4d82b4cd8a4ecd889492f86eac3a3a4 Mon Sep 17 00:00:00 2001 From: Noah Perks Sloan Date: Tue, 8 Apr 2025 15:31:55 -0500 Subject: [PATCH] feat: support multiple user groups (#555) --- service/internal/acl/acl.go | 14 ++++- service/internal/acl/acl_test.go | 37 +++++++++++++ service/internal/httpservers/restapi.go | 8 ++- .../internal/httpservers/restapi_auth_jwt.go | 19 ++++++- .../httpservers/restapi_auth_jwt_test.go | 55 +++++++++++++++++++ 5 files changed, 128 insertions(+), 5 deletions(-) create mode 100644 service/internal/acl/acl_test.go diff --git a/service/internal/acl/acl.go b/service/internal/acl/acl.go index 1a41b934..f1fa32cd 100644 --- a/service/internal/acl/acl.go +++ b/service/internal/acl/acl.go @@ -2,6 +2,7 @@ package acl import ( "context" + "strings" config "github.com/OliveTin/OliveTin/internal/config" log "github.com/sirupsen/logrus" @@ -202,14 +203,23 @@ func buildUserAcls(cfg *config.Config, user *AuthenticatedUser) { continue } - if slices.Contains(acl.MatchUsergroups, user.Usergroup) { + // handle multiple usergroups - groups will be separated by a space + if hasGroupsMatch(acl.MatchUsergroups, user.Usergroup) { user.Acls = append(user.Acls, acl.Name) continue - } } } +func hasGroupsMatch(matchUsergroups []string, usergroup string) bool { + for _, group := range strings.Fields(usergroup) { + if slices.Contains(matchUsergroups, group) { + return true + } + } + return false +} + func isACLRelevantToAction(cfg *config.Config, actionAcls []string, acl *config.AccessControlList, user *AuthenticatedUser) bool { if !slices.Contains(user.Acls, acl.Name) { // If the user does not have this ACL, then it is not relevant diff --git a/service/internal/acl/acl_test.go b/service/internal/acl/acl_test.go new file mode 100644 index 00000000..58f36de0 --- /dev/null +++ b/service/internal/acl/acl_test.go @@ -0,0 +1,37 @@ +package acl + +import "testing" + +func Test_hasGroupsMatch(t *testing.T) { + tests := []struct { + name string + matchUsergroups []string + usergroup string + want bool + }{ + { + name: "No groups match", + matchUsergroups: []string{"group1", "group2"}, + usergroup: "group3", + }, + { + name: "Exact match", + matchUsergroups: []string{"group1", "group2"}, + usergroup: "group1", + want: true, + }, + { + name: "Multiple groups match", + matchUsergroups: []string{"group1", "group2"}, + usergroup: "group1 group2", + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := hasGroupsMatch(tt.matchUsergroups, tt.usergroup); got != tt.want { + t.Errorf("hasGroupsMatch() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/service/internal/httpservers/restapi.go b/service/internal/httpservers/restapi.go index d25de136..1749a021 100644 --- a/service/internal/httpservers/restapi.go +++ b/service/internal/httpservers/restapi.go @@ -55,8 +55,7 @@ func parseRequestMetadata(ctx context.Context, req *http.Request) metadata.MD { sid := "" if cfg.AuthJwtHeader != "" { - // JWTs in the Authorization header are usually prefixed with "Bearer " which is not part of the JWT token. - username, usergroup = parseJwt(strings.TrimPrefix(req.Header.Get(cfg.AuthJwtHeader), "Bearer ")) + username, usergroup = parseJwtHeader(req) provider = "jwt-header" } @@ -92,6 +91,11 @@ func parseRequestMetadata(ctx context.Context, req *http.Request) metadata.MD { return md } +func parseJwtHeader(req *http.Request) (string, string) { + // JWTs in the Authorization header are usually prefixed with "Bearer " which is not part of the JWT token. + return parseJwt(strings.TrimPrefix(req.Header.Get(cfg.AuthJwtHeader), "Bearer ")) +} + func forwardResponseHandler(ctx context.Context, w http.ResponseWriter, msg protoreflect.ProtoMessage) error { md, ok := runtime.ServerMetadataFromContext(ctx) diff --git a/service/internal/httpservers/restapi_auth_jwt.go b/service/internal/httpservers/restapi_auth_jwt.go index 80fe44d3..fc0c7a4f 100644 --- a/service/internal/httpservers/restapi_auth_jwt.go +++ b/service/internal/httpservers/restapi_auth_jwt.go @@ -9,6 +9,7 @@ import ( log "github.com/sirupsen/logrus" "net/http" "os" + "strings" // "github.com/coreos/go-oidc/v3/oidc" "github.com/MicahParks/keyfunc/v3" @@ -153,7 +154,23 @@ func parseJwt(token string) (string, string) { } username := lookupClaimValueOrDefault(claims, cfg.AuthJwtClaimUsername, "") - usergroup := lookupClaimValueOrDefault(claims, cfg.AuthJwtClaimUserGroup, "") + usergroup := parseGroupClaim(cfg.AuthJwtClaimUserGroup, claims) return username, usergroup } + +func parseGroupClaim(groupClaim string, claims jwt.MapClaims) string { + usergroup := "" + if val, ok := claims[groupClaim]; ok { + if array, ok := val.([]interface{}); ok { + groups := make([]string, len(array)) + for i, v := range array { + groups[i] = fmt.Sprintf("%s", v) + } + usergroup = strings.Join(groups, " ") + } else { + usergroup = fmt.Sprintf("%s", val) + } + } + return usergroup +} diff --git a/service/internal/httpservers/restapi_auth_jwt_test.go b/service/internal/httpservers/restapi_auth_jwt_test.go index 38d8bdd1..921daf05 100644 --- a/service/internal/httpservers/restapi_auth_jwt_test.go +++ b/service/internal/httpservers/restapi_auth_jwt_test.go @@ -103,3 +103,58 @@ func TestJWTSignatureVerificationSucceeds(t *testing.T) { func TestJWTSignatureVerificationFails(t *testing.T) { testJwkValidation(t, -500, 403) } + +func TestJWTHeader(t *testing.T) { + privateKey, publicKeyPath := createKeys(t) + + defer os.Remove(publicKeyPath) + + cfg := config.DefaultConfig() + cfg.AuthJwtPubKeyPath = publicKeyPath + cfg.AuthJwtClaimUsername = "sub" + cfg.AuthJwtClaimUserGroup = "olivetinGroup" + cfg.AuthJwtHeader = "Authorization" + SetGlobalRestConfig(cfg) // ugly, setting global var, we should pass configs as params to modules... :/ + + token := jwt.New(jwt.SigningMethodRS256) + + claims := token.Claims.(jwt.MapClaims) + claims["nbf"] = time.Now().Unix() - 1000 + claims["exp"] = time.Now().Unix() + 2000 + claims["sub"] = "test" + claims["olivetinGroup"] = []string{"test", "test2"} + + tokenStr, _ := token.SignedString(privateKey) + + mux := newMux() + mux.HandlePath("GET", "/", func(w http.ResponseWriter, r *http.Request, pathParams map[string]string) { + username, usergroup := parseJwtHeader(r) + + if username == "" { + w.WriteHeader(403) + } + + assert.Equal(t, "test", username) + assert.Equal(t, "test test2", usergroup) + + w.Write([]byte(fmt.Sprintf("username=%v, usergroup=%v", username, usergroup))) + }) + + srv := setupTestingServer(mux, t) + + req, client := newReq("") + req.Header.Set("Authorization", "Bearer "+tokenStr) + + res, err := client.Do(req) + + if err != nil { + t.Fatalf("Client err: %+v", err) + } else { + defer res.Body.Close() + assert.Equal(t, 200, res.StatusCode) + body, _ := io.ReadAll(res.Body) + fmt.Println(string(body)) + } + + srv.Shutdown(context.TODO()) +}