diff --git a/cmd/admin_user_create.go b/cmd/admin_user_create.go index bf8cbc7c4c0b0..5e03d6ca3f6df 100644 --- a/cmd/admin_user_create.go +++ b/cmd/admin_user_create.go @@ -31,6 +31,11 @@ var microcmdUserCreate = &cli.Command{ Name: "username", Usage: "Username", }, + &cli.StringFlag{ + Name: "user-type", + Usage: "Set user's type: individual or bot", + Value: "individual", + }, &cli.StringFlag{ Name: "password", Usage: "User password", @@ -77,6 +82,22 @@ func runCreateUser(c *cli.Context) error { return err } + userTypes := map[string]user_model.UserType{ + "individual": user_model.UserTypeIndividual, + "bot": user_model.UserTypeBot, + } + userType, ok := userTypes[c.String("user-type")] + if !ok { + return fmt.Errorf("invalid user type: %s", c.String("user-type")) + } + if userType != user_model.UserTypeIndividual { + // Some other commands like "change-password" also only support individual users. + // It needs to clarify the "password" behavior for bot users in the future. + // At the moment, we do not allow setting password for bot users. + if c.IsSet("password") || c.IsSet("random-password") { + return errors.New("password can only be set for individual users") + } + } if c.IsSet("name") && c.IsSet("username") { return errors.New("cannot set both --name and --username flags") } @@ -118,16 +139,19 @@ func runCreateUser(c *cli.Context) error { return err } fmt.Printf("generated random password is '%s'\n", password) - } else { + } else if userType == user_model.UserTypeIndividual { return errors.New("must set either password or random-password flag") } isAdmin := c.Bool("admin") mustChangePassword := true // always default to true if c.IsSet("must-change-password") { + if userType != user_model.UserTypeIndividual { + return errors.New("must-change-password flag can only be set for individual users") + } // if the flag is set, use the value provided by the user mustChangePassword = c.Bool("must-change-password") - } else { + } else if userType == user_model.UserTypeIndividual { // check whether there are users in the database hasUserRecord, err := db.IsTableNotEmpty(&user_model.User{}) if err != nil { @@ -151,8 +175,9 @@ func runCreateUser(c *cli.Context) error { u := &user_model.User{ Name: username, Email: c.String("email"), - Passwd: password, IsAdmin: isAdmin, + Type: userType, + Passwd: password, MustChangePassword: mustChangePassword, Visibility: visibility, } diff --git a/cmd/admin_user_create_test.go b/cmd/admin_user_create_test.go index 83754e97b10a4..d8044e8de7ee3 100644 --- a/cmd/admin_user_create_test.go +++ b/cmd/admin_user_create_test.go @@ -13,32 +13,54 @@ import ( user_model "code.gitea.io/gitea/models/user" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestAdminUserCreate(t *testing.T) { app := NewMainApp(AppVersion{}) reset := func() { - assert.NoError(t, db.TruncateBeans(db.DefaultContext, &user_model.User{})) - assert.NoError(t, db.TruncateBeans(db.DefaultContext, &user_model.EmailAddress{})) + require.NoError(t, db.TruncateBeans(db.DefaultContext, &user_model.User{})) + require.NoError(t, db.TruncateBeans(db.DefaultContext, &user_model.EmailAddress{})) } - type createCheck struct{ IsAdmin, MustChangePassword bool } - createUser := func(name, args string) createCheck { - assert.NoError(t, app.Run(strings.Fields(fmt.Sprintf("./gitea admin user create --username %s --email %s@gitea.local %s --password foobar", name, name, args)))) - u := unittest.AssertExistsAndLoadBean(t, &user_model.User{LowerName: name}) - return createCheck{u.IsAdmin, u.MustChangePassword} - } - reset() - assert.Equal(t, createCheck{IsAdmin: false, MustChangePassword: false}, createUser("u", ""), "first non-admin user doesn't need to change password") - - reset() - assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: false}, createUser("u", "--admin"), "first admin user doesn't need to change password") - - reset() - assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: true}, createUser("u", "--admin --must-change-password")) - assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: true}, createUser("u2", "--admin")) - assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: false}, createUser("u3", "--admin --must-change-password=false")) - assert.Equal(t, createCheck{IsAdmin: false, MustChangePassword: true}, createUser("u4", "")) - assert.Equal(t, createCheck{IsAdmin: false, MustChangePassword: false}, createUser("u5", "--must-change-password=false")) + t.Run("MustChangePassword", func(t *testing.T) { + type check struct { + IsAdmin bool + MustChangePassword bool + } + createCheck := func(name, args string) check { + require.NoError(t, app.Run(strings.Fields(fmt.Sprintf("./gitea admin user create --username %s --email %s@gitea.local %s --password foobar", name, name, args)))) + u := unittest.AssertExistsAndLoadBean(t, &user_model.User{LowerName: name}) + return check{IsAdmin: u.IsAdmin, MustChangePassword: u.MustChangePassword} + } + reset() + assert.Equal(t, check{IsAdmin: false, MustChangePassword: false}, createCheck("u", ""), "first non-admin user doesn't need to change password") + + reset() + assert.Equal(t, check{IsAdmin: true, MustChangePassword: false}, createCheck("u", "--admin"), "first admin user doesn't need to change password") + + reset() + assert.Equal(t, check{IsAdmin: true, MustChangePassword: true}, createCheck("u", "--admin --must-change-password")) + assert.Equal(t, check{IsAdmin: true, MustChangePassword: true}, createCheck("u2", "--admin")) + assert.Equal(t, check{IsAdmin: true, MustChangePassword: false}, createCheck("u3", "--admin --must-change-password=false")) + assert.Equal(t, check{IsAdmin: false, MustChangePassword: true}, createCheck("u4", "")) + assert.Equal(t, check{IsAdmin: false, MustChangePassword: false}, createCheck("u5", "--must-change-password=false")) + }) + + t.Run("UserType", func(t *testing.T) { + createUser := func(name, args string) error { + return app.Run(strings.Fields(fmt.Sprintf("./gitea admin user create --username %s --email %s@gitea.local %s", name, name, args))) + } + + reset() + assert.ErrorContains(t, createUser("u", "--user-type invalid"), "invalid user type") + assert.ErrorContains(t, createUser("u", "--user-type bot --password 123"), "can only be set for individual users") + assert.ErrorContains(t, createUser("u", "--user-type bot --must-change-password"), "can only be set for individual users") + + assert.NoError(t, createUser("u", "--user-type bot")) + u := unittest.AssertExistsAndLoadBean(t, &user_model.User{LowerName: "u"}) + assert.Equal(t, user_model.UserTypeBot, u.Type) + assert.Equal(t, "", u.Passwd) + }) } diff --git a/models/user/user.go b/models/user/user.go index e13fb6ab3c3f2..293c8769577ad 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -385,11 +385,12 @@ func (u *User) ValidatePassword(passwd string) bool { } // IsPasswordSet checks if the password is set or left empty +// TODO: It's better to clarify the "password" behavior for different types (individual, bot) func (u *User) IsPasswordSet() bool { - return len(u.Passwd) != 0 + return u.Passwd != "" } -// IsOrganization returns true if user is actually a organization. +// IsOrganization returns true if user is actually an organization. func (u *User) IsOrganization() bool { return u.Type == UserTypeOrganization } @@ -399,13 +400,14 @@ func (u *User) IsIndividual() bool { return u.Type == UserTypeIndividual } -func (u *User) IsUser() bool { - return u.Type == UserTypeIndividual || u.Type == UserTypeBot +// IsTypeBot returns whether the user is of type bot +func (u *User) IsTypeBot() bool { + return u.Type == UserTypeBot } -// IsBot returns whether or not the user is of type bot -func (u *User) IsBot() bool { - return u.Type == UserTypeBot +// IsTokenAccessAllowed returns whether the user is an individual or a bot (which allows for token access) +func (u *User) IsTokenAccessAllowed() bool { + return u.Type == UserTypeIndividual || u.Type == UserTypeBot } // DisplayName returns full name if it's not empty, diff --git a/models/user/user_system.go b/models/user/user_system.go index e54973dc8e597..6fbfd9e69e5b1 100644 --- a/models/user/user_system.go +++ b/models/user/user_system.go @@ -56,7 +56,7 @@ func NewActionsUser() *User { Email: ActionsUserEmail, KeepEmailPrivate: true, LoginName: ActionsUserName, - Type: UserTypeIndividual, + Type: UserTypeBot, AllowCreateOrganization: true, Visibility: structs.VisibleTypePublic, } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 0aa38b8b6abbb..2ffd6b129b9be 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -268,12 +268,12 @@ func checkTokenPublicOnly() func(ctx *context.APIContext) { return } case auth_model.ContainsCategory(requiredScopeCategories, auth_model.AccessTokenScopeCategoryUser): - if ctx.ContextUser != nil && ctx.ContextUser.IsUser() && ctx.ContextUser.Visibility != api.VisibleTypePublic { + if ctx.ContextUser != nil && ctx.ContextUser.IsTokenAccessAllowed() && ctx.ContextUser.Visibility != api.VisibleTypePublic { ctx.Error(http.StatusForbidden, "reqToken", "token scope is limited to public users") return } case auth_model.ContainsCategory(requiredScopeCategories, auth_model.AccessTokenScopeCategoryActivityPub): - if ctx.ContextUser != nil && ctx.ContextUser.IsUser() && ctx.ContextUser.Visibility != api.VisibleTypePublic { + if ctx.ContextUser != nil && ctx.ContextUser.IsTokenAccessAllowed() && ctx.ContextUser.Visibility != api.VisibleTypePublic { ctx.Error(http.StatusForbidden, "reqToken", "token scope is limited to public activitypub") return } diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index aa49d2e1e8648..aeb2fa52b6ea9 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -4,7 +4,6 @@ package repo import ( - stdCtx "context" "fmt" "math/big" "net/http" @@ -40,86 +39,80 @@ import ( ) // roleDescriptor returns the role descriptor for a comment in/with the given repo, poster and issue -func roleDescriptor(ctx stdCtx.Context, repo *repo_model.Repository, poster *user_model.User, permsCache map[int64]access_model.Permission, issue *issues_model.Issue, hasOriginalAuthor bool) (issues_model.RoleDescriptor, error) { - roleDescriptor := issues_model.RoleDescriptor{} - +func roleDescriptor(ctx *context.Context, repo *repo_model.Repository, poster *user_model.User, permsCache map[int64]access_model.Permission, issue *issues_model.Issue, hasOriginalAuthor bool) (roleDesc issues_model.RoleDescriptor, err error) { if hasOriginalAuthor { - return roleDescriptor, nil + // the poster is a migrated user, so no need to detect the role + return roleDesc, nil } - var perm access_model.Permission - var err error - if permsCache != nil { - var ok bool - perm, ok = permsCache[poster.ID] - if !ok { - perm, err = access_model.GetUserRepoPermission(ctx, repo, poster) - if err != nil { - return roleDescriptor, err - } - } - permsCache[poster.ID] = perm - } else { + if poster.IsGhost() || !poster.IsIndividual() { + return roleDesc, nil + } + + roleDesc.IsPoster = issue.IsPoster(poster.ID) // check whether the comment's poster is the issue's poster + + // Guess the role of the poster in the repo by permission + perm, hasPermCache := permsCache[poster.ID] + if !hasPermCache { perm, err = access_model.GetUserRepoPermission(ctx, repo, poster) if err != nil { - return roleDescriptor, err + return roleDesc, err } } - - // If the poster is the actual poster of the issue, enable Poster role. - roleDescriptor.IsPoster = issue.IsPoster(poster.ID) + if permsCache != nil { + permsCache[poster.ID] = perm + } // Check if the poster is owner of the repo. if perm.IsOwner() { - // If the poster isn't an admin, enable the owner role. + // If the poster isn't a site admin, then is must be the repo's owner if !poster.IsAdmin { - roleDescriptor.RoleInRepo = issues_model.RoleRepoOwner - return roleDescriptor, nil + roleDesc.RoleInRepo = issues_model.RoleRepoOwner + return roleDesc, nil } - - // Otherwise check if poster is the real repo admin. - ok, err := access_model.IsUserRealRepoAdmin(ctx, repo, poster) + // Otherwise (poster is site admin), check if poster is the real repo admin. + isRealRepoAdmin, err := access_model.IsUserRealRepoAdmin(ctx, repo, poster) if err != nil { - return roleDescriptor, err + return roleDesc, err } - if ok { - roleDescriptor.RoleInRepo = issues_model.RoleRepoOwner - return roleDescriptor, nil + if isRealRepoAdmin { + roleDesc.RoleInRepo = issues_model.RoleRepoOwner + return roleDesc, nil } } // If repo is organization, check Member role - if err := repo.LoadOwner(ctx); err != nil { - return roleDescriptor, err + if err = repo.LoadOwner(ctx); err != nil { + return roleDesc, err } if repo.Owner.IsOrganization() { if isMember, err := organization.IsOrganizationMember(ctx, repo.Owner.ID, poster.ID); err != nil { - return roleDescriptor, err + return roleDesc, err } else if isMember { - roleDescriptor.RoleInRepo = issues_model.RoleRepoMember - return roleDescriptor, nil + roleDesc.RoleInRepo = issues_model.RoleRepoMember + return roleDesc, nil } } // If the poster is the collaborator of the repo if isCollaborator, err := repo_model.IsCollaborator(ctx, repo.ID, poster.ID); err != nil { - return roleDescriptor, err + return roleDesc, err } else if isCollaborator { - roleDescriptor.RoleInRepo = issues_model.RoleRepoCollaborator - return roleDescriptor, nil + roleDesc.RoleInRepo = issues_model.RoleRepoCollaborator + return roleDesc, nil } hasMergedPR, err := issues_model.HasMergedPullRequestInRepo(ctx, repo.ID, poster.ID) if err != nil { - return roleDescriptor, err + return roleDesc, err } else if hasMergedPR { - roleDescriptor.RoleInRepo = issues_model.RoleRepoContributor + roleDesc.RoleInRepo = issues_model.RoleRepoContributor } else if issue.IsPull { // only display first time contributor in the first opening pull request - roleDescriptor.RoleInRepo = issues_model.RoleRepoFirstTimeContributor + roleDesc.RoleInRepo = issues_model.RoleRepoFirstTimeContributor } - return roleDescriptor, nil + return roleDesc, nil } func getBranchData(ctx *context.Context, issue *issues_model.Issue) { diff --git a/templates/shared/user/authorlink.tmpl b/templates/shared/user/authorlink.tmpl index abe1ab1ce229c..abfee6aae3624 100644 --- a/templates/shared/user/authorlink.tmpl +++ b/templates/shared/user/authorlink.tmpl @@ -1 +1 @@ -{{.GetDisplayName}}{{if .IsBot}}bot{{end}} +{{.GetDisplayName}}{{if .IsTypeBot}}bot{{end}}