Fix bug preventing transfer to private organization (#12497)

* Fix bug preventing transfer to private organization

The code assessing whether a private organization was visible to a user before
allowing transfer was incorrect due to testing membership the wrong way round

This PR fixes this issue and renames the function performing the test to be
clearer.

Further looking at the API for transfer repository - no testing was
performed to ensure that the acting user could actually see the new
owning organization.

Signed-off-by: Andrew Thornton <art27@cantab.net>

* change IsUserPartOfOrg everywhere

6 files changed, 19 insertions, 10 deletions

diff --git a/models/migrations/v111.go b/models/migrations/v111.go
index 66ff4843e5..6a94298ddc 100644
--- a/models/migrations/v111.go
+++ b/models/migrations/v111.go
@@ -148,7 +148,7 @@ func addBranchProtectionCanPushAndEnableWhitelist(x *xorm.Engine) error {
 		if user == nil {
 			hasOrgVisible = repoOwner.Visibility == VisibleTypePublic
 		} else if !user.IsAdmin {
-			isUserPartOfOrg, err := sess.
+			hasMemberWithUserID, err := sess.
 				Where("uid=?", user.ID).
 				And("org_id=?", repoOwner.ID).
 				Table("org_user").
@@ -156,7 +156,7 @@ func addBranchProtectionCanPushAndEnableWhitelist(x *xorm.Engine) error {
 			if err != nil {
 				hasOrgVisible = false
 			}
-			if (repoOwner.Visibility == VisibleTypePrivate || user.IsRestricted) && !isUserPartOfOrg {
+			if (repoOwner.Visibility == VisibleTypePrivate || user.IsRestricted) && !hasMemberWithUserID {
 				hasOrgVisible = false
 			}
 		}
diff --git a/models/org.go b/models/org.go
index 0915e7fd56..31e5cf81c9 100644
--- a/models/org.go
+++ b/models/org.go
@@ -435,7 +435,7 @@ func hasOrgVisible(e Engine, org *User, user *User) bool {
 		return true
 	}
 
-	if (org.Visibility == structs.VisibleTypePrivate || user.IsRestricted) && !org.isUserPartOfOrg(e, user.ID) {
+	if (org.Visibility == structs.VisibleTypePrivate || user.IsRestricted) && !org.hasMemberWithUserID(e, user.ID) {
 		return false
 	}
 	return true
diff --git a/models/user.go b/models/user.go
index 6bd9d18b07..1c17453930 100644
--- a/models/user.go
+++ b/models/user.go
@@ -610,12 +610,12 @@ func (u *User) IsUserOrgOwner(orgID int64) bool {
 	return isOwner
 }
 
-// IsUserPartOfOrg returns true if user with userID is part of the u organisation.
-func (u *User) IsUserPartOfOrg(userID int64) bool {
-	return u.isUserPartOfOrg(x, userID)
+// HasMemberWithUserID returns true if user with userID is part of the u organisation.
+func (u *User) HasMemberWithUserID(userID int64) bool {
+	return u.hasMemberWithUserID(x, userID)
 }
 
-func (u *User) isUserPartOfOrg(e Engine, userID int64) bool {
+func (u *User) hasMemberWithUserID(e Engine, userID int64) bool {
 	isMember, err := isOrganizationMember(e, u.ID, userID)
 	if err != nil {
 		log.Error("IsOrganizationMember: %v", err)
diff --git a/routers/api/v1/repo/transfer.go b/routers/api/v1/repo/transfer.go
index 847028d106..b1271b7721 100644
--- a/routers/api/v1/repo/transfer.go
+++ b/routers/api/v1/repo/transfer.go
@@ -12,6 +12,7 @@ import (
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/convert"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/structs"
 	api "code.gitea.io/gitea/modules/structs"
 	repo_service "code.gitea.io/gitea/services/repository"
 )
@@ -53,13 +54,21 @@ func Transfer(ctx *context.APIContext, opts api.TransferRepoOption) {
 	newOwner, err := models.GetUserByName(opts.NewOwner)
 	if err != nil {
 		if models.IsErrUserNotExist(err) {
-			ctx.Error(http.StatusNotFound, "GetUserByName", err)
+			ctx.Error(http.StatusNotFound, "", "The new owner does not exist or cannot be found")
 			return
 		}
 		ctx.InternalServerError(err)
 		return
 	}
 
+	if newOwner.Type == models.UserTypeOrganization {
+		if !ctx.User.IsAdmin && newOwner.Visibility == structs.VisibleTypePrivate && !newOwner.HasMemberWithUserID(ctx.User.ID) {
+			// The user shouldn't know about this organization
+			ctx.Error(http.StatusNotFound, "", "The new owner does not exist or cannot be found")
+			return
+		}
+	}
+
 	var teams []*models.Team
 	if opts.TeamIDs != nil {
 		if !newOwner.IsOrganization() {
diff --git a/routers/repo/setting.go b/routers/repo/setting.go
index 02331c232b..e03bf556be 100644
--- a/routers/repo/setting.go
+++ b/routers/repo/setting.go
@@ -418,7 +418,7 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) {
 		}
 
 		if newOwner.Type == models.UserTypeOrganization {
-			if !ctx.User.IsAdmin && newOwner.Visibility == structs.VisibleTypePrivate && !ctx.User.IsUserPartOfOrg(newOwner.ID) {
+			if !ctx.User.IsAdmin && newOwner.Visibility == structs.VisibleTypePrivate && !newOwner.HasMemberWithUserID(ctx.User.ID) {
 				// The user shouldn't know about this organization
 				ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_owner_name"), tplSettingsOptions, nil)
 				return
diff --git a/templates/user/profile.tmpl b/templates/user/profile.tmpl
index bc2960b3b9..f453898f94 100644
--- a/templates/user/profile.tmpl
+++ b/templates/user/profile.tmpl
@@ -52,7 +52,7 @@
 							<li>
 								<ul class="user-orgs">
 								{{range .Orgs}}
-									{{if (or .Visibility.IsPublic (and ($.SignedUser) (or .Visibility.IsLimited (and (.IsUserPartOfOrg $.SignedUserID) .Visibility.IsPrivate) ($.IsAdmin))))}}
+									{{if (or .Visibility.IsPublic (and ($.SignedUser) (or .Visibility.IsLimited (and (.HasMemberWithUserID $.SignedUserID) .Visibility.IsPrivate) ($.IsAdmin))))}}
 									<li>
 										<a href="{{.HomeLink}}"><img class="ui image poping up" src="{{.RelAvatarLink}}" data-content="{{.Name}}" data-position="top center" data-variation="tiny inverted"></a>
 									</li>