-
Notifications
You must be signed in to change notification settings - Fork 55
Added support to redefine users of a repository #166
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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)) | ||
router.Get("/repository/{name:[^/]*/?[^/]+}/archive", http.HandlerFunc(getArchive)) | ||
router.Get("/repository/{name:[^/]*/?[^/]+}/contents", http.HandlerFunc(getFileContents)) | ||
router.Get("/repository/{name:[^/]*/?[^/]+}/tree", http.HandlerFunc(getTree)) | ||
|
@@ -82,6 +83,24 @@ func SetupRouter() *pat.Router { | |
return router | ||
} | ||
|
||
func setAccess(w http.ResponseWriter, r *http.Request) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 ofset
.grant
method just append new users to the list of users, ignoring the already existing usersrevoke
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 withrevoke
andgrant
methods, and can be dangerous if one of them does not works for some reason.There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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 inrepository
.There was a problem hiding this comment.
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
inrepository/:name
that serves such a purpose – renaming a repository. Perhaps we should extend it and support changingUsers
,ReadOnlyUsers
andIsPublic
attributes too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #167