Skip to content

Commit

Permalink
Merge remote-tracking branch 'giteaofficial/main'
Browse files Browse the repository at this point in the history
* giteaofficial/main:
  Send mail to issue/pr assignee/reviewer also when OnMention is set (go-gitea#18707)
  Reduce CI go module downloads, add make targets (go-gitea#18708)
  Add number in queue status to monitor page (go-gitea#18712)
  Fix source code line highlighting (go-gitea#18729)
  Fix forked repositories missed tags (go-gitea#18719)
  [skip ci] Updated translations via Crowdin
  Fix release typo (go-gitea#18728)
  Display template path of current page in dev mode (go-gitea#18717)
  Separate the details links of commit-statuses in headers (go-gitea#18661)
  Add LDAP group sync to Teams, fixes go-gitea#1395 (go-gitea#16299)
  Change git.cmd to RunWithContext (go-gitea#18693)
  • Loading branch information
zjjhot committed Feb 12, 2022
2 parents 0f73b87 + a39287c commit be1b906
Show file tree
Hide file tree
Showing 61 changed files with 1,284 additions and 310 deletions.
255 changes: 194 additions & 61 deletions .drone.yml

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ linters:

run:
timeout: 3m
skip-dirs:
- node_modules
- public
- web_src

linters-settings:
gocritic:
Expand Down
13 changes: 13 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ help:
@echo " - watch-backend watch backend files and continuously rebuild"
@echo " - clean delete backend and integration files"
@echo " - clean-all delete backend, frontend and integration files"
@echo " - deps install dependencies"
@echo " - deps-frontend install frontend dependencies"
@echo " - deps-backend install backend dependencies"
@echo " - lint lint everything"
@echo " - lint-frontend lint frontend files"
@echo " - lint-backend lint backend files"
Expand Down Expand Up @@ -662,6 +665,16 @@ docs:
fi
cd docs; make trans-copy clean build-offline;

.PHONY: deps
deps: deps-frontend deps-backend

.PHONY: deps-frontend
deps-frontend: node_modules

.PHONY: deps-backend
deps-backend:
$(GO) mod download

node_modules: package-lock.json
npm install --no-save
@touch node_modules
Expand Down
1 change: 0 additions & 1 deletion cmd/admin_auth_ldap.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@ func parseLdapConfig(c *cli.Context, config *ldap.Source) error {
if c.IsSet("skip-local-2fa") {
config.SkipLocalTwoFA = c.Bool("skip-local-2fa")
}

return nil
}

Expand Down
119 changes: 118 additions & 1 deletion integrations/auth_ldap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import (
"strings"
"testing"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/services/auth"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -97,7 +100,13 @@ func getLDAPServerHost() string {
return host
}

func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string) {
func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string, groupMapParams ...string) {
groupTeamMapRemoval := "off"
groupTeamMap := ""
if len(groupMapParams) == 2 {
groupTeamMapRemoval = groupMapParams[0]
groupTeamMap = groupMapParams[1]
}
session := loginUser(t, "user1")
csrf := GetCSRF(t, session, "/admin/auths/new")
req := NewRequestWithValues(t, "POST", "/admin/auths/new", map[string]string{
Expand All @@ -119,6 +128,12 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string) {
"attribute_ssh_public_key": sshKeyAttribute,
"is_sync_enabled": "on",
"is_active": "on",
"groups_enabled": "on",
"group_dn": "ou=people,dc=planetexpress,dc=com",
"group_member_uid": "member",
"group_team_map": groupTeamMap,
"group_team_map_removal": groupTeamMapRemoval,
"user_uid": "DN",
})
session.MakeRequest(t, req, http.StatusFound)
}
Expand Down Expand Up @@ -294,3 +309,105 @@ func TestLDAPUserSSHKeySync(t *testing.T) {
assert.ElementsMatch(t, u.SSHKeys, syncedKeys, "Unequal number of keys synchronized for user: %s", u.UserName)
}
}

func TestLDAPGroupTeamSyncAddMember(t *testing.T) {
if skipLDAPTests() {
t.Skip()
return
}
defer prepareTestEnv(t)()
addAuthSourceLDAP(t, "", "on", `{"cn=ship_crew,ou=people,dc=planetexpress,dc=com":{"org26": ["team11"]},"cn=admin_staff,ou=people,dc=planetexpress,dc=com": {"non-existent": ["non-existent"]}}`)
org, err := models.GetOrgByName("org26")
assert.NoError(t, err)
team, err := models.GetTeam(org.ID, "team11")
assert.NoError(t, err)
auth.SyncExternalUsers(context.Background(), true)
for _, gitLDAPUser := range gitLDAPUsers {
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{
Name: gitLDAPUser.UserName,
}).(*user_model.User)
usersOrgs, err := models.FindOrgs(models.FindOrgOptions{
UserID: user.ID,
IncludePrivate: true,
})
assert.NoError(t, err)
allOrgTeams, err := models.GetUserOrgTeams(org.ID, user.ID)
assert.NoError(t, err)
if user.Name == "fry" || user.Name == "leela" || user.Name == "bender" {
// assert members of LDAP group "cn=ship_crew" are added to mapped teams
assert.Equal(t, len(usersOrgs), 1, "User [%s] should be member of one organization", user.Name)
assert.Equal(t, usersOrgs[0].Name, "org26", "Membership should be added to the right organization")
isMember, err := models.IsTeamMember(usersOrgs[0].ID, team.ID, user.ID)
assert.NoError(t, err)
assert.True(t, isMember, "Membership should be added to the right team")
err = team.RemoveMember(user.ID)
assert.NoError(t, err)
err = usersOrgs[0].RemoveMember(user.ID)
assert.NoError(t, err)
} else {
// assert members of LDAP group "cn=admin_staff" keep initial team membership since mapped team does not exist
assert.Empty(t, usersOrgs, "User should be member of no organization")
isMember, err := models.IsTeamMember(org.ID, team.ID, user.ID)
assert.NoError(t, err)
assert.False(t, isMember, "User should no be added to this team")
assert.Empty(t, allOrgTeams, "User should not be added to any team")
}
}
}

func TestLDAPGroupTeamSyncRemoveMember(t *testing.T) {
if skipLDAPTests() {
t.Skip()
return
}
defer prepareTestEnv(t)()
addAuthSourceLDAP(t, "", "on", `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`)
org, err := models.GetOrgByName("org26")
assert.NoError(t, err)
team, err := models.GetTeam(org.ID, "team11")
assert.NoError(t, err)
loginUserWithPassword(t, gitLDAPUsers[0].UserName, gitLDAPUsers[0].Password)
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{
Name: gitLDAPUsers[0].UserName,
}).(*user_model.User)
err = org.AddMember(user.ID)
assert.NoError(t, err)
err = team.AddMember(user.ID)
assert.NoError(t, err)
isMember, err := models.IsOrganizationMember(org.ID, user.ID)
assert.NoError(t, err)
assert.True(t, isMember, "User should be member of this organization")
isMember, err = models.IsTeamMember(org.ID, team.ID, user.ID)
assert.NoError(t, err)
assert.True(t, isMember, "User should be member of this team")
// assert team member "professor" gets removed from org26 team11
loginUserWithPassword(t, gitLDAPUsers[0].UserName, gitLDAPUsers[0].Password)
isMember, err = models.IsOrganizationMember(org.ID, user.ID)
assert.NoError(t, err)
assert.False(t, isMember, "User membership should have been removed from organization")
isMember, err = models.IsTeamMember(org.ID, team.ID, user.ID)
assert.NoError(t, err)
assert.False(t, isMember, "User membership should have been removed from team")
}

// Login should work even if Team Group Map contains a broken JSON
func TestBrokenLDAPMapUserSignin(t *testing.T) {
if skipLDAPTests() {
t.Skip()
return
}
defer prepareTestEnv(t)()
addAuthSourceLDAP(t, "", "on", `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`)

u := gitLDAPUsers[0]

session := loginUserWithPassword(t, u.UserName, u.Password)
req := NewRequest(t, "GET", "/user/settings")
resp := session.MakeRequest(t, req, http.StatusOK)

htmlDoc := NewHTMLParser(t, resp.Body)

assert.Equal(t, u.UserName, htmlDoc.GetInputValueByName("name"))
assert.Equal(t, u.FullName, htmlDoc.GetInputValueByName("full_name"))
assert.Equal(t, u.Email, htmlDoc.Find(`label[for="email"]`).Siblings().First().Text())
}
17 changes: 15 additions & 2 deletions integrations/pull_merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,13 @@ func TestCantMergeUnrelated(t *testing.T) {

stdin := bytes.NewBufferString("Unrelated File")
var stdout strings.Builder
err = git.NewCommand(git.DefaultContext, "hash-object", "-w", "--stdin").RunInDirFullPipeline(path, &stdout, nil, stdin)
err = git.NewCommand(git.DefaultContext, "hash-object", "-w", "--stdin").RunWithContext(&git.RunContext{
Timeout: -1,
Dir: path,
Stdin: stdin,
Stdout: &stdout,
})

assert.NoError(t, err)
sha := strings.TrimSpace(stdout.String())

Expand All @@ -301,7 +307,14 @@ func TestCantMergeUnrelated(t *testing.T) {
_, _ = messageBytes.WriteString("\n")

stdout.Reset()
err = git.NewCommand(git.DefaultContext, "commit-tree", treeSha).RunInDirTimeoutEnvFullPipeline(env, -1, path, &stdout, nil, messageBytes)
err = git.NewCommand(git.DefaultContext, "commit-tree", treeSha).
RunWithContext(&git.RunContext{
Env: env,
Timeout: -1,
Dir: path,
Stdin: messageBytes,
Stdout: &stdout,
})
assert.NoError(t, err)
commitSha := strings.TrimSpace(stdout.String())

Expand Down
5 changes: 4 additions & 1 deletion modules/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,10 @@ func (ctx *Context) RedirectToFirst(location ...string) {
func (ctx *Context) HTML(status int, name base.TplName) {
log.Debug("Template: %s", name)
tmplStartTime := time.Now()
ctx.Data["TmplLoadTimes"] = func() string {
if !setting.IsProd {
ctx.Data["TemplateName"] = name
}
ctx.Data["TemplateLoadTimes"] = func() string {
return strconv.FormatInt(time.Since(tmplStartTime).Nanoseconds()/1e6, 10) + "ms"
}
if err := ctx.Render.HTML(ctx.Resp, status, string(name), ctx.Data); err != nil {
Expand Down
22 changes: 19 additions & 3 deletions modules/git/batch_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ func EnsureValidGitRepository(ctx context.Context, repoPath string) error {
stderr := strings.Builder{}
err := NewCommand(ctx, "rev-parse").
SetDescription(fmt.Sprintf("%s rev-parse [repo_path: %s]", GitExecutable, repoPath)).
RunInDirFullPipeline(repoPath, nil, &stderr, nil)
RunWithContext(&RunContext{
Timeout: -1,
Dir: repoPath,
Stderr: &stderr,
})
if err != nil {
return ConcatenateError(err, (&stderr).String())
}
Expand All @@ -61,7 +65,13 @@ func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError,
stderr := strings.Builder{}
err := NewCommand(ctx, "cat-file", "--batch-check").
SetDescription(fmt.Sprintf("%s cat-file --batch-check [repo_path: %s] (%s:%d)", GitExecutable, repoPath, filename, line)).
RunInDirFullPipeline(repoPath, batchStdoutWriter, &stderr, batchStdinReader)
RunWithContext(&RunContext{
Timeout: -1,
Dir: repoPath,
Stdin: batchStdinReader,
Stdout: batchStdoutWriter,
Stderr: &stderr,
})
if err != nil {
_ = batchStdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String()))
_ = batchStdinReader.CloseWithError(ConcatenateError(err, (&stderr).String()))
Expand Down Expand Up @@ -100,7 +110,13 @@ func CatFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufi
stderr := strings.Builder{}
err := NewCommand(ctx, "cat-file", "--batch").
SetDescription(fmt.Sprintf("%s cat-file --batch [repo_path: %s] (%s:%d)", GitExecutable, repoPath, filename, line)).
RunInDirFullPipeline(repoPath, batchStdoutWriter, &stderr, batchStdinReader)
RunWithContext(&RunContext{
Timeout: -1,
Dir: repoPath,
Stdin: batchStdinReader,
Stdout: batchStdoutWriter,
Stderr: &stderr,
})
if err != nil {
_ = batchStdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String()))
_ = batchStdinReader.CloseWithError(ConcatenateError(err, (&stderr).String()))
Expand Down
7 changes: 6 additions & 1 deletion modules/git/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,12 @@ func GetCommitFileStatus(ctx context.Context, repoPath, commitID string) (*Commi
stderr := new(bytes.Buffer)
args := []string{"log", "--name-status", "-c", "--pretty=format:", "--parents", "--no-renames", "-z", "-1", commitID}

err := NewCommand(ctx, args...).RunInDirPipeline(repoPath, w, stderr)
err := NewCommand(ctx, args...).RunWithContext(&RunContext{
Timeout: -1,
Dir: repoPath,
Stdout: w,
Stderr: stderr,
})
w.Close() // Close writer to exit parsing goroutine
if err != nil {
return nil, ConcatenateError(err, stderr.String())
Expand Down
12 changes: 8 additions & 4 deletions modules/git/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,12 @@ func GetAffectedFiles(repo *Repository, oldCommitID, newCommitID string, env []s

// Run `git diff --name-only` to get the names of the changed files
err = NewCommand(repo.Ctx, "diff", "--name-only", oldCommitID, newCommitID).
RunInDirTimeoutEnvFullPipelineFunc(env, -1, repo.Path,
stdoutWriter, nil, nil,
func(ctx context.Context, cancel context.CancelFunc) error {
RunWithContext(&RunContext{
Env: env,
Timeout: -1,
Dir: repo.Path,
Stdout: stdoutWriter,
PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error {
// Close the writer end of the pipe to begin processing
_ = stdoutWriter.Close()
defer func() {
Expand All @@ -320,7 +323,8 @@ func GetAffectedFiles(repo *Repository, oldCommitID, newCommitID string, env []s
affectedFiles = append(affectedFiles, path)
}
return scanner.Err()
})
},
})
if err != nil {
log.Error("Unable to get affected files for commits from %s to %s in %s: %v", oldCommitID, newCommitID, repo.Path, err)
}
Expand Down
7 changes: 6 additions & 1 deletion modules/git/log_name_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,12 @@ func LogNameStatusRepo(ctx context.Context, repository, head, treepath string, p

go func() {
stderr := strings.Builder{}
err := NewCommand(ctx, args...).RunInDirFullPipeline(repository, stdoutWriter, &stderr, nil)
err := NewCommand(ctx, args...).RunWithContext(&RunContext{
Timeout: -1,
Dir: repository,
Stdout: stdoutWriter,
Stderr: &stderr,
})
if err != nil {
_ = stdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String()))
} else {
Expand Down
23 changes: 20 additions & 3 deletions modules/git/pipeline/catfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,13 @@ func CatFileBatchCheck(ctx context.Context, shasToCheckReader *io.PipeReader, ca
stderr := new(bytes.Buffer)
var errbuf strings.Builder
cmd := git.NewCommand(ctx, "cat-file", "--batch-check")
if err := cmd.RunInDirFullPipeline(tmpBasePath, catFileCheckWriter, stderr, shasToCheckReader); err != nil {
if err := cmd.RunWithContext(&git.RunContext{
Timeout: -1,
Dir: tmpBasePath,
Stdin: shasToCheckReader,
Stdout: catFileCheckWriter,
Stderr: stderr,
}); err != nil {
_ = catFileCheckWriter.CloseWithError(fmt.Errorf("git cat-file --batch-check [%s]: %v - %s", tmpBasePath, err, errbuf.String()))
}
}
Expand All @@ -40,7 +46,12 @@ func CatFileBatchCheckAllObjects(ctx context.Context, catFileCheckWriter *io.Pip
stderr := new(bytes.Buffer)
var errbuf strings.Builder
cmd := git.NewCommand(ctx, "cat-file", "--batch-check", "--batch-all-objects")
if err := cmd.RunInDirPipeline(tmpBasePath, catFileCheckWriter, stderr); err != nil {
if err := cmd.RunWithContext(&git.RunContext{
Timeout: -1,
Dir: tmpBasePath,
Stdout: catFileCheckWriter,
Stderr: stderr,
}); err != nil {
log.Error("git cat-file --batch-check --batch-all-object [%s]: %v - %s", tmpBasePath, err, errbuf.String())
err = fmt.Errorf("git cat-file --batch-check --batch-all-object [%s]: %v - %s", tmpBasePath, err, errbuf.String())
_ = catFileCheckWriter.CloseWithError(err)
Expand All @@ -56,7 +67,13 @@ func CatFileBatch(ctx context.Context, shasToBatchReader *io.PipeReader, catFile

stderr := new(bytes.Buffer)
var errbuf strings.Builder
if err := git.NewCommand(ctx, "cat-file", "--batch").RunInDirFullPipeline(tmpBasePath, catFileBatchWriter, stderr, shasToBatchReader); err != nil {
if err := git.NewCommand(ctx, "cat-file", "--batch").RunWithContext(&git.RunContext{
Timeout: -1,
Dir: tmpBasePath,
Stdout: catFileBatchWriter,
Stdin: shasToBatchReader,
Stderr: stderr,
}); err != nil {
_ = shasToBatchReader.CloseWithError(fmt.Errorf("git rev-list [%s]: %v - %s", tmpBasePath, err, errbuf.String()))
}
}
Expand Down
7 changes: 6 additions & 1 deletion modules/git/pipeline/lfs_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,12 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {

go func() {
stderr := strings.Builder{}
err := git.NewCommand(repo.Ctx, "rev-list", "--all").RunInDirPipeline(repo.Path, revListWriter, &stderr)
err := git.NewCommand(repo.Ctx, "rev-list", "--all").RunWithContext(&git.RunContext{
Timeout: -1,
Dir: repo.Path,
Stdout: revListWriter,
Stderr: &stderr,
})
if err != nil {
_ = revListWriter.CloseWithError(git.ConcatenateError(err, (&stderr).String()))
} else {
Expand Down
Loading

0 comments on commit be1b906

Please sign in to comment.