Skip to content

Commit 0a09b13

Browse files
committed
Merge remote-tracking branch 'giteaofficial/release/v1.25'
* giteaofficial/release/v1.25: (77 commits) Add "site admin" back to profile menu (go-gitea#36010) (go-gitea#36013) release notes for 1.25.2 (go-gitea#35986) Allow empty commit when merging pull request with squash style (go-gitea#35989) (go-gitea#36003) Fix various permission & login related bugs (go-gitea#36002) (go-gitea#36004) upgrade golang.org/x/crypto to 0.45.0 (go-gitea#35988) Change project default column icon to 'star' (go-gitea#35967) (go-gitea#35979) Misc CSS fixes (go-gitea#35888) (go-gitea#35981) Fix container push tag overwriting (go-gitea#35936) (go-gitea#35954) Fix corrupted external render content (go-gitea#35946) (go-gitea#35950) Don't show unnecessary error message to end users for DeleteBranchAfterMerge (go-gitea#35937) (go-gitea#35941) Limit read bytes instead of ReadAll (go-gitea#35928) (go-gitea#35934) Load jQuery as early as possible to support custom scripts (go-gitea#35926) (go-gitea#35929) Allow to display embed images/pdfs when SERVE_DIRECT was enabled on MinIO storage (go-gitea#35882) (go-gitea#35917) Use correct form field for allowed force push users in branch protection API (go-gitea#35894) (go-gitea#35908) Make OAuth2 issuer configurable (go-gitea#35915) (go-gitea#35916) Fix go-gitea#35763: Add proper page title for project pages (go-gitea#35773) (go-gitea#35909) Display source code downloads last for release attachments (go-gitea#35897) (go-gitea#35903) Fix team member access check (go-gitea#35899) (go-gitea#35905) Fix conda null depend issue (go-gitea#35900) (go-gitea#35902) Fix avatar upload error handling (go-gitea#35887) (go-gitea#35890) ... # Conflicts: # go.mod # go.sum # models/actions/run_test.go # models/fixtures/action_run.yml # models/fixtures/action_run_job.yml # models/fixtures/action_task.yml # models/fixtures/branch.yml # models/fixtures/repo_unit.yml # modules/git/tree_entry_gogit.go # modules/git/tree_gogit.go # routers/web/repo/actions/view.go # routers/web/repo/issue_comment.go # services/actions/workflow.go # services/doctor/actions_test.go # services/pull/comment.go # services/pull/pull.go # services/pull/temp_repo.go # templates/base/head_navbar.tmpl # templates/swagger/v1_json.tmpl # tests/integration/actions_schedule_test.go # tests/integration/git_lfs_ssh_test.go # tests/integration/pull_create_test.go # tests/integration/pull_merge_test.go # tests/sqlite.ini.tmpl # web_src/js/components/ContextPopup.vue
2 parents 4030b8b + 688430e commit 0a09b13

File tree

12 files changed

+234
-69
lines changed

12 files changed

+234
-69
lines changed

routers/api/v1/admin/user.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ func RenameUser(ctx *context.APIContext) {
480480
newName := web.GetForm(ctx).(*api.RenameUserOption).NewName
481481

482482
// Check if username has been changed
483-
if err := user_service.RenameUser(ctx, ctx.ContextUser, newName); err != nil {
483+
if err := user_service.RenameUser(ctx, ctx.ContextUser, newName, ctx.Doer); err != nil {
484484
if user_model.IsErrUserAlreadyExist(err) || db.IsErrNameReserved(err) || db.IsErrNamePatternNotAllowed(err) || db.IsErrNameCharsNotAllowed(err) {
485485
ctx.APIError(http.StatusUnprocessableEntity, err)
486486
} else {

routers/api/v1/org/org.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ func Rename(ctx *context.APIContext) {
340340

341341
form := web.GetForm(ctx).(*api.RenameOrgOption)
342342
orgUser := ctx.Org.Organization.AsUser()
343-
if err := user_service.RenameUser(ctx, orgUser, form.NewName); err != nil {
343+
if err := user_service.RenameUser(ctx, orgUser, form.NewName, ctx.Doer); err != nil {
344344
if user_model.IsErrUserAlreadyExist(err) || db.IsErrNameReserved(err) || db.IsErrNamePatternNotAllowed(err) || db.IsErrNameCharsNotAllowed(err) {
345345
ctx.APIError(http.StatusUnprocessableEntity, err)
346346
} else {

routers/web/admin/users.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ func EditUserPost(ctx *context.Context) {
345345
}
346346

347347
if form.UserName != "" {
348-
if err := user_service.RenameUser(ctx, u, form.UserName); err != nil {
348+
if err := user_service.RenameUser(ctx, u, form.UserName, ctx.Doer); err != nil {
349349
switch {
350350
case user_model.IsErrUserIsNotLocal(err):
351351
ctx.Data["Err_UserName"] = true

routers/web/org/setting.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ func SettingsRenamePost(ctx *context.Context) {
213213
return
214214
}
215215

216-
if err := user_service.RenameUser(ctx, ctx.Org.Organization.AsUser(), newOrgName); err != nil {
216+
if err := user_service.RenameUser(ctx, ctx.Org.Organization.AsUser(), newOrgName, ctx.Doer); err != nil {
217217
if user_model.IsErrUserAlreadyExist(err) {
218218
ctx.JSONError(ctx.Tr("org.form.name_been_taken", newOrgName))
219219
} else if db.IsErrNameReserved(err) {

routers/web/user/setting/profile.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func ProfilePost(ctx *context.Context) {
7575
ctx.Redirect(setting.AppSubURL + "/user/settings")
7676
return
7777
}
78-
if err := user_service.RenameUser(ctx, ctx.Doer, form.Name); err != nil {
78+
if err := user_service.RenameUser(ctx, ctx.Doer, form.Name, ctx.Doer); err != nil {
7979
switch {
8080
case user_model.IsErrUserIsNotLocal(err):
8181
ctx.Flash.Error(ctx.Tr("form.username_change_not_local_user"))

services/pull/pull.go

Lines changed: 42 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"regexp"
1414
"strings"
1515
"time"
16+
"unicode/utf8"
1617

1718
"code.gitea.io/gitea/models/db"
1819
git_model "code.gitea.io/gitea/models/git"
@@ -838,64 +839,66 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ
838839
stringBuilder := strings.Builder{}
839840

840841
if !setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages {
842+
// use PR's title and description as squash commit message
841843
message := strings.TrimSpace(pr.Issue.Content)
842844
stringBuilder.WriteString(message)
843845
if stringBuilder.Len() > 0 {
844846
stringBuilder.WriteRune('\n')
845847
if !commitMessageTrailersPattern.MatchString(message) {
848+
// TODO: this trailer check doesn't work with the separator line added below for the co-authors
846849
stringBuilder.WriteRune('\n')
847850
}
848851
}
849-
}
850-
851-
// commits list is in reverse chronological order
852-
first := true
853-
for i := len(commits) - 1; i >= 0; i-- {
854-
commit := commits[i]
855-
856-
if setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages {
857-
maxSize := setting.Repository.PullRequest.DefaultMergeMessageSize
858-
if maxSize < 0 || stringBuilder.Len() < maxSize {
859-
var toWrite []byte
860-
if first {
861-
first = false
862-
toWrite = []byte(strings.TrimPrefix(commit.CommitMessage, pr.Issue.Title))
863-
} else {
864-
toWrite = []byte(commit.CommitMessage)
865-
}
866-
867-
if len(toWrite) > maxSize-stringBuilder.Len() && maxSize > -1 {
868-
toWrite = append(toWrite[:maxSize-stringBuilder.Len()], "..."...)
869-
}
870-
if _, err := stringBuilder.Write(toWrite); err != nil {
871-
log.Error("Unable to write commit message Error: %v", err)
872-
return ""
873-
}
852+
} else {
853+
// use PR's commit messages as squash commit message
854+
// commits list is in reverse chronological order
855+
maxMsgSize := setting.Repository.PullRequest.DefaultMergeMessageSize
856+
for i := len(commits) - 1; i >= 0; i-- {
857+
commit := commits[i]
858+
msg := strings.TrimSpace(commit.CommitMessage)
859+
if msg == "" {
860+
continue
861+
}
874862

875-
if _, err := stringBuilder.WriteRune('\n'); err != nil {
876-
log.Error("Unable to write commit message Error: %v", err)
877-
return ""
863+
// This format follows GitHub's squash commit message style,
864+
// even if there are other "* " in the commit message body, they are written as-is.
865+
// Maybe, ideally, we should indent those lines too.
866+
_, _ = fmt.Fprintf(&stringBuilder, "* %s\n\n", msg)
867+
if maxMsgSize > 0 && stringBuilder.Len() >= maxMsgSize {
868+
tmp := stringBuilder.String()
869+
wasValidUtf8 := utf8.ValidString(tmp)
870+
tmp = tmp[:maxMsgSize] + "..."
871+
if wasValidUtf8 {
872+
// If the message was valid UTF-8 before truncation, ensure it remains valid after truncation
873+
// For non-utf8 messages, we can't do much about it, end users should use utf-8 as much as possible
874+
tmp = strings.ToValidUTF8(tmp, "")
878875
}
876+
stringBuilder.Reset()
877+
stringBuilder.WriteString(tmp)
878+
break
879879
}
880880
}
881+
}
881882

883+
// collect co-authors
884+
for _, commit := range commits {
882885
authorString := commit.Author.String()
883886
if uniqueAuthors.Add(authorString) && authorString != posterSig {
884887
// Compare use account as well to avoid adding the same author multiple times
885-
// times when email addresses are private or multiple emails are used.
888+
// when email addresses are private or multiple emails are used.
886889
commitUser, _ := user_model.GetUserByEmail(ctx, commit.Author.Email)
887890
if commitUser == nil || commitUser.ID != pr.Issue.Poster.ID {
888891
authors = append(authors, authorString)
889892
}
890893
}
891894
}
892895

893-
// Consider collecting the remaining authors
896+
// collect the remaining authors
894897
if limit >= 0 && setting.Repository.PullRequest.DefaultMergeMessageAllAuthors {
895898
skip := limit
896899
limit = 30
897900
for {
898-
commits, err := gitRepo.CommitsBetweenLimit(headCommit, mergeBase, limit, skip)
901+
commits, err = gitRepo.CommitsBetweenLimit(headCommit, mergeBase, limit, skip)
899902
if err != nil {
900903
log.Error("Unable to get commits between: %s %s Error: %v", pr.HeadBranch, pr.MergeBase, err)
901904
return ""
@@ -916,19 +919,15 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ
916919
}
917920
}
918921

922+
if stringBuilder.Len() > 0 && len(authors) > 0 {
923+
// TODO: this separator line doesn't work with the trailer check (commitMessageTrailersPattern) above
924+
stringBuilder.WriteString("---------\n\n")
925+
}
926+
919927
for _, author := range authors {
920-
if _, err := stringBuilder.WriteString("Co-authored-by: "); err != nil {
921-
log.Error("Unable to write to string builder Error: %v", err)
922-
return ""
923-
}
924-
if _, err := stringBuilder.WriteString(author); err != nil {
925-
log.Error("Unable to write to string builder Error: %v", err)
926-
return ""
927-
}
928-
if _, err := stringBuilder.WriteRune('\n'); err != nil {
929-
log.Error("Unable to write to string builder Error: %v", err)
930-
return ""
931-
}
928+
stringBuilder.WriteString("Co-authored-by: ")
929+
stringBuilder.WriteString(author)
930+
stringBuilder.WriteRune('\n')
932931
}
933932

934933
return stringBuilder.String()

services/user/user.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,15 @@ import (
3131
)
3232

3333
// RenameUser renames a user
34-
func RenameUser(ctx context.Context, u *user_model.User, newUserName string) error {
34+
func RenameUser(ctx context.Context, u *user_model.User, newUserName string, doer *user_model.User) error {
3535
if newUserName == u.Name {
3636
return nil
3737
}
3838

39-
// Non-local users are not allowed to change their username.
40-
if !u.IsOrganization() && !u.IsLocal() {
41-
return user_model.ErrUserIsNotLocal{
42-
UID: u.ID,
43-
Name: u.Name,
44-
}
39+
// Non-local users are not allowed to change their own username, but admins are
40+
isExternalUser := !u.IsOrganization() && !u.IsLocal()
41+
if isExternalUser && !doer.IsAdmin {
42+
return user_model.ErrUserIsNotLocal{UID: u.ID, Name: u.Name}
4543
}
4644

4745
if err := user_model.IsUsableUsername(newUserName); err != nil {

services/user/user_test.go

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
org_service "code.gitea.io/gitea/services/org"
2121

2222
"github.com/stretchr/testify/assert"
23+
"github.com/stretchr/testify/require"
2324
)
2425

2526
func TestMain(m *testing.M) {
@@ -101,23 +102,31 @@ func TestRenameUser(t *testing.T) {
101102
assert.NoError(t, unittest.PrepareTestDatabase())
102103
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 21})
103104

104-
t.Run("Non-Local", func(t *testing.T) {
105-
u := &user_model.User{
106-
Type: user_model.UserTypeIndividual,
107-
LoginType: auth.OAuth2,
105+
t.Run("External user", func(t *testing.T) {
106+
adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1, IsAdmin: true})
107+
externalUser := &user_model.User{
108+
Name: "external_user",
109+
110+
LoginType: auth.LDAP,
108111
}
109-
assert.ErrorIs(t, RenameUser(t.Context(), u, "user_rename"), user_model.ErrUserIsNotLocal{})
112+
require.NoError(t, user_model.CreateUser(t.Context(), externalUser, &user_model.Meta{}))
113+
114+
err := RenameUser(t.Context(), externalUser, externalUser.Name+"_changed", externalUser)
115+
assert.True(t, user_model.IsErrUserIsNotLocal(err), "external user is not allowed to rename themselves")
116+
117+
err = RenameUser(t.Context(), externalUser, externalUser.Name+"_changed", adminUser)
118+
assert.NoError(t, err, "admin can rename external user")
110119
})
111120

112121
t.Run("Same username", func(t *testing.T) {
113-
assert.NoError(t, RenameUser(t.Context(), user, user.Name))
122+
assert.NoError(t, RenameUser(t.Context(), user, user.Name, user))
114123
})
115124

116125
t.Run("Non usable username", func(t *testing.T) {
117126
usernames := []string{"--diff", ".well-known", "gitea-actions", "aaa.atom", "aa.png"}
118127
for _, username := range usernames {
119128
assert.Error(t, user_model.IsUsableUsername(username), "non-usable username: %s", username)
120-
assert.Error(t, RenameUser(t.Context(), user, username), "non-usable username: %s", username)
129+
assert.Error(t, RenameUser(t.Context(), user, username, user), "non-usable username: %s", username)
121130
}
122131
})
123132

@@ -126,7 +135,7 @@ func TestRenameUser(t *testing.T) {
126135
unittest.AssertNotExistsBean(t, &user_model.User{ID: user.ID, Name: caps})
127136
unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: user.ID, OwnerName: user.Name})
128137

129-
assert.NoError(t, RenameUser(t.Context(), user, caps))
138+
assert.NoError(t, RenameUser(t.Context(), user, caps, user))
130139

131140
unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: user.ID, Name: caps})
132141
unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: user.ID, OwnerName: caps})
@@ -135,17 +144,17 @@ func TestRenameUser(t *testing.T) {
135144
t.Run("Already exists", func(t *testing.T) {
136145
existUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
137146

138-
assert.ErrorIs(t, RenameUser(t.Context(), user, existUser.Name), user_model.ErrUserAlreadyExist{Name: existUser.Name})
139-
assert.ErrorIs(t, RenameUser(t.Context(), user, existUser.LowerName), user_model.ErrUserAlreadyExist{Name: existUser.LowerName})
147+
assert.ErrorIs(t, RenameUser(t.Context(), user, existUser.Name, user), user_model.ErrUserAlreadyExist{Name: existUser.Name})
148+
assert.ErrorIs(t, RenameUser(t.Context(), user, existUser.LowerName, user), user_model.ErrUserAlreadyExist{Name: existUser.LowerName})
140149
newUsername := fmt.Sprintf("uSEr%d", existUser.ID)
141-
assert.ErrorIs(t, RenameUser(t.Context(), user, newUsername), user_model.ErrUserAlreadyExist{Name: newUsername})
150+
assert.ErrorIs(t, RenameUser(t.Context(), user, newUsername, user), user_model.ErrUserAlreadyExist{Name: newUsername})
142151
})
143152

144153
t.Run("Normal", func(t *testing.T) {
145154
oldUsername := user.Name
146155
newUsername := "User_Rename"
147156

148-
assert.NoError(t, RenameUser(t.Context(), user, newUsername))
157+
assert.NoError(t, RenameUser(t.Context(), user, newUsername, user))
149158
unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: user.ID, Name: newUsername, LowerName: strings.ToLower(newUsername)})
150159

151160
redirectUID, err := user_model.LookupUserRedirect(t.Context(), oldUsername)

templates/admin/user/edit.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
{{.CsrfTokenHtml}}
1010
<div class="field {{if .Err_UserName}}error{{end}}">
1111
<label for="user_name">{{ctx.Locale.Tr "username"}}</label>
12-
<input id="user_name" name="user_name" value="{{.User.Name}}" {{if not .User.IsLocal}}disabled{{end}} maxlength="40">
12+
<input id="user_name" name="user_name" value="{{.User.Name}}" maxlength="40">
1313
</div>
1414
<!-- Types and name -->
1515
<div class="inline required field {{if .Err_LoginType}}error{{end}}">

templates/base/head_navbar.tmpl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,13 @@
124124
{{ctx.Locale.Tr "help"}}
125125
</a>
126126
</div>
127+
{{if .IsAdmin}}
128+
<div class="divider"></div>
129+
<a class="{{if .PageIsAdmin}}active {{end}}item" href="{{AppSubUrl}}/-/admin">
130+
{{svg "octicon-server"}}
131+
{{ctx.Locale.Tr "admin_panel"}}
132+
</a>
133+
{{end}}
127134
<div class="divider"></div>
128135
<a class="item link-action" href data-url="{{AppSubUrl}}/user/logout">
129136
{{svg "octicon-sign-out"}}

0 commit comments

Comments
 (0)