diff --git a/.codex-history.md b/.codex-history.md index 126cdd69db..b78abe0688 100644 --- a/.codex-history.md +++ b/.codex-history.md @@ -387,3 +387,13 @@ Project Change ID[date-time] - application-version - Type - Summary: - 1 - I modified `routers/web/admin/users.go` so a GOOD-granted or super-admin user can still edit their own ordinary account fields, while forbidden cross-user edits remain blocked and table action states are computed per target user. - 2 - I modified `routers/api/v1/admin/user.go` so GOOD-protection no longer blocks a user from editing their own account through the admin API. - 3 - I modified `templates/admin/user/list.tmpl`, `templates/admin/user/view.tmpl`, and `templates/admin/user/edit.tmpl` so `Edit` stays enabled for allowed self-edits, while forbidden `Edit` and `Delete` actions are shown disabled with the specific reason tooltip or warning message. + +74 - [2026-04-30 20:11:48] - v1.27.0-dev-72-g43161732e3 - Type: Modified - Added install-time admin management policy choices with direct-grantor and inherited-grantor enforcement. +- 1 - I modified `modules/setting/admin.go`, `services/forms/user_form.go`, `routers/install/install.go`, `templates/install.tmpl`, and `options/locale/locale_en-US.json` so installation now exposes three administrator-management policies: `super_admin_only`, `grantor_only`, and `grantor_inheritance`, while normalizing the old legacy policy names. +- 2 - I modified `models/user/admin_grant.go` so the code can resolve the effective admin grantor by walking the admin-grant chain until it finds an active administrator who still has sign-in enabled. +- 3 - I modified `routers/web/admin/users.go` so regular admins can edit or delete only the administrator accounts allowed by the selected policy, while keeping super-admin protections, GOOD protections, self-edit exceptions, and disabled forbidden actions intact in the admin UI. +- 4 - I modified `routers/api/v1/admin/user.go` so the admin API now applies the same grantor-based restrictions for editing and deleting administrator accounts. +- 5 - I added focused integration coverage in `tests/integration/admin_user_test.go` and `tests/integration/api_admin_test.go` for direct-grantor edits, inherited-grantor edits, and API denial for unrelated admins. + +75 - [2026-04-30 20:37:01] - v1.27.0-dev-72-g43161732e3 - Type: Modified - Updated the example app.ini documentation for the new administrator management policies. +- 1 - I modified `custom/conf/app.example.ini` so the `ADMIN_MANAGEMENT_POLICY` comments now document `super_admin_only`, `grantor_only`, and `grantor_inheritance`, and changed the documented default to `grantor_only`. diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 93e7714b49..974c817048 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1619,10 +1619,10 @@ LEVEL = Info ;; ;; Controls who can change administrator and super administrator permissions. ;; Options: -;; - super_admin_only: only super administrators can grant or revoke administrator and super administrator permissions. -;; - admins_can_promote_users: administrators can promote regular users to administrator, while super administrators manage existing administrators and all super administrator changes. -;; - super_admin_approval: reserved for the approval workflow. -;ADMIN_MANAGEMENT_POLICY = admins_can_promote_users +;; - super_admin_only: only super administrators can modify or delete administrator accounts and manage all administrator privilege changes. +;; - grantor_only: administrators can promote regular users to administrator and later manage only the administrator accounts they directly promoted. +;; - grantor_inheritance: like grantor_only, but if the direct grantor becomes inactive or has sign-in disabled, management passes up the admin grant chain until an eligible administrator is found. +;ADMIN_MANAGEMENT_POLICY = grantor_only ;; Disabled features for users could be "deletion", "manage_ssh_keys", "manage_gpg_keys", "manage_mfa", "manage_credentials" more features can be disabled in future ;; - deletion: a user cannot delete their own account ;; - manage_ssh_keys: a user cannot configure ssh keys diff --git a/models/user/admin_grant.go b/models/user/admin_grant.go index fe5b139017..032577c614 100644 --- a/models/user/admin_grant.go +++ b/models/user/admin_grant.go @@ -64,3 +64,43 @@ func IsGrantorOf(ctx context.Context, grantedUserID, possibleGrantorID int64) (b } return false, nil } + +func IsEligibleAdminGrantor(ctx context.Context, userID int64) (bool, error) { + u, err := GetUserByID(ctx, userID) + if err != nil { + if IsErrUserNotExist(err) { + return false, nil + } + return false, err + } + return u.IsAdmin && u.IsActive && !u.ProhibitLogin, nil +} + +func GetEffectiveAdminGrantorID(ctx context.Context, userID int64) (int64, bool, error) { + visited := map[int64]struct{}{userID: {}} + currentUserID := userID + + for { + grantorID, has, err := GetAdminGrantorID(ctx, currentUserID) + if err != nil { + return 0, false, err + } + if !has { + return 0, false, nil + } + if _, ok := visited[grantorID]; ok { + return 0, false, nil + } + visited[grantorID] = struct{}{} + + eligible, err := IsEligibleAdminGrantor(ctx, grantorID) + if err != nil { + return 0, false, err + } + if eligible { + return grantorID, true, nil + } + + currentUserID = grantorID + } +} diff --git a/modules/setting/admin.go b/modules/setting/admin.go index 060c465222..e0598e08b7 100644 --- a/modules/setting/admin.go +++ b/modules/setting/admin.go @@ -20,13 +20,17 @@ var Admin struct { const ( AdminManagementPolicySuperAdminOnly = "super_admin_only" + AdminManagementPolicyGrantorOnly = "grantor_only" + AdminManagementPolicyGrantorInheritance = "grantor_inheritance" AdminManagementPolicyAdminsCanPromote = "admins_can_promote_users" AdminManagementPolicySuperAdminApproval = "super_admin_approval" - defaultAdminManagementPolicy = AdminManagementPolicyAdminsCanPromote + defaultAdminManagementPolicy = AdminManagementPolicyGrantorOnly ) var validAdminManagementPolicies = container.SetOf( AdminManagementPolicySuperAdminOnly, + AdminManagementPolicyGrantorOnly, + AdminManagementPolicyGrantorInheritance, AdminManagementPolicyAdminsCanPromote, AdminManagementPolicySuperAdminApproval, ) @@ -46,7 +50,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.SuperAdminEnabled = sec.Key("SUPER_ADMIN_ENABLED").MustBool(true) - Admin.AdminManagementPolicy = sec.Key("ADMIN_MANAGEMENT_POLICY").MustString(defaultAdminManagementPolicy) + Admin.AdminManagementPolicy = normalizeAdminManagementPolicy(sec.Key("ADMIN_MANAGEMENT_POLICY").MustString(defaultAdminManagementPolicy)) Admin.UserDisabledFeatures = container.SetOf(sec.Key("USER_DISABLED_FEATURES").Strings(",")...) Admin.ExternalUserDisableFeatures = container.SetOf(sec.Key("EXTERNAL_USER_DISABLE_FEATURES").Strings(",")...).Union(Admin.UserDisabledFeatures) @@ -67,6 +71,17 @@ func loadAdminFrom(rootCfg ConfigProvider) { } } +func normalizeAdminManagementPolicy(policy string) string { + switch policy { + case AdminManagementPolicyAdminsCanPromote: + return AdminManagementPolicyGrantorOnly + case AdminManagementPolicySuperAdminApproval: + return AdminManagementPolicySuperAdminOnly + default: + return policy + } +} + const ( UserFeatureDeletion = "deletion" UserFeatureManageSSHKeys = "manage_ssh_keys" diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 413e983a23..6fbd1e3113 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -309,6 +309,13 @@ "install.admin_password": "Password", "install.confirm_password": "Confirm Password", "install.admin_email": "Email Address", + "install.admin_management_policy": "Administrator Management Policy", + "install.admin_management_policy.super_admin_only": "Only super admins manage admins", + "install.admin_management_policy.super_admin_only_helper": "Regular admins can manage only non-admin accounts. Any admin-to-admin change or deletion requires a super administrator.", + "install.admin_management_policy.grantor_only": "Grantor manages promoted admins", + "install.admin_management_policy.grantor_only_helper": "Regular admins can promote users to administrator and can later modify or delete only the administrators they directly promoted.", + "install.admin_management_policy.grantor_inheritance": "Grantor chain with inheritance", + "install.admin_management_policy.grantor_inheritance_helper": "Like grantor-only, but if the direct grantor becomes inactive or has sign-in disabled, management passes up the admin grant chain until an eligible admin is found. If none is found, super admins keep control.", "install.install_btn_confirm": "Install Gitea", "install.test_git_failed": "Could not test 'git' command: %v", "install.sqlite3_not_available": "This Gitea version does not support SQLite3. Please download the official binary version from %s (not the 'gobuild' version).", @@ -3091,6 +3098,7 @@ "admin.users.delete_account": "Delete User Account", "admin.users.cannot_delete_self": "You cannot delete yourself", "admin.users.cannot_delete_grantor": "You cannot delete the administrator who granted your privileges.", + "admin.users.admin_grantor.required": "You can only manage administrator accounts that belong to your grant chain.", "admin.users.good_immutable": "Accounts granted by GOOD cannot be modified or deleted.", "admin.users.still_own_repo": "This user still owns one or more repositories. Delete or transfer these repositories first.", "admin.users.still_has_org": "This user is a member of an organization. Remove the user from any organizations first.", diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 0787869320..8d005c9d30 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -96,6 +96,57 @@ func countSuperAdmins(ctx *context.APIContext) int { return count } +func canManageAdminUser(ctx *context.APIContext, doer, target *user_model.User) (bool, error) { + if !target.IsAdmin || doer.ID == target.ID { + return true, nil + } + if isSuperAdmin(ctx, doer.ID) { + return true, nil + } + if !doer.IsAdmin { + return false, nil + } + + switch setting.Admin.AdminManagementPolicy { + case setting.AdminManagementPolicyGrantorOnly: + grantorID, has, err := user_model.GetAdminGrantorID(ctx, target.ID) + if err != nil { + return false, err + } + return has && grantorID == doer.ID, nil + case setting.AdminManagementPolicyGrantorInheritance: + grantorID, has, err := user_model.GetEffectiveAdminGrantorID(ctx, target.ID) + if err != nil { + return false, err + } + return has && grantorID == doer.ID, nil + default: + return false, nil + } +} + +func canManageAdminStatus(ctx *context.APIContext, doer, target *user_model.User, targetRequestedAdmin bool, targetIsSuperAdmin bool) (bool, error) { + targetWasAdmin := target.IsAdmin + if !setting.Admin.SuperAdminEnabled || targetWasAdmin == targetRequestedAdmin { + return true, nil + } + if isSuperAdmin(ctx, doer.ID) { + return true, nil + } + if !doer.IsAdmin || targetIsSuperAdmin { + return false, nil + } + switch setting.Admin.AdminManagementPolicy { + case setting.AdminManagementPolicyGrantorOnly, setting.AdminManagementPolicyGrantorInheritance: + if !targetWasAdmin && targetRequestedAdmin { + return true, nil + } + return canManageAdminUser(ctx, doer, target) + default: + return false, nil + } +} + func canDeleteUserFromAdminAPI(ctx *context.APIContext, doer, target *user_model.User) (bool, error) { if target.ID == doer.ID { return false, errors.New("you cannot delete yourself") @@ -103,6 +154,15 @@ func canDeleteUserFromAdminAPI(ctx *context.APIContext, doer, target *user_model if isBootstrapProtectedUser(ctx, target.ID) { return false, errors.New("GOOD-granted accounts cannot be modified or deleted") } + if target.IsAdmin { + canManage, err := canManageAdminUser(ctx, doer, target) + if err != nil { + return false, err + } + if !canManage { + return false, errors.New("you can only manage administrator accounts that belong to your grant chain") + } + } isCreator, err := user_model.IsGrantorOf(ctx, doer.ID, target.ID) if err != nil { return false, err @@ -270,13 +330,61 @@ func EditUser(ctx *context.APIContext) { ctx.APIError(http.StatusForbidden, errors.New("super admin required")) return } + canManageTargetAdmin, err := canManageAdminUser(ctx, ctx.Doer, ctx.ContextUser) + if err != nil { + ctx.APIErrorInternal(err) + return + } + if !canManageTargetAdmin { + ctx.APIError(http.StatusForbidden, errors.New("you can only manage administrator accounts that belong to your grant chain")) + return + } + + wasAdmin := ctx.ContextUser.IsAdmin + wasActive := ctx.ContextUser.IsActive + wasRestricted := ctx.ContextUser.IsRestricted + wasProhibitLogin := ctx.ContextUser.ProhibitLogin + wasSuperAdmin := isSuperAdmin(ctx, ctx.ContextUser.ID) + + requestedAdmin := wasAdmin + if form.Admin != nil { + requestedAdmin = *form.Admin + } + requestedActive := wasActive + if form.Active != nil { + requestedActive = *form.Active + } + requestedRestricted := wasRestricted + if form.Restricted != nil { + requestedRestricted = *form.Restricted + } + requestedProhibitLogin := wasProhibitLogin + if form.ProhibitLogin != nil { + requestedProhibitLogin = *form.ProhibitLogin + } + + if ctx.Doer.ID == ctx.ContextUser.ID { + requestedAdmin = wasAdmin + requestedActive = wasActive + requestedRestricted = wasRestricted + requestedProhibitLogin = wasProhibitLogin + } + + canManageRequestedAdmin, err := canManageAdminStatus(ctx, ctx.Doer, ctx.ContextUser, requestedAdmin, wasSuperAdmin) + if err != nil { + ctx.APIErrorInternal(err) + return + } + if !canManageRequestedAdmin { + requestedAdmin = wasAdmin + } authOpts := &user_service.UpdateAuthOptions{ LoginSource: optional.FromNonDefault(form.SourceID), LoginName: optional.Some(form.LoginName), Password: optional.FromNonDefault(form.Password), MustChangePassword: optional.FromPtr(form.MustChangePassword), - ProhibitLogin: optional.FromPtr(form.ProhibitLogin), + ProhibitLogin: optional.Some(requestedProhibitLogin), } if err := user_service.UpdateAuth(ctx, ctx.ContextUser, authOpts); err != nil { switch { @@ -314,14 +422,14 @@ func EditUser(ctx *context.APIContext) { Website: optional.FromPtr(form.Website), Location: optional.FromPtr(form.Location), Description: optional.FromPtr(form.Description), - IsActive: optional.FromPtr(form.Active), - IsAdmin: user_service.UpdateOptionFieldFromPtr(form.Admin), + IsActive: optional.Some(requestedActive), + IsAdmin: user_service.UpdateOptionFieldFromValue(requestedAdmin), Visibility: optional.FromMapLookup(api.VisibilityModes, form.Visibility), AllowGitHook: optional.FromPtr(form.AllowGitHook), AllowImportLocal: optional.FromPtr(form.AllowImportLocal), MaxRepoCreation: optional.FromPtr(form.MaxRepoCreation), AllowCreateOrganization: optional.FromPtr(form.AllowCreateOrganization), - IsRestricted: optional.FromPtr(form.Restricted), + IsRestricted: optional.Some(requestedRestricted), } if err := user_service.UpdateUser(ctx, ctx.ContextUser, opts); err != nil { diff --git a/routers/install/install.go b/routers/install/install.go index a0f32fb939..1690058306 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -115,6 +115,7 @@ func Install(ctx *context.Context) { form.DefaultEnableTimetracking = setting.Service.DefaultEnableTimetracking form.NoReplyAddress = setting.Service.NoReplyAddress form.PasswordAlgorithm = hash.ConfigHashAlgorithm(setting.PasswordHashAlgo) + form.AdminManagementPolicy = setting.Admin.AdminManagementPolicy middleware.AssignForm(form, ctx.Data) ctx.HTML(http.StatusOK, tplInstall) @@ -199,6 +200,9 @@ func SubmitInstall(ctx *context.Context) { if form.AppURL != "" && form.AppURL[len(form.AppURL)-1] != '/' { form.AppURL += "/" } + if form.AdminManagementPolicy == "" { + form.AdminManagementPolicy = setting.AdminManagementPolicyGrantorOnly + } ctx.Data["CurDbType"] = form.DbType @@ -393,6 +397,8 @@ func SubmitInstall(ctx *context.Context) { cfg.Section("service").Key("DEFAULT_ENABLE_TIMETRACKING").SetValue(strconv.FormatBool(form.DefaultEnableTimetracking)) cfg.Section("service").Key("NO_REPLY_ADDRESS").SetValue(form.NoReplyAddress) cfg.Section("cron.update_checker").Key("ENABLED").SetValue(strconv.FormatBool(form.EnableUpdateChecker)) + cfg.Section("admin").Key("SUPER_ADMIN_ENABLED").SetValue("true") + cfg.Section("admin").Key("ADMIN_MANAGEMENT_POLICY").SetValue(form.AdminManagementPolicy) cfg.Section("session").Key("PROVIDER").SetValue("file") diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index e28d32f1f3..875547e26d 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -572,17 +572,27 @@ func countSuperAdmins(ctx *context.Context) int { return count } -func canManageAdminStatus(ctx *context.Context, targetWasAdmin, targetRequestedAdmin, targetIsSuperAdmin bool) bool { +func canManageAdminStatus(ctx *context.Context, target *user_model.User, targetRequestedAdmin bool, targetIsSuperAdmin bool) bool { + targetWasAdmin := target.IsAdmin if !setting.Admin.SuperAdminEnabled || targetWasAdmin == targetRequestedAdmin { return true } if isSuperAdmin(ctx, ctx.Doer.ID) { return true } - if setting.Admin.AdminManagementPolicy == setting.AdminManagementPolicyAdminsCanPromote && !targetWasAdmin && targetRequestedAdmin && !targetIsSuperAdmin { - return ctx.Doer.IsAdmin + if !ctx.Doer.IsAdmin || targetIsSuperAdmin { + return false + } + switch setting.Admin.AdminManagementPolicy { + case setting.AdminManagementPolicyGrantorOnly, setting.AdminManagementPolicyGrantorInheritance: + if !targetWasAdmin && targetRequestedAdmin { + return true + } + canManage, _ := canManageAdminUser(ctx, ctx.Doer, target) + return canManage + default: + return false } - return false } func canManageSuperAdminStatus(ctx *context.Context, targetWasSuperAdmin, targetRequestedSuperAdmin bool) bool { @@ -599,6 +609,66 @@ func canEditSuperAdminUser(ctx *context.Context, targetIsSuperAdmin bool) bool { return isSuperAdmin(ctx, ctx.Doer.ID) } +func canManageAdminUser(ctx *context.Context, doer, target *user_model.User) (bool, string) { + if !target.IsAdmin || doer.ID == target.ID { + return true, "" + } + if isSuperAdmin(ctx, doer.ID) { + return true, "" + } + if !doer.IsAdmin { + return false, ctx.Locale.TrString("admin.users.super_admin.required") + } + + switch setting.Admin.AdminManagementPolicy { + case setting.AdminManagementPolicyGrantorOnly: + grantorID, has, err := user_model.GetAdminGrantorID(ctx, target.ID) + if err != nil { + log.Error("GetAdminGrantorID for user %d: %v", target.ID, err) + return false, ctx.Locale.TrString("admin.users.admin_grantor.required") + } + if has && grantorID == doer.ID { + return true, "" + } + return false, ctx.Locale.TrString("admin.users.admin_grantor.required") + case setting.AdminManagementPolicyGrantorInheritance: + grantorID, has, err := user_model.GetEffectiveAdminGrantorID(ctx, target.ID) + if err != nil { + log.Error("GetEffectiveAdminGrantorID for user %d: %v", target.ID, err) + return false, ctx.Locale.TrString("admin.users.admin_grantor.required") + } + if has && grantorID == doer.ID { + return true, "" + } + return false, ctx.Locale.TrString("admin.users.admin_grantor.required") + default: + return false, ctx.Locale.TrString("admin.users.super_admin.required") + } +} + +func canEditAdminStatus(ctx *context.Context, target *user_model.User) bool { + if !setting.Admin.SuperAdminEnabled { + return true + } + if isSuperAdmin(ctx, ctx.Doer.ID) { + return true + } + if !ctx.Doer.IsAdmin || isSuperAdmin(ctx, target.ID) { + return false + } + + switch setting.Admin.AdminManagementPolicy { + case setting.AdminManagementPolicyGrantorOnly, setting.AdminManagementPolicyGrantorInheritance: + if !target.IsAdmin { + return true + } + canManage, _ := canManageAdminUser(ctx, ctx.Doer, target) + return canManage + default: + return false + } +} + func isBootstrapProtectedUser(ctx *context.Context, userID int64) bool { protected, err := user_model.HasBootstrapAdminGrant(ctx, userID) if err != nil { @@ -615,6 +685,9 @@ func canEditUserFromAdminPanel(ctx *context.Context, doer, target *user_model.Us if !canEditSuperAdminUser(ctx, isSuperAdmin(ctx, target.ID)) { return false, ctx.Locale.TrString("admin.users.super_admin.required") } + if target.IsAdmin { + return canManageAdminUser(ctx, doer, target) + } return true, "" } @@ -645,6 +718,13 @@ func canDeleteUserFromAdminPanel(ctx *context.Context, doer, target *user_model. return false, ctx.Locale.TrString("admin.users.good_immutable") } + if target.IsAdmin { + canManage, reason := canManageAdminUser(ctx, doer, target) + if !canManage { + return false, reason + } + } + isCreator, err := user_model.IsGrantorOf(ctx, doer.ID, target.ID) if err != nil { log.Error("IsGrantorOf doer %d target %d: %v", doer.ID, target.ID, err) @@ -987,7 +1067,7 @@ func EditUser(ctx *context.Context) { ctx.Data["SuperAdminEnabled"] = setting.Admin.SuperAdminEnabled ctx.Data["IsUserSuperAdmin"] = targetIsSuperAdmin ctx.Data["CanEditSuperAdmin"] = isSuperAdmin(ctx, ctx.Doer.ID) && ctx.Doer.ID != u.ID - ctx.Data["CanEditAdminStatus"] = !setting.Admin.SuperAdminEnabled || isSuperAdmin(ctx, ctx.Doer.ID) || (setting.Admin.AdminManagementPolicy == setting.AdminManagementPolicyAdminsCanPromote && !u.IsAdmin && !targetIsSuperAdmin) + ctx.Data["CanEditAdminStatus"] = canEditAdminStatus(ctx, u) ctx.Data["IsLastSuperAdminUser"] = targetIsSuperAdmin && countSuperAdmins(ctx) <= 1 ctx.HTML(http.StatusOK, tplUserEdit) @@ -1026,7 +1106,7 @@ func renderAdminUserEditReasonErr(ctx *context.Context, u *user_model.User, form ctx.Data["SuperAdminEnabled"] = setting.Admin.SuperAdminEnabled ctx.Data["IsUserSuperAdmin"] = targetIsSuperAdmin ctx.Data["CanEditSuperAdmin"] = isSuperAdmin(ctx, ctx.Doer.ID) && ctx.Doer.ID != u.ID - ctx.Data["CanEditAdminStatus"] = !setting.Admin.SuperAdminEnabled || isSuperAdmin(ctx, ctx.Doer.ID) || (setting.Admin.AdminManagementPolicy == setting.AdminManagementPolicyAdminsCanPromote && !u.IsAdmin && !targetIsSuperAdmin) + ctx.Data["CanEditAdminStatus"] = canEditAdminStatus(ctx, u) ctx.Data["IsLastSuperAdminUser"] = targetIsSuperAdmin && countSuperAdmins(ctx) <= 1 ctx.Data["IsLastAdminUser"] = user_model.IsLastAdminUser(ctx, u) canDeleteUser, deleteDisabledReason := canDeleteUserFromAdminPanel(ctx, ctx.Doer, u) @@ -1105,7 +1185,7 @@ func EditUserPost(ctx *context.Context) { if !requestedAdmin { requestedSuperAdmin = false } - if !canManageAdminStatus(ctx, wasAdmin, requestedAdmin, wasSuperAdmin) { + if !canManageAdminStatus(ctx, u, requestedAdmin, wasSuperAdmin) { requestedAdmin = wasAdmin requestedSuperAdmin = wasSuperAdmin } diff --git a/services/forms/user_form.go b/services/forms/user_form.go index cc514a2e27..f06ea2eba4 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -57,7 +57,8 @@ type InstallForm struct { EnableUpdateChecker bool NoReplyAddress string - PasswordAlgorithm string + PasswordAlgorithm string + AdminManagementPolicy string `binding:"In(,super_admin_only,grantor_only,grantor_inheritance)" locale:"install.admin_management_policy"` AdminName string `binding:"OmitEmpty;Username;MaxSize(30)" locale:"install.admin_name"` AdminPasswd string `binding:"OmitEmpty;MaxSize(255)" locale:"install.admin_password"` diff --git a/templates/install.tmpl b/templates/install.tmpl index bc6fed08e9..cc0e8dba73 100644 --- a/templates/install.tmpl +++ b/templates/install.tmpl @@ -300,6 +300,32 @@ +
+ +
+
+
+ + +
+

{{ctx.Locale.Tr "install.admin_management_policy.super_admin_only_helper"}}

+
+
+
+ + +
+

{{ctx.Locale.Tr "install.admin_management_policy.grantor_only_helper"}}

+
+
+
+ + +
+

{{ctx.Locale.Tr "install.admin_management_policy.grantor_inheritance_helper"}}

+
+
+
diff --git a/tests/integration/admin_user_test.go b/tests/integration/admin_user_test.go index c9db4dfdae..9321e72a54 100644 --- a/tests/integration/admin_user_test.go +++ b/tests/integration/admin_user_test.go @@ -160,3 +160,78 @@ func TestAdminDeleteAdminButNotGrantor(t *testing.T) { assert.Equal(t, "/-/admin/users/4", resp.Header().Get("Location")) unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) } + +func TestAdminGrantorOnlyCanEditGrantedAdmin(t *testing.T) { + defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.Admin.AdminManagementPolicy, setting.AdminManagementPolicyGrantorOnly)() + + makeAdminUser(t, 2) + makeAdminUser(t, 4) + makeAdminUser(t, 8) + setAdminGrantor(t, 2, 4, "user4", "user4@example.com") + + session := loginUser(t, "user4") + resp := session.MakeRequest(t, NewRequestWithValues(t, "POST", "/-/admin/users/2/edit", map[string]string{ + "user_name": "user2", + "login_name": "user2", + "login_type": "0-0", + "email": "grantor@example.com", + }), http.StatusSeeOther) + assert.Equal(t, "/-/admin/users/2", resp.Header().Get("Location")) + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + assert.Equal(t, "grantor@example.com", user2.Email) + + session = loginUser(t, "user8") + resp = session.MakeRequest(t, NewRequestWithValues(t, "POST", "/-/admin/users/2/edit", map[string]string{ + "user_name": "user2", + "login_name": "user2", + "login_type": "0-0", + "email": "blocked@example.com", + }), http.StatusSeeOther) + assert.Equal(t, "/-/admin/users/2", resp.Header().Get("Location")) + + user2 = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + assert.Equal(t, "grantor@example.com", user2.Email) +} + +func TestAdminGrantorInheritanceUsesParentGrantor(t *testing.T) { + defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.Admin.AdminManagementPolicy, setting.AdminManagementPolicyGrantorInheritance)() + + makeAdminUser(t, 2) + makeAdminUser(t, 4) + makeAdminUser(t, 8) + setAdminGrantor(t, 2, 4, "user4", "user4@example.com") + setAdminGrantor(t, 4, 8, "user8", "user8@example.com") + + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + user4.ProhibitLogin = true + _, err := db.GetEngine(t.Context()).ID(user4.ID).Cols("prohibit_login").Update(user4) + require.NoError(t, err) + + session := loginUser(t, "user8") + resp := session.MakeRequest(t, NewRequestWithValues(t, "POST", "/-/admin/users/2/edit", map[string]string{ + "user_name": "user2", + "login_name": "user2", + "login_type": "0-0", + "email": "inherited@example.com", + }), http.StatusSeeOther) + assert.Equal(t, "/-/admin/users/2", resp.Header().Get("Location")) + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + assert.Equal(t, "inherited@example.com", user2.Email) +} + +func makeAdminUser(t *testing.T, userID int64) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userID}) + user.IsAdmin = true + _, err := db.GetEngine(t.Context()).ID(userID).Cols("is_admin").Update(user) + require.NoError(t, err) +} + +func setAdminGrantor(t *testing.T, userID, grantorID int64, grantorName, grantorEmail string) { + require.NoError(t, user_model.SetUserSetting(t.Context(), userID, user_model.SettingsKeyAdminGrantedBy, strconv.FormatInt(grantorID, 10))) + require.NoError(t, user_model.SetUserSetting(t.Context(), userID, user_model.SettingsKeyAdminGrantedByName, grantorName)) + require.NoError(t, user_model.SetUserSetting(t.Context(), userID, user_model.SettingsKeyAdminGrantedByEmail, grantorEmail)) +} diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index e257bd23b2..425126510b 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -18,9 +18,11 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestAPIAdminCreateAndDeleteSSHKey(t *testing.T) { @@ -257,6 +259,42 @@ func TestAPIAdminCannotEditGoodGrantedUser(t *testing.T) { MakeRequest(t, req, http.StatusForbidden) } +func TestAPIAdminGrantorPolicyForAdminEdits(t *testing.T) { + defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.Admin.AdminManagementPolicy, setting.AdminManagementPolicyGrantorOnly)() + + for _, userID := range []int64{2, 4, 8} { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userID}) + user.IsAdmin = true + _, err := db.GetEngine(t.Context()).ID(userID).Cols("is_admin").Update(user) + require.NoError(t, err) + } + require.NoError(t, user_model.SetUserSetting(t.Context(), 2, user_model.SettingsKeyAdminGrantedBy, "4")) + require.NoError(t, user_model.SetUserSetting(t.Context(), 2, user_model.SettingsKeyAdminGrantedByName, "user4")) + require.NoError(t, user_model.SetUserSetting(t.Context(), 2, user_model.SettingsKeyAdminGrantedByEmail, "user4@example.com")) + + blockedToken := getUserToken(t, "user8", auth_model.AccessTokenScopeWriteAdmin) + fullName := "blocked" + req := NewRequestWithJSON(t, "PATCH", "/api/v1/admin/users/user2", api.EditUserOption{ + LoginName: "user2", + SourceID: 0, + FullName: &fullName, + }).AddTokenAuth(blockedToken) + MakeRequest(t, req, http.StatusForbidden) + + grantorToken := getUserToken(t, "user4", auth_model.AccessTokenScopeWriteAdmin) + allowedName := "grantor edit" + req = NewRequestWithJSON(t, "PATCH", "/api/v1/admin/users/user2", api.EditUserOption{ + LoginName: "user2", + SourceID: 0, + FullName: &allowedName, + }).AddTokenAuth(grantorToken) + MakeRequest(t, req, http.StatusOK) + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + assert.Equal(t, allowedName, user2.FullName) +} + func TestAPICreateRepoForUser(t *testing.T) { defer tests.PrepareTestEnv(t)() adminUsername := "user1"