feat: support array-valued OAuth2/JWT group claims (#1104)

Makefile

@@ -1,5 +1,5 @@
 define delete-files
-	python -c "import shutil;shutil.rmtree('$(1)', ignore_errors=True)"
+	python3 -c "import shutil;shutil.rmtree('$(1)', ignore_errors=True)"
 endef
 
 service:

docs/modules/ROOT/pages/security/oauth2.adoc

@@ -27,7 +27,7 @@ authOAuth2Providers:
 * `tokenUrl` - the URL to exchange the code for a token
 * `whoamiUrl` - the URL to fetch user information from
 * `usernameField` - the field in the user information response to use as the username
-* `userGroupField` - the field in the user information response to use as the group. This is a string containing one group name, e.g. `olivetin_group`
+* `userGroupField` - the field in the user information response to use as the group. This can either be a string containing one group name (e.g. `olivetin_group`), or an array of group names (e.g. a `groups` claim returned by many OIDC providers). When multiple groups are found, they are joined into one usergroup value using `authHttpHeaderUserGroupSep` (space-separated by default) - see xref:security/acl.adoc[Access Control Lists] for how this is matched against ACLs.
 * `addToUsergroup` - a group name to add to every user who logs in via this provider. If the user already has a usergroup (e.g. from `userGroupField`), this value is appended to it; otherwise it becomes the user's usergroup. Useful for giving all users from this provider a common group for ACLs, e.g. `addToUsergroup: github`
 * `certBundlePath` - the path to a certificate to add to the truststore for authentication requests, e.g. `/certs/internal.crt`
 * `insecureSkipVerify` - a boolean to disable certificate verfication
@@ -70,7 +70,7 @@ authOAuth2Providers:
     addToUsergroup: github
 ```
 
-Then in your actions you can restrict access with `allowedUserGroups: ["github"]`.
+Then in your ACLs you can restrict access with `matchUsergroups: ["github"]`.
 
 === Combining with userGroupField
 
@@ -103,4 +103,4 @@ authOAuth2Providers:
     addToUsergroup: google
 ```
 
-Then use ACLs such as `allowedUserGroups: ["github"]` for GitHub-only actions or `allowedUserGroups: ["github", "google"]` for any OAuth user.
+Then use ACLs such as `matchUsergroups: ["github"]` for GitHub-only actions or `matchUsergroups: ["github", "google"]` for any OAuth user.

docs/modules/ROOT/pages/security/oauth2_authelia.adoc

@@ -1,6 +1,6 @@
 = OAuth2 - Authelia
 
-Notes contributed by a member of the OliveTin community - many thanks Phampyk! 
+Notes contributed by a member of the OliveTin community - many thanks Phampyk!
 
 [source,yaml]
 .Authelia code
@@ -48,7 +48,7 @@ Digest: $pbkdf2-sha512$310000$yQogpMZvkHoAmOBGiIHVJQ$hxKuvar6Q6pOlkdzQBMWq1i5WjX
 authRequireGuestsToLogin: true
 authOAuth2RedirectURL: https://olivetin.hostname.com/oauth/callback
 authOAuth2Providers:
-  authelia: 
+  authelia:
     name: authelia
     title: Authelia
     clientID: olivetin  #same as authelia
@@ -61,7 +61,7 @@ authOAuth2Providers:
       - profile
     usernameField: preferred_username
     icon: <iconify-icon icon="simple-icons:authelia"></iconify-icon>
-	
+
 accessControlLists:
   - name: john #same as authelia
     matchUserNames:
@@ -73,7 +73,62 @@ accessControlLists:
     addToEveryAction: true
 ----
 
+== Group-based access control
+
+Rather than (or as well as) matching on individual usernames, you can grant access based on Authelia group membership. Authelia's OIDC provider can return a user's groups as a `groups` claim, which is an array (e.g. `["admins", "operators"]`), not a single string.
+
+First, request the `groups` scope for the client in Authelia:
+
+[source,yaml]
+.Authelia code
+----
+identity_providers:
+  oidc:
+    clients:
+      - client_id: "olivetin"
+        # ... other settings as above ...
+        scopes:
+          - openid
+          - profile
+          - groups
+----
+
+Then tell OliveTin which field to read the groups from with `userGroupField`. OliveTin accepts the `groups` claim as a native array - no need to flatten it into a string yourself:
+
+[source,yaml]
+.OliveTin config
+----
+authOAuth2Providers:
+  authelia:
+    name: authelia
+    title: Authelia
+    clientID: olivetin
+    clientSecret: xxxxxxx
+    authURL: https://authelia.hostname.com/api/oidc/authorization
+    tokenURL: https://authelia.hostname.com/api/oidc/token
+    whoamiUrl: https://authelia.hostname.com/api/oidc/userinfo
+    scopes:
+      - openid
+      - profile
+      - groups
+    usernameField: preferred_username
+    userGroupField: groups
+
+accessControlLists:
+  - name: admins
+    matchUsergroups:
+      - admins
+    permissions:
+      view: true
+      exec: true
+      logs: true
+    addToEveryAction: true
+----
+
+A user who is a member of multiple Authelia groups (e.g. `admins` and `operators`) will match any ACL whose `matchUsergroups` contains one of those groups - OliveTin combines the permissions from every matching ACL.
+
+If you need the joined groups to use a specific separator (e.g. because a group name itself contains a space), set `authHttpHeaderUserGroupSep`, e.g. `authHttpHeaderUserGroupSep: ","`. This is optional; the default separator is a space.
+
 == Next steps
 
 Once you have OAuth2 working, you will probably want to configure access control lists in OliveTin. This is described in the xref:security/acl.adoc[Access Control Lists] documentation page.
-

docs/modules/ROOT/pages/security/oauth2_authentik.adoc

@@ -37,6 +37,9 @@ OliveTin `2024.11.24` added support for OAuth2 group mapping for a single group.
 
 The examples below show various ways to map groups from Authentik to OliveTin.
 
+[NOTE]
+OliveTin can also accept groups returned as a native list/array, instead of a comma-separated string. If your scope mapping expression returns `{"olivetin_group_list": groups}` (a plain list) rather than joining it with `",".join(groups)`, OliveTin will read it the same way. The comma-separated approach below still works and is kept for reference.
+
 === Multiple group mapping: Comma-separated list
 
 The below will match all groups the user is a member of and return them as a comma-separated list. If no groups are found then an empty string is returned (no groups)".
@@ -183,9 +186,9 @@ image::authentik_login3.png[]
 
 == Debugging
 
-OliveTin logs OAuth2 flows quite extensively. If you are having trouble with OAuth2, you should check your OliveTin logs. 
+OliveTin logs OAuth2 flows quite extensively. If you are having trouble with OAuth2, you should check your OliveTin logs.
 
-You may see errors such as "OAuth2: Error getting user data" or "Failed to get field from user data". 
+You may see errors such as "OAuth2: Error getting user data" or "Failed to get field from user data".
 
 Sometimes it can be infuriating to debug the user data mapping (username and usergroup), as you cannot easily capture the data that is being sent back from Authentik. To help with this, you can temporarily enable a debug log flag that is INSECURE (do not leave this enabled) to log the user data that is being sent back from Authentik. To do this, add the following to your OliveTin configuration file:
 
@@ -200,4 +203,3 @@ Once you have this working, you can disable the `insecureAllowDumpOAuth2UserData
 == Next steps
 
 Once you have OAuth2 working, you will probably want to configure access control lists in OliveTin. This is described in the xref:security/acl.adoc[Access Control Lists] documentation page.
-

frontend/Makefile

@@ -1,5 +1,5 @@
 define delete-files
-	python -c "import shutil;shutil.rmtree('$(1)', ignore_errors=True)"
+	python3 -c "import shutil;shutil.rmtree('$(1)', ignore_errors=True)"
 endef
 
 codestyle:

service/Makefile

@@ -1,5 +1,5 @@
 define delete-files
-	python -c "import shutil;shutil.rmtree('$(1)', ignore_errors=True)"
+	python3 -c "import shutil;shutil.rmtree('$(1)', ignore_errors=True)"
 endef
 
 compile-currentenv:

service/internal/auth/otjwt/jwt.go

@@ -227,25 +227,41 @@ func parseJwt(cfg *config.Config, token string) *authTypes.AuthenticatedUser {
 
 	user := &authTypes.AuthenticatedUser{
 		Username:      lookupClaimValueOrDefault(claims, cfg.AuthJwtClaimUsername, ""),
-		UsergroupLine: parseGroupClaim(cfg.AuthJwtClaimUserGroup, claims),
+		UsergroupLine: parseGroupClaim(cfg.AuthJwtClaimUserGroup, claims, cfg.AuthHttpHeaderUserGroupSep),
 		Provider:      "jwt",
 	}
 
 	return user
 }
 
-func parseGroupClaim(groupClaim string, claims jwt.MapClaims) string {
-	usergroup := ""
-	if val, ok := claims[groupClaim]; ok {
-		if array, ok := val.([]any); 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)
+func parseGroupClaim(groupClaim string, claims jwt.MapClaims, sep string) string {
+	val, ok := claims[groupClaim]
+	if !ok {
+		return ""
+	}
+
+	array, isArray := val.([]any)
+	if isArray {
+		return joinJWTGroupArray(array, groupClaim, sep)
+	}
+
+	return fmt.Sprintf("%s", val)
+}
+
+func joinJWTGroupArray(arrayVal []any, groupClaim string, sep string) string {
+	if sep == "" {
+		sep = " "
+	}
+
+	groups := make([]string, 0, len(arrayVal))
+	for _, element := range arrayVal {
+		groupName, isString := element.(string)
+		if !isString {
+			log.Warnf("Skipping non-string group entry in JWT claim %v: %v", groupClaim, element)
+			continue
 		}
+		groups = append(groups, groupName)
 	}
-	return usergroup
+
+	return strings.Join(groups, sep)
 }

service/internal/auth/otjwt/jwt_test.go

@@ -202,6 +202,79 @@ func makeJWTRequest(t *testing.T, srv *httptest.Server, tokenStr string) *http.R
 	return res
 }
 
+func TestJWTHeaderSkipsNonStringGroupArrayElements(t *testing.T) {
+	privateKey, publicKeyPath := createKeys(t)
+	defer func() { _ = os.Remove(publicKeyPath) }()
+
+	cfg := config.DefaultConfig()
+	cfg.AuthJwtPubKeyPath = publicKeyPath
+	cfg.AuthJwtClaimUsername = "sub"
+	cfg.AuthJwtClaimUserGroup = "olivetinGroup"
+	cfg.AuthJwtHeader = "Authorization"
+
+	tokenStr := createJWTTokenWithGroups(t, privateKey, []any{"admins", 42, "ops"})
+
+	mux := newMux()
+	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		context := &authpublic.AuthCheckingContext{
+			Request: r,
+			Config:  cfg,
+		}
+		user := CheckUserFromJwtHeader(context)
+
+		if user == nil {
+			w.WriteHeader(http.StatusForbidden)
+			return
+		}
+
+		assert.Equal(t, "test", user.Username)
+		assert.Equal(t, "admins ops", user.UsergroupLine)
+	})
+
+	srv := httptest.NewServer(mux)
+	defer srv.Close()
+
+	res := makeJWTRequest(t, srv, tokenStr) //nolint:bodyclose // closed by verifyJWTResponse
+	verifyJWTResponse(t, res, http.StatusOK)
+}
+
+func TestJWTHeaderWithCustomGroupSeparator(t *testing.T) {
+	privateKey, publicKeyPath := createKeys(t)
+	defer func() { _ = os.Remove(publicKeyPath) }()
+
+	cfg := config.DefaultConfig()
+	cfg.AuthJwtPubKeyPath = publicKeyPath
+	cfg.AuthJwtClaimUsername = "sub"
+	cfg.AuthJwtClaimUserGroup = "olivetinGroup"
+	cfg.AuthJwtHeader = "Authorization"
+	cfg.AuthHttpHeaderUserGroupSep = ","
+
+	tokenStr := createJWTTokenWithGroups(t, privateKey, []string{"test", "test2"})
+
+	mux := newMux()
+	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		context := &authpublic.AuthCheckingContext{
+			Request: r,
+			Config:  cfg,
+		}
+		user := CheckUserFromJwtHeader(context)
+
+		if user == nil {
+			w.WriteHeader(http.StatusForbidden)
+			return
+		}
+
+		assert.Equal(t, "test", user.Username)
+		assert.Equal(t, "test,test2", user.UsergroupLine)
+	})
+
+	srv := httptest.NewServer(mux)
+	defer srv.Close()
+
+	res := makeJWTRequest(t, srv, tokenStr) //nolint:bodyclose // closed by verifyJWTResponse
+	verifyJWTResponse(t, res, http.StatusOK)
+}
+
 func TestJWTHeader(t *testing.T) {
 	privateKey, publicKeyPath := createKeys(t)
 	defer func() { _ = os.Remove(publicKeyPath) }()

service/internal/auth/otoauth2/restapi_auth_oauth2.go

@@ -11,6 +11,7 @@ import (
 	"io"
 	"net/http"
 	"os"
+	"strings"
 	"sync"
 	"time"
 
@@ -288,7 +289,11 @@ func (h *OAuth2Handler) computeUsergroup(userinfo *UserInfo, providerConfig *con
 	usergroup := userinfo.Usergroup
 	if providerConfig != nil && providerConfig.AddToUsergroup != "" {
 		if usergroup != "" {
-			usergroup = usergroup + " " + providerConfig.AddToUsergroup
+			sep := h.cfg.AuthHttpHeaderUserGroupSep
+			if sep == "" {
+				sep = " "
+			}
+			usergroup = usergroup + sep + providerConfig.AddToUsergroup
 		} else {
 			usergroup = providerConfig.AddToUsergroup
 		}
@@ -389,14 +394,14 @@ func getUserInfo(cfg *config.Config, client *http.Client, provider *config.OAuth
 	}
 
 	ret.Username = getDataField(userData, provider.UsernameField)
-	ret.Usergroup = getDataField(userData, provider.UserGroupField)
+	ret.Usergroup = getGroupField(userData, provider.UserGroupField, cfg.AuthHttpHeaderUserGroupSep)
 
 	return ret
 }
 
-func getDataField(data map[string]any, field string) string {
+func lookupRawField(data map[string]any, field string) (any, bool) {
 	if field == "" {
-		return ""
+		return nil, false
 	}
 
 	val, ok := data[field]
@@ -404,6 +409,16 @@ func getDataField(data map[string]any, field string) string {
 	if !ok {
 		log.Errorf("Failed to get field from user data: %v / %v", data, field)
 
+		return nil, false
+	}
+
+	return val, true
+}
+
+func getDataField(data map[string]any, field string) string {
+	val, ok := lookupRawField(data, field)
+
+	if !ok {
 		return ""
 	}
 
@@ -417,6 +432,55 @@ func getDataField(data map[string]any, field string) string {
 	return stringVal
 }
 
+// getGroupField reads a userinfo field that may be either a single string
+// group name, or a JSON array of group names (e.g. an OIDC "groups" claim).
+// Array elements that aren't strings are skipped and logged, rather than
+// discarding the whole claim.
+func getGroupField(data map[string]any, field string, sep string) string {
+	val, found := lookupRawField(data, field)
+
+	if !found {
+		return ""
+	}
+
+	if stringVal, isString := val.(string); isString {
+		return stringVal
+	}
+
+	arrayVal, isArray := val.([]any)
+
+	if !isArray {
+		log.Errorf("Field %v is not a string or array: %v", field, val)
+		return ""
+	}
+
+	return joinGroupArray(arrayVal, field, sep)
+}
+
+// joinGroupArray joins the string elements of a group claim array using sep,
+// defaulting to a space. Non-string elements are skipped and logged, rather
+// than discarding the whole claim.
+func joinGroupArray(arrayVal []any, field string, sep string) string {
+	groups := make([]string, 0, len(arrayVal))
+
+	for _, v := range arrayVal {
+		strVal, isString := v.(string)
+
+		if !isString {
+			log.Warnf("Skipping non-string group entry in field %v: %v", field, v)
+			continue
+		}
+
+		groups = append(groups, strVal)
+	}
+
+	if sep == "" {
+		sep = " "
+	}
+
+	return strings.Join(groups, sep)
+}
+
 func (h *OAuth2Handler) lookupOAuth2UserByState(state string) (*authTypes.AuthenticatedUser, bool) {
 	h.mu.RLock()
 	serverState, found := h.registeredStates[state]

service/internal/auth/otoauth2/restapi_auth_oauth2_test.go

@@ -34,6 +34,99 @@ func TestSweepExpiredOAuthStatesLocked(t *testing.T) {
 	assert.False(t, staleFound)
 }
 
+func TestGetGroupFieldString(t *testing.T) {
+	data := map[string]any{"olivetin_group": "admins"}
+
+	assert.Equal(t, "admins", getGroupField(data, "olivetin_group", ""))
+}
+
+func TestGetGroupFieldMissing(t *testing.T) {
+	data := map[string]any{}
+
+	assert.Equal(t, "", getGroupField(data, "olivetin_group", ""))
+}
+
+func TestGetGroupFieldEmptyFieldName(t *testing.T) {
+	data := map[string]any{"olivetin_group": "admins"}
+
+	assert.Equal(t, "", getGroupField(data, "", ""))
+}
+
+func TestGetGroupFieldArrayDefaultSeparator(t *testing.T) {
+	data := map[string]any{"groups": []any{"admins", "ops"}}
+
+	assert.Equal(t, "admins ops", getGroupField(data, "groups", ""))
+}
+
+func TestGetGroupFieldArrayCustomSeparator(t *testing.T) {
+	data := map[string]any{"groups": []any{"admins", "ops"}}
+
+	assert.Equal(t, "admins,ops", getGroupField(data, "groups", ","))
+}
+
+func TestGetGroupFieldArraySkipsNonStringElements(t *testing.T) {
+	data := map[string]any{"groups": []any{"admins", float64(5), "ops"}}
+
+	assert.Equal(t, "admins ops", getGroupField(data, "groups", ""))
+}
+
+func TestGetGroupFieldArrayAllNonStringElements(t *testing.T) {
+	data := map[string]any{"groups": []any{float64(1), true}}
+
+	assert.Equal(t, "", getGroupField(data, "groups", ""))
+}
+
+func TestGetGroupFieldNotStringOrArray(t *testing.T) {
+	data := map[string]any{"groups": map[string]any{"nested": "value"}}
+
+	assert.Equal(t, "", getGroupField(data, "groups", ""))
+}
+
+func TestGetUserInfoWithArrayGroupsClaim(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"preferred_username":"john","groups":["admins","ops"]}`))
+	}))
+	defer srv.Close()
+
+	cfg := config.DefaultConfig()
+	cfg.AuthHttpHeaderUserGroupSep = ","
+
+	provider := &config.OAuth2Provider{
+		WhoamiUrl:      srv.URL,
+		UsernameField:  "preferred_username",
+		UserGroupField: "groups",
+	}
+
+	userinfo := getUserInfo(cfg, srv.Client(), provider)
+
+	assert.Equal(t, "john", userinfo.Username)
+	assert.Equal(t, "admins,ops", userinfo.Usergroup)
+}
+
+func TestComputeUsergroupUsesConfiguredSeparatorWithAddToUsergroup(t *testing.T) {
+	cfg := config.DefaultConfig()
+	cfg.AuthHttpHeaderUserGroupSep = ","
+
+	h := &OAuth2Handler{cfg: cfg}
+
+	userinfo := &UserInfo{Usergroup: "admins,ops"}
+	providerConfig := &config.OAuth2Provider{AddToUsergroup: "github"}
+
+	assert.Equal(t, "admins,ops,github", h.computeUsergroup(userinfo, providerConfig))
+}
+
+func TestComputeUsergroupDefaultSeparatorWithAddToUsergroup(t *testing.T) {
+	cfg := config.DefaultConfig()
+
+	h := &OAuth2Handler{cfg: cfg}
+
+	userinfo := &UserInfo{Usergroup: "admins"}
+	providerConfig := &config.OAuth2Provider{AddToUsergroup: "github"}
+
+	assert.Equal(t, "admins github", h.computeUsergroup(userinfo, providerConfig))
+}
+
 func TestHandleOAuthLoginRejectsWhenStateMapFull(t *testing.T) {
 	cfg := config.DefaultConfig()
 	cfg.AuthOAuth2Providers = map[string]*config.OAuth2Provider{