finn/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Fix ignored errors when checking if organization, team member (#3177)

Browse source
This commit is contained in:
Ethan Koenig 2017-12-20 23:43:26 -08:00 • committed by Lauris BH
parent 529482135c
commit 515cdaa85d
16 changed files with 281 additions and 144 deletions
Download patch file Download diff file Expand all files Collapse all files

39
models/org.go
View file

@ -21,13 +21,13 @@ var (
)
// IsOwnedBy returns true if given user is in the owner team.
func (org *User) IsOwnedBy(uid int64) bool {
func (org *User) IsOwnedBy(uid int64) (bool, error) {
return IsOrganizationOwner(org.ID, uid)
}
// IsOrgMember returns true if given user is member of organization.
func (org *User) IsOrgMember(uid int64) bool {
return org.IsOrganization() && IsOrganizationMember(org.ID, uid)
func (org *User) IsOrgMember(uid int64) (bool, error) {
return IsOrganizationMember(org.ID, uid)
}
func (org *User) getTeam(e Engine, name string) (*Team, error) {
@ -285,32 +285,32 @@ type OrgUser struct {
}
// IsOrganizationOwner returns true if given user is in the owner team.
func IsOrganizationOwner(orgID, uid int64) bool {
has, _ := x.
func IsOrganizationOwner(orgID, uid int64) (bool, error) {
return x.
Where("is_owner=?", true).
And("uid=?", uid).
And("org_id=?", orgID).
Get(new(OrgUser))
return has
Table("org_user").
Exist()
}
// IsOrganizationMember returns true if given user is member of organization.
func IsOrganizationMember(orgID, uid int64) bool {
has, _ := x.
func IsOrganizationMember(orgID, uid int64) (bool, error) {
return x.
Where("uid=?", uid).
And("org_id=?", orgID).
Get(new(OrgUser))
return has
Table("org_user").
Exist()
}
// IsPublicMembership returns true if given user public his/her membership.
func IsPublicMembership(orgID, uid int64) bool {
has, _ := x.
func IsPublicMembership(orgID, uid int64) (bool, error) {
return x.
Where("uid=?", uid).
And("org_id=?", orgID).
And("is_public=?", true).
Get(new(OrgUser))
return has
Table("org_user").
Exist()
}
func getOrgsByUserID(sess *xorm.Session, userID int64, showAll bool) ([]*User, error) {
@ -401,8 +401,9 @@ func ChangeOrgUserStatus(orgID, uid int64, public bool) error {
// AddOrgUser adds new user to given organization.
func AddOrgUser(orgID, uid int64) error {
if IsOrganizationMember(orgID, uid) {
return nil
isAlreadyMember, err := IsOrganizationMember(orgID, uid)
if err != nil || isAlreadyMember {
return err
}
sess := x.NewSession()
@ -447,7 +448,9 @@ func RemoveOrgUser(orgID, userID int64) error {
}
// Check if the user to delete is the last member in owner team.
if IsOrganizationOwner(orgID, userID) {
if isOwner, err := IsOrganizationOwner(orgID, userID); err != nil {
return err
} else if isOwner {
t, err := org.GetOwnerTeam()
if err != nil {
return err

31
models/org_team.go
View file

@ -8,6 +8,8 @@ import (
"errors"
"fmt"
"strings"
"code.gitea.io/gitea/modules/log"
)
const ownerTeamName = "Owners"
@ -47,7 +49,12 @@ func (t *Team) IsOwnerTeam() bool {
// IsMember returns true if given user is a member of team.
func (t *Team) IsMember(userID int64) bool {
return IsTeamMember(t.OrgID, t.ID, userID)
isMember, err := IsTeamMember(t.OrgID, t.ID, userID)
if err != nil {
log.Error(4, "IsMember: %v", err)
return false
}
return isMember
}
func (t *Team) getRepositories(e Engine) error {
@ -413,17 +420,17 @@ type TeamUser struct {
UID int64 `xorm:"UNIQUE(s)"`
}
func isTeamMember(e Engine, orgID, teamID, userID int64) bool {
has, _ := e.
func isTeamMember(e Engine, orgID, teamID, userID int64) (bool, error) {
return e.
Where("org_id=?", orgID).
And("team_id=?", teamID).
And("uid=?", userID).
Get(new(TeamUser))
return has
Table("team_user").
Exist()
}
// IsTeamMember returns true if given user is a member of team.
func IsTeamMember(orgID, teamID, userID int64) bool {
func IsTeamMember(orgID, teamID, userID int64) (bool, error) {
return isTeamMember(x, orgID, teamID, userID)
}
@ -471,8 +478,9 @@ func GetUserTeams(orgID, userID int64) ([]*Team, error) {
// AddTeamMember adds new membership of given team to given organization,
// the user will have membership to given organization automatically when needed.
func AddTeamMember(team *Team, userID int64) error {
if IsTeamMember(team.OrgID, team.ID, userID) {
return nil
isAlreadyMember, err := IsTeamMember(team.OrgID, team.ID, userID)
if err != nil || isAlreadyMember {
return err
}
if err := AddOrgUser(team.OrgID, userID); err != nil {
@ -529,8 +537,9 @@ func AddTeamMember(team *Team, userID int64) error {
}
func removeTeamMember(e Engine, team *Team, userID int64) error {
if !isTeamMember(e, team.OrgID, team.ID, userID) {
return nil
isMember, err := isTeamMember(e, team.OrgID, team.ID, userID)
if err != nil || !isMember {
return err
}
// Check if the user to delete is the last member in owner team.
@ -566,7 +575,7 @@ func removeTeamMember(e Engine, team *Team, userID int64) error {
// This must exist.
ou := new(OrgUser)
_, err := e.
_, err = e.
Where("uid = ?", userID).
And("org_id = ?", team.OrgID).
Get(ou)

19
models/org_team_test.go
View file

@ -250,16 +250,21 @@ func TestDeleteTeam(t *testing.T) {
func TestIsTeamMember(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
test := func(orgID, teamID, userID int64, expected bool) {
isMember, err := IsTeamMember(orgID, teamID, userID)
assert.NoError(t, err)
assert.Equal(t, expected, isMember)
}
assert.True(t, IsTeamMember(3, 1, 2))
assert.False(t, IsTeamMember(3, 1, 4))
assert.False(t, IsTeamMember(3, 1, NonexistentID))
test(3, 1, 2, true)
test(3, 1, 4, false)
test(3, 1, NonexistentID, false)
assert.True(t, IsTeamMember(3, 2, 2))
assert.True(t, IsTeamMember(3, 2, 4))
test(3, 2, 2, true)
test(3, 2, 4, true)
assert.False(t, IsTeamMember(3, NonexistentID, NonexistentID))
assert.False(t, IsTeamMember(NonexistentID, NonexistentID, NonexistentID))
test(3, NonexistentID, NonexistentID, false)
test(NonexistentID, NonexistentID, NonexistentID, false)
}
func TestGetTeamMembers(t *testing.T) {

101
models/org_test.go
View file

@ -12,28 +12,44 @@ import (
func TestUser_IsOwnedBy(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
org := AssertExistsAndLoadBean(t, &User{ID: 3}).(*User)
assert.True(t, org.IsOwnedBy(2))
assert.False(t, org.IsOwnedBy(1))
assert.False(t, org.IsOwnedBy(3))
assert.False(t, org.IsOwnedBy(4))
nonOrg := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
assert.False(t, nonOrg.IsOwnedBy(2))
assert.False(t, nonOrg.IsOwnedBy(3))
for _, testCase := range []struct {
OrgID int64
UserID int64
ExpectedOwner bool
}{
{3, 2, true},
{3, 1, false},
{3, 3, false},
{3, 4, false},
{2, 2, false}, // user2 is not an organization
{2, 3, false},
} {
org := AssertExistsAndLoadBean(t, &User{ID: testCase.OrgID}).(*User)
isOwner, err := org.IsOwnedBy(testCase.UserID)
assert.NoError(t, err)
assert.Equal(t, testCase.ExpectedOwner, isOwner)
}
}
func TestUser_IsOrgMember(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
org := AssertExistsAndLoadBean(t, &User{ID: 3}).(*User)
assert.True(t, org.IsOrgMember(2))
assert.True(t, org.IsOrgMember(4))
assert.False(t, org.IsOrgMember(1))
assert.False(t, org.IsOrgMember(3))
nonOrg := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
assert.False(t, nonOrg.IsOrgMember(2))
assert.False(t, nonOrg.IsOrgMember(3))
for _, testCase := range []struct {
OrgID int64
UserID int64
ExpectedMember bool
}{
{3, 2, true},
{3, 4, true},
{3, 1, false},
{3, 3, false},
{2, 2, false}, // user2 is not an organization
{2, 3, false},
} {
org := AssertExistsAndLoadBean(t, &User{ID: testCase.OrgID}).(*User)
isMember, err := org.IsOrgMember(testCase.UserID)
assert.NoError(t, err)
assert.Equal(t, testCase.ExpectedMember, isMember)
}
}
func TestUser_GetTeam(t *testing.T) {
@ -257,31 +273,46 @@ func TestDeleteOrganization(t *testing.T) {
func TestIsOrganizationOwner(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
assert.True(t, IsOrganizationOwner(3, 2))
assert.False(t, IsOrganizationOwner(3, 3))
assert.True(t, IsOrganizationOwner(6, 5))
assert.False(t, IsOrganizationOwner(6, 4))
assert.False(t, IsOrganizationOwner(NonexistentID, NonexistentID))
test := func(orgID, userID int64, expected bool) {
isOwner, err := IsOrganizationOwner(orgID, userID)
assert.NoError(t, err)
assert.EqualValues(t, expected, isOwner)
}
test(3, 2, true)
test(3, 3, false)
test(6, 5, true)
test(6, 4, false)
test(NonexistentID, NonexistentID, false)
}
func TestIsOrganizationMember(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
assert.True(t, IsOrganizationMember(3, 2))
assert.False(t, IsOrganizationMember(3, 3))
assert.True(t, IsOrganizationMember(3, 4))
assert.True(t, IsOrganizationMember(6, 5))
assert.False(t, IsOrganizationMember(6, 4))
assert.False(t, IsOrganizationMember(NonexistentID, NonexistentID))
test := func(orgID, userID int64, expected bool) {
isMember, err := IsOrganizationMember(orgID, userID)
assert.NoError(t, err)
assert.EqualValues(t, expected, isMember)
}
test(3, 2, true)
test(3, 3, false)
test(3, 4, true)
test(6, 5, true)
test(6, 4, false)
test(NonexistentID, NonexistentID, false)
}
func TestIsPublicMembership(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
assert.True(t, IsPublicMembership(3, 2))
assert.False(t, IsPublicMembership(3, 3))
assert.False(t, IsPublicMembership(3, 4))
assert.True(t, IsPublicMembership(6, 5))
assert.False(t, IsPublicMembership(6, 4))
assert.False(t, IsPublicMembership(NonexistentID, NonexistentID))
test := func(orgID, userID int64, expected bool) {
isMember, err := IsPublicMembership(orgID, userID)
assert.NoError(t, err)
assert.EqualValues(t, expected, isMember)
}
test(3, 2, true)
test(3, 3, false)
test(3, 4, false)
test(6, 5, true)
test(6, 4, false)
test(NonexistentID, NonexistentID, false)
}
func TestGetOrgsByUserID(t *testing.T) {

14
models/repo.go
View file

@ -1493,12 +1493,18 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error
// Dummy object.
collaboration := &Collaboration{RepoID: repo.ID}
for _, c := range collaborators {
collaboration.UserID = c.ID
if c.ID == newOwner.ID || newOwner.IsOrgMember(c.ID) {
if _, err = sess.Delete(collaboration); err != nil {
return fmt.Errorf("remove collaborator '%d': %v", c.ID, err)
if c.ID != newOwner.ID {
isMember, err := newOwner.IsOrgMember(c.ID)
if err != nil {
return fmt.Errorf("IsOrgMember: %v", err)
} else if !isMember {
continue
}
}
collaboration.UserID = c.ID
if _, err = sess.Delete(collaboration); err != nil {
return fmt.Errorf("remove collaborator '%d': %v", c.ID, err)
}
}
// Remove old team-repository relations.

14
models/user.go
View file

@ -487,12 +487,22 @@ func (u *User) IsOrganization() bool {
// IsUserOrgOwner returns true if user is in the owner team of given organization.
func (u *User) IsUserOrgOwner(orgID int64) bool {
return IsOrganizationOwner(orgID, u.ID)
isOwner, err := IsOrganizationOwner(orgID, u.ID)
if err != nil {
log.Error(4, "IsOrganizationOwner: %v", err)
return false
}
return isOwner
}
// IsPublicMember returns true if user public his/her membership in given organization.
func (u *User) IsPublicMember(orgID int64) bool {
return IsPublicMembership(orgID, u.ID)
isMember, err := IsPublicMembership(orgID, u.ID)
if err != nil {
log.Error(4, "IsPublicMembership: %v", err)
return false
}
return isMember
}
func (u *User) getOrganizationCount(e Engine) (int64, error) {