diff --git a/internal/config/config.go b/internal/config/config.go index 3a9d1174..b9daec2f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -134,6 +134,7 @@ var ( Scopes: []string{"openid", "profile", "email"}, CustomFields: []string{}, QueryUserInfo: false, + RequireVerifiedEmail: false, InsecureSkipSignatureCheck: false, Debug: false, }, @@ -1689,6 +1690,12 @@ func getHTTPDOIDCFromEnv(idx int) (httpd.OIDC, bool) { isSet = true } + requireVerifiedEmail, ok := lookupBoolFromEnv(fmt.Sprintf("SFTPGO_HTTPD__BINDINGS__%v__OIDC__REQUIRE_VERIFIED_EMAIL", idx)) + if ok { + result.RequireVerifiedEmail = requireVerifiedEmail + isSet = true + } + debug, ok := lookupBoolFromEnv(fmt.Sprintf("SFTPGO_HTTPD__BINDINGS__%v__OIDC__DEBUG", idx)) if ok { result.Debug = debug diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 405ec5e2..42196b2b 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1223,6 +1223,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__CUSTOM_FIELDS", "field1,field2") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__INSECURE_SKIP_SIGNATURE_CHECK", "1") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__QUERY_USERINFO", "1") + os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__REQUIRE_VERIFIED_EMAIL", "1") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__DEBUG", "1") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__SECURITY__ENABLED", "true") os.Setenv("SFTPGO_HTTPD__BINDINGS__2__SECURITY__ALLOWED_HOSTS", "*.example.com,*.example.net") @@ -1296,6 +1297,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__CUSTOM_FIELDS") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__INSECURE_SKIP_SIGNATURE_CHECK") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__QUERY_USERINFO") + os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__REQUIRE_VERIFIED_EMAIL") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__OIDC__DEBUG") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__SECURITY__ENABLED") os.Unsetenv("SFTPGO_HTTPD__BINDINGS__2__SECURITY__ALLOWED_HOSTS") @@ -1356,6 +1358,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { require.Len(t, bindings[0].OIDC.Scopes, 3) require.False(t, bindings[0].OIDC.InsecureSkipSignatureCheck) require.False(t, bindings[0].OIDC.QueryUserInfo) + require.False(t, bindings[0].OIDC.RequireVerifiedEmail) require.False(t, bindings[0].OIDC.Debug) require.Empty(t, bindings[0].Security.ReferrerPolicy) require.Equal(t, 8000, bindings[1].Port) @@ -1377,6 +1380,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { require.Len(t, bindings[1].OIDC.Scopes, 3) require.False(t, bindings[1].OIDC.InsecureSkipSignatureCheck) require.False(t, bindings[1].OIDC.QueryUserInfo) + require.False(t, bindings[1].OIDC.RequireVerifiedEmail) require.False(t, bindings[1].OIDC.Debug) require.False(t, bindings[1].Security.Enabled) require.Equal(t, "Web Admin", bindings[1].Branding.WebAdmin.Name) @@ -1425,6 +1429,7 @@ func TestHTTPDBindingsFromEnv(t *testing.T) { require.Equal(t, "field2", bindings[2].OIDC.CustomFields[1]) require.True(t, bindings[2].OIDC.InsecureSkipSignatureCheck) require.True(t, bindings[2].OIDC.QueryUserInfo) + require.True(t, bindings[2].OIDC.RequireVerifiedEmail) require.True(t, bindings[2].OIDC.Debug) require.True(t, bindings[2].Security.Enabled) require.Len(t, bindings[2].Security.AllowedHosts, 2) diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index 6c9b7884..9cc81d4f 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -28397,7 +28397,11 @@ func startOIDCMockServer() { w.WriteHeader(http.StatusInternalServerError) return } - fmt.Fprintf(w, `{"sub":"123","preferred_username":"oidc_user","email":"example@example.com","sftpgo_role":"admin"}`) + if r.Header.Get("Authorization") == "Bearer 789" { + fmt.Fprintf(w, `{"sub":"123","preferred_username":"oidc_user","email":"example@example.com","sftpgo_role":"admin"}`) + return + } + fmt.Fprintf(w, `{"sub":"123","preferred_username":"oidc_user","email":"example@example.com","email_verified":true,"sftpgo_role":"admin"}`) }) http.HandleFunc("/auth/realms/sftpgo/.well-known/openid-configuration", func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") diff --git a/internal/httpd/oidc.go b/internal/httpd/oidc.go index adaaa732..8ef424d8 100644 --- a/internal/httpd/oidc.go +++ b/internal/httpd/oidc.go @@ -43,6 +43,7 @@ const ( adminRoleFieldValue = "admin" authStateValidity = 2 * 60 * 1000 // 2 minutes maxWebClientNextLength = 4096 + emailVerifiedClaim = "email_verified" tokenUpdateInterval = 3 * 60 * 1000 // 3 minutes tokenDeleteInterval = 2 * 3600 * 1000 // 2 hours ) @@ -101,6 +102,10 @@ type OIDC struct { // Non-empty ID token claims take precedence over UserInfo claims with the // same name. The UserInfo subject must match the ID token subject. QueryUserInfo bool `json:"query_userinfo" mapstructure:"query_userinfo"` + // RequireVerifiedEmail defines whether to accept only users whose "email_verified" + // claim is set to true in the ID token or, when QueryUserInfo is enabled, + // in the merged claims. + RequireVerifiedEmail bool `json:"require_verified_email" mapstructure:"require_verified_email"` // InsecureSkipSignatureCheck causes SFTPGo to skip JWT signature validation. // It's intended for special cases where providers, such as Azure, use the "none" // algorithm. Skipping the signature validation can cause security issues @@ -181,6 +186,10 @@ func (o *OIDC) initialize() error { if o.QueryUserInfo && provider.UserInfoEndpoint() == "" { return errors.New("oidc: query_userinfo is enabled but the provider has no userinfo endpoint") } + if o.RequireVerifiedEmail && !slices.Contains(o.Scopes, "email") { + logger.Warn(logSender, "", "oidc: require_verified_email is enabled and the \"email\" scope is not requested, "+ + "ensure the provider returns the %q claim", emailVerifiedClaim) + } o.provider = provider o.verifier = nil o.oauth2Config = &oauth2.Config{ @@ -808,6 +817,14 @@ func (s *httpdServer) handleOIDCRedirect(w http.ResponseWriter, r *http.Request) if !oauth2Token.Expiry.IsZero() { token.ExpiresAt = util.GetTimeAsMsSinceEpoch(oauth2Token.Expiry) } + if err := validateOIDCEmailVerified(claims, s.binding.OIDC.RequireVerifiedEmail); err != nil { + logger.Debug(logSender, "", "unable to validate the OpenID email address: %v", err) + setFlashMessage(w, r, newFlashMessage(fmt.Sprintf("Unable to validate the OpenID email address: %v", err), + util.I18nOIDCEmailNotVerified)) + doRedirect() + doLogout(rawIDToken) + return + } err = token.parseClaims(claims, s.binding.OIDC.UsernameField, s.binding.OIDC.RoleField, s.binding.OIDC.CustomFields, s.binding.OIDC.getForcedRole(authReq.Audience)) if err != nil { @@ -852,6 +869,23 @@ func (s *httpdServer) handleOIDCRedirect(w http.ResponseWriter, r *http.Request) loginOIDCUser(w, r, token, authReq.Next) } +func validateOIDCEmailVerified(claims map[string]any, required bool) error { + if !required { + return nil + } + switch v := claims[emailVerifiedClaim].(type) { + case bool: + if !v { + return errors.New("the email address is not verified by the identity provider") + } + return nil + case nil: + return fmt.Errorf("missing %q claim required to verify the email address", emailVerifiedClaim) + default: + return fmt.Errorf("invalid type for the %q claim: %T", emailVerifiedClaim, v) + } +} + func loginOIDCUser(w http.ResponseWriter, r *http.Request, token oidcToken, next string) { oidcMgr.addToken(token) diff --git a/internal/httpd/oidc_test.go b/internal/httpd/oidc_test.go index beefc71d..497b61ef 100644 --- a/internal/httpd/oidc_test.go +++ b/internal/httpd/oidc_test.go @@ -452,6 +452,170 @@ func TestOIDCQueryUserInfo(t *testing.T) { require.Len(t, oidcMgr.pendingAuths, 0) } +func TestValidateOIDCEmailVerified(t *testing.T) { + tests := []struct { + name string + claims map[string]any + required bool + wantErr bool + }{ + { + name: "check disabled", + claims: map[string]any{"sub": "user1"}, + required: false, + wantErr: false, + }, + { + name: "verified, boolean claim", + claims: map[string]any{"email_verified": true}, + required: true, + wantErr: false, + }, + { + name: "not verified, boolean claim", + claims: map[string]any{"email_verified": false}, + required: true, + wantErr: true, + }, + { + name: "invalid claim type, string", + claims: map[string]any{"email_verified": "true"}, + required: true, + wantErr: true, + }, + { + name: "invalid claim type, number", + claims: map[string]any{"email_verified": float64(1)}, + required: true, + wantErr: true, + }, + { + name: "missing claim", + claims: map[string]any{"sub": "user1"}, + required: true, + wantErr: true, + }, + { + name: "null claim", + claims: map[string]any{"email_verified": nil}, + required: true, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateOIDCEmailVerified(tt.claims, tt.required) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestOIDCRequireVerifiedEmail(t *testing.T) { + oidcMgr, ok := oidcMgr.(*memoryOIDCManager) + require.True(t, ok) + server := getTestOIDCServer() + server.binding.OIDC.RequireVerifiedEmail = true + err := server.binding.OIDC.initialize() + assert.NoError(t, err) + err = server.initializeRouter() + require.NoError(t, err) + + admin := dataprovider.Admin{ + Username: "oidc_user", + Password: "p", + Permissions: []string{dataprovider.PermAdminAny}, + Status: 1, + } + err = dataprovider.AddAdmin(&admin, "", "", "") + assert.NoError(t, err) + defer func() { + err := dataprovider.DeleteAdmin(admin.Username, "", "", "") + assert.NoError(t, err) + }() + + token := (&oauth2.Token{ + AccessToken: "123", + Expiry: time.Now().Add(5 * time.Minute), + }).WithExtra(map[string]any{"id_token": "id_token_val"}) + server.binding.OIDC.oauth2Config = &mockOAuth2Config{ + tokenSource: &mockTokenSource{}, + token: token, + } + + doLogin := func(idTokenClaims string) *httptest.ResponseRecorder { + authReq := newTestOIDCPendingAuth(tokenAudienceWebAdmin) + oidcMgr.addPendingAuth(authReq) + idToken := &oidc.IDToken{ + Nonce: authReq.Nonce, + Expiry: time.Now().Add(5 * time.Minute), + Subject: "123", + } + setIDTokenClaims(idToken, []byte(idTokenClaims)) + server.binding.OIDC.verifier = &mockOIDCVerifier{token: idToken} + rr := httptest.NewRecorder() + r, err := newOIDCRedirectRequest(authReq) + assert.NoError(t, err) + server.router.ServeHTTP(rr, r) + return rr + } + + // a missing email_verified claim rejects the login + rr := doLogin(`{"sub":"123","preferred_username":"oidc_user","sftpgo_role":"admin"}`) + assert.Equal(t, http.StatusFound, rr.Code) + assert.Equal(t, webAdminLoginPath, rr.Header().Get("Location")) + require.Len(t, oidcMgr.tokens, 0) + require.Len(t, oidcMgr.pendingAuths, 0) + + // an email_verified claim set to false rejects the login + rr = doLogin(`{"sub":"123","preferred_username":"oidc_user","sftpgo_role":"admin","email_verified":false}`) + assert.Equal(t, http.StatusFound, rr.Code) + assert.Equal(t, webAdminLoginPath, rr.Header().Get("Location")) + require.Len(t, oidcMgr.tokens, 0) + + // an email_verified claim set to true allows the login + rr = doLogin(`{"sub":"123","preferred_username":"oidc_user","sftpgo_role":"admin","email_verified":true}`) + assert.Equal(t, http.StatusFound, rr.Code, rr.Body.String()) + assert.Equal(t, webUsersPath, rr.Header().Get("Location")) + require.Len(t, oidcMgr.tokens, 1) + for k := range oidcMgr.tokens { + oidcMgr.removeToken(k) + } + + // the claim can also be provided by the user info endpoint + server.binding.OIDC.QueryUserInfo = true + rr = doLogin(`{"sub":"123","preferred_username":"oidc_user","sftpgo_role":"admin"}`) + assert.Equal(t, http.StatusFound, rr.Code, rr.Body.String()) + assert.Equal(t, webUsersPath, rr.Header().Get("Location")) + require.Len(t, oidcMgr.tokens, 1) + for k := range oidcMgr.tokens { + oidcMgr.removeToken(k) + } + + // the ID token claim takes precedence over the user info one, which is set to true + rr = doLogin(`{"sub":"123","preferred_username":"oidc_user","sftpgo_role":"admin","email_verified":false}`) + assert.Equal(t, http.StatusFound, rr.Code) + assert.Equal(t, webAdminLoginPath, rr.Header().Get("Location")) + require.Len(t, oidcMgr.tokens, 0) + + // the login is rejected if neither source provides the claim + server.binding.OIDC.oauth2Config = &mockOAuth2Config{ + tokenSource: &mockTokenSource{}, + token: (&oauth2.Token{ + AccessToken: "789", + Expiry: time.Now().Add(5 * time.Minute), + }).WithExtra(map[string]any{"id_token": "id_token_val"}), + } + rr = doLogin(`{"sub":"123","preferred_username":"oidc_user","sftpgo_role":"admin"}`) + assert.Equal(t, http.StatusFound, rr.Code) + assert.Equal(t, webAdminLoginPath, rr.Header().Get("Location")) + require.Len(t, oidcMgr.tokens, 0) + require.Len(t, oidcMgr.pendingAuths, 0) +} + func TestOIDCLoginLogout(t *testing.T) { tokenValidationMode = 2 diff --git a/internal/util/i18n.go b/internal/util/i18n.go index 70403a1f..9160ee58 100644 --- a/internal/util/i18n.go +++ b/internal/util/i18n.go @@ -186,6 +186,7 @@ const ( I18nOIDCTokenInvalidRoleAdmin = "oidc.role_admin_err" I18nOIDCTokenInvalidRoleUser = "oidc.role_user_err" I18nOIDCErrGetUser = "oidc.get_user_err" + I18nOIDCEmailNotVerified = "oidc.email_not_verified" I18nErrorInvalidQuotaSize = "user.invalid_quota_size" I18nErrorTimeOfDayInvalid = "user.time_of_day_invalid" I18nErrorTimeOfDayConflict = "user.time_of_day_conflict" diff --git a/sftpgo.json b/sftpgo.json index 39fce639..432f47c5 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -305,6 +305,7 @@ "implicit_roles": false, "custom_fields": [], "query_userinfo": false, + "require_verified_email": false, "insecure_skip_signature_check": false, "debug": false }, diff --git a/static/locales/de/translation.json b/static/locales/de/translation.json index bae08382..9c2d84f3 100644 --- a/static/locales/de/translation.json +++ b/static/locales/de/translation.json @@ -676,7 +676,8 @@ "token_invalid": "Ungültiges OpenID-Token", "role_admin_err": "Falsche OpenID-Rolle: Der angemeldete Benutzer ist kein Administrator", "role_user_err": "Falsche OpenID-Rolle: Der angemeldete Benutzer ist ein Administrator", - "get_user_err": "Fehler beim Abrufen des mit dem OpenID-Token verknüpften Benutzers" + "get_user_err": "Fehler beim Abrufen des mit dem OpenID-Token verknüpften Benutzers", + "email_not_verified": "Die E-Mail-Adresse ist vom Identitätsanbieter nicht verifiziert" }, "oauth2": { "auth_verify_error": "OAuth2-Code kann nicht überprüft werden!", diff --git a/static/locales/en/translation.json b/static/locales/en/translation.json index ef3d7d28..5b8d6fae 100644 --- a/static/locales/en/translation.json +++ b/static/locales/en/translation.json @@ -676,7 +676,8 @@ "token_invalid": "Invalid OpenID token", "role_admin_err": "Incorrect OpenID role, logged in user is not an administrator", "role_user_err": "Incorrect OpenID role, logged in user is an administrator", - "get_user_err": "Failed to get user associated with OpenID token" + "get_user_err": "Failed to get user associated with OpenID token", + "email_not_verified": "The email address is not verified by the identity provider" }, "oauth2": { "auth_verify_error": "Unable to verify OAuth2 code", diff --git a/static/locales/es/translation.json b/static/locales/es/translation.json index f581a38c..eaa19560 100644 --- a/static/locales/es/translation.json +++ b/static/locales/es/translation.json @@ -676,7 +676,8 @@ "token_invalid": "Token OpenID inválido", "role_admin_err": "Rol OpenID incorrecto, el usuario conectado no es un administrador", "role_user_err": "Rol OpenID incorrecto, el usuario conectado es un administrador", - "get_user_err": "Error al obtener el usuario asociado con el token OpenID" + "get_user_err": "Error al obtener el usuario asociado con el token OpenID", + "email_not_verified": "La dirección de correo electrónico no está verificada por el proveedor de identidad" }, "oauth2": { "auth_verify_error": "No se puede verificar el código OAuth2", diff --git a/static/locales/fr/translation.json b/static/locales/fr/translation.json index 125b9b31..b2b8a06f 100644 --- a/static/locales/fr/translation.json +++ b/static/locales/fr/translation.json @@ -676,7 +676,8 @@ "token_invalid": "Jeton OpenID invalide", "role_admin_err": "Rôle OpenID incorrect, l'utilisateur connecté n'est pas un administrateur", "role_user_err": "Rôle OpenID incorrect, l'utilisateur connecté est un administrateur", - "get_user_err": "Échec de la récupération de l'utilisateur associé au jeton OpenID" + "get_user_err": "Échec de la récupération de l'utilisateur associé au jeton OpenID", + "email_not_verified": "L'adresse e-mail n'est pas vérifiée par le fournisseur d'identité" }, "oauth2": { "auth_verify_error": "Impossible de vérifier le code OAuth2", diff --git a/static/locales/it/translation.json b/static/locales/it/translation.json index 3f60d7e7..f7b198d4 100644 --- a/static/locales/it/translation.json +++ b/static/locales/it/translation.json @@ -676,7 +676,8 @@ "token_invalid": "Token OpenID non valido", "role_admin_err": "Ruolo OpenID errato, l'utente che ha effettuato l'accesso non è un amministratore", "role_user_err": "Ruolo OpenID errato, l'utente che ha effettuato l'accesso è un amministratore", - "get_user_err": "Impossibile ottenere l'utente associato al token OpenID" + "get_user_err": "Impossibile ottenere l'utente associato al token OpenID", + "email_not_verified": "L'indirizzo email non è verificato dall'identity provider" }, "oauth2": { "auth_verify_error": "Impossibile verificare il codice OAuth2", diff --git a/static/locales/zh-CN/translation.json b/static/locales/zh-CN/translation.json index 04aa5085..d4f46532 100644 --- a/static/locales/zh-CN/translation.json +++ b/static/locales/zh-CN/translation.json @@ -676,7 +676,8 @@ "token_invalid": "无效的 OpenID 令牌", "role_admin_err": "OpenID 角色不正确，登录用户不是管理员", "role_user_err": "OpenID 角色不正确，登录用户是管理员", - "get_user_err": "无法获取与 OpenID 令牌关联的用户" + "get_user_err": "无法获取与 OpenID 令牌关联的用户", + "email_not_verified": "身份提供程序未验证该电子邮件地址" }, "oauth2": { "auth_verify_error": "无法验证 OAuth2 代码",