From 05a1b3cbd8767871116cb1601dd1e246311dfaea Mon Sep 17 00:00:00 2001 From: Anton Markelov Date: Wed, 26 Jul 2017 15:55:34 +1000 Subject: [PATCH 1/7] add admin role, restrict users without it --- api-docs.yml | 2 ++ api/users.go | 45 +++++++++++++++++++++++++++++++++++++- db/User.go | 1 + db/migrations/v2.4.2.sql | 1 + db/versionHistory.go | 1 + public/html/users/add.pug | 9 ++++++++ public/html/users/list.pug | 8 ++++++- public/html/users/user.pug | 4 ++++ 8 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 db/migrations/v2.4.2.sql diff --git a/api-docs.yml b/api-docs.yml index 9cc321d80..ffa63a702 100644 --- a/api-docs.yml +++ b/api-docs.yml @@ -55,6 +55,8 @@ definitions: format: date-time alert: type: boolean + admin: + type: boolean APIToken: type: object properties: diff --git a/api/users.go b/api/users.go index 318eb668f..3b65ba913 100644 --- a/api/users.go +++ b/api/users.go @@ -28,6 +28,13 @@ func addUser(w http.ResponseWriter, r *http.Request) { return } + editor := context.Get(r, "user").(*db.User) + if editor.Admin != true { + log.Warn(editor.Username + " doesn't permitted for user creating") + w.WriteHeader(http.StatusUnauthorized) + return + } + user.Created = time.Now() if err := db.Mysql.Insert(&user); err != nil { @@ -53,23 +60,44 @@ func getUserMiddleware(w http.ResponseWriter, r *http.Request) { panic(err) } + editor := context.Get(r, "user").(*db.User) + if editor.Admin != true && editor.ID != user.ID { + log.Warn(editor.Username + " doesn't permitted for user editing") + w.WriteHeader(http.StatusUnauthorized) + return + } + context.Set(r, "_user", user) } func updateUser(w http.ResponseWriter, r *http.Request) { oldUser := context.Get(r, "_user").(db.User) + editor := context.Get(r, "user").(*db.User) var user db.User if err := mulekick.Bind(w, r, &user); err != nil { return } + if editor.Admin != true && editor.ID != oldUser.ID { + log.Warn(editor.Username + " doesn't permitted for user editing") + w.WriteHeader(http.StatusUnauthorized) + return + } + + if editor.ID == oldUser.ID && oldUser.Admin != user.Admin { + log.Warn("User can't edit his own role") + w.WriteHeader(http.StatusUnauthorized) + return + } + if oldUser.External == true && oldUser.Username != user.Username { log.Warn("Username is not editable for external LDAP users") w.WriteHeader(http.StatusBadRequest) + return } - if _, err := db.Mysql.Exec("update user set name=?, username=?, email=?, alert=? where id=?", user.Name, user.Username, user.Email, user.Alert, oldUser.ID); err != nil { + if _, err := db.Mysql.Exec("update user set name=?, username=?, email=?, alert=?, admin=? where id=?", user.Name, user.Username, user.Email, user.Alert, user.Admin, oldUser.ID); err != nil { panic(err) } @@ -78,10 +106,18 @@ func updateUser(w http.ResponseWriter, r *http.Request) { func updateUserPassword(w http.ResponseWriter, r *http.Request) { user := context.Get(r, "_user").(db.User) + editor := context.Get(r, "user").(*db.User) + var pwd struct { Pwd string `json:"password"` } + if editor.Admin != true && editor.ID != user.ID { + log.Warn(editor.Username + " doesn't permitted for user editing") + w.WriteHeader(http.StatusUnauthorized) + return + } + if user.External == true { log.Warn("Password is not editable for external LDAP users") w.WriteHeader(http.StatusBadRequest) @@ -102,6 +138,13 @@ func updateUserPassword(w http.ResponseWriter, r *http.Request) { func deleteUser(w http.ResponseWriter, r *http.Request) { user := context.Get(r, "_user").(db.User) + editor := context.Get(r, "user").(*db.User) + + if editor.Admin != true && editor.ID != user.ID { + log.Warn(editor.Username + " doesn't permitted for user deletion") + w.WriteHeader(http.StatusUnauthorized) + return + } if _, err := db.Mysql.Exec("delete from project__user where user_id=?", user.ID); err != nil { panic(err) diff --git a/db/User.go b/db/User.go index 4e9bf033f..7e1326b3d 100644 --- a/db/User.go +++ b/db/User.go @@ -11,6 +11,7 @@ type User struct { Name string `db:"name" json:"name" binding:"required"` Email string `db:"email" json:"email" binding:"required"` Password string `db:"password" json:"-"` + Admin bool `db:"admin" json:"admin"` External bool `db:"external" json:"external"` Alert bool `db:"alert" json:"alert"` } diff --git a/db/migrations/v2.4.2.sql b/db/migrations/v2.4.2.sql new file mode 100644 index 000000000..4fd21b530 --- /dev/null +++ b/db/migrations/v2.4.2.sql @@ -0,0 +1 @@ +ALTER TABLE user ADD admin BOOLEAN NOT NULL DEFAULT 1 AFTER password; \ No newline at end of file diff --git a/db/versionHistory.go b/db/versionHistory.go index 370ec4a82..59fa119cf 100644 --- a/db/versionHistory.go +++ b/db/versionHistory.go @@ -71,5 +71,6 @@ func init() { {Major: 2, Minor: 3, Patch: 1}, {Major: 2, Minor: 3, Patch: 2}, {Major: 2, Minor: 4}, + {Major: 2, Minor: 4, Patch: 2}, } } diff --git a/public/html/users/add.pug b/public/html/users/add.pug index eac1eecbe..00d5e429d 100644 --- a/public/html/users/add.pug +++ b/public/html/users/add.pug @@ -20,6 +20,15 @@ .col-sm-6 input#password.form-control(type="password" placeholder="User Password" ng-model="user.password" required) + .form-group + .col-sm-8.col-sm-offset-4: .checkbox: label + input#admin(type="checkbox" title="User have admin privileges" ng-model="user.admin") + | Admin user + .form-group + .col-sm-8.col-sm-offset-4: .checkbox: label + input#alert(type="checkbox" title="Send email alerts about failed tasks" ng-model="user.alert") + | Send alerts + .modal-footer button.btn.btn-default.pull-left(ng-click="$dismiss()") Dismiss button.btn.btn-success(ng-click="$close(user)") Create User diff --git a/public/html/users/list.pug b/public/html/users/list.pug index ceea169ff..7c7d74555 100644 --- a/public/html/users/list.pug +++ b/public/html/users/list.pug @@ -1,15 +1,21 @@ .container-fluid: .row: .col-sm-12 h3.no-top-margin Users - button.btn.btn-primary.pull-right(ng-click="addUser()") New User + button.btn.btn-primary.pull-right(ng-click="addUser()" ng-if="user.admin == true") New User table.table.table-hover thead: tr th Name th Username th Email + th Alert + th Admin + th External tr(ng-repeat="u in users" ng-class="{ info: u.id == user.id }" ui-sref="users.user({ user_id: u.id })" style="cursor: pointer;") td {{ u.name }} td {{ u.username }} td {{ u.email }} + td {{ u.alert }} + td {{ u.admin }} + td {{ u.external }} p(ng-show="users.length == 0") No Users diff --git a/public/html/users/user.pug b/public/html/users/user.pug index a776f5531..33cdb1528 100644 --- a/public/html/users/user.pug +++ b/public/html/users/user.pug @@ -15,6 +15,10 @@ .form-group label.control-label.col-sm-4 Password .col-sm-8: input.form-control(type="password" placeholder="Enter new password" ng-readonly="user.external == true" ng-model="user.password") + .form-group(ng-if="!is_self") + .col-sm-8.col-sm-offset-4: .checkbox: label + input(type="checkbox" title="User have admin privileges" ng-model="user.admin") + | Admin user .form-group .col-sm-8.col-sm-offset-4: .checkbox: label input(type="checkbox" title="Send email alerts about failed tasks" ng-model="user.alert") From 59c10ac7c4b640282d76e6dff8f16321640100f8 Mon Sep 17 00:00:00 2001 From: Anton Markelov Date: Wed, 26 Jul 2017 16:37:16 +1000 Subject: [PATCH 2/7] add admin rights for first user --- cli/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/main.go b/cli/main.go index 6d3f77ae6..3c0276df9 100644 --- a/cli/main.go +++ b/cli/main.go @@ -143,7 +143,7 @@ func doSetup() int { user.Password = readNewline(" > Password: ", stdin) pwdHash, _ := bcrypt.GenerateFromPassword([]byte(user.Password), 11) - if _, err := db.Mysql.Exec("insert into user set name=?, username=?, email=?, password=?, created=UTC_TIMESTAMP()", user.Name, user.Username, user.Email, pwdHash); err != nil { + if _, err := db.Mysql.Exec("insert into user set name=?, username=?, email=?, password=?, admin=1, created=UTC_TIMESTAMP()", user.Name, user.Username, user.Email, pwdHash); err != nil { fmt.Printf(" Inserting user failed. If you already have a user, you can disregard this error.\n %v\n", err.Error()) os.Exit(1) } From fe5dcdc1c8e6982e1850886cb25aab92cbc13c35 Mon Sep 17 00:00:00 2001 From: Anton Markelov Date: Sat, 17 Feb 2018 09:46:23 +1000 Subject: [PATCH 3/7] minor fixes after code review --- api/users.go | 2 +- db/versionHistory.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/users.go b/api/users.go index 3b65ba913..4a4894140 100644 --- a/api/users.go +++ b/api/users.go @@ -30,7 +30,7 @@ func addUser(w http.ResponseWriter, r *http.Request) { editor := context.Get(r, "user").(*db.User) if editor.Admin != true { - log.Warn(editor.Username + " doesn't permitted for user creating") + log.Warn(editor.Username + " doesn't permit user creation") w.WriteHeader(http.StatusUnauthorized) return } diff --git a/db/versionHistory.go b/db/versionHistory.go index 59fa119cf..e47872f43 100644 --- a/db/versionHistory.go +++ b/db/versionHistory.go @@ -71,6 +71,6 @@ func init() { {Major: 2, Minor: 3, Patch: 1}, {Major: 2, Minor: 3, Patch: 2}, {Major: 2, Minor: 4}, - {Major: 2, Minor: 4, Patch: 2}, + {Major: 2, Minor: 5}, } } From 5e26bfb92c6f3764e803dfe512fbdc6433a687ac Mon Sep 17 00:00:00 2001 From: Anton Markelov Date: Sat, 17 Feb 2018 09:52:08 +1000 Subject: [PATCH 4/7] fix for https://github.com/ansible-semaphore/semaphore/issues/158 --- api/tasks/http.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/api/tasks/http.go b/api/tasks/http.go index d066a35c3..2323a2209 100644 --- a/api/tasks/http.go +++ b/api/tasks/http.go @@ -105,6 +105,13 @@ func GetTaskOutput(w http.ResponseWriter, r *http.Request) { func RemoveTask(w http.ResponseWriter, r *http.Request) { task := context.Get(r, "task").(db.Task) + editor := context.Get(r, "user").(*db.User) + + if editor.Admin != true { + log.Warn(editor.Username + " doesn't permit task log deletion") + w.WriteHeader(http.StatusUnauthorized) + return + } statements := []string{ "delete from task__output where task_id=?", From 910c8bc4e9c8472d016a872db1f6621a1d6d47f2 Mon Sep 17 00:00:00 2001 From: Anton Markelov Date: Mon, 19 Feb 2018 08:49:40 +1000 Subject: [PATCH 5/7] another minor fixes after review --- api/tasks/http.go | 1 + api/users.go | 2 +- public/html/users/user.pug | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/api/tasks/http.go b/api/tasks/http.go index 2323a2209..31654f303 100644 --- a/api/tasks/http.go +++ b/api/tasks/http.go @@ -5,6 +5,7 @@ import ( "strconv" "time" + log "github.com/Sirupsen/logrus" "github.com/ansible-semaphore/semaphore/db" "github.com/ansible-semaphore/semaphore/util" "github.com/castawaylabs/mulekick" diff --git a/api/users.go b/api/users.go index 4a4894140..c03d7690e 100644 --- a/api/users.go +++ b/api/users.go @@ -62,7 +62,7 @@ func getUserMiddleware(w http.ResponseWriter, r *http.Request) { editor := context.Get(r, "user").(*db.User) if editor.Admin != true && editor.ID != user.ID { - log.Warn(editor.Username + " doesn't permitted for user editing") + log.Warn(editor.Username + " is not permitted to edit users") w.WriteHeader(http.StatusUnauthorized) return } diff --git a/public/html/users/user.pug b/public/html/users/user.pug index 33cdb1528..e20b51e39 100644 --- a/public/html/users/user.pug +++ b/public/html/users/user.pug @@ -17,7 +17,7 @@ .col-sm-8: input.form-control(type="password" placeholder="Enter new password" ng-readonly="user.external == true" ng-model="user.password") .form-group(ng-if="!is_self") .col-sm-8.col-sm-offset-4: .checkbox: label - input(type="checkbox" title="User have admin privileges" ng-model="user.admin") + input(type="checkbox" title="User has admin privileges" ng-model="user.admin") | Admin user .form-group .col-sm-8.col-sm-offset-4: .checkbox: label From ec41a59539e03ea2faed177ce0e56021b1407a8a Mon Sep 17 00:00:00 2001 From: Anton Markelov Date: Mon, 19 Feb 2018 08:59:28 +1000 Subject: [PATCH 6/7] rename migration asset --- db/migrations/{v2.4.2.sql => v2.5.0.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename db/migrations/{v2.4.2.sql => v2.5.0.sql} (100%) diff --git a/db/migrations/v2.4.2.sql b/db/migrations/v2.5.0.sql similarity index 100% rename from db/migrations/v2.4.2.sql rename to db/migrations/v2.5.0.sql From 10f2b4b41336fcc118c11c3de354970e0efb59f8 Mon Sep 17 00:00:00 2001 From: Anton Markelov Date: Tue, 20 Feb 2018 10:12:19 +1000 Subject: [PATCH 7/7] another minor grammar fixes --- api/tasks/http.go | 2 +- api/users.go | 8 ++++---- public/vendor | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/tasks/http.go b/api/tasks/http.go index 31654f303..99ac9ac34 100644 --- a/api/tasks/http.go +++ b/api/tasks/http.go @@ -109,7 +109,7 @@ func RemoveTask(w http.ResponseWriter, r *http.Request) { editor := context.Get(r, "user").(*db.User) if editor.Admin != true { - log.Warn(editor.Username + " doesn't permit task log deletion") + log.Warn(editor.Username + " is not permitted to delete task logs") w.WriteHeader(http.StatusUnauthorized) return } diff --git a/api/users.go b/api/users.go index c03d7690e..5be081328 100644 --- a/api/users.go +++ b/api/users.go @@ -30,7 +30,7 @@ func addUser(w http.ResponseWriter, r *http.Request) { editor := context.Get(r, "user").(*db.User) if editor.Admin != true { - log.Warn(editor.Username + " doesn't permit user creation") + log.Warn(editor.Username + " is not permitted to create users") w.WriteHeader(http.StatusUnauthorized) return } @@ -80,7 +80,7 @@ func updateUser(w http.ResponseWriter, r *http.Request) { } if editor.Admin != true && editor.ID != oldUser.ID { - log.Warn(editor.Username + " doesn't permitted for user editing") + log.Warn(editor.Username + " is not permitted to edit users") w.WriteHeader(http.StatusUnauthorized) return } @@ -113,7 +113,7 @@ func updateUserPassword(w http.ResponseWriter, r *http.Request) { } if editor.Admin != true && editor.ID != user.ID { - log.Warn(editor.Username + " doesn't permitted for user editing") + log.Warn(editor.Username + " is not permitted to edit users") w.WriteHeader(http.StatusUnauthorized) return } @@ -141,7 +141,7 @@ func deleteUser(w http.ResponseWriter, r *http.Request) { editor := context.Get(r, "user").(*db.User) if editor.Admin != true && editor.ID != user.ID { - log.Warn(editor.Username + " doesn't permitted for user deletion") + log.Warn(editor.Username + " is not permitted to delete users") w.WriteHeader(http.StatusUnauthorized) return } diff --git a/public/vendor b/public/vendor index dcb3abe0d..c4f6280ac 160000 --- a/public/vendor +++ b/public/vendor @@ -1 +1 @@ -Subproject commit dcb3abe0d08a7096c025f32829770e41de1735d2 +Subproject commit c4f6280ac9177587197e1b5017e6181b3fa692e6