Skip to content

Commit

Permalink
Restricted users (go-gitea#4334): initial implementation
Browse files Browse the repository at this point in the history
* Add User.IsRestricted & UI to edit it

* Pass user object instead of user id to places where IsRestricted flag matters

* Restricted users: maintain access rows for all referenced repos (incl public)

* Take logged in user & IsRestricted flag into account in org/repo listings, searches and accesses

* Add basic repo access tests for restricted users

Signed-off-by: Manush Dodunekov <manush@stendahls.se>
  • Loading branch information
mnsh committed Jan 6, 2020
1 parent 5749b26 commit 836f9d8
Show file tree
Hide file tree
Showing 30 changed files with 281 additions and 117 deletions.
40 changes: 27 additions & 13 deletions models/access.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,17 @@ type Access struct {
Mode AccessMode
}

func accessLevel(e Engine, userID int64, repo *Repository) (AccessMode, error) {
func accessLevel(e Engine, user *User, repo *Repository) (AccessMode, error) {
mode := AccessModeNone
if !repo.IsPrivate {
var userID int64
restricted := false

if user != nil {
userID = user.ID
restricted = user.IsRestricted
}

if !restricted && !repo.IsPrivate {
mode = AccessModeRead
}

Expand Down Expand Up @@ -163,17 +171,23 @@ func maxAccessMode(modes ...AccessMode) AccessMode {
}

// FIXME: do cross-comparison so reduce deletions and additions to the minimum?
func (repo *Repository) refreshAccesses(e Engine, accessMap map[int64]AccessMode) (err error) {
func (repo *Repository) refreshAccesses(e Engine, accessMap map[*User]AccessMode) (err error) {
minMode := AccessModeRead
if !repo.IsPrivate {
minMode = AccessModeWrite
}

newAccesses := make([]Access, 0, len(accessMap))
for userID, mode := range accessMap {
if mode < minMode {
// merge accessMap contents into a single ID-keyed map to eliminate duplicates
idMap := make(map[int64]AccessMode, len(accessMap))
for user, mode := range accessMap {
if mode < minMode && !user.IsRestricted {
continue
}
idMap[user.ID] = maxAccessMode(idMap[user.ID], mode)
}

newAccesses := make([]Access, 0, len(idMap))
for userID, mode := range idMap {
newAccesses = append(newAccesses, Access{
UserID: userID,
RepoID: repo.ID,
Expand All @@ -191,13 +205,13 @@ func (repo *Repository) refreshAccesses(e Engine, accessMap map[int64]AccessMode
}

// refreshCollaboratorAccesses retrieves repository collaborations with their access modes.
func (repo *Repository) refreshCollaboratorAccesses(e Engine, accessMap map[int64]AccessMode) error {
collaborations, err := repo.getCollaborations(e)
func (repo *Repository) refreshCollaboratorAccesses(e Engine, accessMap map[*User]AccessMode) error {
collaborators, err := repo.getCollaborators(e)
if err != nil {
return fmt.Errorf("getCollaborations: %v", err)
}
for _, c := range collaborations {
accessMap[c.UserID] = c.Mode
for _, c := range collaborators {
accessMap[c.User] = maxAccessMode(accessMap[c.User], c.Collaboration.Mode)
}
return nil
}
Expand All @@ -206,7 +220,7 @@ func (repo *Repository) refreshCollaboratorAccesses(e Engine, accessMap map[int6
// except the team whose ID is given. It is used to assign a team ID when
// remove repository from that team.
func (repo *Repository) recalculateTeamAccesses(e Engine, ignTeamID int64) (err error) {
accessMap := make(map[int64]AccessMode, 20)
accessMap := make(map[*User]AccessMode, 20)

if err = repo.getOwner(e); err != nil {
return err
Expand Down Expand Up @@ -239,7 +253,7 @@ func (repo *Repository) recalculateTeamAccesses(e Engine, ignTeamID int64) (err
return fmt.Errorf("getMembers '%d': %v", t.ID, err)
}
for _, m := range t.Members {
accessMap[m.ID] = maxAccessMode(accessMap[m.ID], t.Authorize)
accessMap[m] = maxAccessMode(accessMap[m], t.Authorize)
}
}

Expand Down Expand Up @@ -300,7 +314,7 @@ func (repo *Repository) recalculateAccesses(e Engine) error {
return repo.recalculateTeamAccesses(e, 0)
}

accessMap := make(map[int64]AccessMode, 20)
accessMap := make(map[*User]AccessMode, 20)
if err := repo.refreshCollaboratorAccesses(e, accessMap); err != nil {
return fmt.Errorf("refreshCollaboratorAccesses: %v", err)
}
Expand Down
50 changes: 50 additions & 0 deletions models/access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,20 @@ func TestAccessLevel(t *testing.T) {

user2 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
user5 := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
user29 := AssertExistsAndLoadBean(t, &User{ID: 29}).(*User)
// A public repository owned by User 2
repo1 := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
assert.False(t, repo1.IsPrivate)
// A private repository owned by Org 3
repo3 := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository)
assert.True(t, repo3.IsPrivate)

// Another public repository
repo4 := AssertExistsAndLoadBean(t, &Repository{ID: 4}).(*Repository)
assert.False(t, repo4.IsPrivate)
// org. owned private repo
repo24 := AssertExistsAndLoadBean(t, &Repository{ID: 24}).(*Repository)

level, err := AccessLevel(user2, repo1)
assert.NoError(t, err)
assert.Equal(t, AccessModeOwner, level)
Expand All @@ -37,6 +44,21 @@ func TestAccessLevel(t *testing.T) {
level, err = AccessLevel(user5, repo3)
assert.NoError(t, err)
assert.Equal(t, AccessModeNone, level)

// restricted user has no access to a public repo
level, err = AccessLevel(user29, repo1)
assert.NoError(t, err)
assert.Equal(t, AccessModeNone, level)

// ... unless he's a collaborator
level, err = AccessLevel(user29, repo4)
assert.NoError(t, err)
assert.Equal(t, AccessModeWrite, level)

// ... or a team member
level, err = AccessLevel(user29, repo24)
assert.NoError(t, err)
assert.Equal(t, AccessModeRead, level)
}

func TestHasAccess(t *testing.T) {
Expand Down Expand Up @@ -72,6 +94,11 @@ func TestUser_GetRepositoryAccesses(t *testing.T) {
accesses, err := user1.GetRepositoryAccesses()
assert.NoError(t, err)
assert.Len(t, accesses, 0)

user29 := AssertExistsAndLoadBean(t, &User{ID: 29}).(*User)
accesses, err = user29.GetRepositoryAccesses()
assert.NoError(t, err)
assert.Len(t, accesses, 2)
}

func TestUser_GetAccessibleRepositories(t *testing.T) {
Expand All @@ -86,6 +113,11 @@ func TestUser_GetAccessibleRepositories(t *testing.T) {
repos, err = user2.GetAccessibleRepositories(0)
assert.NoError(t, err)
assert.Len(t, repos, 1)

user29 := AssertExistsAndLoadBean(t, &User{ID: 29}).(*User)
repos, err = user29.GetAccessibleRepositories(0)
assert.NoError(t, err)
assert.Len(t, repos, 2)
}

func TestRepository_RecalculateAccesses(t *testing.T) {
Expand Down Expand Up @@ -119,3 +151,21 @@ func TestRepository_RecalculateAccesses2(t *testing.T) {
assert.NoError(t, err)
assert.False(t, has)
}

func TestRepository_RecalculateAccesses3(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
team5 := AssertExistsAndLoadBean(t, &Team{ID: 5}).(*Team)
user29 := AssertExistsAndLoadBean(t, &User{ID: 29}).(*User)

has, err := x.Get(&Access{UserID: 29, RepoID: 23})
assert.NoError(t, err)
assert.False(t, has)

// adding user29 to team5 should add an explicit access row for repo 23
// even though repo 23 is public
assert.NoError(t, AddTeamMember(team5, user29.ID))

has, err = x.Get(&Access{UserID: 29, RepoID: 23})
assert.NoError(t, err)
assert.True(t, has)
}
20 changes: 14 additions & 6 deletions models/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,20 +410,26 @@ func (pc *PushCommits) AvatarLink(email string) string {

// GetFeedsOptions options for retrieving feeds
type GetFeedsOptions struct {
RequestedUser *User
RequestingUserID int64
IncludePrivate bool // include private actions
OnlyPerformedBy bool // only actions performed by requested user
IncludeDeleted bool // include deleted actions
RequestedUser *User // the user we want activity for
Actor *User // the user viewing the activity
IncludePrivate bool // include private actions
OnlyPerformedBy bool // only actions performed by requested user
IncludeDeleted bool // include deleted actions
}

// GetFeeds returns actions according to the provided options
func GetFeeds(opts GetFeedsOptions) ([]*Action, error) {
cond := builder.NewCond()

var repoIDs []int64
var actorID int64

if opts.Actor != nil {
actorID = opts.Actor.ID
}

if opts.RequestedUser.IsOrganization() {
env, err := opts.RequestedUser.AccessibleReposEnv(opts.RequestingUserID)
env, err := opts.RequestedUser.AccessibleReposEnv(actorID)
if err != nil {
return nil, fmt.Errorf("AccessibleReposEnv: %v", err)
}
Expand All @@ -432,6 +438,8 @@ func GetFeeds(opts GetFeedsOptions) ([]*Action, error) {
}

cond = cond.And(builder.In("repo_id", repoIDs))
} else if opts.Actor != nil {
cond = cond.And(builder.In("repo_id", opts.Actor.AccessibleRepoIDsQuery()))
}

cond = cond.And(builder.Eq{"user_id": opts.RequestedUser.ID})
Expand Down
40 changes: 20 additions & 20 deletions models/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,11 @@ func TestGetFeeds(t *testing.T) {
user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)

actions, err := GetFeeds(GetFeedsOptions{
RequestedUser: user,
RequestingUserID: user.ID,
IncludePrivate: true,
OnlyPerformedBy: false,
IncludeDeleted: true,
RequestedUser: user,
Actor: user,
IncludePrivate: true,
OnlyPerformedBy: false,
IncludeDeleted: true,
})
assert.NoError(t, err)
if assert.Len(t, actions, 1) {
Expand All @@ -146,10 +146,10 @@ func TestGetFeeds(t *testing.T) {
}

actions, err = GetFeeds(GetFeedsOptions{
RequestedUser: user,
RequestingUserID: user.ID,
IncludePrivate: false,
OnlyPerformedBy: false,
RequestedUser: user,
Actor: user,
IncludePrivate: false,
OnlyPerformedBy: false,
})
assert.NoError(t, err)
assert.Len(t, actions, 0)
Expand All @@ -159,14 +159,14 @@ func TestGetFeeds2(t *testing.T) {
// test with an organization user
assert.NoError(t, PrepareTestDatabase())
org := AssertExistsAndLoadBean(t, &User{ID: 3}).(*User)
const userID = 2 // user2 is an owner of the organization
user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)

actions, err := GetFeeds(GetFeedsOptions{
RequestedUser: org,
RequestingUserID: userID,
IncludePrivate: true,
OnlyPerformedBy: false,
IncludeDeleted: true,
RequestedUser: org,
Actor: user,
IncludePrivate: true,
OnlyPerformedBy: false,
IncludeDeleted: true,
})
assert.NoError(t, err)
assert.Len(t, actions, 1)
Expand All @@ -176,11 +176,11 @@ func TestGetFeeds2(t *testing.T) {
}

actions, err = GetFeeds(GetFeedsOptions{
RequestedUser: org,
RequestingUserID: userID,
IncludePrivate: false,
OnlyPerformedBy: false,
IncludeDeleted: true,
RequestedUser: org,
Actor: user,
IncludePrivate: false,
OnlyPerformedBy: false,
IncludeDeleted: true,
})
assert.NoError(t, err)
assert.Len(t, actions, 0)
Expand Down
14 changes: 13 additions & 1 deletion models/fixtures/access.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,16 @@
id: 13
user_id: 20
repo_id: 28
mode: 4 # owner
mode: 4 # owner

-
id: 14
user_id: 29
repo_id: 4
mode: 2 # write (collaborator)

-
id: 15
user_id: 29
repo_id: 24
mode: 1 # read
8 changes: 7 additions & 1 deletion models/fixtures/collaboration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,10 @@
id: 3
repo_id: 40
user_id: 4
mode: 2 # write
mode: 2 # write

-
id: 4
repo_id: 4
user_id: 29
mode: 2 # write
5 changes: 5 additions & 0 deletions models/fixtures/org_user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,8 @@
org_id: 6
is_public: true

-
id: 11
uid: 29
org_id: 17
is_public: true
2 changes: 1 addition & 1 deletion models/fixtures/team.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
name: review_team
authorize: 1 # read
num_repos: 1
num_members: 1
num_members: 2

-
id: 10
Expand Down
6 changes: 6 additions & 0 deletions models/fixtures/team_user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,9 @@
org_id: 6
team_id: 13
uid: 28

-
id: 15
org_id: 17
team_id: 9
uid: 29
17 changes: 16 additions & 1 deletion models/fixtures/user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@
avatar_email: user17@example.com
num_repos: 2
is_active: true
num_members: 2
num_members: 3
num_teams: 3

-
Expand Down Expand Up @@ -463,3 +463,18 @@
num_following: 0
is_active: true

-
id: 29
lower_name: user29
name: user29
full_name: User 29
email: user29@example.com
passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password
type: 0 # individual
salt: ZogKvWdyEx
is_admin: false
is_restricted: true
avatar: avatar29
avatar_email: user29@example.com
num_repos: 0
is_active: true
4 changes: 2 additions & 2 deletions models/lfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func LFSObjectAccessible(user *User, oid string) (bool, error) {
count, err := x.Count(&LFSMetaObject{Oid: oid})
return (count > 0), err
}
cond := accessibleRepositoryCondition(user.ID)
cond := accessibleRepositoryCondition(user)
count, err := x.Where(cond).Join("INNER", "repository", "`lfs_meta_object`.repository_id = `repository`.id").Count(&LFSMetaObject{Oid: oid})
return (count > 0), err
}
Expand All @@ -182,7 +182,7 @@ func LFSAutoAssociate(metas []*LFSMetaObject, user *User, repoID int64) error {
cond := builder.NewCond()
if !user.IsAdmin {
cond = builder.In("`lfs_meta_object`.repository_id",
builder.Select("`repository`.id").From("repository").Where(accessibleRepositoryCondition(user.ID)))
builder.Select("`repository`.id").From("repository").Where(accessibleRepositoryCondition(user)))
}
newMetas := make([]*LFSMetaObject, 0, len(metas))
if err := sess.Cols("oid").Where(cond).In("oid", oids...).GroupBy("oid").Find(&newMetas); err != nil {
Expand Down
Loading

0 comments on commit 836f9d8

Please sign in to comment.