diff --git a/server/database/perm.go b/server/database/perm.go index 1e741d761..54eec9a5d 100644 --- a/server/database/perm.go +++ b/server/database/perm.go @@ -116,7 +116,7 @@ func (db *permManager) Find(u *model.User, r *model.Repo) *model.Perm { // if the user is authenticated we'll retireive the // permission details from the database. perm, err := db.find(u, r) - if err != nil { + if err != nil && perm.ID != 0 { return perm } @@ -138,65 +138,20 @@ func (db *permManager) Find(u *model.User, r *model.Repo) *model.Perm { } func (db *permManager) Read(u *model.User, r *model.Repo) (bool, error) { - switch { - // if the repo is public, grant access. - case r.Private == false: - return true, nil - // if the repo is private and the user is nil, deny access - case r.Private == true && u == nil: - return false, nil - // if the user is a system admin, grant access - case u.Admin == true: - return true, nil - } - - // get the permissions from the database - perm, err := db.find(u, r) - return perm.Read, err + return db.Find(u, r).Read, nil } func (db *permManager) Write(u *model.User, r *model.Repo) (bool, error) { - switch { - // if the user is nil, deny access - case u == nil: - return false, nil - // if the user is a system admin, grant access - case u.Admin == true: - return true, nil - } - - // get the permissions from the database - perm, err := db.find(u, r) - return perm.Write, err + return db.Find(u, r).Write, nil } func (db *permManager) Admin(u *model.User, r *model.Repo) (bool, error) { - switch { - // if the user is nil, deny access - case u == nil: - return false, nil - // if the user is a system admin, grant access - case u.Admin == true: - return true, nil - } - - // get the permissions from the database - perm, err := db.find(u, r) - return perm.Admin, err + return db.Find(u, r).Admin, nil } func (db *permManager) Member(u *model.User, r *model.Repo) (bool, error) { - switch { - // if the user is nil, deny access - case u == nil: - return false, nil - case u.ID == r.UserID: - return true, nil - } - - // get the permissions from the database - perm, err := db.find(u, r) - return perm.Read, err + perm := db.Find(u, r) + return perm.Read && !perm.Guest, nil } func (db *permManager) find(u *model.User, r *model.Repo) (*model.Perm, error) { diff --git a/server/database/perm_test.go b/server/database/perm_test.go index cf107741c..90058ad7c 100644 --- a/server/database/perm_test.go +++ b/server/database/perm_test.go @@ -65,6 +65,53 @@ func Test_find(t *testing.T) { } } +func TestPermFind(t *testing.T) { + setup() + defer teardown() + + manager := NewPermManager(db).(*permManager) + + u := model.User{ID: 101, Admin: false} + r := model.Repo{ID: 201, Private: false} + + // public repos should always be accessible + if perm := manager.Find(&u, &r); !perm.Read { + t.Errorf("Public repos should always be READ accessible") + } + + // public repos should always be accessible, even to guest users + if perm := manager.Find(nil, &r); !perm.Read || perm.Write || perm.Admin { + t.Errorf("Public repos should always be READ accessible, even to nil users") + } + + // private repos should not be accessible to nil users + r.Private = true + if perm := manager.Find(nil, &r); perm.Read || perm.Write || perm.Admin { + t.Errorf("Private repos should not be READ accessible to nil users") + } + + // private repos should not be accessible to users without a row in the perm table. + r.Private = true + if perm := manager.Find(&u, &r); perm.Read || perm.Write || perm.Admin { + t.Errorf("Private repos should not be READ accessible to users without a row in the perm table.") + } + + // private repos should be accessible to admins + r.Private = true + u.Admin = true + if perm := manager.Find(&u, &r); !perm.Read || !perm.Write || !perm.Admin { + t.Errorf("Private repos should be READ accessible to admins") + } + + // private repos should be accessible to users with rows in the perm table. + r.ID = 200 + r.Private = true + u.Admin = false + if perm := manager.Find(&u, &r); !perm.Read { + t.Errorf("Private repos should be READ accessible to users with rows in the perm table.") + } +} + func TestPermRead(t *testing.T) { setup() defer teardown() @@ -92,7 +139,7 @@ func TestPermRead(t *testing.T) { // private repos should not be accessible to users without a row in the perm table. r.Private = true - if read, err := manager.Read(&u, &r); read || err != sql.ErrNoRows { + if read, _ := manager.Read(&u, &r); read { t.Errorf("Private repos should not be READ accessible to users without a row in the perm table.") } @@ -128,7 +175,7 @@ func TestPermWrite(t *testing.T) { } // repos should not be accessible to users without a row in the perm table. - if write, err := manager.Write(&u, &r); write || err != sql.ErrNoRows { + if write, _ := manager.Write(&u, &r); write { t.Errorf("Repos should not be WRITE accessible to users without a row in the perm table.") } @@ -169,7 +216,7 @@ func TestPermAdmin(t *testing.T) { } // repos should not be accessible to users without a row in the perm table. - if admin, err := manager.Admin(&u, &r); admin || err != sql.ErrNoRows { + if admin, _ := manager.Admin(&u, &r); admin { t.Errorf("Repos should not be ADMIN accessible to users without a row in the perm table.") } @@ -202,7 +249,7 @@ func TestPermRevoke(t *testing.T) { u := model.User{ID: 101} r := model.Repo{ID: 200} - manager := NewPermManager(db) + manager := NewPermManager(db).(*permManager) admin, err := manager.Admin(&u, &r) if !admin || err != nil { t.Errorf("Want Admin permission, got Admin %v, error %s", admin, err) @@ -213,9 +260,9 @@ func TestPermRevoke(t *testing.T) { t.Errorf("Want revoked permissions, got %s", err) } - admin, err = manager.Admin(&u, &r) - if admin == true || err != sql.ErrNoRows { - t.Errorf("Expected revoked permission, got Admin %v, error %v", admin, err) + perm, err := manager.find(&u, &r) + if perm.Admin == true || err != sql.ErrNoRows { + t.Errorf("Expected revoked permission, got Admin %v, error %v", perm.Admin, err) } }