Do not overwrite empty DefaultBranch (#22708)

Fix #21994. 
And fix #19470.

While generating new repo from a template, it does something like
"commit to git repo, re-fetch repo model from DB, and update default
branch if it's empty".


19d5b2f922/modules/repository/generate.go (L241-L253)

Unfortunately, when load repo from DB, the default branch will be set to
`setting.Repository.DefaultBranch` if it's empty:


19d5b2f922/models/repo/repo.go (L228-L233)

I believe it's a very old temporary patch but has been kept for many
years, see:
[2d2d85bb](https://github.com/go-gitea/gitea/commit/2d2d85bb#diff-1851799b06733db4df3ec74385c1e8850ee5aedee70b8b55366910d22725eea8)

I know it's a risk to delete it, may lead to potential behavioral
changes, but we cannot keep the outdated `FIXME` forever. On the other
hand, an empty `DefaultBranch` does make sense: an empty repo doesn't
have one conceptually (actually, Gitea will still set it to
`setting.Repository.DefaultBranch` to make it safer).

models/asymkey/error.go

@@ -24,6 +24,9 @@ func (err ErrKeyUnableVerify) Error() string {
 	return fmt.Sprintf("Unable to verify key content [result: %s]", err.Result)
 }
 
+// ErrKeyIsPrivate is returned when the provided key is a private key not a public key
+var ErrKeyIsPrivate = util.NewSilentWrapErrorf(util.ErrInvalidArgument, "the provided key is a private key")
+
 // ErrKeyNotExist represents a "KeyNotExist" kind of error.
 type ErrKeyNotExist struct {
 	ID int64

models/asymkey/ssh_key_parse.go

@@ -96,6 +96,9 @@ func parseKeyString(content string) (string, error) {
 			if block == nil {
 				return "", fmt.Errorf("failed to parse PEM block containing the public key")
 			}
+			if strings.Contains(block.Type, "PRIVATE") {
+				return "", ErrKeyIsPrivate
+			}
 
 			pub, err := x509.ParsePKIXPublicKey(block.Bytes)
 			if err != nil {

models/dbfs/dbfs.go

@@ -10,6 +10,35 @@ import (
 	"code.gitea.io/gitea/models/db"
 )
 
+/*
+The reasons behind the DBFS (database-filesystem) package:
+When a Gitea action is running, the Gitea action server should collect and store all the logs.
+
+The requirements are:
+* The running logs must be stored across the cluster if the Gitea servers are deployed as a cluster.
+* The logs will be archived to Object Storage (S3/MinIO, etc.) after a period of time.
+* The Gitea action UI should be able to render the running logs and the archived logs.
+
+Some possible solutions for the running logs:
+* [Not ideal] Using local temp file: it can not be shared across the cluster.
+* [Not ideal] Using shared file in the filesystem of git repository: although at the moment, the Gitea cluster's
+	git repositories must be stored in a shared filesystem, in the future, Gitea may need a dedicated Git Service Server
+	to decouple the shared filesystem. Then the action logs will become a blocker.
+* [Not ideal] Record the logs in a database table line by line: it has a couple of problems:
+	- It's difficult to make multiple increasing sequence (log line number) for different databases.
+	- The database table will have a lot of rows and be affected by the big-table performance problem.
+	- It's difficult to load logs by using the same interface as other storages.
+  - It's difficult to calculate the size of the logs.
+
+The DBFS solution:
+* It can be used in a cluster.
+* It can share the same interface (Read/Write/Seek) as other storages.
+* It's very friendly to database because it only needs to store much fewer rows than the log-line solution.
+* In the future, when Gitea action needs to limit the log size (other CI/CD services also do so), it's easier to calculate the log file size.
+* Even sometimes the UI needs to render the tailing lines, the tailing lines can be found be counting the "\n" from the end of the file by seek.
+  The seeking and finding is not the fastest way, but it's still acceptable and won't affect the performance too much.
+*/
+
 type dbfsMeta struct {
 	ID              int64  `xorm:"pk autoincr"`
 	FullPath        string `xorm:"VARCHAR(500) UNIQUE NOT NULL"`

models/fixtures/repository.yml

@@ -4,6 +4,7 @@
   owner_name: user2
   lower_name: repo1
   name: repo1
+  default_branch: master
   num_watches: 4
   num_stars: 0
   num_forks: 0
@@ -34,6 +35,7 @@
   owner_name: user2
   lower_name: repo2
   name: repo2
+  default_branch: master
   num_watches: 0
   num_stars: 1
   num_forks: 0
@@ -64,6 +66,7 @@
   owner_name: user3
   lower_name: repo3
   name: repo3
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -94,6 +97,7 @@
   owner_name: user5
   lower_name: repo4
   name: repo4
+  default_branch: master
   num_watches: 0
   num_stars: 1
   num_forks: 0
@@ -274,6 +278,7 @@
   owner_name: user12
   lower_name: repo10
   name: repo10
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 1
@@ -304,6 +309,7 @@
   owner_name: user13
   lower_name: repo11
   name: repo11
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -425,6 +431,7 @@
   owner_name: user2
   lower_name: repo15
   name: repo15
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -455,6 +462,7 @@
   owner_name: user2
   lower_name: repo16
   name: repo16
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -905,6 +913,7 @@
   owner_name: user2
   lower_name: repo20
   name: repo20
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -965,6 +974,7 @@
   owner_name: user2
   lower_name: utf8
   name: utf8
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1055,6 +1065,7 @@
   owner_name: user2
   lower_name: commits_search_test
   name: commits_search_test
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1085,6 +1096,7 @@
   owner_name: user2
   lower_name: git_hooks_test
   name: git_hooks_test
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1115,6 +1127,7 @@
   owner_name: limited_org
   lower_name: public_repo_on_limited_org
   name: public_repo_on_limited_org
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1145,6 +1158,7 @@
   owner_name: limited_org
   lower_name: private_repo_on_limited_org
   name: private_repo_on_limited_org
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1175,6 +1189,7 @@
   owner_name: privated_org
   lower_name: public_repo_on_private_org
   name: public_repo_on_private_org
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1205,6 +1220,7 @@
   owner_name: privated_org
   lower_name: private_repo_on_private_org
   name: private_repo_on_private_org
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1235,6 +1251,7 @@
   owner_name: user2
   lower_name: glob
   name: glob
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1295,6 +1312,7 @@
   owner_name: user27
   lower_name: template1
   name: template1
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1355,6 +1373,7 @@
   owner_name: org26
   lower_name: repo_external_tracker
   name: repo_external_tracker
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1385,6 +1404,7 @@
   owner_name: org26
   lower_name: repo_external_tracker_numeric
   name: repo_external_tracker_numeric
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1415,6 +1435,7 @@
   owner_name: org26
   lower_name: repo_external_tracker_alpha
   name: repo_external_tracker_alpha
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1445,6 +1466,7 @@
   owner_name: user27
   lower_name: repo49
   name: repo49
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1475,6 +1497,7 @@
   owner_name: user30
   lower_name: repo50
   name: repo50
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1505,6 +1528,7 @@
   owner_name: user30
   lower_name: repo51
   name: repo51
+  default_branch: master
   num_watches: 0
   num_stars: 0
   num_forks: 0
@@ -1565,6 +1589,7 @@
   owner_name: user30
   lower_name: renderer
   name: renderer
+  default_branch: master
   is_archived: false
   is_empty: false
   is_private: false
@@ -1592,6 +1617,7 @@
   owner_name: user2
   lower_name: lfs
   name: lfs
+  default_branch: master
   is_empty: false
   is_archived: false
   is_private: true

models/repo/repo.go

@@ -227,11 +227,6 @@ func (repo *Repository) IsBroken() bool {
 
 // AfterLoad is invoked from XORM after setting the values of all fields of this object.
 func (repo *Repository) AfterLoad() {
-	// FIXME: use models migration to solve all at once.
-	if len(repo.DefaultBranch) == 0 {
-		repo.DefaultBranch = setting.Repository.DefaultBranch
-	}
-
 	repo.NumOpenIssues = repo.NumIssues - repo.NumClosedIssues
 	repo.NumOpenPulls = repo.NumPulls - repo.NumClosedPulls
 	repo.NumOpenMilestones = repo.NumMilestones - repo.NumClosedMilestones

modules/actions/workflows.go

@@ -75,7 +75,6 @@ func DetectWorkflows(commit *git.Commit, triggedEvent webhook_module.HookEventTy
 			if evt.Name != triggedEvent.Event() {
 				continue
 			}
-
 			if detectMatched(commit, triggedEvent, payload, evt) {
 				workflows[entry.Name()] = content
 			}
@@ -105,8 +104,9 @@ func detectMatched(commit *git.Commit, triggedEvent webhook_module.HookEventType
 		for cond, vals := range evt.Acts {
 			switch cond {
 			case "branches", "tags":
+				refShortName := git.RefName(pushPayload.Ref).ShortName()
 				for _, val := range vals {
-					if glob.MustCompile(val, '/').Match(pushPayload.Ref) {
+					if glob.MustCompile(val, '/').Match(refShortName) {
 						matchTimes++
 						break
 					}
@@ -160,8 +160,9 @@ func detectMatched(commit *git.Commit, triggedEvent webhook_module.HookEventType
 					}
 				}
 			case "branches":
+				refShortName := git.RefName(prPayload.PullRequest.Base.Ref).ShortName()
 				for _, val := range vals {
-					if glob.MustCompile(val, '/').Match(prPayload.PullRequest.Base.Ref) {
+					if glob.MustCompile(val, '/').Match(refShortName) {
 						matchTimes++
 						break
 					}

modules/httpcache/httpcache.go

@@ -19,6 +19,8 @@ import (
 func AddCacheControlToHeader(h http.Header, maxAge time.Duration, additionalDirectives ...string) {
 	directives := make([]string, 0, 2+len(additionalDirectives))
 
+	// "max-age=0 + must-revalidate" (aka "no-cache") is preferred instead of "no-store"
+	// because browsers may restore some input fields after navigate-back / reload a page.
 	if setting.IsProd {
 		if maxAge == 0 {
 			directives = append(directives, "max-age=0", "private", "must-revalidate")

options/locale/locale_en-US.ini

@@ -518,6 +518,7 @@ organization_leave_success = You have successfully left the organization %s.
 invalid_ssh_key = Cannot verify your SSH key: %s
 invalid_gpg_key = Cannot verify your GPG key: %s
 invalid_ssh_principal = Invalid principal: %s
+must_use_public_key = The key you provided is a private key. Please do not upload your private key anywhere. Use your public key instead.
 unable_verify_ssh_key = "Cannot verify the SSH key; double-check it for mistakes."
 auth_failed = Authentication failed: %v
 

routers/web/repo/setting.go

@@ -1158,6 +1158,10 @@ func DeployKeysPost(ctx *context.Context) {
 			ctx.Flash.Info(ctx.Tr("settings.ssh_disabled"))
 		} else if asymkey_model.IsErrKeyUnableVerify(err) {
 			ctx.Flash.Info(ctx.Tr("form.unable_verify_ssh_key"))
+		} else if err == asymkey_model.ErrKeyIsPrivate {
+			ctx.Data["HasError"] = true
+			ctx.Data["Err_Content"] = true
+			ctx.Flash.Error(ctx.Tr("form.must_use_public_key"))
 		} else {
 			ctx.Data["HasError"] = true
 			ctx.Data["Err_Content"] = true

routers/web/user/setting/keys.go

@@ -159,6 +159,8 @@ func KeysPost(ctx *context.Context) {
 				ctx.Flash.Info(ctx.Tr("settings.ssh_disabled"))
 			} else if asymkey_model.IsErrKeyUnableVerify(err) {
 				ctx.Flash.Info(ctx.Tr("form.unable_verify_ssh_key"))
+			} else if err == asymkey_model.ErrKeyIsPrivate {
+				ctx.Flash.Error(ctx.Tr("form.must_use_public_key"))
 			} else {
 				ctx.Flash.Error(ctx.Tr("form.invalid_ssh_key", err.Error()))
 			}

services/auth/source/ldap/source_search.go

@@ -196,22 +196,39 @@ func checkRestricted(l *ldap.Conn, ls *Source, userDN string) bool {
 }
 
 // List all group memberships of a user
-func (source *Source) listLdapGroupMemberships(l *ldap.Conn, uid string) []string {
+func (source *Source) listLdapGroupMemberships(l *ldap.Conn, uid string, applyGroupFilter bool) []string {
 	var ldapGroups []string
-	groupFilter := fmt.Sprintf("(%s=%s)", source.GroupMemberUID, ldap.EscapeFilter(uid))
+	var searchFilter string
+
+	groupFilter, ok := source.sanitizedGroupFilter(source.GroupFilter)
+	if !ok {
+		return ldapGroups
+	}
+
+	groupDN, ok := source.sanitizedGroupDN(source.GroupDN)
+	if !ok {
+		return ldapGroups
+	}
+
+	if applyGroupFilter {
+		searchFilter = fmt.Sprintf("(&(%s)(%s=%s))", groupFilter, source.GroupMemberUID, ldap.EscapeFilter(uid))
+	} else {
+		searchFilter = fmt.Sprintf("(%s=%s)", source.GroupMemberUID, ldap.EscapeFilter(uid))
+	}
+
 	result, err := l.Search(ldap.NewSearchRequest(
-		source.GroupDN,
+		groupDN,
 		ldap.ScopeWholeSubtree,
 		ldap.NeverDerefAliases,
 		0,
 		0,
 		false,
-		groupFilter,
+		searchFilter,
 		[]string{},
 		nil,
 	))
 	if err != nil {
-		log.Error("Failed group search using filter[%s]: %v", groupFilter, err)
+		log.Error("Failed group search in LDAP with filter [%s]: %v", searchFilter, err)
 		return ldapGroups
 	}
 
@@ -238,9 +255,7 @@ func (source *Source) mapLdapGroupsToTeams() map[string]map[string][]string {
 }
 
 // getMappedMemberships : returns the organizations and teams to modify the users membership
-func (source *Source) getMappedMemberships(l *ldap.Conn, uid string) (map[string][]string, map[string][]string) {
-	// get all LDAP group memberships for user
-	usersLdapGroups := source.listLdapGroupMemberships(l, uid)
+func (source *Source) getMappedMemberships(usersLdapGroups []string, uid string) (map[string][]string, map[string][]string) {
 	// unmarshall LDAP group team map from configs
 	ldapGroupsToTeams := source.mapLdapGroupsToTeams()
 	membershipsToAdd := map[string][]string{}
@@ -260,6 +275,14 @@ func (source *Source) getMappedMemberships(l *ldap.Conn, uid string) (map[string
 	return membershipsToAdd, membershipsToRemove
 }
 
+func (source *Source) getUserAttributeListedInGroup(entry *ldap.Entry) string {
+	if strings.ToLower(source.UserUID) == "dn" {
+		return entry.DN
+	}
+
+	return entry.GetAttributeValue(source.UserUID)
+}
+
 // SearchEntry : search an LDAP source if an entry (name, passwd) is valid and in the specific filter
 func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchResult {
 	// See https://tools.ietf.org/search/rfc4513#section-5.1.2
@@ -375,58 +398,30 @@ func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchR
 	firstname := sr.Entries[0].GetAttributeValue(source.AttributeName)
 	surname := sr.Entries[0].GetAttributeValue(source.AttributeSurname)
 	mail := sr.Entries[0].GetAttributeValue(source.AttributeMail)
-	uid := sr.Entries[0].GetAttributeValue(source.UserUID)
-	if source.UserUID == "dn" || source.UserUID == "DN" {
-		uid = sr.Entries[0].DN
-	}
+
+	teamsToAdd := make(map[string][]string)
+	teamsToRemove := make(map[string][]string)
 
 	// Check group membership
-	if source.GroupsEnabled && source.GroupFilter != "" {
-		groupFilter, ok := source.sanitizedGroupFilter(source.GroupFilter)
-		if !ok {
-			return nil
-		}
-		groupDN, ok := source.sanitizedGroupDN(source.GroupDN)
-		if !ok {
+	if source.GroupsEnabled {
+		userAttributeListedInGroup := source.getUserAttributeListedInGroup(sr.Entries[0])
+		usersLdapGroups := source.listLdapGroupMemberships(l, userAttributeListedInGroup, true)
+
+		if source.GroupFilter != "" && len(usersLdapGroups) == 0 {
 			return nil
 		}
 
-		log.Trace("Fetching groups '%v' with filter '%s' and base '%s'", source.GroupMemberUID, groupFilter, groupDN)
-		groupSearch := ldap.NewSearchRequest(
-			groupDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases, 0, 0, false, groupFilter,
-			[]string{source.GroupMemberUID},
-			nil)
-
-		srg, err := l.Search(groupSearch)
-		if err != nil {
-			log.Error("LDAP group search failed: %v", err)
-			return nil
-		} else if len(srg.Entries) < 1 {
-			log.Error("LDAP group search failed: 0 entries")
-			return nil
-		}
-
-		isMember := false
-	Entries:
-		for _, group := range srg.Entries {
-			for _, member := range group.GetAttributeValues(source.GroupMemberUID) {
-				if (source.UserUID == "dn" && member == sr.Entries[0].DN) || member == uid {
-					isMember = true
-					break Entries
-				}
-			}
-		}
-
-		if !isMember {
-			log.Error("LDAP group membership test failed")
-			return nil
+		if source.GroupTeamMap != "" || source.GroupTeamMapRemoval {
+			teamsToAdd, teamsToRemove = source.getMappedMemberships(usersLdapGroups, userAttributeListedInGroup)
 		}
 	}
 
 	if isAttributeSSHPublicKeySet {
 		sshPublicKey = sr.Entries[0].GetAttributeValues(source.AttributeSSHPublicKey)
 	}
+
 	isAdmin := checkAdmin(l, source, userDN)
+
 	var isRestricted bool
 	if !isAdmin {
 		isRestricted = checkRestricted(l, source, userDN)
@@ -436,12 +431,6 @@ func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchR
 		Avatar = sr.Entries[0].GetRawAttributeValue(source.AttributeAvatar)
 	}
 
-	teamsToAdd := make(map[string][]string)
-	teamsToRemove := make(map[string][]string)
-	if source.GroupsEnabled && (source.GroupTeamMap != "" || source.GroupTeamMapRemoval) {
-		teamsToAdd, teamsToRemove = source.getMappedMemberships(l, uid)
-	}
-
 	if !directBind && source.AttributesInBind {
 		// binds user (checking password) after looking-up attributes in BindDN context
 		err = bindUser(l, userDN, passwd)
@@ -520,19 +509,29 @@ func (source *Source) SearchEntries() ([]*SearchResult, error) {
 		return nil, err
 	}
 
-	result := make([]*SearchResult, len(sr.Entries))
+	result := make([]*SearchResult, 0, len(sr.Entries))
 
-	for i, v := range sr.Entries {
+	for _, v := range sr.Entries {
 		teamsToAdd := make(map[string][]string)
 		teamsToRemove := make(map[string][]string)
-		if source.GroupsEnabled && (source.GroupTeamMap != "" || source.GroupTeamMapRemoval) {
-			userAttributeListedInGroup := v.GetAttributeValue(source.UserUID)
-			if source.UserUID == "dn" || source.UserUID == "DN" {
-				userAttributeListedInGroup = v.DN
+
+		if source.GroupsEnabled {
+			userAttributeListedInGroup := source.getUserAttributeListedInGroup(v)
+
+			if source.GroupFilter != "" {
+				usersLdapGroups := source.listLdapGroupMemberships(l, userAttributeListedInGroup, true)
+				if len(usersLdapGroups) == 0 {
+					continue
+				}
+			}
+
+			if source.GroupTeamMap != "" || source.GroupTeamMapRemoval {
+				usersLdapGroups := source.listLdapGroupMemberships(l, userAttributeListedInGroup, false)
+				teamsToAdd, teamsToRemove = source.getMappedMemberships(usersLdapGroups, userAttributeListedInGroup)
 			}
-			teamsToAdd, teamsToRemove = source.getMappedMemberships(l, userAttributeListedInGroup)
 		}
-		result[i] = &SearchResult{
+
+		user := &SearchResult{
 			Username:       v.GetAttributeValue(source.AttributeUsername),
 			Name:           v.GetAttributeValue(source.AttributeName),
 			Surname:        v.GetAttributeValue(source.AttributeSurname),
@@ -541,16 +540,22 @@ func (source *Source) SearchEntries() ([]*SearchResult, error) {
 			LdapTeamAdd:    teamsToAdd,
 			LdapTeamRemove: teamsToRemove,
 		}
-		if !result[i].IsAdmin {
-			result[i].IsRestricted = checkRestricted(l, source, v.DN)
+
+		if !user.IsAdmin {
+			user.IsRestricted = checkRestricted(l, source, v.DN)
 		}
+
 		if isAttributeSSHPublicKeySet {
-			result[i].SSHPublicKey = v.GetAttributeValues(source.AttributeSSHPublicKey)
+			user.SSHPublicKey = v.GetAttributeValues(source.AttributeSSHPublicKey)
 		}
+
 		if isAtributeAvatarSet {
-			result[i].Avatar = v.GetRawAttributeValue(source.AttributeAvatar)
+			user.Avatar = v.GetRawAttributeValue(source.AttributeAvatar)
 		}
-		result[i].LowerName = strings.ToLower(result[i].Username)
+
+		user.LowerName = strings.ToLower(user.Username)
+
+		result = append(result, user)
 	}
 
 	return result, nil

templates/repo/diff/box.tmpl

@@ -76,19 +76,18 @@
 						{{$isImage := or (call $.IsBlobAnImage $blobBase) (call $.IsBlobAnImage $blobHead)}}
 						{{$isCsv := (call $.IsCsvFile $file)}}
 						{{$showFileViewToggle := or $isImage (and (not $file.IsIncomplete) $isCsv)}}
-						<div class="diff-file-box diff-box file-content {{TabSizeClass $.Editorconfig $file.Name}} mt-3" id="diff-{{$file.NameHash}}" data-old-filename="{{$file.OldName}}" data-new-filename="{{$file.Name}}" {{if $file.ShouldBeHidden}}data-folded="true"{{end}}>
+						{{$isExpandable := or (gt $file.Addition 0) (gt $file.Deletion 0) $file.IsBin}}
+						<div class="diff-file-box diff-box file-content {{TabSizeClass $.Editorconfig $file.Name}} mt-3" id="diff-{{$file.NameHash}}" data-old-filename="{{$file.OldName}}" data-new-filename="{{$file.Name}}" {{if or ($file.ShouldBeHidden) (not $isExpandable)}}data-folded="true"{{end}}>
 							<h4 class="diff-file-header sticky-2nd-row ui top attached normal header df ac sb">
 								<div class="df ac">
-									{{if or (gt $file.Addition 0) (gt $file.Deletion 0) $file.IsBin}}
-										<a role="button" class="fold-file muted mr-2">
-											{{if $file.ShouldBeHidden}}
-												{{svg "octicon-chevron-right" 18}}
-											{{else}}
-												{{svg "octicon-chevron-down" 18}}
-											{{end}}
-										</a>
-									{{end}}
-									<div class="bold df ac">
+									<a role="button" class="fold-file muted mr-2" {{if not $isExpandable}}style="visibility: hidden"{{end}}>
+										{{if $file.ShouldBeHidden}}
+											{{svg "octicon-chevron-right" 18}}
+										{{else}}
+											{{svg "octicon-chevron-down" 18}}
+										{{end}}
+									</a>
+									<div class="bold df ac mono">
 										{{if $file.IsBin}}
 											<span class="ml-1 mr-3">
 												{{$.locale.Tr "repo.diff.bin"}}

templates/user/dashboard/navbar.tmpl

@@ -7,8 +7,8 @@
 					<span class="truncated-item-name">{{.ContextUser.ShortName 40}}</span>
 					{{if .ContextUser.IsOrganization}}
 						<span class="org-visibility">
-							{{if .ContextUser.Visibility.IsLimited}}<div class="ui orange tiny horizontal label">{{.locale.Tr "org.settings.visibility.limited_shortname"}}</div>{{end}}
-							{{if .ContextUser.Visibility.IsPrivate}}<div class="ui red tiny horizontal label">{{.locale.Tr "org.settings.visibility.private_shortname"}}</div>{{end}}
+							{{if .ContextUser.Visibility.IsLimited}}<div class="ui basic tiny horizontal label">{{.locale.Tr "org.settings.visibility.limited_shortname"}}</div>{{end}}
+							{{if .ContextUser.Visibility.IsPrivate}}<div class="ui basic tiny horizontal label">{{.locale.Tr "org.settings.visibility.private_shortname"}}</div>{{end}}
 						</span>
 					{{end}}
 					{{svg "octicon-triangle-down" 14 "dropdown icon"}}
@@ -27,8 +27,8 @@
 								{{avatar .}}
 								<span class="truncated-item-name">{{.ShortName 40}}</span>
 								<span class="org-visibility">
-									{{if .Visibility.IsLimited}}<div class="ui orange tiny horizontal label">{{$.locale.Tr "org.settings.visibility.limited_shortname"}}</div>{{end}}
-									{{if .Visibility.IsPrivate}}<div class="ui red tiny horizontal label">{{$.locale.Tr "org.settings.visibility.private_shortname"}}</div>{{end}}
+									{{if .Visibility.IsLimited}}<div class="ui basic tiny horizontal label">{{$.locale.Tr "org.settings.visibility.limited_shortname"}}</div>{{end}}
+									{{if .Visibility.IsPrivate}}<div class="ui basic tiny horizontal label">{{$.locale.Tr "org.settings.visibility.private_shortname"}}</div>{{end}}
 								</span>
 							</a>
 						{{end}}

tests/integration/auth_ldap_test.go

@@ -11,12 +11,14 @@ import (
 	"testing"
 
 	"code.gitea.io/gitea/models"
+	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/organization"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/translation"
 	"code.gitea.io/gitea/services/auth"
+	"code.gitea.io/gitea/services/auth/source/ldap"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
@@ -102,13 +104,28 @@ func getLDAPServerHost() string {
 	return host
 }
 
-func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string, groupMapParams ...string) {
+func getLDAPServerPort() string {
+	port := os.Getenv("TEST_LDAP_PORT")
+	if len(port) == 0 {
+		port = "389"
+	}
+	return port
+}
+
+func addAuthSourceLDAP(t *testing.T, sshKeyAttribute, groupFilter string, groupMapParams ...string) {
 	groupTeamMapRemoval := "off"
 	groupTeamMap := ""
 	if len(groupMapParams) == 2 {
 		groupTeamMapRemoval = groupMapParams[0]
 		groupTeamMap = groupMapParams[1]
 	}
+
+	// Modify user filter to test group filter explicitly
+	userFilter := "(&(objectClass=inetOrgPerson)(memberOf=cn=git,ou=people,dc=planetexpress,dc=com)(uid=%s))"
+	if groupFilter != "" {
+		userFilter = "(&(objectClass=inetOrgPerson)(uid=%s))"
+	}
+
 	session := loginUser(t, "user1")
 	csrf := GetCSRF(t, session, "/admin/auths/new")
 	req := NewRequestWithValues(t, "POST", "/admin/auths/new", map[string]string{
@@ -116,11 +133,11 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string, groupMapParams ...s
 		"type":                     "2",
 		"name":                     "ldap",
 		"host":                     getLDAPServerHost(),
-		"port":                     "389",
+		"port":                     getLDAPServerPort(),
 		"bind_dn":                  "uid=gitea,ou=service,dc=planetexpress,dc=com",
 		"bind_password":            "password",
 		"user_base":                "ou=people,dc=planetexpress,dc=com",
-		"filter":                   "(&(objectClass=inetOrgPerson)(memberOf=cn=git,ou=people,dc=planetexpress,dc=com)(uid=%s))",
+		"filter":                   userFilter,
 		"admin_filter":             "(memberOf=cn=admin_staff,ou=people,dc=planetexpress,dc=com)",
 		"restricted_filter":        "(uid=leela)",
 		"attribute_username":       "uid",
@@ -133,6 +150,7 @@ func addAuthSourceLDAP(t *testing.T, sshKeyAttribute string, groupMapParams ...s
 		"groups_enabled":           "on",
 		"group_dn":                 "ou=people,dc=planetexpress,dc=com",
 		"group_member_uid":         "member",
+		"group_filter":             groupFilter,
 		"group_team_map":           groupTeamMap,
 		"group_team_map_removal":   groupTeamMapRemoval,
 		"user_uid":                 "DN",
@@ -146,7 +164,7 @@ func TestLDAPUserSignin(t *testing.T) {
 		return
 	}
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "")
+	addAuthSourceLDAP(t, "", "")
 
 	u := gitLDAPUsers[0]
 
@@ -163,7 +181,7 @@ func TestLDAPUserSignin(t *testing.T) {
 
 func TestLDAPAuthChange(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "")
+	addAuthSourceLDAP(t, "", "")
 
 	session := loginUser(t, "user1")
 	req := NewRequest(t, "GET", "/admin/auths")
@@ -221,7 +239,7 @@ func TestLDAPUserSync(t *testing.T) {
 		return
 	}
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "")
+	addAuthSourceLDAP(t, "", "")
 	auth.SyncExternalUsers(context.Background(), true)
 
 	session := loginUser(t, "user1")
@@ -266,13 +284,72 @@ func TestLDAPUserSync(t *testing.T) {
 	}
 }
 
+func TestLDAPUserSyncWithGroupFilter(t *testing.T) {
+	if skipLDAPTests() {
+		t.Skip()
+		return
+	}
+	defer tests.PrepareTestEnv(t)()
+	addAuthSourceLDAP(t, "", "(cn=git)")
+
+	// Assert a user not a member of the LDAP group "cn=git" cannot login
+	// This test may look like TestLDAPUserSigninFailed but it is not.
+	// The later test uses user filter containing group membership filter (memberOf)
+	// This test is for the case when LDAP user records may not be linked with
+	// all groups the user is a member of, the user filter is modified accordingly inside
+	// the addAuthSourceLDAP based on the value of the groupFilter
+	u := otherLDAPUsers[0]
+	testLoginFailed(t, u.UserName, u.Password, translation.NewLocale("en-US").Tr("form.username_password_incorrect"))
+
+	auth.SyncExternalUsers(context.Background(), true)
+
+	// Assert members of LDAP group "cn=git" are added
+	for _, gitLDAPUser := range gitLDAPUsers {
+		unittest.BeanExists(t, &user_model.User{
+			Name: gitLDAPUser.UserName,
+		})
+	}
+
+	// Assert everyone else is not added
+	for _, gitLDAPUser := range otherLDAPUsers {
+		unittest.AssertNotExistsBean(t, &user_model.User{
+			Name: gitLDAPUser.UserName,
+		})
+	}
+
+	ldapSource := unittest.AssertExistsAndLoadBean(t, &auth_model.Source{
+		Name: "ldap",
+	})
+	ldapConfig := ldapSource.Cfg.(*ldap.Source)
+	ldapConfig.GroupFilter = "(cn=ship_crew)"
+	auth_model.UpdateSource(ldapSource)
+
+	auth.SyncExternalUsers(context.Background(), true)
+
+	for _, gitLDAPUser := range gitLDAPUsers {
+		if gitLDAPUser.UserName == "fry" || gitLDAPUser.UserName == "leela" || gitLDAPUser.UserName == "bender" {
+			// Assert members of the LDAP group "cn-ship_crew" are still active
+			user := unittest.AssertExistsAndLoadBean(t, &user_model.User{
+				Name: gitLDAPUser.UserName,
+			})
+			assert.True(t, user.IsActive, "User %s should be active", gitLDAPUser.UserName)
+		} else {
+			// Assert everyone else is inactive
+			user := unittest.AssertExistsAndLoadBean(t, &user_model.User{
+				Name: gitLDAPUser.UserName,
+			})
+			assert.False(t, user.IsActive, "User %s should be inactive", gitLDAPUser.UserName)
+		}
+	}
+}
+
 func TestLDAPUserSigninFailed(t *testing.T) {
 	if skipLDAPTests() {
 		t.Skip()
 		return
 	}
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "")
+	addAuthSourceLDAP(t, "", "")
 
 	u := otherLDAPUsers[0]
 	testLoginFailed(t, u.UserName, u.Password, translation.NewLocale("en-US").Tr("form.username_password_incorrect"))
@@ -284,7 +361,7 @@ func TestLDAPUserSSHKeySync(t *testing.T) {
 		return
 	}
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "sshPublicKey")
+	addAuthSourceLDAP(t, "sshPublicKey", "")
 
 	auth.SyncExternalUsers(context.Background(), true)
 
@@ -317,7 +394,7 @@ func TestLDAPGroupTeamSyncAddMember(t *testing.T) {
 		return
 	}
 	defer tests.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"]}}`)
+	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 := organization.GetOrgByName("org26")
 	assert.NoError(t, err)
 	team, err := organization.GetTeam(db.DefaultContext, org.ID, "team11")
@@ -362,7 +439,7 @@ func TestLDAPGroupTeamSyncRemoveMember(t *testing.T) {
 		return
 	}
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "", "on", `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`)
+	addAuthSourceLDAP(t, "", "", "on", `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`)
 	org, err := organization.GetOrgByName("org26")
 	assert.NoError(t, err)
 	team, err := organization.GetTeam(db.DefaultContext, org.ID, "team11")
@@ -398,7 +475,7 @@ func TestBrokenLDAPMapUserSignin(t *testing.T) {
 		return
 	}
 	defer tests.PrepareTestEnv(t)()
-	addAuthSourceLDAP(t, "", "on", `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`)
+	addAuthSourceLDAP(t, "", "", "on", `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`)
 
 	u := gitLDAPUsers[0]
 

tests/mysql.ini.tmpl

@@ -126,6 +126,7 @@ INTERNAL_TOKEN = eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJuYmYiOjE0OTU1NTE2MTh9.h
 ENABLED = true
 
 [email.incoming]
+; temporarily disabled because the incoming mail tests are flaky due to the IMAP server (during integration tests) couldn't be not ready in time sometimes.
 ENABLED = false
 HOST = smtpimap
 PORT = 993