diff --git a/CHANGELOG.md b/CHANGELOG.md index 3613d91d7..4c6e7b579 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,16 +4,18 @@ All notable changes to this project will be documented in this file. For commit ## v0.7.8-beta +Note: if using oidc, please update from 0.7.7 to resolve invalid_grant issue. Also - oidc no longer creates users automatically by default -- must be enabled. **New Features**: - - - - **Notes**: - - + - More oidc user creation options https://github.com/gtsteffaniak/filebrowser/issues/685 + - `auth.methods.oidc.createUser` must be true to automatically create user, defaults to false. + - `auth.methods.oidc.adminGroup` allows using oidc provider group name to enable admin user creation. **BugFixes**: - fix save editor info sometimes saves wrong file. https://github.com/gtsteffaniak/filebrowser/issues/701 - make ctrl select work on mac or windows. https://github.com/gtsteffaniak/filebrowser/issues/739 + - oidc login failures introduced in 0.7.6 https://github.com/gtsteffaniak/filebrowser/issues/731 + - oidc respects non-default baseURL ## v0.7.7-beta diff --git a/backend/common/settings/auth.go b/backend/common/settings/auth.go index d03301a32..bab8af3b1 100644 --- a/backend/common/settings/auth.go +++ b/backend/common/settings/auth.go @@ -60,6 +60,8 @@ type OidcConfig struct { UserIdentifier string `json:"userIdentifier"` // the user identifier to use for authentication. Default is "username", can be "email" or "username", or "phone" DisableVerifyTLS bool `json:"disableVerifyTLS"` // disable TLS verification for the OIDC provider. This is insecure and should only be used for testing. LogoutRedirectUrl string `json:"logoutRedirectUrl"` // if provider logout url is provided, filebrowser will also redirect to logout url. Custom logout query params are respected. + CreateUser bool `json:"createUser"` // create user if not exists + AdminGroup string `json:"adminGroup"` // if set, users in this group will be granted admin privileges. Provider *oidc.Provider `json:"-"` // OIDC provider Verifier *oidc.IDTokenVerifier `json:"-"` // OIDC verifier } diff --git a/backend/http/oidc.go b/backend/http/oidc.go index d9b7df6ef..1210e576b 100644 --- a/backend/http/oidc.go +++ b/backend/http/oidc.go @@ -4,6 +4,8 @@ import ( "crypto/tls" "fmt" "net/http" + "net/url" + "slices" "strings" "time" @@ -18,11 +20,42 @@ import ( // userInfo struct to hold user claims from either UserInfo or ID token type userInfo struct { - Name string `json:"name"` - PreferredUsername string `json:"preferred_username"` - Email string `json:"email"` - Sub string `json:"sub"` - Phone string `json:"phone_number"` + Name string `json:"name"` + PreferredUsername string `json:"preferred_username"` + Email string `json:"email"` + Sub string `json:"sub"` + Phone string `json:"phone_number"` + Groups []string `json:"groups"` +} + +// oidcLoginHandler redirects the user to the OIDC provider's authorization endpoint. +// This function remains largely the same, but includes the 'fb_redirect' parameter +// to redirect the user back to the original page after successful login. +func oidcLoginHandler(w http.ResponseWriter, r *http.Request, d *requestContext) (int, error) { + oidcCfg := settings.Config.Auth.Methods.OidcAuth + if !oidcCfg.Enabled { + return http.StatusForbidden, fmt.Errorf("oidc authentication is not enabled") + } + + origin := r.Header.Get("Origin") + if origin == "" { + origin = fmt.Sprintf("%s://%s", getScheme(r), r.Host) + } + oauth2Config := &oauth2.Config{ + ClientID: oidcCfg.ClientID, + ClientSecret: oidcCfg.ClientSecret, + Endpoint: oidcCfg.Provider.Endpoint(), + RedirectURL: fmt.Sprintf("%s%sapi/auth/oidc/callback", origin, config.Server.BaseURL), + Scopes: strings.Split(oidcCfg.Scopes, " "), + } + + nonce := utils.InsecureRandomIdentifier(16) + fbRedirect := r.URL.Query().Get("redirect") + state := fmt.Sprintf("%s:%s", nonce, fbRedirect) + + authURL := oauth2Config.AuthCodeURL(state) + http.Redirect(w, r, authURL, http.StatusFound) + return 0, nil } // oidcCallbackHandler handles the OIDC callback after the user authenticates with the provider. @@ -49,14 +82,14 @@ func oidcCallbackHandler(w http.ResponseWriter, r *http.Request, d *requestConte } ctx = oidc.ClientContext(ctx, customClient) } - code := r.Header.Get("X-Secret") + code := r.URL.Query().Get("code") // state := r.URL.Query().Get("state") // You might want to validate the state parameter for CSRF protection // The redirect URI MUST match the one registered with the OIDC provider // and used in the initial /api/auth/oidc/login handler. // Using r.Host here might be tricky if running behind a proxy. // Consider using a fixed redirect URL from settings if possible. - redirectURL := fmt.Sprintf("%s://%s/api/auth/oidc/callback", getScheme(r), r.Host) + redirectURL := fmt.Sprintf("%s://%s%sapi/auth/oidc/callback", getScheme(r), r.Host, config.Server.BaseURL) oauth2Config := &oauth2.Config{ ClientID: oidcCfg.ClientID, @@ -88,7 +121,8 @@ func oidcCallbackHandler(w http.ResponseWriter, r *http.Request, d *requestConte // This uses the verifier initialized with the provider's JWKS endpoint and client ID idToken, err := oidcCfg.Verifier.Verify(ctx, rawIDToken) if err != nil { - logger.Warningf("failed to verify ID token: %v. Falling back to UserInfo endpoint.", err) + // this might not be necessary for certain providers like authentik + logger.Debugf("failed to verify ID token: %v. This might be expected, falling back to UserInfo endpoint.", err) // Verification failed, claimsFromIDToken remains false } else { var claims map[string]interface{} @@ -173,64 +207,43 @@ func oidcCallbackHandler(w http.ResponseWriter, r *http.Request, d *requestConte logger.Error("No valid username found in ID token or UserInfo response.") return http.StatusInternalServerError, fmt.Errorf("no valid username found in ID token or UserInfo response from claims") } + isAdmin := false // Default to non-admin user + if config.Auth.Methods.OidcAuth.AdminGroup != "" { + if slices.Contains(userdata.Groups, config.Auth.Methods.OidcAuth.AdminGroup) { + isAdmin = true // User is in the admin group, grant admin privileges + logger.Debugf("User %s is in admin group %s, granting admin privileges.", loginUsername, config.Auth.Methods.OidcAuth.AdminGroup) + } + } // Proceed to log the user in with the OIDC data // userdata struct now contains info from either verified ID token or UserInfo endpoint - return loginWithOidcUser(w, r, loginUsername) -} - -// oidcLoginHandler redirects the user to the OIDC provider's authorization endpoint. -// This function remains largely the same, but includes the 'fb_redirect' parameter -// to redirect the user back to the original page after successful login. -func oidcLoginHandler(w http.ResponseWriter, r *http.Request, d *requestContext) (int, error) { - oidcCfg := settings.Config.Auth.Methods.OidcAuth - if !oidcCfg.Enabled { - return http.StatusForbidden, fmt.Errorf("oidc authentication is not enabled") - } - - origin := r.Header.Get("Origin") - if origin == "" { - origin = fmt.Sprintf("%s://%s", getScheme(r), r.Host) - } - - oauth2Config := &oauth2.Config{ - ClientID: oidcCfg.ClientID, - ClientSecret: oidcCfg.ClientSecret, - Endpoint: oidcCfg.Provider.Endpoint(), - RedirectURL: fmt.Sprintf("%s/api/auth/oidc/callback", origin), - Scopes: strings.Split(oidcCfg.Scopes, " "), - } - - nonce := utils.InsecureRandomIdentifier(16) - fbRedirect := r.URL.Query().Get("fb_redirect") - state := fmt.Sprintf("%s:%s", nonce, fbRedirect) - - authURL := oauth2Config.AuthCodeURL(state) - http.Redirect(w, r, authURL, http.StatusFound) - return 0, nil + return loginWithOidcUser(w, r, loginUsername, isAdmin) } // loginWithOidcUser extracts the username from the user claims (userInfo) // based on the configured UserIdentifier and logs the user into the application. // It creates a new user if one doesn't exist. -func loginWithOidcUser(w http.ResponseWriter, r *http.Request, username string) (int, error) { - logger.Debugf("Successfully authenticated OIDC username: %s", username) +func loginWithOidcUser(w http.ResponseWriter, r *http.Request, username string, isAdmin bool) (int, error) { + logger.Debugf("Successfully authenticated OIDC username: %s isAdmin: %v", username, isAdmin) // Retrieve the user from the store and store it in the context user, err := store.Users.Get(username) if err != nil { if err.Error() != "the resource does not exist" { return http.StatusInternalServerError, err } - - err = storage.CreateUser(users.User{ - LoginMethod: users.LoginMethodOidc, - Username: username, - }, false) - if err != nil { - return http.StatusInternalServerError, err - } - user, err = store.Users.Get(username) - if err != nil { - return http.StatusInternalServerError, err + if config.Auth.Methods.OidcAuth.CreateUser { + err = storage.CreateUser(users.User{ + LoginMethod: users.LoginMethodOidc, + Username: username, + }, isAdmin) + if err != nil { + return http.StatusInternalServerError, err + } + user, err = store.Users.Get(username) + if err != nil { + return http.StatusInternalServerError, err + } + } else { + return http.StatusForbidden, fmt.Errorf("user %s does not exist and createUser is disabled. Your admin needs to create your user before you can access this application", username) } } if user.LoginMethod != users.LoginMethodOidc { @@ -271,20 +284,31 @@ func loginWithOidcUser(w http.ResponseWriter, r *http.Request, username string) // or to the root ("/") if no specific redirect was requested. // The 'fb_redirect' parameter is extracted from the 'state' parameter for security. state := r.URL.Query().Get("state") - fbRedirect := "/" // Default redirect path + + fbRedirect := config.Server.BaseURL // Default redirect to the base URL if state != "" { - // Assuming state is in the format "nonce:fb_redirect" parts := strings.SplitN(state, ":", 2) - if len(parts) == 2 { - // TODO: Validate the nonce part against the stored nonce for CSRF protection - // For this example, we'll just extract the redirect part - extractedRedirect := parts[1] - if extractedRedirect != "" { - fbRedirect = extractedRedirect + + // 2. Validate the nonce + // receivedNonce := parts[0] + // if receivedNonce != nonceCookie.Value { + // // Handle error: nonce mismatch (possible CSRF attack) + // return http.StatusBadRequest, fmt.Errorf("invalid state nonce") + // } + + if len(parts) == 2 && parts[1] != "" { + // 3. Prevent Open Redirect vulnerability + // Ensure the redirect is to a local path. + potentialRedirect, err := url.QueryUnescape(parts[1]) + if err == nil && strings.HasPrefix(potentialRedirect, "/") { + fbRedirect = potentialRedirect + } else { + logger.Warningf("Blocked potentially malicious redirect to: %s", parts[1]) } } } + // Clean up http.Redirect(w, r, fbRedirect, http.StatusFound) // Return 0 to indicate that the response has been handled by the redirect diff --git a/backend/swagger/docs/docs.go b/backend/swagger/docs/docs.go index 1abbe7711..8069dcfd4 100644 --- a/backend/swagger/docs/docs.go +++ b/backend/swagger/docs/docs.go @@ -1910,6 +1910,10 @@ const docTemplate = `{ "settings.OidcConfig": { "type": "object", "properties": { + "adminGroup": { + "description": "if set, users in this group will be granted admin privileges.", + "type": "string" + }, "clientId": { "description": "client id of the OIDC application", "type": "string" @@ -1918,6 +1922,10 @@ const docTemplate = `{ "description": "client secret of the OIDC application", "type": "string" }, + "createUser": { + "description": "create user if not exists", + "type": "boolean" + }, "disableVerifyTLS": { "description": "disable TLS verification for the OIDC provider. This is insecure and should only be used for testing.", "type": "boolean" diff --git a/backend/swagger/docs/swagger.json b/backend/swagger/docs/swagger.json index 897c8d151..5f256af47 100644 --- a/backend/swagger/docs/swagger.json +++ b/backend/swagger/docs/swagger.json @@ -1899,6 +1899,10 @@ "settings.OidcConfig": { "type": "object", "properties": { + "adminGroup": { + "description": "if set, users in this group will be granted admin privileges.", + "type": "string" + }, "clientId": { "description": "client id of the OIDC application", "type": "string" @@ -1907,6 +1911,10 @@ "description": "client secret of the OIDC application", "type": "string" }, + "createUser": { + "description": "create user if not exists", + "type": "boolean" + }, "disableVerifyTLS": { "description": "disable TLS verification for the OIDC provider. This is insecure and should only be used for testing.", "type": "boolean" diff --git a/backend/swagger/docs/swagger.yaml b/backend/swagger/docs/swagger.yaml index bbf0511e5..5bc4b728c 100644 --- a/backend/swagger/docs/swagger.yaml +++ b/backend/swagger/docs/swagger.yaml @@ -209,12 +209,18 @@ definitions: type: object settings.OidcConfig: properties: + adminGroup: + description: if set, users in this group will be granted admin privileges. + type: string clientId: description: client id of the OIDC application type: string clientSecret: description: client secret of the OIDC application type: string + createUser: + description: create user if not exists + type: boolean disableVerifyTLS: description: disable TLS verification for the OIDC provider. This is insecure and should only be used for testing. diff --git a/frontend/.eslintrc.json b/frontend/.eslintrc.json index 7ccc24883..6c46ea082 100644 --- a/frontend/.eslintrc.json +++ b/frontend/.eslintrc.json @@ -30,7 +30,7 @@ "@intlify/vue-i18n/no-missing-keys": "error", // Optional: Warn about unused keys in your 'en.json' // This requires configuring the `localeDir` setting below. - "@intlify/vue-i18n/no-unused-keys": ["warn", { + "@intlify/vue-i18n/no-unused-keys": ["error", { "src": "./src", // Path to your source files "extensions": [".js", ".vue"] // Important: This tells the rule to check unused keys specifically in en.json @@ -40,7 +40,7 @@ }], "@intlify/vue-i18n/no-raw-text": [ - "warn", + "error", { "ignoreNodes": ["i", "v-icon"] } diff --git a/frontend/public/config.generated.yaml b/frontend/public/config.generated.yaml index ed6294cf6..a51060339 100644 --- a/frontend/public/config.generated.yaml +++ b/frontend/public/config.generated.yaml @@ -69,6 +69,8 @@ auth: userIdentifier: "" # the user identifier to use for authentication. Default is "username", can be "email" or "username", or "phone" disableVerifyTLS: false # disable TLS verification for the OIDC provider. This is insecure and should only be used for testing. logoutRedirectUrl: "" # if provider logout url is provided, filebrowser will also redirect to logout url. Custom logout query params are respected. + createUser: false # create user if not exists + adminGroup: "" # if set, users in this group will be granted admin privileges. key: "" # the key used to sign the JWT tokens. If not set, a random key will be generated. adminUsername: admin # the username of the admin user. If not set, the default is "admin". adminPassword: admin # the password of the admin user. If not set, the default is "admin". diff --git a/frontend/src/views/Login.vue b/frontend/src/views/Login.vue index 507f4cc7f..ef6611f8c 100644 --- a/frontend/src/views/Login.vue +++ b/frontend/src/views/Login.vue @@ -43,7 +43,7 @@
{{ $t("login.or") }}
- + OpenID Connect @@ -60,6 +60,7 @@ import Prompts from "@/components/prompts/Prompts.vue"; import Icon from "@/components/files/Icon.vue"; import { usersApi } from "@/api"; import { initAuth } from "@/utils/auth"; +import { removeLeadingSlash } from "@/utils/url"; import { name, logoURL, @@ -69,6 +70,7 @@ import { darkMode, oidcAvailable, passwordAvailable, + baseURL, } from "@/utils/constants"; export default { @@ -98,9 +100,19 @@ export default { password: "", recaptcha: recaptcha, passwordConfirm: "", + loginURL: baseURL + "api/auth/oidc/login", }; }, mounted() { + let redirect = state.route.query.redirect; + if (redirect === "" || redirect === undefined || redirect === null) { + redirect = baseURL + "files/"; + } else { + redirect = removeLeadingSlash(redirect); + redirect = baseURL + redirect; + } + this.loginURL += `?redirect=${encodeURIComponent(redirect)}`; + if (!recaptcha) return; window.grecaptcha.ready(function () { window.grecaptcha.render("recaptcha", { @@ -249,4 +261,4 @@ export default { .or::after { right: 0; } - + \ No newline at end of file diff --git a/frontend/src/views/files/DocViewer.vue b/frontend/src/views/files/DocViewer.vue index 5575e0fba..3b27aba3a 100644 --- a/frontend/src/views/files/DocViewer.vue +++ b/frontend/src/views/files/DocViewer.vue @@ -1,6 +1,6 @@