Browse Source

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
for-closed-social
zeripath 4 years ago
committed by GitHub
parent
commit
d1e67d7ade
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 19 additions and 10 deletions
  1. +2
    -2
      models/migrations/v111.go
  2. +1
    -1
      models/org.go
  3. +4
    -4
      models/user.go
  4. +10
    -1
      routers/api/v1/repo/transfer.go
  5. +1
    -1
      routers/repo/setting.go
  6. +1
    -1
      templates/user/profile.tmpl

+ 2
- 2
models/migrations/v111.go View File

@ -148,7 +148,7 @@ func addBranchProtectionCanPushAndEnableWhitelist(x *xorm.Engine) error {
if user == nil { if user == nil {
hasOrgVisible = repoOwner.Visibility == VisibleTypePublic hasOrgVisible = repoOwner.Visibility == VisibleTypePublic
} else if !user.IsAdmin { } else if !user.IsAdmin {
isUserPartOfOrg, err := sess.
hasMemberWithUserID, err := sess.
Where("uid=?", user.ID). Where("uid=?", user.ID).
And("org_id=?", repoOwner.ID). And("org_id=?", repoOwner.ID).
Table("org_user"). Table("org_user").
@ -156,7 +156,7 @@ func addBranchProtectionCanPushAndEnableWhitelist(x *xorm.Engine) error {
if err != nil { if err != nil {
hasOrgVisible = false hasOrgVisible = false
} }
if (repoOwner.Visibility == VisibleTypePrivate || user.IsRestricted) && !isUserPartOfOrg {
if (repoOwner.Visibility == VisibleTypePrivate || user.IsRestricted) && !hasMemberWithUserID {
hasOrgVisible = false hasOrgVisible = false
} }
} }

+ 1
- 1
models/org.go View File

@ -435,7 +435,7 @@ func hasOrgVisible(e Engine, org *User, user *User) bool {
return true 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 false
} }
return true return true

+ 4
- 4
models/user.go View File

@ -610,12 +610,12 @@ func (u *User) IsUserOrgOwner(orgID int64) bool {
return isOwner 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) isMember, err := isOrganizationMember(e, u.ID, userID)
if err != nil { if err != nil {
log.Error("IsOrganizationMember: %v", err) log.Error("IsOrganizationMember: %v", err)

+ 10
- 1
routers/api/v1/repo/transfer.go View File

@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/convert" "code.gitea.io/gitea/modules/convert"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/structs"
api "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs"
repo_service "code.gitea.io/gitea/services/repository" 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) newOwner, err := models.GetUserByName(opts.NewOwner)
if err != nil { if err != nil {
if models.IsErrUserNotExist(err) { 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 return
} }
ctx.InternalServerError(err) ctx.InternalServerError(err)
return 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 var teams []*models.Team
if opts.TeamIDs != nil { if opts.TeamIDs != nil {
if !newOwner.IsOrganization() { if !newOwner.IsOrganization() {

+ 1
- 1
routers/repo/setting.go View File

@ -418,7 +418,7 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) {
} }
if newOwner.Type == models.UserTypeOrganization { 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 // The user shouldn't know about this organization
ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_owner_name"), tplSettingsOptions, nil) ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_owner_name"), tplSettingsOptions, nil)
return return

+ 1
- 1
templates/user/profile.tmpl View File

@ -52,7 +52,7 @@
<li> <li>
<ul class="user-orgs"> <ul class="user-orgs">
{{range .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> <li>
<a href="{{.HomeLink}}"><img class="ui image poping up" src="{{.RelAvatarLink}}" data-content="{{.Name}}" data-position="top center" data-variation="tiny inverted"></a> <a href="{{.HomeLink}}"><img class="ui image poping up" src="{{.RelAvatarLink}}" data-content="{{.Name}}" data-position="top center" data-variation="tiny inverted"></a>
</li> </li>

Loading…
Cancel
Save