fix(oauth2): read the OIDC group claim from the ID token too

The admin role mapping only looked for the group claim in the user
info response. Providers such as Microsoft Entra ID, or Keycloak when
the groups mapper is not added to the user info, only include it in
the ID token, so every administrator lost the admin role at their
next login without any trace in the logs.

Look for the claim in the user info response first, then in the
verified ID token. A user without the claim is still not an
administrator, but a warning is logged, and every admin role change
made at login is logged as well. A claim that is neither a string nor
a list of strings is now rejected instead of being ignored.

internal/oauth2/oidc.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"log/slog"
 	"slices"
 
 	"miniflux.app/v2/internal/config"
@@ -109,30 +110,42 @@ func (o *oidcProvider) Profile(ctx context.Context, code, codeVerifier string) (
 		}
 	}
 
+	if profile.Username == "" {
+		return nil, ErrEmptyUsername
+	}
+
 	if groupClaim := config.Opts.Oauth2GroupClaim(); groupClaim != "" {
 		var rawUserClaims map[string]any
 		if err := userInfo.Claims(&rawUserClaims); err != nil {
 			return nil, fmt.Errorf(`oidc: failed to parse user claims: %w`, err)
 		}
 
-		rawValue := rawUserClaims[groupClaim]
-		switch value := rawValue.(type) {
-		case string:
-			profile.Groups = []string{value}
-		case []any:
-			profile.Groups = make([]string, len(value))
-			for i, rawEntry := range value {
-				if entry, ok := rawEntry.(string); ok {
-					profile.Groups[i] = entry
-				} else {
-					return nil, fmt.Errorf(`oidc: failed to parse user claims: %q must only contain strings`, groupClaim)
-				}
+		groups, found, err := groupsFromClaims(rawUserClaims, groupClaim)
+		if err != nil {
+			return nil, err
+		}
+
+		// Some providers only include the groups in the ID token.
+		if !found {
+			var rawIDTokenClaims map[string]any
+			if err := idToken.Claims(&rawIDTokenClaims); err != nil {
+				return nil, fmt.Errorf(`oidc: failed to parse id token claims: %w`, err)
+			}
+
+			groups, found, err = groupsFromClaims(rawIDTokenClaims, groupClaim)
+			if err != nil {
+				return nil, err
 			}
 		}
-	}
 
-	if profile.Username == "" {
-		return nil, ErrEmptyUsername
+		if !found {
+			slog.Warn("[OIDC] Group claim not found in the ID token or user info",
+				slog.String("claim", groupClaim),
+				slog.String("username", profile.Username),
+			)
+		}
+
+		profile.Groups = groups
 	}
 
 	return profile, nil
@@ -153,3 +166,28 @@ func (o *oidcProvider) UserProfileID(user *model.User) string {
 func (o *oidcProvider) UnsetUserProfileID(user *model.User) {
 	user.OpenIDConnectID = ""
 }
+
+// groupsFromClaims returns the groups stored in the given claim, and whether the claim is present.
+func groupsFromClaims(claims map[string]any, claimName string) ([]string, bool, error) {
+	rawValue, found := claims[claimName]
+	if !found {
+		return nil, false, nil
+	}
+
+	switch value := rawValue.(type) {
+	case string:
+		return []string{value}, true, nil
+	case []any:
+		groups := make([]string, 0, len(value))
+		for _, rawEntry := range value {
+			entry, ok := rawEntry.(string)
+			if !ok {
+				return nil, true, fmt.Errorf(`oidc: failed to parse user claims: %q must only contain strings`, claimName)
+			}
+			groups = append(groups, entry)
+		}
+		return groups, true, nil
+	default:
+		return nil, true, fmt.Errorf(`oidc: failed to parse user claims: %q must be a string or a list of strings`, claimName)
+	}
+}

internal/oauth2/oidc_test.go

@@ -0,0 +1,72 @@
+// SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package oauth2 // import "miniflux.app/v2/internal/oauth2"
+
+import (
+	"slices"
+	"testing"
+)
+
+func TestGroupsFromClaims(t *testing.T) {
+	scenarios := []struct {
+		name           string
+		claims         map[string]any
+		expectedGroups []string
+		expectedFound  bool
+		expectedError  bool
+	}{
+		{
+			name:   "missing claim",
+			claims: map[string]any{"email": "user@example.org"},
+		},
+		{
+			name:           "single group",
+			claims:         map[string]any{"groups": "admins"},
+			expectedGroups: []string{"admins"},
+			expectedFound:  true,
+		},
+		{
+			name:           "list of groups",
+			claims:         map[string]any{"groups": []any{"users", "admins"}},
+			expectedGroups: []string{"users", "admins"},
+			expectedFound:  true,
+		},
+		{
+			name:           "empty list",
+			claims:         map[string]any{"groups": []any{}},
+			expectedGroups: []string{},
+			expectedFound:  true,
+		},
+		{
+			name:          "list with a non-string entry",
+			claims:        map[string]any{"groups": []any{"admins", 42.0}},
+			expectedFound: true,
+			expectedError: true,
+		},
+		{
+			name:          "unsupported type",
+			claims:        map[string]any{"groups": map[string]any{"name": "admins"}},
+			expectedFound: true,
+			expectedError: true,
+		},
+	}
+
+	for _, scenario := range scenarios {
+		t.Run(scenario.name, func(t *testing.T) {
+			groups, found, err := groupsFromClaims(scenario.claims, "groups")
+
+			if (err != nil) != scenario.expectedError {
+				t.Fatalf("Expected error %v, got %v", scenario.expectedError, err)
+			}
+
+			if found != scenario.expectedFound {
+				t.Errorf("Expected found %v, got %v", scenario.expectedFound, found)
+			}
+
+			if !slices.Equal(groups, scenario.expectedGroups) {
+				t.Errorf("Expected groups %v, got %v", scenario.expectedGroups, groups)
+			}
+		})
+	}
+}

internal/ui/oauth2_callback.go

@@ -157,6 +157,13 @@ func (h *handler) oauth2Callback(w http.ResponseWriter, r *http.Request) {
 			response.HTMLServerError(w, r, err)
 			return
 		}
+
+		slog.Info("Updated admin role from OIDC group membership",
+			slog.Int64("user_id", user.ID),
+			slog.String("username", user.Username),
+			slog.Bool("is_admin", isAdmin),
+			slog.String("admin_group", adminGroup),
+		)
 	}
 
 	h.store.SetLastLogin(user.ID)

miniflux.1

@@ -547,6 +547,10 @@ Claim that contains user groups\&.
 .br
 Used for obtaining user groups, which can be used for mapping the admin role\&.
 .br
+The claim is read from the user info response, then from the ID token\&.
+.br
+Users without this claim are not administrators\&.
+.br
 Default is empty\&.
 .TP
 .B OAUTH2_OIDC_DISCOVERY_ENDPOINT