Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add non-privileged user role #405

Merged
merged 7 commits into from
Feb 27, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 2 additions & 0 deletions api-docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ definitions:
format: date-time
alert:
type: boolean
admin:
type: boolean
APIToken:
type: object
properties:
Expand Down
45 changes: 44 additions & 1 deletion api/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Contributor

Choose a reason for hiding this comment

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

should be log.Warn(editor.Username + " doesn't permit user creation")
Please correct this for all other instances/variations of this text as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

w.WriteHeader(http.StatusUnauthorized)
return
}

user.Created = time.Now()

if err := db.Mysql.Insert(&user); err != nil {
Expand All @@ -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")
Copy link
Contributor

@twhiston twhiston Feb 18, 2018

Choose a reason for hiding this comment

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

should be editor.Username + " is not permitted to edit users" here and in other locations.
Sorry for the pedantry about english sentence construction!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. That's alright, UI should have clear sentences, without any mess.

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")
Copy link
Contributor

Choose a reason for hiding this comment

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

editor.Username + " is not permitted to edit users"

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)
}

Expand All @@ -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")
Copy link
Contributor

Choose a reason for hiding this comment

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

editor.Username + " is not permitted to edit users"

w.WriteHeader(http.StatusUnauthorized)
return
}

if user.External == true {
log.Warn("Password is not editable for external LDAP users")
w.WriteHeader(http.StatusBadRequest)
Expand All @@ -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")
Copy link
Contributor

Choose a reason for hiding this comment

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

editor.Username + " is not permitted to delete users"

w.WriteHeader(http.StatusUnauthorized)
return
}

if _, err := db.Mysql.Exec("delete from project__user where user_id=?", user.ID); err != nil {
panic(err)
Expand Down
2 changes: 1 addition & 1 deletion cli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have to have admin as default? I would guess most people would prefer admin not to be the default user mode (especially in enterprise usage)

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 think, the first system user always should be an admin. That code executes only for initial setup of Semaphore.

fmt.Printf(" Inserting user failed. If you already have a user, you can disregard this error.\n %v\n", err.Error())
os.Exit(1)
}
Expand Down
1 change: 1 addition & 0 deletions db/User.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand Down
1 change: 1 addition & 0 deletions db/migrations/v2.4.2.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE user ADD admin BOOLEAN NOT NULL DEFAULT 1 AFTER password;
1 change: 1 addition & 0 deletions db/versionHistory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Copy link
Contributor

Choose a reason for hiding this comment

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

Please can you update this to 2.5.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}
}
9 changes: 9 additions & 0 deletions public/html/users/add.pug
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 7 additions & 1 deletion public/html/users/list.pug
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions public/html/users/user.pug
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Contributor

Choose a reason for hiding this comment

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

"User has admin privileges"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

| 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")
Expand Down