Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
015c03d
Added the optional oauth allowed roles environment variable
JimKnoxx Feb 25, 2026
5868e26
Fixed error page for existing users that are blocked from accessing now
JimKnoxx Feb 27, 2026
bda3044
Merge branch 'main' into oauth-allowed-roles
mattwoberts Mar 5, 2026
9fbf448
Merge branch 'getfider:main' into oauth-allowed-roles
JimKnoxx Mar 6, 2026
c60d23a
Fixed role check being used for all oauth provider
JimKnoxx Mar 6, 2026
1cf53c3
Changed allowed roles form env to db level configuration, to allow pe…
JimKnoxx Mar 6, 2026
63b1baa
Move JSON array extraction into jsonq package, simplify extractRolesF…
mattwoberts Mar 8, 2026
820eb47
Merge branch 'main' into oauth-allowed-roles
mattwoberts Mar 9, 2026
3d0a833
User with the role Administrator or Collaborator can now log in, rega…
JimKnoxx Mar 14, 2026
ea5979b
Added security stamp system to invalidate user sessions when oauth al…
JimKnoxx Mar 24, 2026
1e0de53
Merge branch 'main' into oauth-allowed-roles
mattwoberts Apr 2, 2026
bc84939
Exclude current user from security stamp rotation when allowed_roles …
JimKnoxx Apr 2, 2026
ed81a6f
Don't rotate security stamp on profile updates (name/avatar)
mattwoberts Apr 3, 2026
ef2d6f8
Deactivate oauth role check by removing all allowed roles or the role…
JimKnoxx Apr 7, 2026
0358b38
Update tenant tests to validate ID checks against expected values
JimKnoxx Apr 7, 2026
aa2c1c6
Merge branch 'main' into oauth-allowed-roles
mattwoberts Apr 12, 2026
0993f5c
Merge branch 'main' into oauth-allowed-roles
mattwoberts Apr 12, 2026
0bbc173
Merge branch 'main' into oauth-allowed-roles
mattwoberts Apr 14, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions app/actions/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ type CreateEditOAuthConfig struct {
JSONUserIDPath string `json:"jsonUserIDPath"`
JSONUserNamePath string `json:"jsonUserNamePath"`
JSONUserEmailPath string `json:"jsonUserEmailPath"`
JSONUserRolesPath string `json:"jsonUserRolesPath"`
AllowedRoles string `json:"allowedRoles"`
}

func NewCreateEditOAuthConfig() *CreateEditOAuthConfig {
Expand Down Expand Up @@ -184,5 +186,13 @@ 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.")
}

if len(action.AllowedRoles) > 500 {
result.AddFieldFailure("allowedRoles", "Allowed Roles must have less than 500 characters.")
}

return result
}
1 change: 1 addition & 0 deletions app/cmd/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 2 additions & 0 deletions app/handlers/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ func SaveOAuthConfig() web.HandlerFunc {
JSONUserIDPath: action.JSONUserIDPath,
JSONUserNamePath: action.JSONUserNamePath,
JSONUserEmailPath: action.JSONUserEmailPath,
JSONUserRolesPath: action.JSONUserRolesPath,
AllowedRoles: action.AllowedRoles,
},
); err != nil {
return c.Failure(err)
Expand Down
115 changes: 105 additions & 10 deletions app/handlers/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,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,
},
})
}
Expand Down Expand Up @@ -91,20 +101,52 @@ func OAuthToken() web.HandlerFunc {
return c.Failure(err)
}

// 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)
}

// 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)
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
}
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,
"UserRoles": oauthUser.Result.Roles,
"AllowedRoles": providerAllowedRoles,
})
return c.Redirect("/access-denied")
}
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")
}
Expand Down Expand Up @@ -144,13 +186,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
}
q := &query.GetCustomOAuthConfigByProvider{Provider: provider}
if err := bus.Dispatch(ctx, q); err != nil {
return nil, err
}
return customOAuthConfigByProvider.Result.IsTrusted
return q.Result, nil
}

// OAuthCallback handles the redirect back from the OAuth provider
Expand Down Expand Up @@ -264,3 +314,48 @@ func SignInByOAuth() web.HandlerFunc {
return c.Redirect(authURL.Result)
}
}

// 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 on this provider, allow all users
if allowedRolesConfig == "" {
return true
}

// 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)
allowedRolesList := strings.Split(allowedRolesConfig, ",")
allowedRolesMap := make(map[string]bool)
for _, role := range allowedRolesList {
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
}

119 changes: 119 additions & 0 deletions app/handlers/oauth_roles_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package handlers

// Internal test file for hasAllowedRole — uses package handlers (not handlers_test)
// so the unexported function is accessible without exporting it.

import (
"testing"
)

// --- empty / unconfigured allowed roles ---

func TestHasAllowedRole_EmptyConfig_AllowsAll(t *testing.T) {
if !hasAllowedRole([]string{"ROLE_ADMIN"}, "roles", "") {
t.Error("expected true when allowedRoles is empty")
}
}

func TestHasAllowedRole_EmptyConfig_AllowsEmptyRoles(t *testing.T) {
if !hasAllowedRole([]string{}, "roles", "") {
t.Error("expected true when allowedRoles is empty and user has no roles")
}
}

func TestHasAllowedRole_WhitespaceOnlyConfig_AllowsAll(t *testing.T) {
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) {
// Provider without a roles path must always be allowed through,
// regardless of whether the user carries matching roles or not.
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) {
if !hasAllowedRole([]string{}, " ", "ROLE_ADMIN") {
t.Error("expected true when JSONUserRolesPath is whitespace only (check should be skipped)")
}
}

// --- matching role ---

func TestHasAllowedRole_MatchingSingleRole(t *testing.T) {
if !hasAllowedRole([]string{"ROLE_ADMIN"}, "roles", "ROLE_ADMIN") {
t.Error("expected true when user has the required role")
}
}

func TestHasAllowedRole_MatchingOneOfMultipleAllowed(t *testing.T) {
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) {
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) {
if !hasAllowedRole([]string{"ROLE_TEACHER"}, "roles", " ROLE_ADMIN , ROLE_TEACHER ") {
t.Error("expected true when allowed roles config has surrounding whitespace")
}
}

// --- no matching role ---

func TestHasAllowedRole_NoMatchingRole(t *testing.T) {
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) {
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) {
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) {
// Role names are matched case-sensitively.
if hasAllowedRole([]string{"role_admin"}, "roles", "ROLE_ADMIN") {
t.Error("expected false: role matching must be case-sensitive")
}
}

func TestHasAllowedRole_MultipleDefinedNoMatch(t *testing.T) {
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) {
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.
if !hasAllowedRole([]string{}, "roles", ",,, ,") {
t.Error("expected true when config contains only separators (no valid roles)")
}
}

11 changes: 11 additions & 0 deletions app/handlers/signin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 21 additions & 0 deletions app/middlewares/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package middlewares

import (
"fmt"
"net/url"
"strconv"
"strings"

Expand Down Expand Up @@ -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")
Expand Down
Loading
Loading