Skip to content

Commit

Permalink
Add setting to disable user features when user login type is not plain (
Browse files Browse the repository at this point in the history
#29615)

## Changes
- 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
  • Loading branch information
jackHay22 authored Mar 29, 2024
1 parent 849eee8 commit 59d4aad
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 16 deletions.
5 changes: 5 additions & 0 deletions custom/conf/app.example.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1485,6 +1485,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 =

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Expand Down
4 changes: 4 additions & 0 deletions docs/content/administration/config-cheat-sheet.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,10 @@ And the following unique queues:
- `deletion`: User cannot delete their own account.
- `manage_ssh_keys`: User cannot configure ssh keys.
- `manage_gpg_keys`: User cannot configure gpg keys.
- `EXTERNAL_USER_DISABLE_FEATURES`: **_empty_**: 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`: User cannot delete their own account.
- `manage_ssh_keys`: User cannot configure ssh keys.
- `manage_gpg_keys`: User cannot configure gpg keys.

## Security (`security`)

Expand Down
18 changes: 18 additions & 0 deletions models/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -1232,3 +1232,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
}
35 changes: 35 additions & 0 deletions models/user/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -526,3 +527,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))
}
}
12 changes: 8 additions & 4 deletions modules/setting/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,24 @@

package setting

import "code.gitea.io/gitea/modules/container"
import (
"code.gitea.io/gitea/modules/container"
)

// Admin settings
var Admin struct {
DisableRegularOrgCreation bool
DefaultEmailNotification string
UserDisabledFeatures container.Set[string]
DisableRegularOrgCreation bool
DefaultEmailNotification string
UserDisabledFeatures container.Set[string]
ExternalUserDisableFeatures container.Set[string]
}

func loadAdminFrom(rootCfg ConfigProvider) {
sec := rootCfg.Section("admin")
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 (
Expand Down
5 changes: 3 additions & 2 deletions routers/api/v1/user/gpg_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/user/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions routers/web/user/setting/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,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
}
Expand Down Expand Up @@ -319,7 +319,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()
Expand Down
13 changes: 7 additions & 6 deletions routers/web/user/setting/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}

0 comments on commit 59d4aad

Please sign in to comment.