From 015c03d4251362301beeeaffe835a63263c9fb5d Mon Sep 17 00:00:00 2001 From: Albert Nimtz Date: Wed, 25 Feb 2026 15:27:18 +0100 Subject: [PATCH 01/11] Added the optional oauth allowed roles environment variable - Setting this prevents users without the specified roles from accessing the Fider instance Generated with Claude Code Co-Authored-By: Claude --- .example.env | 2 + app/actions/oauth.go | 5 + app/cmd/routes.go | 1 + app/handlers/admin.go | 1 + app/handlers/oauth.go | 51 +++++++ app/handlers/signin.go | 11 ++ app/models/cmd/oauth.go | 1 + app/models/dto/oauth.go | 7 +- app/models/entity/oauth.go | 2 + app/pkg/env/env.go | 1 + app/services/oauth/oauth.go | 132 ++++++++++++++++++ app/services/oauth/roles_test.go | 76 ++++++++++ app/services/sqlstore/dbEntities/oauth.go | 39 ++++-- app/services/sqlstore/postgres/oauth.go | 14 +- docs/OAUTH_ROLE_RESTRICTION.md | 119 ++++++++++++++++ locale/ar/client.json | 3 + locale/de/client.json | 129 ++++++++--------- locale/el/client.json | 3 + locale/en/client.json | 3 + locale/es-ES/client.json | 3 + locale/fa/client.json | 3 + locale/fr/client.json | 3 + locale/it/client.json | 3 + locale/ja/client.json | 3 + locale/nl/client.json | 3 + locale/pl/client.json | 3 + locale/pt-BR/client.json | 3 + locale/ru/client.json | 3 + locale/sk/client.json | 3 + locale/sv-SE/client.json | 3 + locale/tr/client.json | 3 + locale/zh-CN/client.json | 3 + .../202602251400_add_oauth_roles_path.sql | 2 + public/models/settings.ts | 1 + .../Administration/components/OAuthForm.tsx | 14 +- public/pages/Error/AccessDenied.page.tsx | 19 +++ public/pages/OAuthEcho/OAuthEcho.page.tsx | 9 ++ public/services/actions/tenant.ts | 1 + 38 files changed, 596 insertions(+), 89 deletions(-) create mode 100644 app/services/oauth/roles_test.go create mode 100644 docs/OAUTH_ROLE_RESTRICTION.md create mode 100644 migrations/202602251400_add_oauth_roles_path.sql create mode 100644 public/pages/Error/AccessDenied.page.tsx diff --git a/.example.env b/.example.env index a99492ca5..4d0ee1818 100644 --- a/.example.env +++ b/.example.env @@ -23,6 +23,8 @@ OAUTH_GOOGLE_SECRET= OAUTH_GITHUB_CLIENTID= OAUTH_GITHUB_SECRET= +OAUTH_ALLOWED_ROLES= + EMAIL_NOREPLY=noreply@yourdomain.com #EMAIL_MAILGUN_API= diff --git a/app/actions/oauth.go b/app/actions/oauth.go index 1b259c60f..67274a3d3 100644 --- a/app/actions/oauth.go +++ b/app/actions/oauth.go @@ -33,6 +33,7 @@ type CreateEditOAuthConfig struct { JSONUserIDPath string `json:"jsonUserIDPath"` JSONUserNamePath string `json:"jsonUserNamePath"` JSONUserEmailPath string `json:"jsonUserEmailPath"` + JSONUserRolesPath string `json:"jsonUserRolesPath"` } func NewCreateEditOAuthConfig() *CreateEditOAuthConfig { @@ -184,5 +185,9 @@ func (action *CreateEditOAuthConfig) Validate(ctx context.Context, user *entity. result.AddFieldFailure("jsonUserEmailPath", "JSON User Email Path must have less than 100 characters.") } + if len(action.JSONUserRolesPath) > 100 { + result.AddFieldFailure("jsonUserRolesPath", "JSON User Roles Path must have less than 100 characters.") + } + return result } diff --git a/app/cmd/routes.go b/app/cmd/routes.go index b55a39c98..97e4fe1f7 100644 --- a/app/cmd/routes.go +++ b/app/cmd/routes.go @@ -120,6 +120,7 @@ func routes(r *web.Engine) *web.Engine { r.Get("/signin/complete", handlers.CompleteSignInProfilePage()) r.Get("/loginemailsent", handlers.LoginEmailSentPage()) r.Get("/not-invited", handlers.NotInvitedPage()) + r.Get("/access-denied", handlers.AccessDeniedPage()) r.Get("/signin/verify", handlers.VerifySignInKey(enum.EmailVerificationKindSignIn)) r.Get("/invite/verify", handlers.VerifySignInKey(enum.EmailVerificationKindUserInvitation)) r.Post("/_api/signin/complete", handlers.CompleteSignInProfile()) diff --git a/app/handlers/admin.go b/app/handlers/admin.go index e21d3724a..17cea6d2e 100644 --- a/app/handlers/admin.go +++ b/app/handlers/admin.go @@ -242,6 +242,7 @@ func SaveOAuthConfig() web.HandlerFunc { JSONUserIDPath: action.JSONUserIDPath, JSONUserNamePath: action.JSONUserNamePath, JSONUserEmailPath: action.JSONUserEmailPath, + JSONUserRolesPath: action.JSONUserRolesPath, }, ); err != nil { return c.Failure(err) diff --git a/app/handlers/oauth.go b/app/handlers/oauth.go index 2a8672097..6a46e50e2 100644 --- a/app/handlers/oauth.go +++ b/app/handlers/oauth.go @@ -17,6 +17,7 @@ import ( "github.com/getfider/fider/app/pkg/bus" "github.com/getfider/fider/app" + "github.com/getfider/fider/app/pkg/env" "github.com/getfider/fider/app/pkg/errors" "github.com/getfider/fider/app/pkg/jwt" "github.com/getfider/fider/app/pkg/log" @@ -91,6 +92,17 @@ func OAuthToken() web.HandlerFunc { return c.Failure(err) } + // Check if user has required roles (if OAUTH_ALLOWED_ROLES is configured) + if !hasAllowedRole(oauthUser.Result.Roles) { + log.Warnf(c, "User @{UserID} attempted OAuth login but does not have required role. User roles: @{UserRoles}, Allowed roles: @{AllowedRoles}", + dto.Props{ + "UserID": oauthUser.Result.ID, + "UserRoles": oauthUser.Result.Roles, + "AllowedRoles": env.Config.OAuth.AllowedRoles, + }) + return c.Redirect("/access-denied") + } + var user *entity.User userByProvider := &query.GetUserByProvider{Provider: provider, UID: oauthUser.Result.ID} @@ -264,3 +276,42 @@ func SignInByOAuth() web.HandlerFunc { return c.Redirect(authURL.Result) } } + +// hasAllowedRole checks if the user has any of the allowed roles configured in OAUTH_ALLOWED_ROLES +// If OAUTH_ALLOWED_ROLES is not set or empty, all users are allowed (returns true) +// If set, user must have at least one of the specified roles +func hasAllowedRole(userRoles []string) bool { + allowedRolesConfig := strings.TrimSpace(env.Config.OAuth.AllowedRoles) + + // If no roles restriction is configured, allow all users + if allowedRolesConfig == "" { + return true + } + + // Parse allowed roles from config (semicolon-separated) + allowedRoles := strings.Split(allowedRolesConfig, ";") + allowedRolesMap := make(map[string]bool) + for _, role := range allowedRoles { + role = strings.TrimSpace(role) + if role != "" { + allowedRolesMap[role] = true + } + } + + // If no valid roles in config, allow all + if len(allowedRolesMap) == 0 { + return true + } + + // Check if user has any of the allowed roles + for _, userRole := range userRoles { + userRole = strings.TrimSpace(userRole) + if allowedRolesMap[userRole] { + return true + } + } + + // User doesn't have any of the required roles + return false +} + diff --git a/app/handlers/signin.go b/app/handlers/signin.go index df03aa133..5c350096f 100644 --- a/app/handlers/signin.go +++ b/app/handlers/signin.go @@ -71,6 +71,17 @@ func NotInvitedPage() web.HandlerFunc { } } +// AccessDeniedPage renders the access denied page for OAuth role mismatches +func AccessDeniedPage() web.HandlerFunc { + return func(c *web.Context) error { + return c.Page(http.StatusForbidden, web.Props{ + Page: "Error/AccessDenied.page", + Title: "Access Denied", + Description: "You do not have the required permissions to access this site.", + }) + } +} + // SignInByEmail checks if user exists and sends code only for existing users func SignInByEmail() web.HandlerFunc { return func(c *web.Context) error { diff --git a/app/models/cmd/oauth.go b/app/models/cmd/oauth.go index 2a05f9d47..db1947ab0 100644 --- a/app/models/cmd/oauth.go +++ b/app/models/cmd/oauth.go @@ -20,6 +20,7 @@ type SaveCustomOAuthConfig struct { JSONUserIDPath string JSONUserNamePath string JSONUserEmailPath string + JSONUserRolesPath string } type ParseOAuthRawProfile struct { diff --git a/app/models/dto/oauth.go b/app/models/dto/oauth.go index bde602df6..d6b9b27c2 100644 --- a/app/models/dto/oauth.go +++ b/app/models/dto/oauth.go @@ -2,9 +2,10 @@ package dto //OAuthUserProfile represents an OAuth user profile type OAuthUserProfile struct { - ID string `json:"id"` - Name string `json:"name"` - Email string `json:"email"` + ID string `json:"id"` + Name string `json:"name"` + Email string `json:"email"` + Roles []string `json:"roles"` } //OAuthProviderOption represents an OAuth provider that can be used to authenticate diff --git a/app/models/entity/oauth.go b/app/models/entity/oauth.go index d0a49fd41..08a451712 100644 --- a/app/models/entity/oauth.go +++ b/app/models/entity/oauth.go @@ -27,6 +27,7 @@ type OAuthConfig struct { JSONUserIDPath string JSONUserNamePath string JSONUserEmailPath string + JSONUserRolesPath string } // MarshalJSON returns the JSON encoding of OAuthConfig @@ -51,5 +52,6 @@ func (o OAuthConfig) MarshalJSON() ([]byte, error) { "jsonUserIDPath": o.JSONUserIDPath, "jsonUserNamePath": o.JSONUserNamePath, "jsonUserEmailPath": o.JSONUserEmailPath, + "jsonUserRolesPath": o.JSONUserRolesPath, }) } diff --git a/app/pkg/env/env.go b/app/pkg/env/env.go index fc1fee739..2edd69977 100644 --- a/app/pkg/env/env.go +++ b/app/pkg/env/env.go @@ -99,6 +99,7 @@ type config struct { ClientID string `env:"OAUTH_GITHUB_CLIENTID"` Secret string `env:"OAUTH_GITHUB_SECRET"` } + AllowedRoles string `env:"OAUTH_ALLOWED_ROLES"` } Email struct { Type string `env:"EMAIL"` // possible values: smtp, mailgun, awsses diff --git a/app/services/oauth/oauth.go b/app/services/oauth/oauth.go index 87086d9c6..bfffa84ae 100644 --- a/app/services/oauth/oauth.go +++ b/app/services/oauth/oauth.go @@ -3,6 +3,7 @@ package oauth import ( "context" "encoding/base64" + "encoding/json" "fmt" "net/url" "strings" @@ -120,10 +121,17 @@ func parseOAuthRawProfile(ctx context.Context, c *cmd.ParseOAuthRawProfile) erro // Extract and combine name parts name := extractCompositeName(query, config.JSONUserNamePath) + // Extract roles if path is configured + var roles []string + if config.JSONUserRolesPath != "" { + roles = extractRolesFromJSON(c.Body, config.JSONUserRolesPath) + } + profile := &dto.OAuthUserProfile{ ID: strings.TrimSpace(query.String(config.JSONUserIDPath)), Name: name, Email: strings.ToLower(strings.TrimSpace(query.String(config.JSONUserEmailPath))), + Roles: roles, } if profile.ID == "" { @@ -193,6 +201,130 @@ func extractCompositeName(query *jsonq.Query, namePath string) string { return "" } +// Supports formats: +// - "roles" for array of strings: ["ROLE_ADMIN", "ROLE_USER"] +// - "roles[].id" for array of objects: [{"id": "ROLE_ADMIN"}, {"id": "ROLE_USER"}] +// - "user.roles[].name" for nested array of objects +func extractRolesFromJSON(jsonBody string, rolesPath string) []string { + rolesPath = strings.TrimSpace(rolesPath) + if rolesPath == "" { + return nil + } + + // Parse the JSON body + var data map[string]interface{} + if err := json.Unmarshal([]byte(jsonBody), &data); err != nil { + return nil + } + + // Check if we need to extract a field from array of objects (e.g., "roles[].id") + var fieldToExtract string + var actualPath string + + if strings.Contains(rolesPath, "[].") { + parts := strings.Split(rolesPath, "[].") + if len(parts) == 2 { + actualPath = parts[0] + fieldToExtract = parts[1] + } + } else { + actualPath = rolesPath + } + + // Navigate to the value using the path + value := navigateJSONPath(data, actualPath) + if value == nil { + return nil + } + + // If it's an array + if arr, ok := value.([]interface{}); ok { + roles := make([]string, 0) + + // If we need to extract a field from objects + if fieldToExtract != "" { + for _, item := range arr { + if obj, ok := item.(map[string]interface{}); ok { + if fieldValue, exists := obj[fieldToExtract]; exists { + if roleStr, ok := fieldValue.(string); ok && roleStr != "" { + roles = append(roles, strings.TrimSpace(roleStr)) + } + } + } + } + } else { + // Array of strings + for _, item := range arr { + if roleStr, ok := item.(string); ok && roleStr != "" { + roles = append(roles, strings.TrimSpace(roleStr)) + } + } + } + + if len(roles) > 0 { + return roles + } + } + + // If it's a string, try splitting + if str, ok := value.(string); ok { + str = strings.TrimSpace(str) + if str != "" { + // Try splitting by semicolon first, then comma + var roles []string + if strings.Contains(str, ";") { + roles = strings.Split(str, ";") + } else if strings.Contains(str, ",") { + roles = strings.Split(str, ",") + } else { + roles = []string{str} + } + + // Trim whitespace from each role + cleanRoles := make([]string, 0) + for _, role := range roles { + role = strings.TrimSpace(role) + if role != "" { + cleanRoles = append(cleanRoles, role) + } + } + return cleanRoles + } + } + + return nil +} + +// navigateJSONPath navigates through nested JSON structure using dot notation +// e.g., "user.profile.roles" will navigate data["user"]["profile"]["roles"] +func navigateJSONPath(data map[string]interface{}, path string) interface{} { + if path == "" { + return nil + } + + parts := strings.Split(path, ".") + var current interface{} = data + + for _, part := range parts { + part = strings.TrimSpace(part) + if part == "" { + continue + } + + if m, ok := current.(map[string]interface{}); ok { + if value, exists := m[part]; exists { + current = value + } else { + return nil + } + } else { + return nil + } + } + + return current +} + func getOAuthAuthorizationURL(ctx context.Context, q *query.GetOAuthAuthorizationURL) error { config, err := getConfig(ctx, q.Provider) if err != nil { diff --git a/app/services/oauth/roles_test.go b/app/services/oauth/roles_test.go new file mode 100644 index 000000000..ae07583a7 --- /dev/null +++ b/app/services/oauth/roles_test.go @@ -0,0 +1,76 @@ +package oauth + +import ( + "testing" +) + +func TestExtractRolesFromJSON_ArrayOfObjects(t *testing.T) { + // IServ-style response + jsonBody := `{ + "email": "test@example.com", + "roles": [ + { + "uuid": "c9b85a22-1a6c-46e1-99eb-36cb88001fda", + "id": "ROLE_TEACHER", + "displayName": "Lehrer" + }, + { + "uuid": "f66aee04-c335-4299-9cfe-ca7176cc0213", + "id": "ROLE_ADMIN", + "displayName": "Administrator" + } + ] + }` + + roles := extractRolesFromJSON(jsonBody, "roles[].id") + + if len(roles) != 2 { + t.Errorf("Expected 2 roles, got %d", len(roles)) + } + + if roles[0] != "ROLE_TEACHER" { + t.Errorf("Expected first role to be ROLE_TEACHER, got %s", roles[0]) + } + + if roles[1] != "ROLE_ADMIN" { + t.Errorf("Expected second role to be ROLE_ADMIN, got %s", roles[1]) + } +} + +func TestExtractRolesFromJSON_ArrayOfStrings(t *testing.T) { + jsonBody := `{ + "email": "test@example.com", + "roles": ["ROLE_ADMIN", "ROLE_USER"] + }` + + roles := extractRolesFromJSON(jsonBody, "roles") + + if len(roles) != 2 { + t.Errorf("Expected 2 roles, got %d", len(roles)) + } + + if roles[0] != "ROLE_ADMIN" { + t.Errorf("Expected first role to be ROLE_ADMIN, got %s", roles[0]) + } +} + +func TestExtractRolesFromJSON_NestedPath(t *testing.T) { + jsonBody := `{ + "user": { + "profile": { + "roles": ["ROLE_ADMIN"] + } + } + }` + + roles := extractRolesFromJSON(jsonBody, "user.profile.roles") + + if len(roles) != 1 { + t.Errorf("Expected 1 role, got %d", len(roles)) + } + + if roles[0] != "ROLE_ADMIN" { + t.Errorf("Expected role to be ROLE_ADMIN, got %s", roles[0]) + } +} + diff --git a/app/services/sqlstore/dbEntities/oauth.go b/app/services/sqlstore/dbEntities/oauth.go index 32c973af1..1d41823d4 100644 --- a/app/services/sqlstore/dbEntities/oauth.go +++ b/app/services/sqlstore/dbEntities/oauth.go @@ -1,28 +1,36 @@ package dbEntities import ( + "database/sql" + "github.com/getfider/fider/app/models/entity" ) type OAuthConfig struct { - ID int `db:"id"` - Provider string `db:"provider"` - DisplayName string `db:"display_name"` - LogoBlobKey string `db:"logo_bkey"` - Status int `db:"status"` - IsTrusted bool `db:"is_trusted"` - ClientID string `db:"client_id"` - ClientSecret string `db:"client_secret"` - AuthorizeURL string `db:"authorize_url"` - TokenURL string `db:"token_url"` - Scope string `db:"scope"` - ProfileURL string `db:"profile_url"` - JSONUserIDPath string `db:"json_user_id_path"` - JSONUserNamePath string `db:"json_user_name_path"` - JSONUserEmailPath string `db:"json_user_email_path"` + ID int `db:"id"` + Provider string `db:"provider"` + DisplayName string `db:"display_name"` + LogoBlobKey string `db:"logo_bkey"` + Status int `db:"status"` + IsTrusted bool `db:"is_trusted"` + ClientID string `db:"client_id"` + ClientSecret string `db:"client_secret"` + AuthorizeURL string `db:"authorize_url"` + TokenURL string `db:"token_url"` + Scope string `db:"scope"` + ProfileURL string `db:"profile_url"` + JSONUserIDPath string `db:"json_user_id_path"` + JSONUserNamePath string `db:"json_user_name_path"` + JSONUserEmailPath string `db:"json_user_email_path"` + JSONUserRolesPath sql.NullString `db:"json_user_roles_path"` } func (m *OAuthConfig) ToModel() *entity.OAuthConfig { + rolesPath := "" + if m.JSONUserRolesPath.Valid { + rolesPath = m.JSONUserRolesPath.String + } + return &entity.OAuthConfig{ ID: m.ID, Provider: m.Provider, @@ -39,5 +47,6 @@ func (m *OAuthConfig) ToModel() *entity.OAuthConfig { JSONUserIDPath: m.JSONUserIDPath, JSONUserNamePath: m.JSONUserNamePath, JSONUserEmailPath: m.JSONUserEmailPath, + JSONUserRolesPath: rolesPath, } } diff --git a/app/services/sqlstore/postgres/oauth.go b/app/services/sqlstore/postgres/oauth.go index 16bca0964..916eec43e 100644 --- a/app/services/sqlstore/postgres/oauth.go +++ b/app/services/sqlstore/postgres/oauth.go @@ -24,7 +24,7 @@ func getCustomOAuthConfigByProvider(ctx context.Context, q *query.GetCustomOAuth SELECT id, provider, display_name, status, is_trusted, logo_bkey, client_id, client_secret, authorize_url, profile_url, token_url, scope, json_user_id_path, - json_user_name_path, json_user_email_path + json_user_name_path, json_user_email_path, json_user_roles_path FROM oauth_providers WHERE tenant_id = $1 AND provider = $2 `, tenant.ID, q.Provider) @@ -49,7 +49,7 @@ func listCustomOAuthConfig(ctx context.Context, q *query.ListCustomOAuthConfig) SELECT id, provider, display_name, status, is_trusted, logo_bkey, client_id, client_secret, authorize_url, profile_url, token_url, scope, json_user_id_path, - json_user_name_path, json_user_email_path + json_user_name_path, json_user_email_path, json_user_roles_path FROM oauth_providers WHERE tenant_id = $1 ORDER BY id`, tenant.ID) @@ -79,29 +79,29 @@ func saveCustomOAuthConfig(ctx context.Context, c *cmd.SaveCustomOAuthConfig) er tenant_id, provider, display_name, status, is_trusted, client_id, client_secret, authorize_url, profile_url, token_url, scope, json_user_id_path, - json_user_name_path, json_user_email_path, logo_bkey - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15) + json_user_name_path, json_user_email_path, json_user_roles_path, logo_bkey + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16) RETURNING id` err = trx.Get(&c.ID, query, tenant.ID, c.Provider, c.DisplayName, c.Status, c.IsTrusted, c.ClientID, c.ClientSecret, c.AuthorizeURL, c.ProfileURL, c.TokenURL, c.Scope, c.JSONUserIDPath, c.JSONUserNamePath, - c.JSONUserEmailPath, c.Logo.BlobKey) + c.JSONUserEmailPath, c.JSONUserRolesPath, c.Logo.BlobKey) } else { query := ` UPDATE oauth_providers SET display_name = $3, status = $4, client_id = $5, client_secret = $6, authorize_url = $7, profile_url = $8, token_url = $9, scope = $10, json_user_id_path = $11, json_user_name_path = $12, json_user_email_path = $13, - logo_bkey = $14, is_trusted = $15 + json_user_roles_path = $14, logo_bkey = $15, is_trusted = $16 WHERE tenant_id = $1 AND id = $2` _, err = trx.Execute(query, tenant.ID, c.ID, c.DisplayName, c.Status, c.ClientID, c.ClientSecret, c.AuthorizeURL, c.ProfileURL, c.TokenURL, c.Scope, c.JSONUserIDPath, c.JSONUserNamePath, - c.JSONUserEmailPath, c.Logo.BlobKey, c.IsTrusted) + c.JSONUserEmailPath, c.JSONUserRolesPath, c.Logo.BlobKey, c.IsTrusted) } if err != nil { diff --git a/docs/OAUTH_ROLE_RESTRICTION.md b/docs/OAUTH_ROLE_RESTRICTION.md new file mode 100644 index 000000000..cca21c36e --- /dev/null +++ b/docs/OAUTH_ROLE_RESTRICTION.md @@ -0,0 +1,119 @@ +# OAuth Role-Based Access Control + +This feature allows you to restrict OAuth login to users who have specific roles provided by your OAuth provider. + +## Configuration + +### Environment Variable + +Add the following environment variable to your `.env` file: + +```bash +OAUTH_ALLOWED_ROLES=ROLE_ADMIN;ROLE_TEACHER +``` + +- **Format**: Semicolon-separated list of role names +- **Case-sensitive**: Role names are matched exactly as they appear in the OAuth response +- **Optional**: If not set or empty, all users are allowed to login (default behavior) + +### Examples + +#### Allow only admins: +```bash +OAUTH_ALLOWED_ROLES=ROLE_ADMIN +``` + +#### Allow admins and teachers: +```bash +OAUTH_ALLOWED_ROLES=ROLE_ADMIN;ROLE_TEACHER +``` + +#### Allow all users (default): +```bash +# Don't set OAUTH_ALLOWED_ROLES or leave it empty +OAUTH_ALLOWED_ROLES= +``` + +## OAuth Provider Configuration + +For custom OAuth providers, you need to configure the JSON path to extract roles from the OAuth response. + +### Admin Panel Configuration + +1. Go to **Site Settings** → **Authentication** → **OAuth Providers** +2. Edit or create a custom OAuth provider +3. Configure the **JSON User Roles Path** field + +### JSON User Roles Path Examples + +The roles path supports various OAuth response formats: + +#### Array of strings: +```json +{ + "id": "12345", + "name": "John Doe", + "email": "john@example.com", + "roles": ["ROLE_ADMIN", "ROLE_TEACHER"] +} +``` +**JSON User Roles Path**: `roles` + +#### Array of objects (extract specific field): +```json +{ + "id": "12345", + "roles": [ + { "id": "ROLE_ADMIN", "displayName": "Administrator" }, + { "id": "ROLE_TEACHER", "displayName": "Teacher" } + ] +} +``` +**JSON User Roles Path**: `roles[].id` + +This extracts the `id` field from each object in the `roles` array. + +#### Nested array: +```json +{ + "id": "12345", + "user": { + "profile": { + "roles": ["ROLE_ADMIN"] + } + } +} +``` +**JSON User Roles Path**: `user.profile.roles` + +#### Nested array of objects: +```json +{ + "id": "12345", + "user": { + "groups": [ + { "name": "ROLE_ADMIN" }, + { "name": "ROLE_USER" } + ] + } +} +``` +**JSON User Roles Path**: `user.groups[].name` + +#### Comma or semicolon-separated string: +```json +{ + "id": "12345", + "roles": "ROLE_ADMIN,ROLE_TEACHER" +} +``` +**JSON User Roles Path**: `roles` + +#### Single role as string: +```json +{ + "id": "12345", + "role": "ROLE_ADMIN" +} +``` +**JSON User Roles Path**: `role` diff --git a/locale/ar/client.json b/locale/ar/client.json index 4098b3a0f..b4aadaa5c 100644 --- a/locale/ar/client.json +++ b/locale/ar/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "فتح", "enum.poststatus.planned": "مخطط", "enum.poststatus.started": "بدأت", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "انتهت صَلاحِيَة الرابط الذي ضغطت عليه.", "error.expired.title": "منتهية الصَّلاحِيَة", "error.forbidden.text": "لا تملك الصلاحيات لتصفح هذه الصفحة.", diff --git a/locale/de/client.json b/locale/de/client.json index 52a2e63c5..58772c867 100644 --- a/locale/de/client.json +++ b/locale/de/client.json @@ -6,18 +6,18 @@ "action.confirm": "Bestätigen", "action.copylink": "Link kopieren", "action.delete": "Löschen", - "action.delete.block": "", + "action.delete.block": "Löschen & Blockieren", "action.edit": "Bearbeiten", "action.markallasread": "Alle als gelesen markieren", "action.ok": "OK", - "action.postcomment": "", + "action.postcomment": "Senden", "action.postsfeed": "Beiträge-Feed", - "action.publish": "", - "action.publish.verify": "", + "action.publish": "Veröffentlichen", + "action.publish.verify": "Veröffentlichen & Vertrauen", "action.respond": "Antworten", "action.save": "Sichern", "action.signin": "Anmelden", - "action.signup": "Melden Sie sich an", + "action.signup": "Registrieren", "action.submit": "Absenden", "action.vote": "Abstimmen", "action.voted": "Abgestimmt!", @@ -30,6 +30,9 @@ "enum.poststatus.open": "Offen", "enum.poststatus.planned": "Geplant", "enum.poststatus.started": "Gestartet", + "error.accessdenied.contact": "Wenn du glaubst, dass dies ein Fehler ist, wende dich bitte an deinen Administrator.", + "error.accessdenied.text": "Du hast nicht die erforderlichen Berechtigungen, um auf diese Seite zuzugreifen.", + "error.accessdenied.title": "Zugriff verweigert", "error.expired.text": "Der Link, dem du gefolgt bist, ist abgelaufen.", "error.expired.title": "Abgelaufen", "error.forbidden.text": "Du bist nicht berechtigt diese Seite anzuschauen.", @@ -44,11 +47,11 @@ "error.unauthorized.title": "Nicht berechtigt", "home.filter.label": "Filter", "home.filter.search.label": "Filter durchsuchen...", - "home.form.defaultinvitation": "Gib hier Deinen Vorschlag ein...", - "home.form.defaultwelcomemessage": "Wir würden gerne erfahren, worüber Du nachdenkst.\n\nWas können wir verbessern? Hier kannst Du abstimmen, diskutieren und neue Ideen vorschlagen.", + "home.form.defaultinvitation": "Gib hier deinen Vorschlag ein...", + "home.form.defaultwelcomemessage": "Wir würden gerne erfahren, worüber du nachdenkst.\n\nWas können wir verbessern? Hier kannst du abstimmen, diskutieren und neue Ideen vorschlagen.", "home.lonely.suggestion": "Es wird empfohlen, dass du <0>mindestens 3 Vorschläge hier erstellst, bevor du diese Seite teilst. Der anfängliche Inhalt ist wichtig, um die Interaktion mit deiner Zielgruppe zu fördern.", "home.lonely.text": "Es wurden noch keine Beiträge erstellt.", - "home.postfilter.label.moderation": "", + "home.postfilter.label.moderation": "Moderation", "home.postfilter.label.myactivity": "Meine Aktivität", "home.postfilter.label.status": "Status", "home.postfilter.option.mostdiscussed": "Am häufigsten diskutiert", @@ -82,9 +85,9 @@ "label.tags": "Tags", "label.unread": "Ungelesen", "label.unsubscribe": "Abbestellen", - "label.vote": "", + "label.vote": "Abstimmen", "label.voters": "Wähler", - "label.votes": "", + "label.votes": "Stimmen", "labels.notagsavailable": "Keine Tags verfügbar", "legal.agreement": "Ich habe die <0/> und <1/> gelesen und akzeptiere sie.", "legal.notice": "Mit der Anmeldung stimmst du den <0/><1/> und <2/> zu.", @@ -94,17 +97,17 @@ "linkmodal.text.label": "Anzuzeigender Text", "linkmodal.text.placeholder": "Linktext eingeben", "linkmodal.title": "Link einfügen", - "linkmodal.url.label": "", - "linkmodal.url.placeholder": "", + "linkmodal.url.label": "URL", + "linkmodal.url.placeholder": "https://example.com", "menu.administration": "Verwaltung", "menu.mysettings": "Meine Einstellungen", "menu.signout": "Abmelden", "menu.sitesettings": "Seiteneinstellungen", "modal.changeemail.header": "Bestätige deine neue E-Mail-Adresse", "modal.changeemail.text": "Wir haben soeben einen Bestätigungslink an <0>{0} gesendet. <1/> Klicke auf den Link, um deine E-Mail zu aktualisieren.", - "modal.completeprofile.header": "Vervollständige Dein Profil", + "modal.completeprofile.header": "Vervollständige dein Profil", "modal.completeprofile.name.placeholder": "Name", - "modal.completeprofile.text": "Da dies dein erster Login ist, gebe bitte einen Namen an.", + "modal.completeprofile.text": "Da dies dein erster Login ist, gib bitte deinen Namen ein.", "modal.deleteaccount.header": "Account löschen", "modal.deleteaccount.text": "<0>Wenn du dein Konto löschst, werden wir all deine persönlichen Daten für immer löschen. Der von dir veröffentlichte Inhalt bleibt erhalten, wird aber anonymisiert.<1>Dieser Prozess ist unumkehrbar. <2>Bist du sicher? ", "modal.deletecomment.header": "Kommentar löschen", @@ -117,26 +120,26 @@ "modal.showvotes.message.zeromatches": "Keine Benutzer gefunden, die <0>{0} entsprechen.", "modal.showvotes.query.placeholder": "Suche nach Benutzern nach Namen...", "modal.signin.header": "Reiche dein Feedback ein", - "moderation.comment.delete.block.error": "", - "moderation.comment.delete.error": "", - "moderation.comment.deleted": "", - "moderation.comment.deleted.blocked": "", - "moderation.comment.publish.error": "", - "moderation.comment.publish.verify.error": "", - "moderation.comment.published": "", - "moderation.comment.published.verified": "", - "moderation.empty": "", - "moderation.fetch.error": "", - "moderation.post.delete.block.error": "", - "moderation.post.delete.error": "", - "moderation.post.deleted": "", - "moderation.post.deleted.blocked": "", - "moderation.post.publish.error": "", - "moderation.post.publish.verify.error": "", - "moderation.post.published": "", - "moderation.post.published.verified": "", - "moderation.subtitle": "", - "moderation.title": "", + "moderation.comment.delete.block.error": "Fehler beim Löschen des Kommentars und Blockieren des Benutzers", + "moderation.comment.delete.error": "Fehler beim Löschen des Kommentars", + "moderation.comment.deleted": "Kommentar erfolgreich gelöscht", + "moderation.comment.deleted.blocked": "Kommentar gelöscht und Benutzer blockiert", + "moderation.comment.publish.error": "Fehler beim Veröffentlichen des Kommentars", + "moderation.comment.publish.verify.error": "Fehler beim Veröffentlichen des Kommentars und Verifizieren des Benutzers", + "moderation.comment.published": "Kommentar erfolgreich veröffentlicht", + "moderation.comment.published.verified": "Kommentar veröffentlicht und Benutzer verifiziert", + "moderation.empty": "Alle Inhalte wurden moderiert. Du bist auf dem neuesten Stand!", + "moderation.fetch.error": "Fehler beim Abrufen der Moderationselemente", + "moderation.post.delete.block.error": "Fehler beim Löschen des Beitrags und Blockieren des Benutzers", + "moderation.post.delete.error": "Fehler beim Löschen des Beitrags", + "moderation.post.deleted": "Beitrag erfolgreich gelöscht", + "moderation.post.deleted.blocked": "Beitrag gelöscht und Benutzer blockiert", + "moderation.post.publish.error": "Fehler beim Veröffentlichen des Beitrags", + "moderation.post.publish.verify.error": "Fehler beim Veröffentlichen des Beitrags und Verifizieren des Benutzers", + "moderation.post.published": "Beitrag erfolgreich veröffentlicht", + "moderation.post.published.verified": "Beitrag veröffentlicht und Benutzer verifiziert", + "moderation.subtitle": "Diese Ideen und Kommentare stammen von Personen außerhalb deiner Liste vertrauenswürdiger Benutzer. Du entscheidest, ob sie veröffentlicht werden.", + "moderation.title": "Moderationswarteschlange", "mynotifications.label.readrecently": "Lies, was in den letzten 30 Tagen geschrieben wurde.", "mynotifications.message.nounread": "Keine ungelesenen Benachrichtigungen.", "mynotifications.page.subtitle": "Bleibe immer auf dem Laufenden", @@ -172,53 +175,53 @@ "newpost.modal.title.label": "Gib deiner Idee einen Titel", "newpost.modal.title.placeholder": "Eine kurze und knappe Zusammenfassung in wenigen Worten", "page.backhome": "Zurück zur <0>{0} Homepage.", - "page.pendingactivation.didntreceive": "Haben Sie die E-Mail nicht erhalten?", + "page.pendingactivation.didntreceive": "Hast du die E-Mail nicht erhalten?", "page.pendingactivation.resend": "Bestätigungs-E-Mail erneut senden", "page.pendingactivation.resending": "Erneut gesendet...", - "page.pendingactivation.text": "Wir haben Ihnen eine Bestätigungs-E-Mail mit einem Link zur Aktivierung Ihrer Website geschickt.", + "page.pendingactivation.text": "Wir haben dir eine Bestätigungs-E-Mail mit einem Link zur Aktivierung deiner Website geschickt.", "page.pendingactivation.text2": "Bitte überprüfe deinen Posteingang, um ihn zu aktivieren.", "page.pendingactivation.title": "Dein Account ist nicht aktiviert", - "pagination.next": "", - "pagination.prev": "", - "post.pending": "", - "postdetails.backtoall": "", + "pagination.next": "Weiter", + "pagination.prev": "Zurück", + "post.pending": "ausstehend", + "postdetails.backtoall": "Zurück zu allen Vorschlägen", "showpost.comment.copylink.error": "Kommentar-Link konnte nicht kopiert werden, bitte URL der Webseite kopieren", "showpost.comment.copylink.success": "Kommentar-Link in die Zwischenablage kopiert", "showpost.comment.unknownhighlighted": "Ungültige Kommentar ID #{id}", "showpost.commentinput.placeholder": "Kommentar hinzufügen", "showpost.copylink.success": "Link in die Zwischenablage kopiert", - "showpost.loading": "", + "showpost.loading": "Lädt...", "showpost.message.nodescription": "Keine Beschreibung angegeben.", - "showpost.moderation.admin.description": "", - "showpost.moderation.admin.title": "", - "showpost.moderation.approved": "", - "showpost.moderation.approveerror": "", - "showpost.moderation.awaiting": "", - "showpost.moderation.comment.admin.description": "", - "showpost.moderation.comment.approved": "", - "showpost.moderation.comment.approveerror": "", - "showpost.moderation.comment.awaiting": "", - "showpost.moderation.comment.declined": "", - "showpost.moderation.comment.declineerror": "", - "showpost.moderation.commentsuccess": "", - "showpost.moderation.declined": "", - "showpost.moderation.declineerror": "", - "showpost.moderation.postsuccess": "", + "showpost.moderation.admin.description": "Diese Idee benötigt deine Genehmigung, bevor sie veröffentlicht wird", + "showpost.moderation.admin.title": "Moderation", + "showpost.moderation.approved": "Beitrag erfolgreich genehmigt", + "showpost.moderation.approveerror": "Fehler beim Genehmigen des Beitrags", + "showpost.moderation.awaiting": "Wartet auf Moderation.", + "showpost.moderation.comment.admin.description": "Dieser Kommentar benötigt deine Genehmigung, bevor er veröffentlicht wird", + "showpost.moderation.comment.approved": "Kommentar erfolgreich genehmigt", + "showpost.moderation.comment.approveerror": "Fehler beim Genehmigen des Kommentars", + "showpost.moderation.comment.awaiting": "Wartet auf Moderation.", + "showpost.moderation.comment.declined": "Kommentar erfolgreich abgelehnt", + "showpost.moderation.comment.declineerror": "Fehler beim Ablehnen des Kommentars", + "showpost.moderation.commentsuccess": "Dein Kommentar wartet auf Moderation 📝", + "showpost.moderation.declined": "Beitrag erfolgreich abgelehnt", + "showpost.moderation.declineerror": "Fehler beim Ablehnen des Beitrags", + "showpost.moderation.postsuccess": "Deine Idee wartet auf Moderation 📝", "showpost.moderationpanel.text.help": "Diese Aktion <0>kann nicht rückgängig gemacht werden.", "showpost.moderationpanel.text.placeholder": "Warum löschst du diesen Beitrag? (optional)", "showpost.notificationspanel.message.subscribed": "Du erhältst Benachrichtigungen über Aktivitäten auf diesem Beitrag.", "showpost.notificationspanel.message.unsubscribed": "Du erhältst keine Benachrichtigungen über diesen Beitrag.", - "showpost.postedby": "", + "showpost.postedby": "Veröffentlicht von", "showpost.postsearch.numofvotes": "{0} Stimmen", "showpost.postsearch.query.placeholder": "Originalbeitrag suchen...", "showpost.responseform.message.mergedvotes": "Stimmen aus diesem Beitrag werden mit den Stimmen vom ursprünglichen Beitrag zusammengeführt.", "showpost.responseform.text.placeholder": "Was passiert in diesem Beitrag? Lass deine Benutzer wissen, was deine Pläne sind...", - "showpost.save.success": "", + "showpost.save.success": "Beitrag erfolgreich aktualisiert", "signin.code.edit": "Bearbeiten", "signin.code.getnew": "Neuen Code anfordern", - "signin.code.instruction": "Bitte geben Sie den soeben gesendeten Code an <0>{email} ein.", - "signin.code.placeholder": "Geben Sie hier den Code ein.", - "signin.code.sent": "Ein neuer Code wurde an Ihre E-Mail-Adresse gesendet.", + "signin.code.instruction": "Bitte gib den soeben an <0>{email} gesendeten Code ein.", + "signin.code.placeholder": "Gib hier den Code ein.", + "signin.code.sent": "Ein neuer Code wurde an deine E-Mail-Adresse gesendet.", "signin.email.placeholder": "E-Mail-Adresse", "signin.message.email": "Mit E-Mail fortfahren", "signin.message.emaildisabled": "Die E-Mail-Authentifizierung wurde von einem Administrator deaktiviert. Wenn du ein Administrator-Konto hast und diese Einschränkung umgehen musst, klicke bitte <0>hier.", @@ -229,7 +232,7 @@ "signin.message.private.text": "Wenn du ein Konto oder eine Einladung hast, kannst du folgende Optionen nutzen, um dich anzumelden.", "signin.message.private.title": "<0>{0} ist ein privater Raum, du musst dich anmelden, um teilzunehmen und abstimmen zu können.", "signin.message.socialbutton.intro": "Einloggen mit", - "signin.name.placeholder": "Ihr Name", + "signin.name.placeholder": "Dein Name", "validation.custom.maxattachments": "Es sind maximal {number} Anhänge zulässig.", "validation.custom.maximagesize": "Die Bildgröße muss kleiner als {kilobytes}KB sein." -} \ No newline at end of file +} diff --git a/locale/el/client.json b/locale/el/client.json index 6c954bd8f..e17b85fa7 100644 --- a/locale/el/client.json +++ b/locale/el/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "Άνοιγμα", "enum.poststatus.planned": "Προγραμματισμένο", "enum.poststatus.started": "Ξεκίνησε", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "Ο σύνδεσμος που κάνατε κλικ έχει λήξει.", "error.expired.title": "Ληγμένο", "error.forbidden.text": "Δεν έχετε δικαίωμα πρόσβασης σε αυτή τη σελίδα.", diff --git a/locale/en/client.json b/locale/en/client.json index 518853923..5a77ed622 100644 --- a/locale/en/client.json +++ b/locale/en/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "Open", "enum.poststatus.planned": "Planned", "enum.poststatus.started": "Started", + "error.accessdenied.contact": "If you believe this is an error, please contact your administrator.", + "error.accessdenied.text": "You do not have the required permissions to access this site.", + "error.accessdenied.title": "Access Denied", "error.expired.text": "The link you clicked has expired.", "error.expired.title": "Expired", "error.forbidden.text": "You are not authorized to view this page.", diff --git a/locale/es-ES/client.json b/locale/es-ES/client.json index 5b8cbfb1a..7d48d5df6 100644 --- a/locale/es-ES/client.json +++ b/locale/es-ES/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "Abierto", "enum.poststatus.planned": "Planificado", "enum.poststatus.started": "Iniciado", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "El enlace en el que hizo clic ha expirado.", "error.expired.title": "Caducado", "error.forbidden.text": "No estás autorizado para ver esta página.", diff --git a/locale/fa/client.json b/locale/fa/client.json index 78a815877..367386a73 100644 --- a/locale/fa/client.json +++ b/locale/fa/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "باز", "enum.poststatus.planned": "برنامه‌ریزی شده", "enum.poststatus.started": "شروع شده", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "لینکی که کلیک کردید منقضی شده است.", "error.expired.title": "منقضی شده", "error.forbidden.text": "شما اجازهٔ مشاهدهٔ این صفحه را ندارید.", diff --git a/locale/fr/client.json b/locale/fr/client.json index e1723a726..c7d00c16a 100644 --- a/locale/fr/client.json +++ b/locale/fr/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "Ouvert", "enum.poststatus.planned": "Planifié", "enum.poststatus.started": "Démarré", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "Le lien que vous avez cliqué a expiré.", "error.expired.title": "Expiré", "error.forbidden.text": "Vous n'êtes pas autorisé à accéder à cette page.", diff --git a/locale/it/client.json b/locale/it/client.json index ed92099a0..d6b51c2ab 100644 --- a/locale/it/client.json +++ b/locale/it/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "Aperto", "enum.poststatus.planned": "Pianificato", "enum.poststatus.started": "Iniziato", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "Il link che hai cliccato è scaduto.", "error.expired.title": "Scaduto", "error.forbidden.text": "Non sei autorizzato a visualizzare questa pagina.", diff --git a/locale/ja/client.json b/locale/ja/client.json index 7b4e04dbf..04408a57d 100644 --- a/locale/ja/client.json +++ b/locale/ja/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "開く", "enum.poststatus.planned": "準備中", "enum.poststatus.started": "開始済み", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "クリックしたリンクは期限切れです。", "error.expired.title": "期限切れ", "error.forbidden.text": "このページを閲覧する権限がありません。", diff --git a/locale/nl/client.json b/locale/nl/client.json index 7b780c6fe..692fc7dae 100644 --- a/locale/nl/client.json +++ b/locale/nl/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "Open", "enum.poststatus.planned": "Gepland", "enum.poststatus.started": "Gestart", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "De link waarop je hebt geklikt is verlopen.", "error.expired.title": "Verlopen", "error.forbidden.text": "Je bent niet gemachtigd om deze pagina te openen.", diff --git a/locale/pl/client.json b/locale/pl/client.json index bf8e5e2e9..696062207 100644 --- a/locale/pl/client.json +++ b/locale/pl/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "Otwarte", "enum.poststatus.planned": "Zaplanowane", "enum.poststatus.started": "Rozpoczęte", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "Kliknięty przez Ciebie link wygasł.", "error.expired.title": "Wygasły", "error.forbidden.text": "Nie masz uprawnień do wyświetlania tej strony.", diff --git a/locale/pt-BR/client.json b/locale/pt-BR/client.json index 0e7feb022..2877b9950 100644 --- a/locale/pt-BR/client.json +++ b/locale/pt-BR/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "Aberto", "enum.poststatus.planned": "Planejado", "enum.poststatus.started": "Iniciado", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "O link que você acessou expirou.", "error.expired.title": "Expirado", "error.forbidden.text": "Você não está autorizado a visualizar esta página.", diff --git a/locale/ru/client.json b/locale/ru/client.json index 6dbb12b8b..f033439b6 100644 --- a/locale/ru/client.json +++ b/locale/ru/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "Открыто", "enum.poststatus.planned": "Запланировано", "enum.poststatus.started": "Начато", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "Срок действия ссылки, по которой вы перешли, истёк.", "error.expired.title": "Срок действия истёк", "error.forbidden.text": "Вы не можете просматривать эту страницу.", diff --git a/locale/sk/client.json b/locale/sk/client.json index 4bc6cd851..eac28dcc2 100644 --- a/locale/sk/client.json +++ b/locale/sk/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "Otvorené", "enum.poststatus.planned": "Plánované", "enum.poststatus.started": "Začaté", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "Platnosť odkazu, na ktorý ste klikli, vypršala.", "error.expired.title": "Platnosť vypršala", "error.forbidden.text": "Nemáte oprávnenie na zobrazenie tejto stránky.", diff --git a/locale/sv-SE/client.json b/locale/sv-SE/client.json index 994f09ed4..91b6dd342 100644 --- a/locale/sv-SE/client.json +++ b/locale/sv-SE/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "Öppen", "enum.poststatus.planned": "Planerad", "enum.poststatus.started": "Startad", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "Länken du klickade på har gått ut.", "error.expired.title": "Utgången", "error.forbidden.text": "Du har inte behörighet att se den här sidan.", diff --git a/locale/tr/client.json b/locale/tr/client.json index 09414eed9..bd9ea6944 100644 --- a/locale/tr/client.json +++ b/locale/tr/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "Açık", "enum.poststatus.planned": "Planlandı", "enum.poststatus.started": "Başlatıldı", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "Tıkladığınız bağlantının süresi doldu.", "error.expired.title": "Süresi Bitmiş", "error.forbidden.text": "Bu sayfayı görüntülemek için yetkiniz yok.", diff --git a/locale/zh-CN/client.json b/locale/zh-CN/client.json index 05ad3059f..8bb9dae19 100644 --- a/locale/zh-CN/client.json +++ b/locale/zh-CN/client.json @@ -30,6 +30,9 @@ "enum.poststatus.open": "开启", "enum.poststatus.planned": "计划中", "enum.poststatus.started": "进行中", + "error.accessdenied.contact": "", + "error.accessdenied.text": "", + "error.accessdenied.title": "", "error.expired.text": "您点击的链接已过期.", "error.expired.title": "过期", "error.forbidden.text": "您无权查看此页面.", diff --git a/migrations/202602251400_add_oauth_roles_path.sql b/migrations/202602251400_add_oauth_roles_path.sql new file mode 100644 index 000000000..f21ce4ade --- /dev/null +++ b/migrations/202602251400_add_oauth_roles_path.sql @@ -0,0 +1,2 @@ +ALTER TABLE oauth_providers ADD COLUMN json_user_roles_path TEXT; + diff --git a/public/models/settings.ts b/public/models/settings.ts index 98c129440..507cb250e 100644 --- a/public/models/settings.ts +++ b/public/models/settings.ts @@ -47,6 +47,7 @@ export interface OAuthConfig { jsonUserIDPath: string jsonUserNamePath: string jsonUserEmailPath: string + jsonUserRolesPath: string isTrusted: boolean } diff --git a/public/pages/Administration/components/OAuthForm.tsx b/public/pages/Administration/components/OAuthForm.tsx index e08b8bd5f..128ff3062 100644 --- a/public/pages/Administration/components/OAuthForm.tsx +++ b/public/pages/Administration/components/OAuthForm.tsx @@ -27,6 +27,7 @@ export const OAuthForm: React.FC = (props) => { const [jsonUserIDPath, setJSONUserIDPath] = useState((props.config && props.config.jsonUserIDPath) || "") const [jsonUserNamePath, setJSONUserNamePath] = useState((props.config && props.config.jsonUserNamePath) || "") const [jsonUserEmailPath, setJSONUserEmailPath] = useState((props.config && props.config.jsonUserEmailPath) || "") + const [jsonUserRolesPath, setJSONUserRolesPath] = useState((props.config && props.config.jsonUserRolesPath) || "") const [logo, setLogo] = useState() const [logoURL, setLogoURL] = useState() const [logoBlobKey, setLogoBlobKey] = useState((props.config && props.config.logoBlobKey) || "") @@ -47,6 +48,7 @@ export const OAuthForm: React.FC = (props) => { jsonUserIDPath, jsonUserNamePath, jsonUserEmailPath, + jsonUserRolesPath, logo, }) if (result.ok) { @@ -157,7 +159,7 @@ export const OAuthForm: React.FC = (props) => { .

-
+
= (props) => { Optional, but highly recommended.

+ +

Optional. Used for role-based access control.

+
diff --git a/public/pages/Error/AccessDenied.page.tsx b/public/pages/Error/AccessDenied.page.tsx new file mode 100644 index 000000000..320844942 --- /dev/null +++ b/public/pages/Error/AccessDenied.page.tsx @@ -0,0 +1,19 @@ +import { Trans } from "@lingui/react/macro" +import React from "react" +import { ErrorPageWrapper } from "./components/ErrorPageWrapper" + +export const AccessDenied = () => { + return ( + +

+ Access Denied +

+

+ You do not have the required permissions to access this site. +

+

+ If you believe this is an error, please contact your administrator. +

+ + ) +} diff --git a/public/pages/OAuthEcho/OAuthEcho.page.tsx b/public/pages/OAuthEcho/OAuthEcho.page.tsx index 8ada76f11..b835f438f 100644 --- a/public/pages/OAuthEcho/OAuthEcho.page.tsx +++ b/public/pages/OAuthEcho/OAuthEcho.page.tsx @@ -14,6 +14,7 @@ interface OAuthEchoPageProps { id: string name: string email: string + roles: string[] } } @@ -39,6 +40,7 @@ export default class OAuthEchoPage extends React.Component 0 let responseBody = "" try { @@ -82,6 +84,13 @@ export default class OAuthEchoPage extends React.Component )} + + + {hasRoles ? ok : warn} + Roles: {hasRoles ? this.props.profile.roles.join(", ") : "(none)"} + + Roles are optional and used for role-based access control when OAUTH_ALLOWED_ROLES is configured. + ) diff --git a/public/services/actions/tenant.ts b/public/services/actions/tenant.ts index 546db011d..fd2ecd135 100644 --- a/public/services/actions/tenant.ts +++ b/public/services/actions/tenant.ts @@ -118,6 +118,7 @@ export interface CreateEditOAuthConfigRequest { jsonUserIDPath: string jsonUserNamePath: string jsonUserEmailPath: string + jsonUserRolesPath: string logo?: ImageUpload isTrusted: boolean } From 5868e26145485e414f59316db29c9223dad04260 Mon Sep 17 00:00:00 2001 From: Albert Nimtz Date: Fri, 27 Feb 2026 09:01:10 +0100 Subject: [PATCH 02/11] Fixed error page for existing users that are blocked from accessing now --- public/pages/Error/AccessDenied.page.tsx | 4 +++- public/ssr.tsx | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/public/pages/Error/AccessDenied.page.tsx b/public/pages/Error/AccessDenied.page.tsx index 320844942..4c655ab44 100644 --- a/public/pages/Error/AccessDenied.page.tsx +++ b/public/pages/Error/AccessDenied.page.tsx @@ -2,7 +2,7 @@ import { Trans } from "@lingui/react/macro" import React from "react" import { ErrorPageWrapper } from "./components/ErrorPageWrapper" -export const AccessDenied = () => { +const AccessDenied = () => { return (

@@ -17,3 +17,5 @@ export const AccessDenied = () => { ) } + +export default AccessDenied diff --git a/public/ssr.tsx b/public/ssr.tsx index e64e9e499..f8491a543 100644 --- a/public/ssr.tsx +++ b/public/ssr.tsx @@ -47,6 +47,7 @@ const pages: { [key: string]: any } = { "Error/Error410.page": require(`./pages/Error/Error410.page`), "Error/Error500.page": require(`./pages/Error/Error500.page`), "Error/NotInvited.page": require(`./pages/Error/NotInvited.page`), + "Error/AccessDenied.page": require(`./pages/Error/AccessDenied.page`), } function ssrRender(url: string, args: any) { From c60d23aca8d7cdded22e13e7905b64451977f3f4 Mon Sep 17 00:00:00 2001 From: Albert Nimtz Date: Fri, 6 Mar 2026 09:19:08 +0100 Subject: [PATCH 03/11] Fixed role check being used for all oauth provider - fixed duplication of jsonq code - fixed env var semicolon separator - added tests --- app/handlers/oauth.go | 36 ++++++-- app/handlers/oauth_roles_test.go | 147 +++++++++++++++++++++++++++++++ app/pkg/jsonq/jsonq.go | 10 +++ app/services/oauth/oauth.go | 119 ++++++++----------------- docs/OAUTH_ROLE_RESTRICTION.md | 9 +- 5 files changed, 228 insertions(+), 93 deletions(-) create mode 100644 app/handlers/oauth_roles_test.go diff --git a/app/handlers/oauth.go b/app/handlers/oauth.go index 6a46e50e2..eccebf1d0 100644 --- a/app/handlers/oauth.go +++ b/app/handlers/oauth.go @@ -92,8 +92,9 @@ func OAuthToken() web.HandlerFunc { return c.Failure(err) } - // Check if user has required roles (if OAUTH_ALLOWED_ROLES is configured) - if !hasAllowedRole(oauthUser.Result.Roles) { + // Check if user has required roles (if OAUTH_ALLOWED_ROLES is configured AND provider has a roles path) + providerRolesPath := getProviderJSONUserRolesPath(c, provider) + if !hasAllowedRole(oauthUser.Result.Roles, providerRolesPath) { log.Warnf(c, "User @{UserID} attempted OAuth login but does not have required role. User roles: @{UserRoles}, Allowed roles: @{AllowedRoles}", dto.Props{ "UserID": oauthUser.Result.ID, @@ -165,6 +166,17 @@ func isTrustedOAuthProvider(ctx context.Context, provider string) bool { return customOAuthConfigByProvider.Result.IsTrusted } +// getProviderJSONUserRolesPath returns the JSONUserRolesPath configured for a custom OAuth provider. +// Returns empty string if the provider is a built-in provider or if the config cannot be retrieved. +func getProviderJSONUserRolesPath(ctx context.Context, provider string) string { + customOAuthConfigByProvider := &query.GetCustomOAuthConfigByProvider{Provider: provider} + err := bus.Dispatch(ctx, customOAuthConfigByProvider) + if err != nil { + return "" + } + return customOAuthConfigByProvider.Result.JSONUserRolesPath +} + // OAuthCallback handles the redirect back from the OAuth provider // This callback can run on either Tenant or Login address // If the request is for a sign in, we redirect the user to the tenant address @@ -277,10 +289,13 @@ func SignInByOAuth() web.HandlerFunc { } } -// hasAllowedRole checks if the user has any of the allowed roles configured in OAUTH_ALLOWED_ROLES -// If OAUTH_ALLOWED_ROLES is not set or empty, all users are allowed (returns true) -// If set, user must have at least one of the specified roles -func hasAllowedRole(userRoles []string) bool { +// hasAllowedRole checks if the user has any of the allowed roles configured in OAUTH_ALLOWED_ROLES. +// If OAUTH_ALLOWED_ROLES is not set or empty, all users are allowed (returns true). +// If the provider has no JSONUserRolesPath configured, the role check is skipped (returns true). +// This ensures that providers without a roles path (e.g. built-in Google/GitHub OAuth) are never +// accidentally blocked in a multi-provider setup where OAUTH_ALLOWED_ROLES is configured for a +// different provider that does return roles. +func hasAllowedRole(userRoles []string, jsonUserRolesPath string) bool { allowedRolesConfig := strings.TrimSpace(env.Config.OAuth.AllowedRoles) // If no roles restriction is configured, allow all users @@ -288,8 +303,13 @@ func hasAllowedRole(userRoles []string) bool { return true } - // Parse allowed roles from config (semicolon-separated) - allowedRoles := strings.Split(allowedRolesConfig, ";") + // If the provider has no roles path configured, skip the role check for this provider + if strings.TrimSpace(jsonUserRolesPath) == "" { + return true + } + + // Parse allowed roles from config (comma-separated) + allowedRoles := strings.Split(allowedRolesConfig, ",") allowedRolesMap := make(map[string]bool) for _, role := range allowedRoles { role = strings.TrimSpace(role) diff --git a/app/handlers/oauth_roles_test.go b/app/handlers/oauth_roles_test.go new file mode 100644 index 000000000..c522dcbd0 --- /dev/null +++ b/app/handlers/oauth_roles_test.go @@ -0,0 +1,147 @@ +package handlers + +// Internal test file for hasAllowedRole — uses package handlers (not handlers_test) +// so the unexported function is accessible without exporting it. + +import ( + "testing" + + "github.com/getfider/fider/app/pkg/env" +) + +// setAllowedRoles sets env.Config.OAuth.AllowedRoles for the duration of a test +// and automatically restores the original value when the test finishes. +func setAllowedRoles(t *testing.T, value string) { + t.Helper() + original := env.Config.OAuth.AllowedRoles + env.Config.OAuth.AllowedRoles = value + t.Cleanup(func() { env.Config.OAuth.AllowedRoles = original }) +} + +// --- empty / unconfigured OAUTH_ALLOWED_ROLES --- + +func TestHasAllowedRole_EmptyConfig_AllowsAll(t *testing.T) { + setAllowedRoles(t, "") + if !hasAllowedRole([]string{"ROLE_ADMIN"}, "roles") { + t.Error("expected true when OAUTH_ALLOWED_ROLES is empty") + } +} + +func TestHasAllowedRole_EmptyConfig_AllowsEmptyRoles(t *testing.T) { + setAllowedRoles(t, "") + if !hasAllowedRole([]string{}, "roles") { + t.Error("expected true when OAUTH_ALLOWED_ROLES is empty and user has no roles") + } +} + +func TestHasAllowedRole_WhitespaceOnlyConfig_AllowsAll(t *testing.T) { + setAllowedRoles(t, " ") + if !hasAllowedRole([]string{"ROLE_ADMIN"}, "roles") { + t.Error("expected true when OAUTH_ALLOWED_ROLES is whitespace only") + } +} + +// --- provider has no JSONUserRolesPath (skip check) --- + +func TestHasAllowedRole_NoRolesPath_SkipsCheck(t *testing.T) { + setAllowedRoles(t, "ROLE_ADMIN") + // Provider without a roles path must always be allowed through, + // regardless of whether the user carries matching roles or not. + if !hasAllowedRole([]string{}, "") { + t.Error("expected true when provider has no JSONUserRolesPath (check should be skipped)") + } +} + +func TestHasAllowedRole_WhitespaceRolesPath_SkipsCheck(t *testing.T) { + setAllowedRoles(t, "ROLE_ADMIN") + if !hasAllowedRole([]string{}, " ") { + t.Error("expected true when JSONUserRolesPath is whitespace only (check should be skipped)") + } +} + +// --- matching role --- + +func TestHasAllowedRole_MatchingSingleRole(t *testing.T) { + setAllowedRoles(t, "ROLE_ADMIN") + if !hasAllowedRole([]string{"ROLE_ADMIN"}, "roles") { + t.Error("expected true when user has the required role") + } +} + +func TestHasAllowedRole_MatchingOneOfMultipleAllowed(t *testing.T) { + setAllowedRoles(t, "ROLE_ADMIN,ROLE_TEACHER") + if !hasAllowedRole([]string{"ROLE_TEACHER"}, "roles") { + t.Error("expected true when user has one of the allowed roles") + } +} + +func TestHasAllowedRole_MatchingMultipleAllowed(t *testing.T) { + setAllowedRoles(t, "ROLE_ADMIN,ROLE_TEACHER") + if !hasAllowedRole([]string{"ROLE_ADMIN", "ROLE_TEACHER"}, "roles") { + t.Error("expected true when user has all of the allowed roles") + } +} + +func TestHasAllowedRole_AllowedRolesWithWhitespace(t *testing.T) { + setAllowedRoles(t, " ROLE_ADMIN , ROLE_TEACHER ") + if !hasAllowedRole([]string{"ROLE_TEACHER"}, "roles") { + t.Error("expected true when allowed roles config has surrounding whitespace") + } +} + +// --- no matching role --- + +func TestHasAllowedRole_NoMatchingRole(t *testing.T) { + setAllowedRoles(t, "ROLE_ADMIN") + if hasAllowedRole([]string{"ROLE_STUDENT"}, "roles") { + t.Error("expected false when user does not have any allowed role") + } +} + +func TestHasAllowedRole_EmptyUserRoles_WithConfig(t *testing.T) { + setAllowedRoles(t, "ROLE_ADMIN") + if hasAllowedRole([]string{}, "roles") { + t.Error("expected false when user has no roles and OAUTH_ALLOWED_ROLES is set") + } +} + +func TestHasAllowedRole_NilUserRoles_WithConfig(t *testing.T) { + setAllowedRoles(t, "ROLE_ADMIN") + if hasAllowedRole(nil, "roles") { + t.Error("expected false when user roles slice is nil and OAUTH_ALLOWED_ROLES is set") + } +} + +func TestHasAllowedRole_CaseSensitiveNoMatch(t *testing.T) { + setAllowedRoles(t, "ROLE_ADMIN") + // Role names are matched case-sensitively. + if hasAllowedRole([]string{"role_admin"}, "roles") { + t.Error("expected false: role matching must be case-sensitive") + } +} + +func TestHasAllowedRole_MultipleDefinedNoMatch(t *testing.T) { + setAllowedRoles(t, "ROLE_ADMIN,ROLE_TEACHER") + if hasAllowedRole([]string{"ROLE_GUEST"}, "roles") { + t.Error("expected false when user has multiple roles but none match the allowed roles") + } +} + +func TestHasAllowedRole_MultipleNoMatch(t *testing.T) { + setAllowedRoles(t, "ROLE_ADMIN,ROLE_TEACHER") + if hasAllowedRole([]string{"ROLE_STUDENT", "ROLE_GUEST"}, "roles") { + t.Error("expected false when user has multiple roles but none match the allowed roles") + } +} + +// --- edge cases --- + +func TestHasAllowedRole_ConfigWithOnlyCommas_AllowsAll(t *testing.T) { + // A config of only separators produces no valid role entries → allow all. + setAllowedRoles(t, ",,, ,") + if !hasAllowedRole([]string{}, "roles") { + t.Error("expected true when config contains only separators (no valid roles)") + } +} + + diff --git a/app/pkg/jsonq/jsonq.go b/app/pkg/jsonq/jsonq.go index 16812b518..942f04c82 100644 --- a/app/pkg/jsonq/jsonq.go +++ b/app/pkg/jsonq/jsonq.go @@ -86,6 +86,16 @@ func (q *Query) Contains(selector string) bool { return q.get(selector) != nil } +//Raw returns the raw JSON bytes at the given dot-notation selector, or nil if not found. +func (q *Query) Raw(selector string) []byte { + msg := q.get(selector) + if msg == nil { + return nil + } + b, _ := msg.MarshalJSON() + return b +} + func (q *Query) get(selector string) *json.RawMessage { if selector == "" { return nil diff --git a/app/services/oauth/oauth.go b/app/services/oauth/oauth.go index bfffa84ae..ecd16c345 100644 --- a/app/services/oauth/oauth.go +++ b/app/services/oauth/oauth.go @@ -211,118 +211,75 @@ func extractRolesFromJSON(jsonBody string, rolesPath string) []string { return nil } - // Parse the JSON body - var data map[string]interface{} - if err := json.Unmarshal([]byte(jsonBody), &data); err != nil { - return nil - } - - // Check if we need to extract a field from array of objects (e.g., "roles[].id") + // Split "roles[].id" into actualPath="roles" and fieldToExtract="id" var fieldToExtract string var actualPath string - if strings.Contains(rolesPath, "[].") { - parts := strings.Split(rolesPath, "[].") - if len(parts) == 2 { - actualPath = parts[0] - fieldToExtract = parts[1] - } + parts := strings.SplitN(rolesPath, "[].", 2) + actualPath = parts[0] + fieldToExtract = parts[1] } else { actualPath = rolesPath } - // Navigate to the value using the path - value := navigateJSONPath(data, actualPath) - if value == nil { + // Use jsonq to navigate to the value at actualPath + rawBytes := jsonq.New(jsonBody).Raw(actualPath) + if rawBytes == nil { return nil } - // If it's an array - if arr, ok := value.([]interface{}); ok { - roles := make([]string, 0) - - // If we need to extract a field from objects + // Try to unmarshal as an array + var arr []json.RawMessage + if err := json.Unmarshal(rawBytes, &arr); err == nil { + roles := make([]string, 0, len(arr)) if fieldToExtract != "" { + // Array of objects — extract the named field from each element for _, item := range arr { - if obj, ok := item.(map[string]interface{}); ok { - if fieldValue, exists := obj[fieldToExtract]; exists { - if roleStr, ok := fieldValue.(string); ok && roleStr != "" { - roles = append(roles, strings.TrimSpace(roleStr)) - } - } + obj := jsonq.New(string(item)) + if s := strings.TrimSpace(obj.String(fieldToExtract)); s != "" { + roles = append(roles, s) } } } else { - // Array of strings + // Array of plain strings for _, item := range arr { - if roleStr, ok := item.(string); ok && roleStr != "" { - roles = append(roles, strings.TrimSpace(roleStr)) + var s string + if err := json.Unmarshal(item, &s); err == nil { + if s = strings.TrimSpace(s); s != "" { + roles = append(roles, s) + } } } } - if len(roles) > 0 { return roles } + return nil } - // If it's a string, try splitting - if str, ok := value.(string); ok { + // Try to unmarshal as a plain string (comma-separated or single value) + var str string + if err := json.Unmarshal(rawBytes, &str); err == nil { str = strings.TrimSpace(str) - if str != "" { - // Try splitting by semicolon first, then comma - var roles []string - if strings.Contains(str, ";") { - roles = strings.Split(str, ";") - } else if strings.Contains(str, ",") { - roles = strings.Split(str, ",") - } else { - roles = []string{str} - } - - // Trim whitespace from each role - cleanRoles := make([]string, 0) - for _, role := range roles { - role = strings.TrimSpace(role) - if role != "" { - cleanRoles = append(cleanRoles, role) - } - } - return cleanRoles + if str == "" { + return nil } - } - - return nil -} - -// navigateJSONPath navigates through nested JSON structure using dot notation -// e.g., "user.profile.roles" will navigate data["user"]["profile"]["roles"] -func navigateJSONPath(data map[string]interface{}, path string) interface{} { - if path == "" { - return nil - } - - parts := strings.Split(path, ".") - var current interface{} = data - - for _, part := range parts { - part = strings.TrimSpace(part) - if part == "" { - continue + var parts []string + if strings.Contains(str, ",") { + parts = strings.Split(str, ",") + } else { + parts = []string{str} } - - if m, ok := current.(map[string]interface{}); ok { - if value, exists := m[part]; exists { - current = value - } else { - return nil + roles := make([]string, 0, len(parts)) + for _, r := range parts { + if r = strings.TrimSpace(r); r != "" { + roles = append(roles, r) } - } else { - return nil } + return roles } - return current + return nil } func getOAuthAuthorizationURL(ctx context.Context, q *query.GetOAuthAuthorizationURL) error { diff --git a/docs/OAUTH_ROLE_RESTRICTION.md b/docs/OAUTH_ROLE_RESTRICTION.md index cca21c36e..ffb78b55e 100644 --- a/docs/OAUTH_ROLE_RESTRICTION.md +++ b/docs/OAUTH_ROLE_RESTRICTION.md @@ -9,12 +9,13 @@ This feature allows you to restrict OAuth login to users who have specific roles Add the following environment variable to your `.env` file: ```bash -OAUTH_ALLOWED_ROLES=ROLE_ADMIN;ROLE_TEACHER +OAUTH_ALLOWED_ROLES=ROLE_ADMIN,ROLE_TEACHER ``` -- **Format**: Semicolon-separated list of role names +- **Format**: Comma-separated list of role names - **Case-sensitive**: Role names are matched exactly as they appear in the OAuth response - **Optional**: If not set or empty, all users are allowed to login (default behavior) +- **Per-provider enforcement**: The role check is only applied to OAuth providers that have a **JSON User Roles Path** configured. Providers without a roles path (e.g. built-in Google or GitHub OAuth) are always allowed through, even when `OAUTH_ALLOWED_ROLES` is set. This prevents accidentally blocking users from providers that don't return roles. ### Examples @@ -25,7 +26,7 @@ OAUTH_ALLOWED_ROLES=ROLE_ADMIN #### Allow admins and teachers: ```bash -OAUTH_ALLOWED_ROLES=ROLE_ADMIN;ROLE_TEACHER +OAUTH_ALLOWED_ROLES=ROLE_ADMIN,ROLE_TEACHER ``` #### Allow all users (default): @@ -100,7 +101,7 @@ This extracts the `id` field from each object in the `roles` array. ``` **JSON User Roles Path**: `user.groups[].name` -#### Comma or semicolon-separated string: +#### Comma-separated string: ```json { "id": "12345", From 1cf53c38884a643c5749cadd3e489a4a553ef52a Mon Sep 17 00:00:00 2001 From: Albert Nimtz Date: Fri, 6 Mar 2026 12:12:13 +0100 Subject: [PATCH 04/11] Changed allowed roles form env to db level configuration, to allow per provider config - also fixed documentation URLS --- .example.env | 2 - app/actions/oauth.go | 5 ++ app/handlers/admin.go | 1 + app/handlers/oauth.go | 90 +++++++++++-------- app/handlers/oauth_roles_test.go | 80 ++++++----------- app/models/cmd/oauth.go | 1 + app/models/entity/oauth.go | 2 + app/pkg/env/env.go | 1 - app/services/sqlstore/dbEntities/oauth.go | 6 ++ app/services/sqlstore/postgres/oauth.go | 16 ++-- docs/OAUTH_ROLE_RESTRICTION.md | 84 ++++++++--------- ... 202603061000_add_oauth_allowed_roles.sql} | 2 +- public/models/settings.ts | 1 + .../Administration/components/OAuthForm.tsx | 20 ++++- .../pages/ManageAuthentication.page.tsx | 2 +- .../pages/ManageWebhooks.page.tsx | 2 +- .../MySettings/components/APIKeyForm.tsx | 2 +- public/pages/OAuthEcho/OAuthEcho.page.tsx | 33 ++++++- public/services/actions/tenant.ts | 1 + 19 files changed, 199 insertions(+), 152 deletions(-) rename migrations/{202602251400_add_oauth_roles_path.sql => 202603061000_add_oauth_allowed_roles.sql} (52%) diff --git a/.example.env b/.example.env index 4d0ee1818..a99492ca5 100644 --- a/.example.env +++ b/.example.env @@ -23,8 +23,6 @@ OAUTH_GOOGLE_SECRET= OAUTH_GITHUB_CLIENTID= OAUTH_GITHUB_SECRET= -OAUTH_ALLOWED_ROLES= - EMAIL_NOREPLY=noreply@yourdomain.com #EMAIL_MAILGUN_API= diff --git a/app/actions/oauth.go b/app/actions/oauth.go index 67274a3d3..5ba7f94bc 100644 --- a/app/actions/oauth.go +++ b/app/actions/oauth.go @@ -34,6 +34,7 @@ type CreateEditOAuthConfig struct { JSONUserNamePath string `json:"jsonUserNamePath"` JSONUserEmailPath string `json:"jsonUserEmailPath"` JSONUserRolesPath string `json:"jsonUserRolesPath"` + AllowedRoles string `json:"allowedRoles"` } func NewCreateEditOAuthConfig() *CreateEditOAuthConfig { @@ -189,5 +190,9 @@ func (action *CreateEditOAuthConfig) Validate(ctx context.Context, user *entity. result.AddFieldFailure("jsonUserRolesPath", "JSON User Roles Path must have less than 100 characters.") } + if len(action.AllowedRoles) > 500 { + result.AddFieldFailure("allowedRoles", "Allowed Roles must have less than 500 characters.") + } + return result } diff --git a/app/handlers/admin.go b/app/handlers/admin.go index 17cea6d2e..6b487f9a7 100644 --- a/app/handlers/admin.go +++ b/app/handlers/admin.go @@ -243,6 +243,7 @@ func SaveOAuthConfig() web.HandlerFunc { JSONUserNamePath: action.JSONUserNamePath, JSONUserEmailPath: action.JSONUserEmailPath, JSONUserRolesPath: action.JSONUserRolesPath, + AllowedRoles: action.AllowedRoles, }, ); err != nil { return c.Failure(err) diff --git a/app/handlers/oauth.go b/app/handlers/oauth.go index eccebf1d0..4b2c7d3fa 100644 --- a/app/handlers/oauth.go +++ b/app/handlers/oauth.go @@ -17,7 +17,6 @@ import ( "github.com/getfider/fider/app/pkg/bus" "github.com/getfider/fider/app" - "github.com/getfider/fider/app/pkg/env" "github.com/getfider/fider/app/pkg/errors" "github.com/getfider/fider/app/pkg/jwt" "github.com/getfider/fider/app/pkg/log" @@ -56,12 +55,22 @@ func OAuthEcho() web.HandlerFunc { parseRawProfile := &cmd.ParseOAuthRawProfile{Provider: provider, Body: rawProfile.Result} _ = bus.Dispatch(c, parseRawProfile) + // Fetch provider config to show configured allowedRoles on the test page. + // Errors are intentionally ignored here — this is a non-critical diagnostic fetch. + var configuredAllowedRoles, configuredRolesPath string + if providerConfig, err := getCustomOAuthConfig(c, provider); err == nil && providerConfig != nil { + configuredAllowedRoles = providerConfig.AllowedRoles + configuredRolesPath = providerConfig.JSONUserRolesPath + } + return c.Page(http.StatusOK, web.Props{ Page: "OAuthEcho/OAuthEcho.page", Title: "OAuth Test Page", Data: web.Map{ - "body": rawProfile.Result, - "profile": parseRawProfile.Result, + "body": rawProfile.Result, + "profile": parseRawProfile.Result, + "configuredRolesPath": configuredRolesPath, + "configuredAllowedRoles": configuredAllowedRoles, }, }) } @@ -92,14 +101,28 @@ func OAuthToken() web.HandlerFunc { return c.Failure(err) } - // Check if user has required roles (if OAUTH_ALLOWED_ROLES is configured AND provider has a roles path) - providerRolesPath := getProviderJSONUserRolesPath(c, provider) - if !hasAllowedRole(oauthUser.Result.Roles, providerRolesPath) { + // Fetch custom provider config once — used for role checking and trust check below. + // Returns nil for built-in providers (Google, Facebook, …). + // Returns an error if the DB is unavailable — we fail hard rather than silently + // bypassing access controls. + customConfig, err := getCustomOAuthConfig(c, provider) + if err != nil { + return c.Failure(err) + } + + // Check if user has the required roles for this provider. + // Both AllowedRoles and JSONUserRolesPath must be set on the provider for the check to run. + var providerRolesPath, providerAllowedRoles string + if customConfig != nil { + providerRolesPath = customConfig.JSONUserRolesPath + providerAllowedRoles = customConfig.AllowedRoles + } + if !hasAllowedRole(oauthUser.Result.Roles, providerRolesPath, providerAllowedRoles) { log.Warnf(c, "User @{UserID} attempted OAuth login but does not have required role. User roles: @{UserRoles}, Allowed roles: @{AllowedRoles}", dto.Props{ "UserID": oauthUser.Result.ID, "UserRoles": oauthUser.Result.Roles, - "AllowedRoles": env.Config.OAuth.AllowedRoles, + "AllowedRoles": providerAllowedRoles, }) return c.Redirect("/access-denied") } @@ -107,7 +130,7 @@ func OAuthToken() web.HandlerFunc { var user *entity.User userByProvider := &query.GetUserByProvider{Provider: provider, UID: oauthUser.Result.ID} - err := bus.Dispatch(c, userByProvider) + err = bus.Dispatch(c, userByProvider) user = userByProvider.Result if errors.Cause(err) == app.ErrNotFound && oauthUser.Result.Email != "" { @@ -117,7 +140,7 @@ func OAuthToken() web.HandlerFunc { } if err != nil { if errors.Cause(err) == app.ErrNotFound { - isTrusted := isTrustedOAuthProvider(c, provider) + isTrusted := customConfig != nil && customConfig.IsTrusted if c.Tenant().IsPrivate && !isTrusted { return c.Redirect("/not-invited") } @@ -157,24 +180,21 @@ func OAuthToken() web.HandlerFunc { } } -func isTrustedOAuthProvider(ctx context.Context, provider string) bool { - customOAuthConfigByProvider := &query.GetCustomOAuthConfigByProvider{Provider: provider} - err := bus.Dispatch(ctx, customOAuthConfigByProvider) - if err != nil { - return false +// getCustomOAuthConfig fetches the custom OAuth provider config for the given provider. +// Built-in providers (Google, Facebook, GitHub, …) are identified by the absence of a +// leading "_" and never have a custom config row, so the bus dispatch is skipped for them. +// Returns (nil, nil) for built-in providers. +// Returns (nil, err) if the DB lookup fails — callers must treat this as a hard error so +// that a transient DB outage cannot silently bypass access controls. +func getCustomOAuthConfig(ctx context.Context, provider string) (*entity.OAuthConfig, error) { + if len(provider) == 0 || provider[0] != '_' { + return nil, nil } - return customOAuthConfigByProvider.Result.IsTrusted -} - -// getProviderJSONUserRolesPath returns the JSONUserRolesPath configured for a custom OAuth provider. -// Returns empty string if the provider is a built-in provider or if the config cannot be retrieved. -func getProviderJSONUserRolesPath(ctx context.Context, provider string) string { - customOAuthConfigByProvider := &query.GetCustomOAuthConfigByProvider{Provider: provider} - err := bus.Dispatch(ctx, customOAuthConfigByProvider) - if err != nil { - return "" + q := &query.GetCustomOAuthConfigByProvider{Provider: provider} + if err := bus.Dispatch(ctx, q); err != nil { + return nil, err } - return customOAuthConfigByProvider.Result.JSONUserRolesPath + return q.Result, nil } // OAuthCallback handles the redirect back from the OAuth provider @@ -289,16 +309,14 @@ func SignInByOAuth() web.HandlerFunc { } } -// hasAllowedRole checks if the user has any of the allowed roles configured in OAUTH_ALLOWED_ROLES. -// If OAUTH_ALLOWED_ROLES is not set or empty, all users are allowed (returns true). -// If the provider has no JSONUserRolesPath configured, the role check is skipped (returns true). -// This ensures that providers without a roles path (e.g. built-in Google/GitHub OAuth) are never -// accidentally blocked in a multi-provider setup where OAUTH_ALLOWED_ROLES is configured for a -// different provider that does return roles. -func hasAllowedRole(userRoles []string, jsonUserRolesPath string) bool { - allowedRolesConfig := strings.TrimSpace(env.Config.OAuth.AllowedRoles) +// hasAllowedRole checks if the user has any of the allowed roles configured on the provider. +// If allowedRoles is empty, all users are allowed (returns true). +// If jsonUserRolesPath is empty, the role check is skipped (returns true) — this ensures +// providers without a roles path are never accidentally blocked. +func hasAllowedRole(userRoles []string, jsonUserRolesPath string, allowedRoles string) bool { + allowedRolesConfig := strings.TrimSpace(allowedRoles) - // If no roles restriction is configured, allow all users + // If no roles restriction is configured on this provider, allow all users if allowedRolesConfig == "" { return true } @@ -309,9 +327,9 @@ func hasAllowedRole(userRoles []string, jsonUserRolesPath string) bool { } // Parse allowed roles from config (comma-separated) - allowedRoles := strings.Split(allowedRolesConfig, ",") + allowedRolesList := strings.Split(allowedRolesConfig, ",") allowedRolesMap := make(map[string]bool) - for _, role := range allowedRoles { + for _, role := range allowedRolesList { role = strings.TrimSpace(role) if role != "" { allowedRolesMap[role] = true diff --git a/app/handlers/oauth_roles_test.go b/app/handlers/oauth_roles_test.go index c522dcbd0..55b554d70 100644 --- a/app/handlers/oauth_roles_test.go +++ b/app/handlers/oauth_roles_test.go @@ -5,56 +5,40 @@ package handlers import ( "testing" - - "github.com/getfider/fider/app/pkg/env" ) -// setAllowedRoles sets env.Config.OAuth.AllowedRoles for the duration of a test -// and automatically restores the original value when the test finishes. -func setAllowedRoles(t *testing.T, value string) { - t.Helper() - original := env.Config.OAuth.AllowedRoles - env.Config.OAuth.AllowedRoles = value - t.Cleanup(func() { env.Config.OAuth.AllowedRoles = original }) -} - -// --- empty / unconfigured OAUTH_ALLOWED_ROLES --- +// --- empty / unconfigured allowed roles --- func TestHasAllowedRole_EmptyConfig_AllowsAll(t *testing.T) { - setAllowedRoles(t, "") - if !hasAllowedRole([]string{"ROLE_ADMIN"}, "roles") { - t.Error("expected true when OAUTH_ALLOWED_ROLES is empty") + if !hasAllowedRole([]string{"ROLE_ADMIN"}, "roles", "") { + t.Error("expected true when allowedRoles is empty") } } func TestHasAllowedRole_EmptyConfig_AllowsEmptyRoles(t *testing.T) { - setAllowedRoles(t, "") - if !hasAllowedRole([]string{}, "roles") { - t.Error("expected true when OAUTH_ALLOWED_ROLES is empty and user has no roles") + if !hasAllowedRole([]string{}, "roles", "") { + t.Error("expected true when allowedRoles is empty and user has no roles") } } func TestHasAllowedRole_WhitespaceOnlyConfig_AllowsAll(t *testing.T) { - setAllowedRoles(t, " ") - if !hasAllowedRole([]string{"ROLE_ADMIN"}, "roles") { - t.Error("expected true when OAUTH_ALLOWED_ROLES is whitespace only") + if !hasAllowedRole([]string{"ROLE_ADMIN"}, "roles", " ") { + t.Error("expected true when allowedRoles is whitespace only") } } // --- provider has no JSONUserRolesPath (skip check) --- func TestHasAllowedRole_NoRolesPath_SkipsCheck(t *testing.T) { - setAllowedRoles(t, "ROLE_ADMIN") // Provider without a roles path must always be allowed through, // regardless of whether the user carries matching roles or not. - if !hasAllowedRole([]string{}, "") { + if !hasAllowedRole([]string{}, "", "ROLE_ADMIN") { t.Error("expected true when provider has no JSONUserRolesPath (check should be skipped)") } } func TestHasAllowedRole_WhitespaceRolesPath_SkipsCheck(t *testing.T) { - setAllowedRoles(t, "ROLE_ADMIN") - if !hasAllowedRole([]string{}, " ") { + if !hasAllowedRole([]string{}, " ", "ROLE_ADMIN") { t.Error("expected true when JSONUserRolesPath is whitespace only (check should be skipped)") } } @@ -62,29 +46,25 @@ func TestHasAllowedRole_WhitespaceRolesPath_SkipsCheck(t *testing.T) { // --- matching role --- func TestHasAllowedRole_MatchingSingleRole(t *testing.T) { - setAllowedRoles(t, "ROLE_ADMIN") - if !hasAllowedRole([]string{"ROLE_ADMIN"}, "roles") { + if !hasAllowedRole([]string{"ROLE_ADMIN"}, "roles", "ROLE_ADMIN") { t.Error("expected true when user has the required role") } } func TestHasAllowedRole_MatchingOneOfMultipleAllowed(t *testing.T) { - setAllowedRoles(t, "ROLE_ADMIN,ROLE_TEACHER") - if !hasAllowedRole([]string{"ROLE_TEACHER"}, "roles") { + if !hasAllowedRole([]string{"ROLE_TEACHER"}, "roles", "ROLE_ADMIN,ROLE_TEACHER") { t.Error("expected true when user has one of the allowed roles") } } func TestHasAllowedRole_MatchingMultipleAllowed(t *testing.T) { - setAllowedRoles(t, "ROLE_ADMIN,ROLE_TEACHER") - if !hasAllowedRole([]string{"ROLE_ADMIN", "ROLE_TEACHER"}, "roles") { + if !hasAllowedRole([]string{"ROLE_ADMIN", "ROLE_TEACHER"}, "roles", "ROLE_ADMIN,ROLE_TEACHER") { t.Error("expected true when user has all of the allowed roles") } } func TestHasAllowedRole_AllowedRolesWithWhitespace(t *testing.T) { - setAllowedRoles(t, " ROLE_ADMIN , ROLE_TEACHER ") - if !hasAllowedRole([]string{"ROLE_TEACHER"}, "roles") { + if !hasAllowedRole([]string{"ROLE_TEACHER"}, "roles", " ROLE_ADMIN , ROLE_TEACHER ") { t.Error("expected true when allowed roles config has surrounding whitespace") } } @@ -92,56 +72,48 @@ func TestHasAllowedRole_AllowedRolesWithWhitespace(t *testing.T) { // --- no matching role --- func TestHasAllowedRole_NoMatchingRole(t *testing.T) { - setAllowedRoles(t, "ROLE_ADMIN") - if hasAllowedRole([]string{"ROLE_STUDENT"}, "roles") { + if hasAllowedRole([]string{"ROLE_STUDENT"}, "roles", "ROLE_ADMIN") { t.Error("expected false when user does not have any allowed role") } } func TestHasAllowedRole_EmptyUserRoles_WithConfig(t *testing.T) { - setAllowedRoles(t, "ROLE_ADMIN") - if hasAllowedRole([]string{}, "roles") { - t.Error("expected false when user has no roles and OAUTH_ALLOWED_ROLES is set") + if hasAllowedRole([]string{}, "roles", "ROLE_ADMIN") { + t.Error("expected false when user has no roles and allowedRoles is set") } } func TestHasAllowedRole_NilUserRoles_WithConfig(t *testing.T) { - setAllowedRoles(t, "ROLE_ADMIN") - if hasAllowedRole(nil, "roles") { - t.Error("expected false when user roles slice is nil and OAUTH_ALLOWED_ROLES is set") + if hasAllowedRole(nil, "roles", "ROLE_ADMIN") { + t.Error("expected false when user roles slice is nil and allowedRoles is set") } } func TestHasAllowedRole_CaseSensitiveNoMatch(t *testing.T) { - setAllowedRoles(t, "ROLE_ADMIN") // Role names are matched case-sensitively. - if hasAllowedRole([]string{"role_admin"}, "roles") { + if hasAllowedRole([]string{"role_admin"}, "roles", "ROLE_ADMIN") { t.Error("expected false: role matching must be case-sensitive") } } func TestHasAllowedRole_MultipleDefinedNoMatch(t *testing.T) { - setAllowedRoles(t, "ROLE_ADMIN,ROLE_TEACHER") - if hasAllowedRole([]string{"ROLE_GUEST"}, "roles") { - t.Error("expected false when user has multiple roles but none match the allowed roles") - } + if hasAllowedRole([]string{"ROLE_GUEST"}, "roles", "ROLE_ADMIN,ROLE_TEACHER") { + t.Error("expected false when user role does not match any allowed role") + } } func TestHasAllowedRole_MultipleNoMatch(t *testing.T) { - setAllowedRoles(t, "ROLE_ADMIN,ROLE_TEACHER") - if hasAllowedRole([]string{"ROLE_STUDENT", "ROLE_GUEST"}, "roles") { - t.Error("expected false when user has multiple roles but none match the allowed roles") - } + if hasAllowedRole([]string{"ROLE_STUDENT", "ROLE_GUEST"}, "roles", "ROLE_ADMIN,ROLE_TEACHER") { + t.Error("expected false when user has multiple roles but none match the allowed roles") + } } // --- edge cases --- func TestHasAllowedRole_ConfigWithOnlyCommas_AllowsAll(t *testing.T) { // A config of only separators produces no valid role entries → allow all. - setAllowedRoles(t, ",,, ,") - if !hasAllowedRole([]string{}, "roles") { + if !hasAllowedRole([]string{}, "roles", ",,, ,") { t.Error("expected true when config contains only separators (no valid roles)") } } - diff --git a/app/models/cmd/oauth.go b/app/models/cmd/oauth.go index db1947ab0..8ef37a33e 100644 --- a/app/models/cmd/oauth.go +++ b/app/models/cmd/oauth.go @@ -21,6 +21,7 @@ type SaveCustomOAuthConfig struct { JSONUserNamePath string JSONUserEmailPath string JSONUserRolesPath string + AllowedRoles string } type ParseOAuthRawProfile struct { diff --git a/app/models/entity/oauth.go b/app/models/entity/oauth.go index 08a451712..12d0304c1 100644 --- a/app/models/entity/oauth.go +++ b/app/models/entity/oauth.go @@ -28,6 +28,7 @@ type OAuthConfig struct { JSONUserNamePath string JSONUserEmailPath string JSONUserRolesPath string + AllowedRoles string } // MarshalJSON returns the JSON encoding of OAuthConfig @@ -53,5 +54,6 @@ func (o OAuthConfig) MarshalJSON() ([]byte, error) { "jsonUserNamePath": o.JSONUserNamePath, "jsonUserEmailPath": o.JSONUserEmailPath, "jsonUserRolesPath": o.JSONUserRolesPath, + "allowedRoles": o.AllowedRoles, }) } diff --git a/app/pkg/env/env.go b/app/pkg/env/env.go index 2edd69977..fc1fee739 100644 --- a/app/pkg/env/env.go +++ b/app/pkg/env/env.go @@ -99,7 +99,6 @@ type config struct { ClientID string `env:"OAUTH_GITHUB_CLIENTID"` Secret string `env:"OAUTH_GITHUB_SECRET"` } - AllowedRoles string `env:"OAUTH_ALLOWED_ROLES"` } Email struct { Type string `env:"EMAIL"` // possible values: smtp, mailgun, awsses diff --git a/app/services/sqlstore/dbEntities/oauth.go b/app/services/sqlstore/dbEntities/oauth.go index 1d41823d4..50d0480f1 100644 --- a/app/services/sqlstore/dbEntities/oauth.go +++ b/app/services/sqlstore/dbEntities/oauth.go @@ -23,6 +23,7 @@ type OAuthConfig struct { JSONUserNamePath string `db:"json_user_name_path"` JSONUserEmailPath string `db:"json_user_email_path"` JSONUserRolesPath sql.NullString `db:"json_user_roles_path"` + AllowedRoles sql.NullString `db:"allowed_roles"` } func (m *OAuthConfig) ToModel() *entity.OAuthConfig { @@ -30,6 +31,10 @@ func (m *OAuthConfig) ToModel() *entity.OAuthConfig { if m.JSONUserRolesPath.Valid { rolesPath = m.JSONUserRolesPath.String } + allowedRoles := "" + if m.AllowedRoles.Valid { + allowedRoles = m.AllowedRoles.String + } return &entity.OAuthConfig{ ID: m.ID, @@ -48,5 +53,6 @@ func (m *OAuthConfig) ToModel() *entity.OAuthConfig { JSONUserNamePath: m.JSONUserNamePath, JSONUserEmailPath: m.JSONUserEmailPath, JSONUserRolesPath: rolesPath, + AllowedRoles: allowedRoles, } } diff --git a/app/services/sqlstore/postgres/oauth.go b/app/services/sqlstore/postgres/oauth.go index 916eec43e..409a5e93d 100644 --- a/app/services/sqlstore/postgres/oauth.go +++ b/app/services/sqlstore/postgres/oauth.go @@ -24,7 +24,8 @@ func getCustomOAuthConfigByProvider(ctx context.Context, q *query.GetCustomOAuth SELECT id, provider, display_name, status, is_trusted, logo_bkey, client_id, client_secret, authorize_url, profile_url, token_url, scope, json_user_id_path, - json_user_name_path, json_user_email_path, json_user_roles_path + json_user_name_path, json_user_email_path, json_user_roles_path, + allowed_roles FROM oauth_providers WHERE tenant_id = $1 AND provider = $2 `, tenant.ID, q.Provider) @@ -49,7 +50,8 @@ func listCustomOAuthConfig(ctx context.Context, q *query.ListCustomOAuthConfig) SELECT id, provider, display_name, status, is_trusted, logo_bkey, client_id, client_secret, authorize_url, profile_url, token_url, scope, json_user_id_path, - json_user_name_path, json_user_email_path, json_user_roles_path + json_user_name_path, json_user_email_path, json_user_roles_path, + allowed_roles FROM oauth_providers WHERE tenant_id = $1 ORDER BY id`, tenant.ID) @@ -79,29 +81,29 @@ func saveCustomOAuthConfig(ctx context.Context, c *cmd.SaveCustomOAuthConfig) er tenant_id, provider, display_name, status, is_trusted, client_id, client_secret, authorize_url, profile_url, token_url, scope, json_user_id_path, - json_user_name_path, json_user_email_path, json_user_roles_path, logo_bkey - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16) + json_user_name_path, json_user_email_path, json_user_roles_path, allowed_roles, logo_bkey + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17) RETURNING id` err = trx.Get(&c.ID, query, tenant.ID, c.Provider, c.DisplayName, c.Status, c.IsTrusted, c.ClientID, c.ClientSecret, c.AuthorizeURL, c.ProfileURL, c.TokenURL, c.Scope, c.JSONUserIDPath, c.JSONUserNamePath, - c.JSONUserEmailPath, c.JSONUserRolesPath, c.Logo.BlobKey) + c.JSONUserEmailPath, c.JSONUserRolesPath, c.AllowedRoles, c.Logo.BlobKey) } else { query := ` UPDATE oauth_providers SET display_name = $3, status = $4, client_id = $5, client_secret = $6, authorize_url = $7, profile_url = $8, token_url = $9, scope = $10, json_user_id_path = $11, json_user_name_path = $12, json_user_email_path = $13, - json_user_roles_path = $14, logo_bkey = $15, is_trusted = $16 + json_user_roles_path = $14, allowed_roles = $15, logo_bkey = $16, is_trusted = $17 WHERE tenant_id = $1 AND id = $2` _, err = trx.Execute(query, tenant.ID, c.ID, c.DisplayName, c.Status, c.ClientID, c.ClientSecret, c.AuthorizeURL, c.ProfileURL, c.TokenURL, c.Scope, c.JSONUserIDPath, c.JSONUserNamePath, - c.JSONUserEmailPath, c.JSONUserRolesPath, c.Logo.BlobKey, c.IsTrusted) + c.JSONUserEmailPath, c.JSONUserRolesPath, c.AllowedRoles, c.Logo.BlobKey, c.IsTrusted) } if err != nil { diff --git a/docs/OAUTH_ROLE_RESTRICTION.md b/docs/OAUTH_ROLE_RESTRICTION.md index ffb78b55e..272590a5f 100644 --- a/docs/OAUTH_ROLE_RESTRICTION.md +++ b/docs/OAUTH_ROLE_RESTRICTION.md @@ -2,52 +2,65 @@ This feature allows you to restrict OAuth login to users who have specific roles provided by your OAuth provider. +Role restrictions are configured **per custom OAuth provider** in the Admin UI. +This means different providers can have completely independent role requirements. + ## Configuration -### Environment Variable +### Admin Panel + +1. Go to **Site Settings** → **Authentication** → **OAuth Providers** +2. Edit or create a custom OAuth provider +3. Configure two fields together: + - **JSON User Roles Path** — the path in the provider's profile JSON where roles are found + - **Allowed Roles** — a comma-separated list of roles that are permitted to sign in -Add the following environment variable to your `.env` file: +Both fields must be set for the role check to run. If either is empty, all users are allowed through for that provider. -```bash -OAUTH_ALLOWED_ROLES=ROLE_ADMIN,ROLE_TEACHER -``` +### Allowed Roles field format -- **Format**: Comma-separated list of role names -- **Case-sensitive**: Role names are matched exactly as they appear in the OAuth response -- **Optional**: If not set or empty, all users are allowed to login (default behavior) -- **Per-provider enforcement**: The role check is only applied to OAuth providers that have a **JSON User Roles Path** configured. Providers without a roles path (e.g. built-in Google or GitHub OAuth) are always allowed through, even when `OAUTH_ALLOWED_ROLES` is set. This prevents accidentally blocking users from providers that don't return roles. +- Comma-separated list of role names, e.g. `ROLE_ADMIN,ROLE_TEACHER` +- Case-sensitive: role names are matched exactly as they appear in the OAuth response +- Leave empty to allow all users (default) ### Examples #### Allow only admins: -```bash -OAUTH_ALLOWED_ROLES=ROLE_ADMIN +``` +ROLE_ADMIN ``` #### Allow admins and teachers: -```bash -OAUTH_ALLOWED_ROLES=ROLE_ADMIN,ROLE_TEACHER ``` - -#### Allow all users (default): -```bash -# Don't set OAUTH_ALLOWED_ROLES or leave it empty -OAUTH_ALLOWED_ROLES= +ROLE_ADMIN,ROLE_TEACHER ``` -## OAuth Provider Configuration +#### Allow all users (default): +Leave the **Allowed Roles** field empty. -For custom OAuth providers, you need to configure the JSON path to extract roles from the OAuth response. +## JSON User Roles Path -### Admin Panel Configuration +The roles path tells Fider where to find roles in the OAuth profile response. -1. Go to **Site Settings** → **Authentication** → **OAuth Providers** -2. Edit or create a custom OAuth provider -3. Configure the **JSON User Roles Path** field +### Examples -### JSON User Roles Path Examples +#### Single role as string: +```json +{ + "id": "12345", + "role": "ROLE_ADMIN" +} +``` +**JSON User Roles Path**: `role` -The roles path supports various OAuth response formats: +#### Comma-separated string: +```json +{ + "id": "12345", + "roles": "ROLE_ADMIN,ROLE_TEACHER" +} +``` +**JSON User Roles Path**: `roles` #### Array of strings: ```json @@ -77,7 +90,6 @@ This extracts the `id` field from each object in the `roles` array. #### Nested array: ```json { - "id": "12345", "user": { "profile": { "roles": ["ROLE_ADMIN"] @@ -100,21 +112,3 @@ This extracts the `id` field from each object in the `roles` array. } ``` **JSON User Roles Path**: `user.groups[].name` - -#### Comma-separated string: -```json -{ - "id": "12345", - "roles": "ROLE_ADMIN,ROLE_TEACHER" -} -``` -**JSON User Roles Path**: `roles` - -#### Single role as string: -```json -{ - "id": "12345", - "role": "ROLE_ADMIN" -} -``` -**JSON User Roles Path**: `role` diff --git a/migrations/202602251400_add_oauth_roles_path.sql b/migrations/202603061000_add_oauth_allowed_roles.sql similarity index 52% rename from migrations/202602251400_add_oauth_roles_path.sql rename to migrations/202603061000_add_oauth_allowed_roles.sql index f21ce4ade..84ca0adc2 100644 --- a/migrations/202602251400_add_oauth_roles_path.sql +++ b/migrations/202603061000_add_oauth_allowed_roles.sql @@ -1,2 +1,2 @@ ALTER TABLE oauth_providers ADD COLUMN json_user_roles_path TEXT; - +ALTER TABLE oauth_providers ADD COLUMN allowed_roles TEXT; diff --git a/public/models/settings.ts b/public/models/settings.ts index 507cb250e..05085200c 100644 --- a/public/models/settings.ts +++ b/public/models/settings.ts @@ -48,6 +48,7 @@ export interface OAuthConfig { jsonUserNamePath: string jsonUserEmailPath: string jsonUserRolesPath: string + allowedRoles: string isTrusted: boolean } diff --git a/public/pages/Administration/components/OAuthForm.tsx b/public/pages/Administration/components/OAuthForm.tsx index 128ff3062..2c05ab930 100644 --- a/public/pages/Administration/components/OAuthForm.tsx +++ b/public/pages/Administration/components/OAuthForm.tsx @@ -28,6 +28,7 @@ export const OAuthForm: React.FC = (props) => { const [jsonUserNamePath, setJSONUserNamePath] = useState((props.config && props.config.jsonUserNamePath) || "") const [jsonUserEmailPath, setJSONUserEmailPath] = useState((props.config && props.config.jsonUserEmailPath) || "") const [jsonUserRolesPath, setJSONUserRolesPath] = useState((props.config && props.config.jsonUserRolesPath) || "") + const [allowedRoles, setAllowedRoles] = useState((props.config && props.config.allowedRoles) || "") const [logo, setLogo] = useState() const [logoURL, setLogoURL] = useState() const [logoBlobKey, setLogoBlobKey] = useState((props.config && props.config.logoBlobKey) || "") @@ -49,6 +50,7 @@ export const OAuthForm: React.FC = (props) => { jsonUserNamePath, jsonUserEmailPath, jsonUserRolesPath, + allowedRoles, logo, }) if (result.ok) { @@ -153,7 +155,7 @@ export const OAuthForm: React.FC = (props) => {

JSON Path

Find out more about{" "} - + configuring the JSON Paths . @@ -202,10 +204,24 @@ export const OAuthForm: React.FC = (props) => { disabled={!fider.session.user.isAdministrator} onChange={setJSONUserRolesPath} > -

Optional. Used for role-based access control.

+

Optional. JSON path to extract roles from the provider profile.

+ +

+ Optional. Comma-separated list of roles allowed to sign in, e.g. ROLE_ADMIN,ROLE_TEACHER. Only enforced when a Roles JSON path is + also configured. Leave empty to allow all roles. +

+ +

diff --git a/public/pages/Administration/pages/ManageAuthentication.page.tsx b/public/pages/Administration/pages/ManageAuthentication.page.tsx index 46ee1a05b..6618a3b87 100644 --- a/public/pages/Administration/pages/ManageAuthentication.page.tsx +++ b/public/pages/Administration/pages/ManageAuthentication.page.tsx @@ -138,7 +138,7 @@ export default class ManageAuthenticationPage extends AdminBasePageOAuth Providers

You can use these section to add any authentication provider thats supports the OAuth2 protocol. Additional information is available in our{" "} - + OAuth Documentation . diff --git a/public/pages/Administration/pages/ManageWebhooks.page.tsx b/public/pages/Administration/pages/ManageWebhooks.page.tsx index ff3042763..2b4141272 100644 --- a/public/pages/Administration/pages/ManageWebhooks.page.tsx +++ b/public/pages/Administration/pages/ManageWebhooks.page.tsx @@ -129,7 +129,7 @@ const ManageWebhooksPage = (props: ManageWebhooksPageProps) => {

Use webhooks to integrate Fider with other applications like Slack, Discord, Zapier and many others.{" "} - + Learn more in our documentation . diff --git a/public/pages/MySettings/components/APIKeyForm.tsx b/public/pages/MySettings/components/APIKeyForm.tsx index 5ab54de44..e37bf1903 100644 --- a/public/pages/MySettings/components/APIKeyForm.tsx +++ b/public/pages/MySettings/components/APIKeyForm.tsx @@ -49,7 +49,7 @@ export class APIKeyForm extends React.Component {

To learn how to use the API, read the{" "} - + official documentation . diff --git a/public/pages/OAuthEcho/OAuthEcho.page.tsx b/public/pages/OAuthEcho/OAuthEcho.page.tsx index b835f438f..490d41597 100644 --- a/public/pages/OAuthEcho/OAuthEcho.page.tsx +++ b/public/pages/OAuthEcho/OAuthEcho.page.tsx @@ -16,6 +16,8 @@ interface OAuthEchoPageProps { email: string roles: string[] } + configuredRolesPath: string + configuredAllowedRoles: string } const ok = @@ -42,6 +44,17 @@ export default class OAuthEchoPage extends React.Component 0 + const { configuredRolesPath, configuredAllowedRoles } = this.props + const roleCheckConfigured = configuredRolesPath && configuredAllowedRoles + let roleCheckPasses = true + if (roleCheckConfigured && this.props.profile) { + const allowed = configuredAllowedRoles + .split(",") + .map((r) => r.trim()) + .filter((r) => r !== "") + roleCheckPasses = allowed.length === 0 || (this.props.profile.roles || []).some((r) => allowed.includes(r.trim())) + } + let responseBody = "" try { responseBody = JSON.stringify(JSON.parse(this.props.body), null, " ") @@ -89,8 +102,26 @@ export default class OAuthEchoPage extends React.ComponentRoles: {hasRoles ? this.props.profile.roles.join(", ") : "(none)"} - Roles are optional and used for role-based access control when OAUTH_ALLOWED_ROLES is configured. + + Roles are optional and used for role-based access control when Allowed Roles is configured on this provider. + + {roleCheckConfigured && ( + + + {roleCheckPasses ? ok : error} + Role check:{" "} + {roleCheckPasses ? ( + Pass + ) : ( + Fail — this user would be redirected to /access-denied + )} + + + Configured roles path: {configuredRolesPath} · Allowed roles: {configuredAllowedRoles} + + + )} ) diff --git a/public/services/actions/tenant.ts b/public/services/actions/tenant.ts index fd2ecd135..2ea49da70 100644 --- a/public/services/actions/tenant.ts +++ b/public/services/actions/tenant.ts @@ -119,6 +119,7 @@ export interface CreateEditOAuthConfigRequest { jsonUserNamePath: string jsonUserEmailPath: string jsonUserRolesPath: string + allowedRoles: string logo?: ImageUpload isTrusted: boolean } From 63b1baa9ddeed6710074fe65160de364ce0f2867 Mon Sep 17 00:00:00 2001 From: Matt Roberts Date: Sun, 8 Mar 2026 22:14:33 +0000 Subject: [PATCH 05/11] Move JSON array extraction into jsonq package, simplify extractRolesFromJSON Add Strings() and ArrayFieldStrings() methods to jsonq so that extractRolesFromJSON no longer needs direct encoding/json usage. Co-Authored-By: Claude Opus 4.6 --- app/pkg/jsonq/jsonq.go | 62 ++++++++++++++++++++++++++ app/pkg/jsonq/jsonq_test.go | 63 ++++++++++++++++++++++++++ app/services/oauth/oauth.go | 88 ++++++++++++------------------------- 3 files changed, 154 insertions(+), 59 deletions(-) diff --git a/app/pkg/jsonq/jsonq.go b/app/pkg/jsonq/jsonq.go index 942f04c82..7dfc36b04 100644 --- a/app/pkg/jsonq/jsonq.go +++ b/app/pkg/jsonq/jsonq.go @@ -96,6 +96,68 @@ func (q *Query) Raw(selector string) []byte { return b } +//Strings returns a slice of strings at the given selector. +//If the value is a JSON array of strings, each element is returned. +//If the value is a single string, a one-element slice is returned. +//Returns nil if the selector doesn't exist or the value is not a string/array. +func (q *Query) Strings(selector string) []string { + raw := q.Raw(selector) + if raw == nil { + return nil + } + + // Try array of strings first + var arr []json.RawMessage + if err := json.Unmarshal(raw, &arr); err == nil { + result := make([]string, 0, len(arr)) + for _, item := range arr { + var s string + if err := json.Unmarshal(item, &s); err == nil && s != "" { + result = append(result, s) + } + } + if len(result) > 0 { + return result + } + return nil + } + + // Try single string + var s string + if err := json.Unmarshal(raw, &s); err == nil && s != "" { + return []string{s} + } + + return nil +} + +//ArrayFieldStrings iterates the JSON array at selector and extracts the named +//field from each object element, returning the values as a string slice. +//Returns nil if the selector doesn't exist or the value is not an array. +func (q *Query) ArrayFieldStrings(selector string, field string) []string { + raw := q.Raw(selector) + if raw == nil { + return nil + } + + var arr []json.RawMessage + if err := json.Unmarshal(raw, &arr); err != nil { + return nil + } + + result := make([]string, 0, len(arr)) + for _, item := range arr { + obj := New(string(item)) + if s := obj.String(field); s != "" { + result = append(result, s) + } + } + if len(result) > 0 { + return result + } + return nil +} + func (q *Query) get(selector string) *json.RawMessage { if selector == "" { return nil diff --git a/app/pkg/jsonq/jsonq_test.go b/app/pkg/jsonq/jsonq_test.go index 42ae1e40f..6b47c4c53 100644 --- a/app/pkg/jsonq/jsonq_test.go +++ b/app/pkg/jsonq/jsonq_test.go @@ -136,3 +136,66 @@ func TestContainsNested(t *testing.T) { Expect(query.Contains("name")).IsFalse() Expect(query.Contains("failures.what")).IsFalse() } + +func TestStrings_ArrayOfStrings(t *testing.T) { + RegisterT(t) + + query := jsonq.New(`{ "roles": ["ROLE_ADMIN", "ROLE_USER"] }`) + Expect(query.Strings("roles")).Equals([]string{"ROLE_ADMIN", "ROLE_USER"}) +} + +func TestStrings_SingleString(t *testing.T) { + RegisterT(t) + + query := jsonq.New(`{ "role": "ROLE_ADMIN" }`) + Expect(query.Strings("role")).Equals([]string{"ROLE_ADMIN"}) +} + +func TestStrings_NestedPath(t *testing.T) { + RegisterT(t) + + query := jsonq.New(`{ "user": { "profile": { "roles": ["ROLE_ADMIN"] } } }`) + Expect(query.Strings("user.profile.roles")).Equals([]string{"ROLE_ADMIN"}) +} + +func TestStrings_MissingSelectorReturnsNil(t *testing.T) { + RegisterT(t) + + query := jsonq.New(`{ "name": "Jon" }`) + Expect(query.Strings("roles")).IsNil() +} + +func TestStrings_EmptyArrayReturnsNil(t *testing.T) { + RegisterT(t) + + query := jsonq.New(`{ "roles": [] }`) + Expect(query.Strings("roles")).IsNil() +} + +func TestArrayFieldStrings_ExtractField(t *testing.T) { + RegisterT(t) + + query := jsonq.New(`{ "roles": [{"id": "ROLE_ADMIN", "name": "Admin"}, {"id": "ROLE_USER", "name": "User"}] }`) + Expect(query.ArrayFieldStrings("roles", "id")).Equals([]string{"ROLE_ADMIN", "ROLE_USER"}) +} + +func TestArrayFieldStrings_NestedPath(t *testing.T) { + RegisterT(t) + + query := jsonq.New(`{ "user": { "groups": [{"name": "ROLE_ADMIN"}, {"name": "ROLE_USER"}] } }`) + Expect(query.ArrayFieldStrings("user.groups", "name")).Equals([]string{"ROLE_ADMIN", "ROLE_USER"}) +} + +func TestArrayFieldStrings_MissingSelectorReturnsNil(t *testing.T) { + RegisterT(t) + + query := jsonq.New(`{ "name": "Jon" }`) + Expect(query.ArrayFieldStrings("roles", "id")).IsNil() +} + +func TestArrayFieldStrings_NotArrayReturnsNil(t *testing.T) { + RegisterT(t) + + query := jsonq.New(`{ "roles": "not an array" }`) + Expect(query.ArrayFieldStrings("roles", "id")).IsNil() +} diff --git a/app/services/oauth/oauth.go b/app/services/oauth/oauth.go index ecd16c345..b9e118f96 100644 --- a/app/services/oauth/oauth.go +++ b/app/services/oauth/oauth.go @@ -3,7 +3,7 @@ package oauth import ( "context" "encoding/base64" - "encoding/json" + "fmt" "net/url" "strings" @@ -201,85 +201,55 @@ func extractCompositeName(query *jsonq.Query, namePath string) string { return "" } +// extractRolesFromJSON extracts role strings from a JSON body at the given path. // Supports formats: // - "roles" for array of strings: ["ROLE_ADMIN", "ROLE_USER"] // - "roles[].id" for array of objects: [{"id": "ROLE_ADMIN"}, {"id": "ROLE_USER"}] // - "user.roles[].name" for nested array of objects +// - "role" for a single string or comma-separated value func extractRolesFromJSON(jsonBody string, rolesPath string) []string { rolesPath = strings.TrimSpace(rolesPath) if rolesPath == "" { return nil } - // Split "roles[].id" into actualPath="roles" and fieldToExtract="id" - var fieldToExtract string - var actualPath string + q := jsonq.New(jsonBody) + + // "roles[].id" — array of objects, extract field from each if strings.Contains(rolesPath, "[].") { parts := strings.SplitN(rolesPath, "[].", 2) - actualPath = parts[0] - fieldToExtract = parts[1] - } else { - actualPath = rolesPath + return trimNonEmpty(q.ArrayFieldStrings(parts[0], parts[1])) } - // Use jsonq to navigate to the value at actualPath - rawBytes := jsonq.New(jsonBody).Raw(actualPath) - if rawBytes == nil { + // "roles" — array of strings or single (possibly comma-separated) string + values := q.Strings(rolesPath) + if len(values) == 0 { return nil } - // Try to unmarshal as an array - var arr []json.RawMessage - if err := json.Unmarshal(rawBytes, &arr); err == nil { - roles := make([]string, 0, len(arr)) - if fieldToExtract != "" { - // Array of objects — extract the named field from each element - for _, item := range arr { - obj := jsonq.New(string(item)) - if s := strings.TrimSpace(obj.String(fieldToExtract)); s != "" { - roles = append(roles, s) - } - } - } else { - // Array of plain strings - for _, item := range arr { - var s string - if err := json.Unmarshal(item, &s); err == nil { - if s = strings.TrimSpace(s); s != "" { - roles = append(roles, s) - } - } - } - } - if len(roles) > 0 { - return roles - } - return nil + // Single string may contain comma-separated roles + if len(values) == 1 && strings.Contains(values[0], ",") { + values = strings.Split(values[0], ",") } - // Try to unmarshal as a plain string (comma-separated or single value) - var str string - if err := json.Unmarshal(rawBytes, &str); err == nil { - str = strings.TrimSpace(str) - if str == "" { - return nil - } - var parts []string - if strings.Contains(str, ",") { - parts = strings.Split(str, ",") - } else { - parts = []string{str} - } - roles := make([]string, 0, len(parts)) - for _, r := range parts { - if r = strings.TrimSpace(r); r != "" { - roles = append(roles, r) - } + return trimNonEmpty(values) +} + +// trimNonEmpty trims whitespace from each string and returns only non-empty values. +func trimNonEmpty(ss []string) []string { + if ss == nil { + return nil + } + result := make([]string, 0, len(ss)) + for _, s := range ss { + if s = strings.TrimSpace(s); s != "" { + result = append(result, s) } - return roles } - - return nil + if len(result) == 0 { + return nil + } + return result } func getOAuthAuthorizationURL(ctx context.Context, q *query.GetOAuthAuthorizationURL) error { From 3d0a833a3e05d02918b54becd23287f1db10a7c5 Mon Sep 17 00:00:00 2001 From: Albert Nimtz Date: Sat, 14 Mar 2026 10:22:27 +0100 Subject: [PATCH 06/11] User with the role Administrator or Collaborator can now log in, regardless of the oauth role Co-Authored-By: Claude --- app/handlers/oauth.go | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/app/handlers/oauth.go b/app/handlers/oauth.go index 4b2c7d3fa..24a97dcf2 100644 --- a/app/handlers/oauth.go +++ b/app/handlers/oauth.go @@ -110,14 +110,32 @@ func OAuthToken() web.HandlerFunc { return c.Failure(err) } + // Look up the existing Fider user first (by provider UID, then by email). + // We need this before the role check so that administrators and collaborators + // can always sign in regardless of OAuth role changes. + var user *entity.User + + userByProvider := &query.GetUserByProvider{Provider: provider, UID: oauthUser.Result.ID} + err = bus.Dispatch(c, userByProvider) + user = userByProvider.Result + + if errors.Cause(err) == app.ErrNotFound && oauthUser.Result.Email != "" { + userByEmail := &query.GetUserByEmail{Email: oauthUser.Result.Email} + err = bus.Dispatch(c, userByEmail) + user = userByEmail.Result + } + // Check if user has the required roles for this provider. // Both AllowedRoles and JSONUserRolesPath must be set on the provider for the check to run. + // Administrators and collaborators already trusted in Fider are always allowed through, + // regardless of their current OAuth roles. var providerRolesPath, providerAllowedRoles string if customConfig != nil { providerRolesPath = customConfig.JSONUserRolesPath providerAllowedRoles = customConfig.AllowedRoles } - if !hasAllowedRole(oauthUser.Result.Roles, providerRolesPath, providerAllowedRoles) { + isFiderPrivileged := user != nil && (user.Role == enum.RoleAdministrator || user.Role == enum.RoleCollaborator) + if !isFiderPrivileged && !hasAllowedRole(oauthUser.Result.Roles, providerRolesPath, providerAllowedRoles) { log.Warnf(c, "User @{UserID} attempted OAuth login but does not have required role. User roles: @{UserRoles}, Allowed roles: @{AllowedRoles}", dto.Props{ "UserID": oauthUser.Result.ID, @@ -126,18 +144,6 @@ func OAuthToken() web.HandlerFunc { }) return c.Redirect("/access-denied") } - - var user *entity.User - - userByProvider := &query.GetUserByProvider{Provider: provider, UID: oauthUser.Result.ID} - err = bus.Dispatch(c, userByProvider) - user = userByProvider.Result - - if errors.Cause(err) == app.ErrNotFound && oauthUser.Result.Email != "" { - userByEmail := &query.GetUserByEmail{Email: oauthUser.Result.Email} - err = bus.Dispatch(c, userByEmail) - user = userByEmail.Result - } if err != nil { if errors.Cause(err) == app.ErrNotFound { isTrusted := customConfig != nil && customConfig.IsTrusted From ea5979b1f3620595f5ba77e3d759e164e279cd2c Mon Sep 17 00:00:00 2001 From: Albert Nimtz Date: Tue, 24 Mar 2026 14:34:27 +0100 Subject: [PATCH 07/11] Added security stamp system to invalidate user sessions when oauth allowed roles change Co-Authored-By: Claude --- app/middlewares/user.go | 21 ++ app/middlewares/user_test.go | 181 ++++++++++++++++++ app/models/cmd/user.go | 2 + app/models/entity/user.go | 1 + app/pkg/jwt/jwt.go | 9 +- app/pkg/web/util/webutil.go | 9 +- app/services/sqlstore/dbEntities/user.go | 2 + app/services/sqlstore/postgres/oauth.go | 24 ++- app/services/sqlstore/postgres/postgres.go | 1 + app/services/sqlstore/postgres/user.go | 52 +++-- .../202603201300_add_security_stamp.sql | 2 + public/services/http.ts | 12 +- 12 files changed, 289 insertions(+), 27 deletions(-) create mode 100644 migrations/202603201300_add_security_stamp.sql diff --git a/app/middlewares/user.go b/app/middlewares/user.go index c29cbea59..320856871 100644 --- a/app/middlewares/user.go +++ b/app/middlewares/user.go @@ -2,6 +2,7 @@ package middlewares import ( "fmt" + "net/url" "strconv" "strings" @@ -55,6 +56,26 @@ func User() web.MiddlewareFunc { } return err } + + // Security stamp check: if the JWT contains a stamp (new tokens only), + // validate it against the current DB stamp. A mismatch means the user's + // security-relevant data has changed (e.g. role changed, account blocked, + // or OAuth allowed-roles updated) and they must re-authenticate so that + // access controls are re-evaluated. + if claims.SecurityStamp != "" && user != nil && claims.SecurityStamp != user.SecurityStamp { + c.RemoveCookie(web.CookieAuthName) + if c.IsAjax() { + return c.JSON(401, web.Map{}) + } + redirectTarget := c.Request.URL.RequestURI() + if redirectTarget != "" && + redirectTarget != "/" && + !strings.HasPrefix(redirectTarget, "/signin") && + !strings.HasPrefix(redirectTarget, "/signout") { + return c.Redirect("/signin?redirect=" + url.QueryEscape(redirectTarget)) + } + return c.Redirect("/signin") + } } else if c.Request.IsAPI() { authHeader := c.Request.GetHeader("Authorization") parts := strings.Split(authHeader, "Bearer") diff --git a/app/middlewares/user_test.go b/app/middlewares/user_test.go index b570c1ce5..b1e09495a 100644 --- a/app/middlewares/user_test.go +++ b/app/middlewares/user_test.go @@ -426,3 +426,184 @@ func TestUser_Impersonation_ValidUser(t *testing.T) { Expect(status).Equals(http.StatusOK) Expect(response.Body.String()).Equals("Arya Stark") } + +// TestUser_SecurityStamp_Match verifies that a token whose security stamp +// matches the DB value grants access normally. +func TestUser_SecurityStamp_Match(t *testing.T) { + RegisterT(t) + + userWithStamp := &entity.User{ + ID: mock.JonSnow.ID, + Name: mock.JonSnow.Name, + Email: mock.JonSnow.Email, + Tenant: mock.DemoTenant, + Status: enum.UserActive, + Role: enum.RoleAdministrator, + SecurityStamp: "stamp-abc123", + Providers: mock.JonSnow.Providers, + } + + token, _ := jwt.Encode(jwt.FiderClaims{ + UserID: userWithStamp.ID, + UserName: userWithStamp.Name, + SecurityStamp: "stamp-abc123", + }) + + bus.AddHandler(func(ctx context.Context, q *query.GetUserByID) error { + if q.UserID == userWithStamp.ID { + q.Result = userWithStamp + return nil + } + return app.ErrNotFound + }) + + server := mock.NewServer() + server.Use(middlewares.User()) + status, response := server. + OnTenant(mock.DemoTenant). + AddCookie(web.CookieAuthName, token). + Execute(func(c *web.Context) error { + return c.String(http.StatusOK, c.User().Name) + }) + + Expect(status).Equals(http.StatusOK) + Expect(response.Body.String()).Equals("Jon Snow") +} + +// TestUser_SecurityStamp_Mismatch verifies that a browser session is redirected to +// /signin when the stamp in the JWT no longer matches the DB stamp (e.g. after a +// role change or an OAuth provider allowed-roles update). +func TestUser_SecurityStamp_Mismatch(t *testing.T) { + RegisterT(t) + + userWithStamp := &entity.User{ + ID: mock.JonSnow.ID, + Name: mock.JonSnow.Name, + Email: mock.JonSnow.Email, + Tenant: mock.DemoTenant, + Status: enum.UserActive, + Role: enum.RoleAdministrator, + SecurityStamp: "stamp-new", // DB has been updated + Providers: mock.JonSnow.Providers, + } + + // Token carries the OLD stamp + token, _ := jwt.Encode(jwt.FiderClaims{ + UserID: userWithStamp.ID, + UserName: userWithStamp.Name, + SecurityStamp: "stamp-old", + }) + + bus.AddHandler(func(ctx context.Context, q *query.GetUserByID) error { + if q.UserID == userWithStamp.ID { + q.Result = userWithStamp + return nil + } + return app.ErrNotFound + }) + + server := mock.NewServer() + server.Use(middlewares.User()) + status, response := server. + OnTenant(mock.DemoTenant). + WithURL("http://demo.test.fider.io/settings"). + AddCookie(web.CookieAuthName, token). + Execute(func(c *web.Context) error { + return c.String(http.StatusOK, c.User().Name) + }) + + // Browser request: should redirect to /signin with the current path as return URL + Expect(status).Equals(http.StatusTemporaryRedirect) + Expect(response.Header().Get("Location")).ContainsSubstring("/signin") + Expect(response.Header().Get("Set-Cookie")).ContainsSubstring(web.CookieAuthName + "=;") +} + +// TestUser_SecurityStamp_Mismatch_AJAX verifies that an AJAX request receives a 401 +// JSON response (not a redirect) when the security stamp is stale. +func TestUser_SecurityStamp_Mismatch_AJAX(t *testing.T) { + RegisterT(t) + + userWithStamp := &entity.User{ + ID: mock.JonSnow.ID, + Name: mock.JonSnow.Name, + Email: mock.JonSnow.Email, + Tenant: mock.DemoTenant, + Status: enum.UserActive, + Role: enum.RoleAdministrator, + SecurityStamp: "stamp-new", + Providers: mock.JonSnow.Providers, + } + + token, _ := jwt.Encode(jwt.FiderClaims{ + UserID: userWithStamp.ID, + UserName: userWithStamp.Name, + SecurityStamp: "stamp-old", + }) + + bus.AddHandler(func(ctx context.Context, q *query.GetUserByID) error { + if q.UserID == userWithStamp.ID { + q.Result = userWithStamp + return nil + } + return app.ErrNotFound + }) + + server := mock.NewServer() + server.Use(middlewares.User()) + status, _ := server. + OnTenant(mock.DemoTenant). + AddHeader("Accept", "application/json"). + AddCookie(web.CookieAuthName, token). + Execute(func(c *web.Context) error { + return c.String(http.StatusOK, c.User().Name) + }) + + // AJAX request: should get a 401 JSON, not a redirect + Expect(status).Equals(http.StatusUnauthorized) +} + +// TestUser_SecurityStamp_EmptyInToken verifies backward compatibility: +// old tokens without a stamp embedded must still be accepted. +func TestUser_SecurityStamp_EmptyInToken(t *testing.T) { + RegisterT(t) + + userWithStamp := &entity.User{ + ID: mock.JonSnow.ID, + Name: mock.JonSnow.Name, + Email: mock.JonSnow.Email, + Tenant: mock.DemoTenant, + Status: enum.UserActive, + Role: enum.RoleAdministrator, + SecurityStamp: "stamp-in-db", + Providers: mock.JonSnow.Providers, + } + + // Old token: no SecurityStamp field + token, _ := jwt.Encode(jwt.FiderClaims{ + UserID: userWithStamp.ID, + UserName: userWithStamp.Name, + // SecurityStamp deliberately omitted (simulates pre-stamp tokens) + }) + + bus.AddHandler(func(ctx context.Context, q *query.GetUserByID) error { + if q.UserID == userWithStamp.ID { + q.Result = userWithStamp + return nil + } + return app.ErrNotFound + }) + + server := mock.NewServer() + server.Use(middlewares.User()) + status, response := server. + OnTenant(mock.DemoTenant). + AddCookie(web.CookieAuthName, token). + Execute(func(c *web.Context) error { + return c.String(http.StatusOK, c.User().Name) + }) + + // Old tokens without a stamp must still work (backward compatible) + Expect(status).Equals(http.StatusOK) + Expect(response.Body.String()).Equals("Jon Snow") +} + diff --git a/app/models/cmd/user.go b/app/models/cmd/user.go index 81e25f1dd..5bae7c0df 100644 --- a/app/models/cmd/user.go +++ b/app/models/cmd/user.go @@ -54,3 +54,5 @@ type UpdateCurrentUser struct { AvatarType enum.AvatarType Avatar *dto.ImageUpload } + +type RotateAllUserSecurityStamps struct {} diff --git a/app/models/entity/user.go b/app/models/entity/user.go index 9f3393865..f9897c7bf 100644 --- a/app/models/entity/user.go +++ b/app/models/entity/user.go @@ -19,6 +19,7 @@ type User struct { AvatarURL string `json:"avatarURL,omitempty"` Status enum.UserStatus `json:"status"` IsTrusted bool `json:"isTrusted"` + SecurityStamp string `json:"-"` } // HasProvider returns true if current user has registered with given provider diff --git a/app/pkg/jwt/jwt.go b/app/pkg/jwt/jwt.go index 24e333170..b934a990d 100644 --- a/app/pkg/jwt/jwt.go +++ b/app/pkg/jwt/jwt.go @@ -27,10 +27,11 @@ const ( // FiderClaims represents what goes into JWT tokens type FiderClaims struct { - UserID int `json:"user/id"` - UserName string `json:"user/name"` - UserEmail string `json:"user/email"` - Origin string `json:"origin"` + UserID int `json:"user/id"` + UserName string `json:"user/name"` + UserEmail string `json:"user/email"` + Origin string `json:"origin"` + SecurityStamp string `json:"user/security_stamp,omitempty"` Metadata } diff --git a/app/pkg/web/util/webutil.go b/app/pkg/web/util/webutil.go index 863d0554c..e1dff7611 100644 --- a/app/pkg/web/util/webutil.go +++ b/app/pkg/web/util/webutil.go @@ -13,10 +13,11 @@ import ( func encode(user *entity.User) string { token, err := jwt.Encode(jwt.FiderClaims{ - UserID: user.ID, - UserName: user.Name, - UserEmail: user.Email, - Origin: jwt.FiderClaimsOriginUI, + UserID: user.ID, + UserName: user.Name, + UserEmail: user.Email, + Origin: jwt.FiderClaimsOriginUI, + SecurityStamp: user.SecurityStamp, Metadata: jwt.Metadata{ ExpiresAt: jwt.Time(time.Now().Add(365 * 24 * time.Hour)), }, diff --git a/app/services/sqlstore/dbEntities/user.go b/app/services/sqlstore/dbEntities/user.go index b255f3ca5..15d056cf6 100644 --- a/app/services/sqlstore/dbEntities/user.go +++ b/app/services/sqlstore/dbEntities/user.go @@ -22,6 +22,7 @@ type User struct { AvatarType sql.NullInt64 `db:"avatar_type"` AvatarBlobKey sql.NullString `db:"avatar_bkey"` IsTrusted sql.NullBool `db:"is_trusted"` + SecurityStamp sql.NullString `db:"security_stamp"` Providers []*UserProvider } @@ -64,6 +65,7 @@ func (u *User) ToModel(ctx context.Context) *entity.User { AvatarBlobKey: u.AvatarBlobKey.String, AvatarURL: avatarURL, IsTrusted: u.IsTrusted.Bool, + SecurityStamp: u.SecurityStamp.String, } if u.Providers != nil { diff --git a/app/services/sqlstore/postgres/oauth.go b/app/services/sqlstore/postgres/oauth.go index 409a5e93d..2f5e52b1c 100644 --- a/app/services/sqlstore/postgres/oauth.go +++ b/app/services/sqlstore/postgres/oauth.go @@ -91,12 +91,20 @@ func saveCustomOAuthConfig(ctx context.Context, c *cmd.SaveCustomOAuthConfig) er c.Scope, c.JSONUserIDPath, c.JSONUserNamePath, c.JSONUserEmailPath, c.JSONUserRolesPath, c.AllowedRoles, c.Logo.BlobKey) } else { + // Detect if allowed_roles is being changed. If so, we must rotate all + // user security stamps so that currently-logged-in users are forced to + // re-authenticate and have their OAuth roles re-evaluated. + var prevAllowedRoles string + _ = trx.Scalar(&prevAllowedRoles, + "SELECT COALESCE(allowed_roles, '') FROM oauth_providers WHERE tenant_id = $1 AND id = $2", + tenant.ID, c.ID) + query := ` UPDATE oauth_providers SET display_name = $3, status = $4, client_id = $5, client_secret = $6, - authorize_url = $7, profile_url = $8, token_url = $9, scope = $10, - json_user_id_path = $11, json_user_name_path = $12, json_user_email_path = $13, - json_user_roles_path = $14, allowed_roles = $15, logo_bkey = $16, is_trusted = $17 + authorize_url = $7, profile_url = $8, token_url = $9, scope = $10, + json_user_id_path = $11, json_user_name_path = $12, json_user_email_path = $13, + json_user_roles_path = $14, allowed_roles = $15, logo_bkey = $16, is_trusted = $17 WHERE tenant_id = $1 AND id = $2` _, err = trx.Execute(query, tenant.ID, c.ID, @@ -104,6 +112,16 @@ func saveCustomOAuthConfig(ctx context.Context, c *cmd.SaveCustomOAuthConfig) er c.AuthorizeURL, c.ProfileURL, c.TokenURL, c.Scope, c.JSONUserIDPath, c.JSONUserNamePath, c.JSONUserEmailPath, c.JSONUserRolesPath, c.AllowedRoles, c.Logo.BlobKey, c.IsTrusted) + + if err == nil && prevAllowedRoles != c.AllowedRoles { + // Rotate stamps inside the same transaction so the change is atomic. + if _, stampErr := trx.Execute( + "UPDATE users SET security_stamp = md5(random()::text || id::text) WHERE tenant_id = $1", + tenant.ID, + ); stampErr != nil { + return errors.Wrap(stampErr, "failed to rotate security stamps after allowed_roles change") + } + } } if err != nil { diff --git a/app/services/sqlstore/postgres/postgres.go b/app/services/sqlstore/postgres/postgres.go index ea34d695a..67ae5cf9b 100644 --- a/app/services/sqlstore/postgres/postgres.go +++ b/app/services/sqlstore/postgres/postgres.go @@ -104,6 +104,7 @@ func (s Service) Init() { bus.AddHandler(getAllUsers) bus.AddHandler(getAllUsersNames) bus.AddHandler(searchUsers) + bus.AddHandler(rotateAllUserSecurityStamps) bus.AddHandler(createTenant) bus.AddHandler(getFirstTenant) diff --git a/app/services/sqlstore/postgres/user.go b/app/services/sqlstore/postgres/user.go index d765927a5..3f064048e 100644 --- a/app/services/sqlstore/postgres/user.go +++ b/app/services/sqlstore/postgres/user.go @@ -14,10 +14,17 @@ import ( "github.com/getfider/fider/app/models/query" "github.com/getfider/fider/app/pkg/dbx" "github.com/getfider/fider/app/pkg/errors" + "github.com/getfider/fider/app/pkg/rand" "github.com/getfider/fider/app/services/sqlstore/dbEntities" "github.com/lib/pq" ) +// generateSecurityStamp creates a new random security stamp for a user. +// Rotating the stamp invalidates all existing sessions for that user. +func generateSecurityStamp() string { + return rand.String(64) +} + func countUsers(ctx context.Context, q *query.CountUsers) error { return using(ctx, func(trx *dbx.Trx, tenant *entity.Tenant, user *entity.User) error { var count int @@ -33,8 +40,8 @@ func countUsers(ctx context.Context, q *query.CountUsers) error { func blockUser(ctx context.Context, c *cmd.BlockUser) error { return using(ctx, func(trx *dbx.Trx, tenant *entity.Tenant, user *entity.User) error { if _, err := trx.Execute( - "UPDATE users SET status = $3 WHERE id = $1 AND tenant_id = $2", - c.UserID, tenant.ID, enum.UserBlocked, + "UPDATE users SET status = $3, security_stamp = $4 WHERE id = $1 AND tenant_id = $2", + c.UserID, tenant.ID, enum.UserBlocked, generateSecurityStamp(), ); err != nil { return errors.Wrap(err, "failed to block user") } @@ -45,8 +52,8 @@ func blockUser(ctx context.Context, c *cmd.BlockUser) error { func unblockUser(ctx context.Context, c *cmd.UnblockUser) error { return using(ctx, func(trx *dbx.Trx, tenant *entity.Tenant, user *entity.User) error { if _, err := trx.Execute( - "UPDATE users SET status = $3 WHERE id = $1 AND tenant_id = $2", - c.UserID, tenant.ID, enum.UserActive, + "UPDATE users SET status = $3, security_stamp = $4 WHERE id = $1 AND tenant_id = $2", + c.UserID, tenant.ID, enum.UserActive, generateSecurityStamp(), ); err != nil { return errors.Wrap(err, "failed to unblock user") } @@ -166,8 +173,8 @@ func userSubscribedTo(ctx context.Context, q *query.UserSubscribedTo) error { func changeUserRole(ctx context.Context, c *cmd.ChangeUserRole) error { return using(ctx, func(trx *dbx.Trx, tenant *entity.Tenant, user *entity.User) error { - cmd := "UPDATE users SET role = $3 WHERE id = $1 AND tenant_id = $2" - _, err := trx.Execute(cmd, c.UserID, tenant.ID, c.Role) + cmd := "UPDATE users SET role = $3, security_stamp = $4 WHERE id = $1 AND tenant_id = $2" + _, err := trx.Execute(cmd, c.UserID, tenant.ID, c.Role, generateSecurityStamp()) if err != nil { return errors.Wrap(err, "failed to change user's role") } @@ -237,11 +244,13 @@ func registerUser(ctx context.Context, c *cmd.RegisterUser) error { now := time.Now() c.User.Status = enum.UserActive c.User.Email = strings.ToLower(strings.TrimSpace(c.User.Email)) + stamp := generateSecurityStamp() if err := trx.Get(&c.User.ID, - "INSERT INTO users (name, email, created_at, tenant_id, role, status, avatar_type, avatar_bkey) VALUES ($1, $2, $3, $4, $5, $6, $7, '') RETURNING id", - c.User.Name, c.User.Email, now, tenant.ID, c.User.Role, enum.UserActive, enum.AvatarTypeGravatar); err != nil { + "INSERT INTO users (name, email, created_at, tenant_id, role, status, avatar_type, avatar_bkey, security_stamp) VALUES ($1, $2, $3, $4, $5, $6, $7, '', $8) RETURNING id", + c.User.Name, c.User.Email, now, tenant.ID, c.User.Role, enum.UserActive, enum.AvatarTypeGravatar, stamp); err != nil { return errors.Wrap(err, "failed to register new user") } + c.User.SecurityStamp = stamp for _, provider := range c.User.Providers { cmd := "INSERT INTO user_providers (tenant_id, user_id, provider, provider_uid, created_at) VALUES ($1, $2, $3, $4, $5)" @@ -270,8 +279,8 @@ func updateCurrentUser(ctx context.Context, c *cmd.UpdateCurrentUser) error { if c.Avatar.Remove { c.Avatar.BlobKey = "" } - cmd := "UPDATE users SET name = $3, avatar_type = $4, avatar_bkey = $5 WHERE id = $1 AND tenant_id = $2" - _, err := trx.Execute(cmd, user.ID, tenant.ID, c.Name, c.AvatarType, c.Avatar.BlobKey) + cmd := "UPDATE users SET name = $3, avatar_type = $4, avatar_bkey = $5, security_stamp = $6 WHERE id = $1 AND tenant_id = $2" + _, err := trx.Execute(cmd, user.ID, tenant.ID, c.Name, c.AvatarType, c.Avatar.BlobKey, generateSecurityStamp()) if err != nil { return errors.Wrap(err, "failed to update user") } @@ -328,7 +337,7 @@ func getAllUsers(ctx context.Context, q *query.GetAllUsers) error { return using(ctx, func(trx *dbx.Trx, tenant *entity.Tenant, user *entity.User) error { var users []*dbEntities.User err := trx.Select(&users, ` - SELECT id, name, email, tenant_id, role, status, avatar_type, avatar_bkey + SELECT id, name, email, tenant_id, role, status, avatar_type, avatar_bkey, is_trusted, security_stamp FROM users WHERE tenant_id = $1 AND status != $2 @@ -370,7 +379,7 @@ func getAllUsersNames(ctx context.Context, q *query.GetAllUsersNames) error { func queryUser(ctx context.Context, trx *dbx.Trx, filter string, args ...any) (*entity.User, error) { user := dbEntities.User{} - sql := fmt.Sprintf("SELECT id, name, email, tenant_id, role, status, avatar_type, avatar_bkey, is_trusted FROM users WHERE status != %d AND ", enum.UserDeleted) + sql := fmt.Sprintf("SELECT id, name, email, tenant_id, role, status, avatar_type, avatar_bkey, is_trusted, security_stamp FROM users WHERE status != %d AND ", enum.UserDeleted) err := trx.Get(&user, sql+filter, args...) if err != nil { return nil, err @@ -397,9 +406,9 @@ func searchUsers(ctx context.Context, q *query.SearchUsers) error { } baseQuery := ` - SELECT id, name, email, tenant_id, role, status, avatar_type, avatar_bkey, is_trusted - FROM users - WHERE tenant_id = $1 AND status != $2 + SELECT id, name, email, tenant_id, role, status, avatar_type, avatar_bkey, is_trusted, security_stamp + FROM users + WHERE tenant_id = $1 AND status != $2 ` args := []interface{}{tenant.ID, enum.UserDeleted} argIndex := 3 @@ -486,3 +495,16 @@ func searchUsers(ctx context.Context, q *query.SearchUsers) error { return nil }) } + +func rotateAllUserSecurityStamps(ctx context.Context, c *cmd.RotateAllUserSecurityStamps) error { + return using(ctx, func(trx *dbx.Trx, tenant *entity.Tenant, user *entity.User) error { + _, err := trx.Execute( + "UPDATE users SET security_stamp = md5(random()::text || id::text) WHERE tenant_id = $1", + tenant.ID, + ) + if err != nil { + return errors.Wrap(err, "failed to rotate all user security stamps") + } + return nil + }) +} diff --git a/migrations/202603201300_add_security_stamp.sql b/migrations/202603201300_add_security_stamp.sql new file mode 100644 index 000000000..11e0729aa --- /dev/null +++ b/migrations/202603201300_add_security_stamp.sql @@ -0,0 +1,2 @@ +ALTER TABLE users ADD COLUMN security_stamp VARCHAR(64) NOT NULL DEFAULT ''; +UPDATE users SET security_stamp = md5(random()::text || id::text || tenant_id::text); diff --git a/public/services/http.ts b/public/services/http.ts index f17d2e011..36d678e38 100644 --- a/public/services/http.ts +++ b/public/services/http.ts @@ -1,4 +1,4 @@ -import { analytics, notify, truncate } from "@fider/services" +import { analytics, notify, truncate, Fider } from "@fider/services" export interface ErrorItem { field?: string @@ -28,6 +28,16 @@ async function toResult(response: Response): Promise> { if (response.status === 500) { notify.error("An unexpected error occurred while processing your request.") } else if (response.status === 401) { + // If the user was authenticated but received a 401 it means their session was + // invalidated (e.g. security stamp rotated after an OAuth allowed-roles change). + // Redirect to /signin so they re-authenticate and the role check runs again. + if (Fider.session.isAuthenticated) { + const redirect = encodeURIComponent(window.location.pathname + window.location.search) + window.location.href = `/signin?redirect=${redirect}` + // Return a never-resolving promise so no further code runs while navigating. + // eslint-disable-next-line @typescript-eslint/no-empty-function + return new Promise>(() => {}) + } notify.error("You need to be authenticated to perform this operation.") } else if (response.status === 403) { notify.error("You are not authorized to perform this operation.") From bc8493918b74683694ef217b87749065b3e8cdb8 Mon Sep 17 00:00:00 2001 From: Albert Nimtz Date: Thu, 2 Apr 2026 22:21:11 +0200 Subject: [PATCH 08/11] Exclude current user from security stamp rotation when allowed_roles change Co-Authored-By: Claude --- app/services/sqlstore/postgres/oauth.go | 27 ++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/app/services/sqlstore/postgres/oauth.go b/app/services/sqlstore/postgres/oauth.go index 2f5e52b1c..ce02bc453 100644 --- a/app/services/sqlstore/postgres/oauth.go +++ b/app/services/sqlstore/postgres/oauth.go @@ -91,9 +91,11 @@ func saveCustomOAuthConfig(ctx context.Context, c *cmd.SaveCustomOAuthConfig) er c.Scope, c.JSONUserIDPath, c.JSONUserNamePath, c.JSONUserEmailPath, c.JSONUserRolesPath, c.AllowedRoles, c.Logo.BlobKey) } else { - // Detect if allowed_roles is being changed. If so, we must rotate all - // user security stamps so that currently-logged-in users are forced to - // re-authenticate and have their OAuth roles re-evaluated. + // Detect if allowed_roles is being changed. If so, we must rotate + // security stamps for all other users so that currently-logged-in users + // are forced to re-authenticate and have their OAuth roles re-evaluated. + // The user making this change is excluded so their own session is not + // invalidated. var prevAllowedRoles string _ = trx.Scalar(&prevAllowedRoles, "SELECT COALESCE(allowed_roles, '') FROM oauth_providers WHERE tenant_id = $1 AND id = $2", @@ -115,10 +117,21 @@ func saveCustomOAuthConfig(ctx context.Context, c *cmd.SaveCustomOAuthConfig) er if err == nil && prevAllowedRoles != c.AllowedRoles { // Rotate stamps inside the same transaction so the change is atomic. - if _, stampErr := trx.Execute( - "UPDATE users SET security_stamp = md5(random()::text || id::text) WHERE tenant_id = $1", - tenant.ID, - ); stampErr != nil { + // Exclude the current user so they are not forced to re-authenticate + // after making the change — their session should remain valid. + var stampErr error + if user != nil { + _, stampErr = trx.Execute( + "UPDATE users SET security_stamp = md5(random()::text || id::text) WHERE tenant_id = $1 AND id != $2", + tenant.ID, user.ID, + ) + } else { + _, stampErr = trx.Execute( + "UPDATE users SET security_stamp = md5(random()::text || id::text) WHERE tenant_id = $1", + tenant.ID, + ) + } + if stampErr != nil { return errors.Wrap(stampErr, "failed to rotate security stamps after allowed_roles change") } } From ed81a6faf3da5b8a4a24724c8c5b66380a3c7fe4 Mon Sep 17 00:00:00 2001 From: Matt Roberts Date: Fri, 3 Apr 2026 21:19:09 +0100 Subject: [PATCH 09/11] Don't rotate security stamp on profile updates (name/avatar) Profile changes are not security-sensitive events and should not invalidate user sessions. Co-Authored-By: Claude Opus 4.6 --- app/services/sqlstore/postgres/user.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/sqlstore/postgres/user.go b/app/services/sqlstore/postgres/user.go index 3f064048e..8d430f022 100644 --- a/app/services/sqlstore/postgres/user.go +++ b/app/services/sqlstore/postgres/user.go @@ -279,8 +279,8 @@ func updateCurrentUser(ctx context.Context, c *cmd.UpdateCurrentUser) error { if c.Avatar.Remove { c.Avatar.BlobKey = "" } - cmd := "UPDATE users SET name = $3, avatar_type = $4, avatar_bkey = $5, security_stamp = $6 WHERE id = $1 AND tenant_id = $2" - _, err := trx.Execute(cmd, user.ID, tenant.ID, c.Name, c.AvatarType, c.Avatar.BlobKey, generateSecurityStamp()) + cmd := "UPDATE users SET name = $3, avatar_type = $4, avatar_bkey = $5 WHERE id = $1 AND tenant_id = $2" + _, err := trx.Execute(cmd, user.ID, tenant.ID, c.Name, c.AvatarType, c.Avatar.BlobKey) if err != nil { return errors.Wrap(err, "failed to update user") } From ef2d6f8ee8a534561e613bb17df7f66fde435397 Mon Sep 17 00:00:00 2001 From: Albert Nimtz Date: Tue, 7 Apr 2026 09:18:07 +0200 Subject: [PATCH 10/11] Deactivate oauth role check by removing all allowed roles or the roles path no longer logs out users. Changing roles path with existing allowed roles will trigger a security stamp reroll. Co-Authored-By: Claude --- app/services/sqlstore/postgres/oauth.go | 55 ++-- app/services/sqlstore/postgres/oauth_test.go | 253 +++++++++++++++++++ 2 files changed, 285 insertions(+), 23 deletions(-) create mode 100644 app/services/sqlstore/postgres/oauth_test.go diff --git a/app/services/sqlstore/postgres/oauth.go b/app/services/sqlstore/postgres/oauth.go index ce02bc453..3b8ed11d0 100644 --- a/app/services/sqlstore/postgres/oauth.go +++ b/app/services/sqlstore/postgres/oauth.go @@ -91,15 +91,20 @@ func saveCustomOAuthConfig(ctx context.Context, c *cmd.SaveCustomOAuthConfig) er c.Scope, c.JSONUserIDPath, c.JSONUserNamePath, c.JSONUserEmailPath, c.JSONUserRolesPath, c.AllowedRoles, c.Logo.BlobKey) } else { - // Detect if allowed_roles is being changed. If so, we must rotate - // security stamps for all other users so that currently-logged-in users - // are forced to re-authenticate and have their OAuth roles re-evaluated. - // The user making this change is excluded so their own session is not - // invalidated. + // Detect if role-related fields are being changed. If the new configuration + // is active (both allowed_roles and json_user_roles_path are non-empty) and + // either field changed, we must rotate security stamps for all other users so + // that currently-logged-in users are forced to re-authenticate and have their + // OAuth roles re-evaluated. The user making this change is excluded so their + // own session is not invalidated. var prevAllowedRoles string _ = trx.Scalar(&prevAllowedRoles, "SELECT COALESCE(allowed_roles, '') FROM oauth_providers WHERE tenant_id = $1 AND id = $2", tenant.ID, c.ID) + var prevJSONUserRolesPath string + _ = trx.Scalar(&prevJSONUserRolesPath, + "SELECT COALESCE(json_user_roles_path, '') FROM oauth_providers WHERE tenant_id = $1 AND id = $2", + tenant.ID, c.ID) query := ` UPDATE oauth_providers @@ -115,24 +120,28 @@ func saveCustomOAuthConfig(ctx context.Context, c *cmd.SaveCustomOAuthConfig) er c.Scope, c.JSONUserIDPath, c.JSONUserNamePath, c.JSONUserEmailPath, c.JSONUserRolesPath, c.AllowedRoles, c.Logo.BlobKey, c.IsTrusted) - if err == nil && prevAllowedRoles != c.AllowedRoles { - // Rotate stamps inside the same transaction so the change is atomic. - // Exclude the current user so they are not forced to re-authenticate - // after making the change — their session should remain valid. - var stampErr error - if user != nil { - _, stampErr = trx.Execute( - "UPDATE users SET security_stamp = md5(random()::text || id::text) WHERE tenant_id = $1 AND id != $2", - tenant.ID, user.ID, - ) - } else { - _, stampErr = trx.Execute( - "UPDATE users SET security_stamp = md5(random()::text || id::text) WHERE tenant_id = $1", - tenant.ID, - ) - } - if stampErr != nil { - return errors.Wrap(stampErr, "failed to rotate security stamps after allowed_roles change") + if err == nil && (prevAllowedRoles != c.AllowedRoles || prevJSONUserRolesPath != c.JSONUserRolesPath) { + // Only rotate security stamps if the new configuration still enforces + // role-based access control. Both allowed_roles and json_user_roles_path + // must be non-empty for restrictions to be active. If either is empty, + // access control is effectively deactivated and all users can log in, + // so forcing re-authentication is unnecessary. + if c.AllowedRoles != "" && c.JSONUserRolesPath != "" { + var stampErr error + if user != nil { + _, stampErr = trx.Execute( + "UPDATE users SET security_stamp = md5(random()::text || id::text) WHERE tenant_id = $1 AND id != $2", + tenant.ID, user.ID, + ) + } else { + _, stampErr = trx.Execute( + "UPDATE users SET security_stamp = md5(random()::text || id::text) WHERE tenant_id = $1", + tenant.ID, + ) + } + if stampErr != nil { + return errors.Wrap(stampErr, "failed to rotate security stamps after allowed_roles change") + } } } } diff --git a/app/services/sqlstore/postgres/oauth_test.go b/app/services/sqlstore/postgres/oauth_test.go new file mode 100644 index 000000000..1ec678763 --- /dev/null +++ b/app/services/sqlstore/postgres/oauth_test.go @@ -0,0 +1,253 @@ +package postgres_test + +import ( + "testing" + + "github.com/getfider/fider/app/models/cmd" + "github.com/getfider/fider/app/models/dto" + "github.com/getfider/fider/app/pkg/bus" + . "github.com/getfider/fider/app/pkg/assert" +) + +// stampRow is used to read security_stamp directly from the DB. +type stampRow struct { + ID int `db:"id"` + SecurityStamp string `db:"security_stamp"` +} + +// getSecurityStamps returns a map of userID → security_stamp for all users in the tenant. +func getSecurityStamps(tenantID int) map[int]string { + var rows []*stampRow + _ = trx.Select(&rows, "SELECT id, security_stamp FROM users WHERE tenant_id = $1", tenantID) + result := make(map[int]string) + for _, r := range rows { + result[r.ID] = r.SecurityStamp + } + return result +} + +// newOAuthConfig builds a minimal SaveCustomOAuthConfig suitable for INSERT (ID = 0). +// Callers set JSONUserRolesPath and AllowedRoles to control whether role access is active. +func newOAuthConfig(rolesPath, allowedRoles string) *cmd.SaveCustomOAuthConfig { + return &cmd.SaveCustomOAuthConfig{ + Logo: &dto.ImageUpload{}, + Provider: "_TEST_ROLES", + DisplayName: "Test Roles Provider", + ClientID: "client-id", + ClientSecret: "client-secret", + AuthorizeURL: "http://provider/authorize", + TokenURL: "http://provider/token", + ProfileURL: "http://provider/profile", + JSONUserIDPath: "id", + JSONUserNamePath: "name", + JSONUserEmailPath: "email", + JSONUserRolesPath: rolesPath, + AllowedRoles: allowedRoles, + } +} + +// TestOAuthConfig_SecurityStamp_ActiveToActive checks that when allowed_roles is +// changed between two non-empty values (and json_user_roles_path is set), security +// stamps are rotated for all users except the one making the change. +func TestOAuthConfig_SecurityStamp_ActiveToActive(t *testing.T) { + SetupDatabaseTest(t) + defer TeardownDatabaseTest() + + // Insert initial active config (AllowedRoles="admin", JSONUserRolesPath="roles") + insertCmd := newOAuthConfig("roles", "admin") + err := bus.Dispatch(jonSnowCtx, insertCmd) + Expect(err).IsNil() + + stampsBefore := getSecurityStamps(demoTenant.ID) + + // Update: change AllowedRoles to a different non-empty value — still active. + updateCmd := newOAuthConfig("roles", "superadmin") + updateCmd.ID = insertCmd.ID + err = bus.Dispatch(jonSnowCtx, updateCmd) + Expect(err).IsNil() + + stampsAfter := getSecurityStamps(demoTenant.ID) + + // Other users' stamps must have been rotated. + Expect(stampsAfter[aryaStark.ID]).NotEquals(stampsBefore[aryaStark.ID]) + Expect(stampsAfter[sansaStark.ID]).NotEquals(stampsBefore[sansaStark.ID]) + + // The acting user's stamp must NOT have changed. + Expect(stampsAfter[jonSnow.ID]).Equals(stampsBefore[jonSnow.ID]) +} + +// TestOAuthConfig_SecurityStamp_InactiveToActive checks that when a provider +// transitions from inactive (empty AllowedRoles) to active, security stamps are +// rotated for all users except the one making the change. +func TestOAuthConfig_SecurityStamp_InactiveToActive(t *testing.T) { + SetupDatabaseTest(t) + defer TeardownDatabaseTest() + + // Insert initial inactive config: JSONUserRolesPath set but AllowedRoles empty. + insertCmd := newOAuthConfig("roles", "") + err := bus.Dispatch(jonSnowCtx, insertCmd) + Expect(err).IsNil() + + stampsBefore := getSecurityStamps(demoTenant.ID) + + // Update: add AllowedRoles — transitions to active. + updateCmd := newOAuthConfig("roles", "admin") + updateCmd.ID = insertCmd.ID + err = bus.Dispatch(jonSnowCtx, updateCmd) + Expect(err).IsNil() + + stampsAfter := getSecurityStamps(demoTenant.ID) + + // Other users' stamps must have been rotated. + Expect(stampsAfter[aryaStark.ID]).NotEquals(stampsBefore[aryaStark.ID]) + Expect(stampsAfter[sansaStark.ID]).NotEquals(stampsBefore[sansaStark.ID]) + + // The acting user's stamp must NOT have changed. + Expect(stampsAfter[jonSnow.ID]).Equals(stampsBefore[jonSnow.ID]) +} + +// TestOAuthConfig_SecurityStamp_ActiveToInactive_ClearAllowedRoles checks that +// clearing AllowedRoles (deactivating role access) does NOT rotate security stamps, +// because all users are allowed to log in when restrictions are lifted. +func TestOAuthConfig_SecurityStamp_ActiveToInactive_ClearAllowedRoles(t *testing.T) { + SetupDatabaseTest(t) + defer TeardownDatabaseTest() + + // Insert initial active config. + insertCmd := newOAuthConfig("roles", "admin") + err := bus.Dispatch(jonSnowCtx, insertCmd) + Expect(err).IsNil() + + stampsBefore := getSecurityStamps(demoTenant.ID) + + // Update: clear AllowedRoles — transitions to inactive (JSONUserRolesPath still set). + updateCmd := newOAuthConfig("roles", "") + updateCmd.ID = insertCmd.ID + err = bus.Dispatch(jonSnowCtx, updateCmd) + Expect(err).IsNil() + + stampsAfter := getSecurityStamps(demoTenant.ID) + + // No stamps should have changed. + Expect(stampsAfter[jonSnow.ID]).Equals(stampsBefore[jonSnow.ID]) + Expect(stampsAfter[aryaStark.ID]).Equals(stampsBefore[aryaStark.ID]) + Expect(stampsAfter[sansaStark.ID]).Equals(stampsBefore[sansaStark.ID]) +} + +// TestOAuthConfig_SecurityStamp_ActiveToInactive_ClearRolesPath checks that +// when json_user_roles_path is cleared (making role evaluation impossible) while +// AllowedRoles also changes, security stamps are NOT rotated. +func TestOAuthConfig_SecurityStamp_ActiveToInactive_ClearRolesPath(t *testing.T) { + SetupDatabaseTest(t) + defer TeardownDatabaseTest() + + // Insert initial active config. + insertCmd := newOAuthConfig("roles", "admin") + err := bus.Dispatch(jonSnowCtx, insertCmd) + Expect(err).IsNil() + + stampsBefore := getSecurityStamps(demoTenant.ID) + + // Update: clear JSONUserRolesPath and change AllowedRoles — inactive because no path. + updateCmd := newOAuthConfig("", "superadmin") + updateCmd.ID = insertCmd.ID + err = bus.Dispatch(jonSnowCtx, updateCmd) + Expect(err).IsNil() + + stampsAfter := getSecurityStamps(demoTenant.ID) + + // No stamps should have changed. + Expect(stampsAfter[jonSnow.ID]).Equals(stampsBefore[jonSnow.ID]) + Expect(stampsAfter[aryaStark.ID]).Equals(stampsBefore[aryaStark.ID]) + Expect(stampsAfter[sansaStark.ID]).Equals(stampsBefore[sansaStark.ID]) +} + +// TestOAuthConfig_SecurityStamp_ActiveToInactive_ClearBoth checks that clearing +// both AllowedRoles and json_user_roles_path does NOT rotate security stamps. +func TestOAuthConfig_SecurityStamp_ActiveToInactive_ClearBoth(t *testing.T) { + SetupDatabaseTest(t) + defer TeardownDatabaseTest() + + // Insert initial active config. + insertCmd := newOAuthConfig("roles", "admin") + err := bus.Dispatch(jonSnowCtx, insertCmd) + Expect(err).IsNil() + + stampsBefore := getSecurityStamps(demoTenant.ID) + + // Update: clear both fields — fully inactive. + updateCmd := newOAuthConfig("", "") + updateCmd.ID = insertCmd.ID + err = bus.Dispatch(jonSnowCtx, updateCmd) + Expect(err).IsNil() + + stampsAfter := getSecurityStamps(demoTenant.ID) + + // No stamps should have changed. + Expect(stampsAfter[jonSnow.ID]).Equals(stampsBefore[jonSnow.ID]) + Expect(stampsAfter[aryaStark.ID]).Equals(stampsBefore[aryaStark.ID]) + Expect(stampsAfter[sansaStark.ID]).Equals(stampsBefore[sansaStark.ID]) +} + +// TestOAuthConfig_SecurityStamp_InactiveToActive_ViaRolesPath checks that when +// AllowedRoles is already set but json_user_roles_path is empty (so access control +// is inactive), adding json_user_roles_path (without changing AllowedRoles) DOES +// rotate security stamps, because the provider just became active. +func TestOAuthConfig_SecurityStamp_InactiveToActive_ViaRolesPath(t *testing.T) { + SetupDatabaseTest(t) + defer TeardownDatabaseTest() + + // Insert initial inactive config: AllowedRoles set but JSONUserRolesPath empty. + insertCmd := newOAuthConfig("", "admin") + err := bus.Dispatch(jonSnowCtx, insertCmd) + Expect(err).IsNil() + + stampsBefore := getSecurityStamps(demoTenant.ID) + + // Update: add JSONUserRolesPath only — AllowedRoles stays "admin", transitions to active. + updateCmd := newOAuthConfig("roles", "admin") + updateCmd.ID = insertCmd.ID + err = bus.Dispatch(jonSnowCtx, updateCmd) + Expect(err).IsNil() + + stampsAfter := getSecurityStamps(demoTenant.ID) + + // Other users' stamps must have been rotated. + Expect(stampsAfter[aryaStark.ID]).NotEquals(stampsBefore[aryaStark.ID]) + Expect(stampsAfter[sansaStark.ID]).NotEquals(stampsBefore[sansaStark.ID]) + + // The acting user's stamp must NOT have changed. + Expect(stampsAfter[jonSnow.ID]).Equals(stampsBefore[jonSnow.ID]) +} + +// TestOAuthConfig_SecurityStamp_InactiveToInactive checks that updating a provider +// that has role access disabled (no AllowedRoles, no JSONUserRolesPath) without +// enabling it does NOT rotate security stamps. +func TestOAuthConfig_SecurityStamp_InactiveToInactive(t *testing.T) { + SetupDatabaseTest(t) + defer TeardownDatabaseTest() + + // Insert initial inactive config — no role fields set at all. + insertCmd := newOAuthConfig("", "") + err := bus.Dispatch(jonSnowCtx, insertCmd) + Expect(err).IsNil() + + stampsBefore := getSecurityStamps(demoTenant.ID) + + // Update: change an unrelated field — role access remains inactive. + updateCmd := newOAuthConfig("", "") + updateCmd.ID = insertCmd.ID + updateCmd.DisplayName = "Updated Display Name" + err = bus.Dispatch(jonSnowCtx, updateCmd) + Expect(err).IsNil() + + stampsAfter := getSecurityStamps(demoTenant.ID) + + // No stamps should have changed. + Expect(stampsAfter[jonSnow.ID]).Equals(stampsBefore[jonSnow.ID]) + Expect(stampsAfter[aryaStark.ID]).Equals(stampsBefore[aryaStark.ID]) + Expect(stampsAfter[sansaStark.ID]).Equals(stampsBefore[sansaStark.ID]) +} + + + From 0358b38e284378ebca42e3d4c5340f1b3acfdac5 Mon Sep 17 00:00:00 2001 From: Albert Nimtz Date: Tue, 7 Apr 2026 09:33:44 +0200 Subject: [PATCH 11/11] Update tenant tests to validate ID checks against expected values --- app/services/sqlstore/postgres/tenant_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/sqlstore/postgres/tenant_test.go b/app/services/sqlstore/postgres/tenant_test.go index 4011b318c..59872bcb0 100644 --- a/app/services/sqlstore/postgres/tenant_test.go +++ b/app/services/sqlstore/postgres/tenant_test.go @@ -356,7 +356,7 @@ func TestTenantStorage_Save_Get_ListOAuthConfig(t *testing.T) { err = bus.Dispatch(demoTenantCtx, getConfig) Expect(err).IsNil() - Expect(getConfig.Result.ID).Equals(1) + Expect(getConfig.Result.ID).NotEquals(0) Expect(getConfig.Result.LogoBlobKey).Equals("uploads/my-logo-key.png") Expect(getConfig.Result.Provider).Equals("_TEST") Expect(getConfig.Result.DisplayName).Equals("My Provider") @@ -395,7 +395,7 @@ func TestTenantStorage_Save_Get_ListOAuthConfig(t *testing.T) { Expect(err).IsNil() Expect(customConfigs.Result).HasLen(1) - Expect(customConfigs.Result[0].ID).Equals(1) + Expect(customConfigs.Result[0].ID).Equals(getConfig.Result.ID) Expect(customConfigs.Result[0].LogoBlobKey).Equals("") Expect(customConfigs.Result[0].Provider).Equals("_TEST") Expect(customConfigs.Result[0].DisplayName).Equals("New My Provider")