Skip to content
This repository has been archived by the owner on Apr 26, 2021. It is now read-only.

Added support to redefine users of a repository #166

Merged
merged 1 commit into from
Sep 10, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions api/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func SetupRouter() *pat.Router {
router.Post("/user", http.HandlerFunc(newUser))
router.Delete("/user/{name}", http.HandlerFunc(removeUser))
router.Delete("/repository/revoke", http.HandlerFunc(revokeAccess))
router.Put("/repository/set", http.HandlerFunc(setAccess))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about grant instead of set?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A route called repository/grant already exists and its purpose is different of set.
grant method just append new users to the list of users, ignoring the already existing users
revoke does the reverse, but also ignores the others too.
The set method, forces the users list to be the same as the users argument. Reseting all the user array. This method is important to garantee a correct updating permission flux in-one-action that normally will be implemented with revoke and grant methods, and can be dangerous if one of them does not works for some reason.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, that makes sense. Thanks for clarifying!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that instead "set" we should just create a route to change/update the repository attributes. A put in repository.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I agree. There's already a PUT in repository/:name that serves such a purpose – renaming a repository. Perhaps we should extend it and support changing Users, ReadOnlyUsers and IsPublic attributes too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #167

router.Get("/repository/{name:[^/]*/?[^/]+}/archive", http.HandlerFunc(getArchive))
router.Get("/repository/{name:[^/]*/?[^/]+}/contents", http.HandlerFunc(getFileContents))
router.Get("/repository/{name:[^/]*/?[^/]+}/tree", http.HandlerFunc(getTree))
Expand All @@ -82,6 +83,24 @@ func SetupRouter() *pat.Router {
return router
}

func setAccess(w http.ResponseWriter, r *http.Request) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

repositories, users, err := accessParameters(r.Body)
readOnly := r.URL.Query().Get("readonly") == "yes"
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
if err := repository.SetAccess(repositories, users, readOnly); err != nil {
http.Error(w, err.Error(), http.StatusNotFound)
return
}
if readOnly {
fmt.Fprintf(w, "Successfully set read-only access to users \"%s\" into repository \"%s\"", users, repositories)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you can use just %q instead of "%s". This will not result in the same output, but I believe that the result of using %q is better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just maintain the pattern of others handlers, to do this change I think is better to change them all in another patch, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, definitely. We can go with the current version and change it all in another pull request. I merging this one right now. Thanks for contributing!

} else {
fmt.Fprintf(w, "Successfully set full access to users \"%s\" into repository \"%s\"", users, repositories)
}
}

func grantAccess(w http.ResponseWriter, r *http.Request) {
repositories, users, err := accessParameters(r.Body)
readOnly := r.URL.Query().Get("readonly") == "yes"
Expand Down
65 changes: 65 additions & 0 deletions api/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ func post(url string, b io.Reader, c *gocheck.C) (*httptest.ResponseRecorder, *h
return request("POST", url, b, c)
}

func put(url string, b io.Reader, c *gocheck.C) (*httptest.ResponseRecorder, *http.Request) {
return request("PUT", url, b, c)
}

func del(url string, b io.Reader, c *gocheck.C) (*httptest.ResponseRecorder, *http.Request) {
return request("DELETE", url, b, c)
}
Expand Down Expand Up @@ -338,6 +342,67 @@ func (s *S) TestNewRepositoryShouldReturnErrorWhenBodyIsEmpty(c *gocheck.C) {
c.Assert(recorder.Code, gocheck.Equals, 400)
}

func (s *S) TestSetAccessShouldReturnErrorWhenBodyIsEmpty(c *gocheck.C) {
b := strings.NewReader("")
recorder, request := put("/repository/set", b, c)
s.router.ServeHTTP(recorder, request)
c.Assert(recorder.Code, gocheck.Equals, 400)
}

func (s *S) TestSetAccessUpdatesReposDocument(c *gocheck.C) {
u, err := user.New("pippin", map[string]string{})
conn, err := db.Conn()
c.Assert(err, gocheck.IsNil)
defer conn.Close()
defer conn.User().Remove(bson.M{"_id": "pippin"})
c.Assert(err, gocheck.IsNil)
r := repository.Repository{Name: "onerepo", Users: []string{"oneuser"}}
err = conn.Repository().Insert(&r)
c.Assert(err, gocheck.IsNil)
defer conn.Repository().Remove(bson.M{"_id": r.Name})
r2 := repository.Repository{Name: "otherepo", Users: []string{"otheruser"}}
err = conn.Repository().Insert(&r2)
c.Assert(err, gocheck.IsNil)
defer conn.Repository().Remove(bson.M{"_id": r2.Name})
b := bytes.NewBufferString(fmt.Sprintf(`{"repositories": ["%s", "%s"], "users": ["%s"]}`, r.Name, r2.Name, u.Name))
rec, req := put("/repository/set", b, c)
s.router.ServeHTTP(rec, req)
var repos []repository.Repository
err = conn.Repository().Find(bson.M{"_id": bson.M{"$in": []string{r.Name, r2.Name}}}).All(&repos)
c.Assert(err, gocheck.IsNil)
c.Assert(rec.Code, gocheck.Equals, 200)
for _, repo := range repos {
c.Assert(repo.Users, gocheck.DeepEquals, []string{u.Name})
}
}

func (s *S) TestSetReadonlyAccessUpdatesReposDocument(c *gocheck.C) {
u, err := user.New("pippin", map[string]string{})
conn, err := db.Conn()
c.Assert(err, gocheck.IsNil)
defer conn.Close()
defer conn.User().Remove(bson.M{"_id": "pippin"})
c.Assert(err, gocheck.IsNil)
r := repository.Repository{Name: "onerepo", ReadOnlyUsers: []string{"oneuser"}}
err = conn.Repository().Insert(&r)
c.Assert(err, gocheck.IsNil)
defer conn.Repository().Remove(bson.M{"_id": r.Name})
r2 := repository.Repository{Name: "otherepo", ReadOnlyUsers: []string{"otheruser"}}
err = conn.Repository().Insert(&r2)
c.Assert(err, gocheck.IsNil)
defer conn.Repository().Remove(bson.M{"_id": r2.Name})
b := bytes.NewBufferString(fmt.Sprintf(`{"repositories": ["%s", "%s"], "users": ["%s"]}`, r.Name, r2.Name, u.Name))
rec, req := put("/repository/set?readonly=yes", b, c)
s.router.ServeHTTP(rec, req)
var repos []repository.Repository
err = conn.Repository().Find(bson.M{"_id": bson.M{"$in": []string{r.Name, r2.Name}}}).All(&repos)
c.Assert(err, gocheck.IsNil)
c.Assert(rec.Code, gocheck.Equals, 200)
for _, repo := range repos {
c.Assert(repo.ReadOnlyUsers, gocheck.DeepEquals, []string{u.Name})
}
}

func (s *S) TestGrantAccessUpdatesReposDocument(c *gocheck.C) {
u, err := user.New("pippin", map[string]string{})
conn, err := db.Conn()
Expand Down
21 changes: 21 additions & 0 deletions docs/source/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,27 @@ Repository retrieval

Retrieves information about a repository.

Access set in repository
--------------------------

Redefines collections of users with read and write access into a repository. Specify ``readonly=yes`` if you'd like to set read-only access.

* Method: PUT
* URI: /repository/set
* Format: JSON

Example URL for **read/write** access (http://gandalf-server omitted for clarity)::

$ curl -XPUT /repository/set \ # PUT to /repository/set
-d '{"repositories": ["myrepo"], \ # Collection of repositories
"users": ["john", "james"]}' # Users with read/write access

Example URL for **read-only** access (http://gandalf-server omitted for clarity)::

$ curl -XPUT /repository/set?readonly=yes \ # PUT to /repository/set
-d '{"repositories": ["myrepo"], \ # Collection of repositories
"users": ["bob", "alice"]}' # Users with read-only access

Access grant in repository
--------------------------

Expand Down
16 changes: 16 additions & 0 deletions repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,22 @@ func (r *Repository) isValid() (bool, error) {
return true, nil
}

// SetAccess gives full or read-only permission for users in all specified repositories.
// It redefines all users permissions, replacing the respective user collection
func SetAccess(rNames, uNames []string, readOnly bool) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

conn, err := db.Conn()
if err != nil {
return err
}
defer conn.Close()
if readOnly {
_, err = conn.Repository().UpdateAll(bson.M{"_id": bson.M{"$in": rNames}}, bson.M{"$set": bson.M{"readonlyusers": uNames}})
} else {
_, err = conn.Repository().UpdateAll(bson.M{"_id": bson.M{"$in": rNames}}, bson.M{"$set": bson.M{"users": uNames}})
}
return err
}

// GrantAccess gives full or read-only permission for users in all specified repositories.
// If any of the repositories/users does not exist, GrantAccess just skips it.
func GrantAccess(rNames, uNames []string, readOnly bool) error {
Expand Down
62 changes: 62 additions & 0 deletions repository/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,68 @@ func (s *S) TestReadWriteURLUseUidFromConfigFile(c *gocheck.C) {
c.Assert(remote, gocheck.Equals, fmt.Sprintf("test@%s:f#.git", host))
}

func (s *S) TestSetAccessShouldAddUserToListOfRepositories(c *gocheck.C) {
tmpdir, err := commandmocker.Add("git", "$*")
c.Assert(err, gocheck.IsNil)
defer commandmocker.Remove(tmpdir)
r, err := New("proj1", []string{"someuser"}, []string{"otheruser"}, true)
c.Assert(err, gocheck.IsNil)
conn, err := db.Conn()
c.Assert(err, gocheck.IsNil)
defer conn.Close()
defer conn.Repository().RemoveId(r.Name)
r2, err := New("proj2", []string{"otheruser"}, []string{"someuser"}, true)
c.Assert(err, gocheck.IsNil)
defer conn.Repository().RemoveId(r2.Name)
u := struct {
Name string `bson:"_id"`
}{Name: "lolcat"}
err = conn.User().Insert(&u)
c.Assert(err, gocheck.IsNil)
defer conn.User().RemoveId(u.Name)
err = SetAccess([]string{r.Name, r2.Name}, []string{u.Name}, false)
c.Assert(err, gocheck.IsNil)
err = conn.Repository().FindId(r.Name).One(&r)
c.Assert(err, gocheck.IsNil)
err = conn.Repository().FindId(r2.Name).One(&r2)
c.Assert(err, gocheck.IsNil)
c.Assert(r.Users, gocheck.DeepEquals, []string{u.Name})
c.Assert(r2.Users, gocheck.DeepEquals, []string{u.Name})
c.Assert(r.ReadOnlyUsers, gocheck.DeepEquals, []string{"otheruser"})
c.Assert(r2.ReadOnlyUsers, gocheck.DeepEquals, []string{"someuser"})
}

func (s *S) TestSetReadonlyAccessShouldAddUserToListOfRepositories(c *gocheck.C) {
tmpdir, err := commandmocker.Add("git", "$*")
c.Assert(err, gocheck.IsNil)
defer commandmocker.Remove(tmpdir)
r, err := New("proj1", []string{"someuser"}, []string{"otheruser"}, true)
c.Assert(err, gocheck.IsNil)
conn, err := db.Conn()
c.Assert(err, gocheck.IsNil)
defer conn.Close()
defer conn.Repository().RemoveId(r.Name)
r2, err := New("proj2", []string{"otheruser"}, []string{"someuser"}, true)
c.Assert(err, gocheck.IsNil)
defer conn.Repository().RemoveId(r2.Name)
u := struct {
Name string `bson:"_id"`
}{Name: "lolcat"}
err = conn.User().Insert(&u)
c.Assert(err, gocheck.IsNil)
defer conn.User().RemoveId(u.Name)
err = SetAccess([]string{r.Name, r2.Name}, []string{u.Name}, true)
c.Assert(err, gocheck.IsNil)
err = conn.Repository().FindId(r.Name).One(&r)
c.Assert(err, gocheck.IsNil)
err = conn.Repository().FindId(r2.Name).One(&r2)
c.Assert(err, gocheck.IsNil)
c.Assert(r.Users, gocheck.DeepEquals, []string{"someuser"})
c.Assert(r2.Users, gocheck.DeepEquals, []string{"otheruser"})
c.Assert(r.ReadOnlyUsers, gocheck.DeepEquals, []string{u.Name})
c.Assert(r2.ReadOnlyUsers, gocheck.DeepEquals, []string{u.Name})
}

func (s *S) TestGrantAccessShouldAddUserToListOfRepositories(c *gocheck.C) {
tmpdir, err := commandmocker.Add("git", "$*")
c.Assert(err, gocheck.IsNil)
Expand Down