From e08f05b069267678623d458eaff4fa762df0be13 Mon Sep 17 00:00:00 2001 From: Jack Hay Date: Fri, 29 Mar 2024 11:05:41 -0400 Subject: [PATCH] Add setting to disable user features when user login type is not plain (#29615) - Adds setting `EXTERNAL_USER_DISABLE_FEATURES` to disable any supported user features when login type is not plain - In general, this is necessary for SSO implementations to avoid inconsistencies between the external account management and the linked account - Adds helper functions to encourage correct use (cherry picked from commit 59d4aadba5c15d02f3b9f0e61abb7476870c20a5) Conflicts: - docs/content/administration/config-cheat-sheet.en-us.md Removed. - modules/setting/admin.go Trivial resolution: pick the newly added struct member. --- custom/conf/app.example.ini | 5 +++++ models/user/user.go | 18 +++++++++++++++ models/user/user_test.go | 35 +++++++++++++++++++++++++++++ modules/setting/admin.go | 6 ++++- routers/api/v1/user/gpg_key.go | 5 +++-- routers/api/v1/user/key.go | 4 ++-- routers/web/user/setting/account.go | 4 ++-- routers/web/user/setting/keys.go | 13 ++++++----- 8 files changed, 77 insertions(+), 13 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 74999b5bb3..d00dc4eb64 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1503,6 +1503,11 @@ LEVEL = Info ;; - manage_ssh_keys: a user cannot configure ssh keys ;; - manage_gpg_keys: a user cannot configure gpg keys ;USER_DISABLED_FEATURES = +;; Comma separated list of disabled features ONLY if the user has an external login type (eg. LDAP, Oauth, etc.), could be `deletion`, `manage_ssh_keys`, `manage_gpg_keys`. This setting is independent from `USER_DISABLED_FEATURES` and supplements its behavior. +;; - deletion: a user cannot delete their own account +;; - manage_ssh_keys: a user cannot configure ssh keys +;; - manage_gpg_keys: a user cannot configure gpg keys +;;EXTERNAL_USER_DISABLE_FEATURES = ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/models/user/user.go b/models/user/user.go index ff85c2cfa0..8452e3c7cf 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1246,3 +1246,21 @@ func GetOrderByName() string { } return "name" } + +// IsFeatureDisabledWithLoginType checks if a user feature is disabled, taking into account the login type of the +// user if applicable +func IsFeatureDisabledWithLoginType(user *User, feature string) bool { + // NOTE: in the long run it may be better to check the ExternalLoginUser table rather than user.LoginType + return (user != nil && user.LoginType > auth.Plain && setting.Admin.ExternalUserDisableFeatures.Contains(feature)) || + setting.Admin.UserDisabledFeatures.Contains(feature) +} + +// DisabledFeaturesWithLoginType returns the set of user features disabled, taking into account the login type +// of the user if applicable +func DisabledFeaturesWithLoginType(user *User) *container.Set[string] { + // NOTE: in the long run it may be better to check the ExternalLoginUser table rather than user.LoginType + if user != nil && user.LoginType > auth.Plain { + return &setting.Admin.ExternalUserDisableFeatures + } + return &setting.Admin.UserDisabledFeatures +} diff --git a/models/user/user_test.go b/models/user/user_test.go index bb72743301..7888c2638f 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/auth/password/hash" + "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" @@ -542,3 +543,37 @@ func Test_NormalizeUserFromEmail(t *testing.T) { } } } + +func TestDisabledUserFeatures(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + testValues := container.SetOf(setting.UserFeatureDeletion, + setting.UserFeatureManageSSHKeys, + setting.UserFeatureManageGPGKeys) + + oldSetting := setting.Admin.ExternalUserDisableFeatures + defer func() { + setting.Admin.ExternalUserDisableFeatures = oldSetting + }() + setting.Admin.ExternalUserDisableFeatures = testValues + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + assert.Len(t, setting.Admin.UserDisabledFeatures.Values(), 0) + + // no features should be disabled with a plain login type + assert.LessOrEqual(t, user.LoginType, auth.Plain) + assert.Len(t, user_model.DisabledFeaturesWithLoginType(user).Values(), 0) + for _, f := range testValues.Values() { + assert.False(t, user_model.IsFeatureDisabledWithLoginType(user, f)) + } + + // check disabled features with external login type + user.LoginType = auth.OAuth2 + + // all features should be disabled + assert.NotEmpty(t, user_model.DisabledFeaturesWithLoginType(user).Values()) + for _, f := range testValues.Values() { + assert.True(t, user_model.IsFeatureDisabledWithLoginType(user, f)) + } +} diff --git a/modules/setting/admin.go b/modules/setting/admin.go index 35ffa9efbf..07b8ef0899 100644 --- a/modules/setting/admin.go +++ b/modules/setting/admin.go @@ -3,7 +3,9 @@ package setting -import "code.gitea.io/gitea/modules/container" +import ( + "code.gitea.io/gitea/modules/container" +) // Admin settings var Admin struct { @@ -11,6 +13,7 @@ var Admin struct { DefaultEmailNotification string SendNotificationEmailOnNewUser bool UserDisabledFeatures container.Set[string] + ExternalUserDisableFeatures container.Set[string] } func loadAdminFrom(rootCfg ConfigProvider) { @@ -18,6 +21,7 @@ func loadAdminFrom(rootCfg ConfigProvider) { Admin.DisableRegularOrgCreation = sec.Key("DISABLE_REGULAR_ORG_CREATION").MustBool(false) Admin.DefaultEmailNotification = sec.Key("DEFAULT_EMAIL_NOTIFICATIONS").MustString("enabled") Admin.UserDisabledFeatures = container.SetOf(sec.Key("USER_DISABLED_FEATURES").Strings(",")...) + Admin.ExternalUserDisableFeatures = container.SetOf(sec.Key("EXTERNAL_USER_DISABLE_FEATURES").Strings(",")...) } const ( diff --git a/routers/api/v1/user/gpg_key.go b/routers/api/v1/user/gpg_key.go index dcf5da0b2e..5a2f995e1b 100644 --- a/routers/api/v1/user/gpg_key.go +++ b/routers/api/v1/user/gpg_key.go @@ -10,6 +10,7 @@ import ( asymkey_model "code.gitea.io/gitea/models/asymkey" "code.gitea.io/gitea/models/db" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" @@ -133,7 +134,7 @@ func GetGPGKey(ctx *context.APIContext) { // CreateUserGPGKey creates new GPG key to given user by ID. func CreateUserGPGKey(ctx *context.APIContext, form api.CreateGPGKeyOption, uid int64) { - if setting.Admin.UserDisabledFeatures.Contains(setting.UserFeatureManageGPGKeys) { + if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageGPGKeys) { ctx.NotFound("Not Found", fmt.Errorf("gpg keys setting is not allowed to be visited")) return } @@ -274,7 +275,7 @@ func DeleteGPGKey(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - if setting.Admin.UserDisabledFeatures.Contains(setting.UserFeatureManageGPGKeys) { + if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageGPGKeys) { ctx.NotFound("Not Found", fmt.Errorf("gpg keys setting is not allowed to be visited")) return } diff --git a/routers/api/v1/user/key.go b/routers/api/v1/user/key.go index bcbfd93bd3..d9456e7ec6 100644 --- a/routers/api/v1/user/key.go +++ b/routers/api/v1/user/key.go @@ -199,7 +199,7 @@ func GetPublicKey(ctx *context.APIContext) { // CreateUserPublicKey creates new public key to given user by ID. func CreateUserPublicKey(ctx *context.APIContext, form api.CreateKeyOption, uid int64) { - if setting.Admin.UserDisabledFeatures.Contains(setting.UserFeatureManageSSHKeys) { + if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageSSHKeys) { ctx.NotFound("Not Found", fmt.Errorf("ssh keys setting is not allowed to be visited")) return } @@ -269,7 +269,7 @@ func DeletePublicKey(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - if setting.Admin.UserDisabledFeatures.Contains(setting.UserFeatureManageSSHKeys) { + if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageSSHKeys) { ctx.NotFound("Not Found", fmt.Errorf("ssh keys setting is not allowed to be visited")) return } diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go index 386309aa71..9cf93320a3 100644 --- a/routers/web/user/setting/account.go +++ b/routers/web/user/setting/account.go @@ -244,7 +244,7 @@ func DeleteEmail(ctx *context.Context) { // DeleteAccount render user suicide page and response for delete user himself func DeleteAccount(ctx *context.Context) { - if setting.Admin.UserDisabledFeatures.Contains(setting.UserFeatureDeletion) { + if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureDeletion) { ctx.Error(http.StatusNotFound) return } @@ -328,7 +328,7 @@ func loadAccountData(ctx *context.Context) { ctx.Data["EmailNotificationsPreference"] = ctx.Doer.EmailNotificationsPreference ctx.Data["ActivationsPending"] = pendingActivation ctx.Data["CanAddEmails"] = !pendingActivation || !setting.Service.RegisterEmailConfirm - ctx.Data["UserDisabledFeatures"] = &setting.Admin.UserDisabledFeatures + ctx.Data["UserDisabledFeatures"] = user_model.DisabledFeaturesWithLoginType(ctx.Doer) if setting.Service.UserDeleteWithCommentsMaxTime != 0 { ctx.Data["UserDeleteWithCommentsMaxTime"] = setting.Service.UserDeleteWithCommentsMaxTime.String() diff --git a/routers/web/user/setting/keys.go b/routers/web/user/setting/keys.go index d2b60fc809..9462be71c2 100644 --- a/routers/web/user/setting/keys.go +++ b/routers/web/user/setting/keys.go @@ -10,6 +10,7 @@ import ( asymkey_model "code.gitea.io/gitea/models/asymkey" "code.gitea.io/gitea/models/db" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" @@ -78,7 +79,7 @@ func KeysPost(ctx *context.Context) { ctx.Flash.Success(ctx.Tr("settings.add_principal_success", form.Content)) ctx.Redirect(setting.AppSubURL + "/user/settings/keys") case "gpg": - if setting.Admin.UserDisabledFeatures.Contains(setting.UserFeatureManageGPGKeys) { + if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageGPGKeys) { ctx.NotFound("Not Found", fmt.Errorf("gpg keys setting is not allowed to be visited")) return } @@ -159,7 +160,7 @@ func KeysPost(ctx *context.Context) { ctx.Flash.Success(ctx.Tr("settings.verify_gpg_key_success", keyID)) ctx.Redirect(setting.AppSubURL + "/user/settings/keys") case "ssh": - if setting.Admin.UserDisabledFeatures.Contains(setting.UserFeatureManageSSHKeys) { + if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageSSHKeys) { ctx.NotFound("Not Found", fmt.Errorf("ssh keys setting is not allowed to be visited")) return } @@ -203,7 +204,7 @@ func KeysPost(ctx *context.Context) { ctx.Flash.Success(ctx.Tr("settings.add_key_success", form.Title)) ctx.Redirect(setting.AppSubURL + "/user/settings/keys") case "verify_ssh": - if setting.Admin.UserDisabledFeatures.Contains(setting.UserFeatureManageSSHKeys) { + if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageSSHKeys) { ctx.NotFound("Not Found", fmt.Errorf("ssh keys setting is not allowed to be visited")) return } @@ -240,7 +241,7 @@ func KeysPost(ctx *context.Context) { func DeleteKey(ctx *context.Context) { switch ctx.FormString("type") { case "gpg": - if setting.Admin.UserDisabledFeatures.Contains(setting.UserFeatureManageGPGKeys) { + if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageGPGKeys) { ctx.NotFound("Not Found", fmt.Errorf("gpg keys setting is not allowed to be visited")) return } @@ -250,7 +251,7 @@ func DeleteKey(ctx *context.Context) { ctx.Flash.Success(ctx.Tr("settings.gpg_key_deletion_success")) } case "ssh": - if setting.Admin.UserDisabledFeatures.Contains(setting.UserFeatureManageSSHKeys) { + if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageSSHKeys) { ctx.NotFound("Not Found", fmt.Errorf("ssh keys setting is not allowed to be visited")) return } @@ -333,5 +334,5 @@ func loadKeysData(ctx *context.Context) { ctx.Data["VerifyingID"] = ctx.FormString("verify_gpg") ctx.Data["VerifyingFingerprint"] = ctx.FormString("verify_ssh") - ctx.Data["UserDisabledFeatures"] = &setting.Admin.UserDisabledFeatures + ctx.Data["UserDisabledFeatures"] = user_model.DisabledFeaturesWithLoginType(ctx.Doer) }