Add non-privileged user role #405

strangeman · 2017-07-26T07:02:18Z

Fix for #318 and #198

I added admin property for User. By default, all users after migration will be Admins (for backward compatibility).

Non-Admin Users can:

Change his own information (login, email, name, password, alert flag), except Admin flag
View all users list

Non-Admin Users can't (will get 401 Unauthorized when trying):

Add a new user
Edit another user
Delete another user

I hide 'add user' button and user editing dialog for Non-Admin User. The first user, created by setup, have admin privileges. Also, I improved User List page, by adding all flags (Admin, Alert, External) to id.

twhiston

great feature. Please see the very minor change requests

twhiston · 2018-02-16T14:49:16Z

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


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

twhiston · 2018-02-16T14:52:41Z

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 {


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)

I think, the first system user always should be an admin. That code executes only for initial setup of Semaphore.

twhiston · 2018-02-16T14:53:00Z

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


Please can you update this to 2.5.0

twhiston · 2018-02-16T17:35:51Z

Would it be possible to also take care of #158 in this pull request and add some extra authentication against log deletion for non admin users (if it's totally out of scope no worries)

strangeman · 2018-02-16T23:53:43Z

@twhiston all done :)

twhiston · 2018-02-18T12:48:49Z

api/users.go

@@ -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")


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

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

twhiston · 2018-02-18T12:50:20Z

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


"User has admin privileges"

twhiston · 2018-02-18T12:51:05Z

api/tasks/http.go

+	editor := context.Get(r, "user").(*db.User)
+
+	if editor.Admin != true {
+		log.Warn(editor.Username + " doesn't permit task log deletion")


looks like this file is missing a log import making the tests fail currently

Oops, my mistake. Fixed for now.

twhiston · 2018-02-18T12:52:35Z

looks really good. Another couple of small things and fixing a missing import and this is good to go. Thanks very much for your work

twhiston

I feel like a real tyrant to keep asking you for these tiny changes to sentences, but it would be great if we can make it correct and consistent everywhere. Then I promise this PR is done!

twhiston · 2018-02-19T18:01:24Z

api/users.go


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


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

twhiston · 2018-02-19T18:01:37Z

api/users.go

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


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

twhiston · 2018-02-19T18:01:47Z

api/users.go

+	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")


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

twhiston · 2018-02-19T18:02:18Z

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 permit user creation")


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

twhiston · 2018-02-19T18:02:38Z

api/tasks/http.go

+	editor := context.Get(r, "user").(*db.User)
+
+	if editor.Admin != true {
+		log.Warn(editor.Username + " doesn't permit task log deletion")


editor.Username + " is not permitted to delete task logs"

strangeman · 2018-02-20T00:14:40Z

All done. I need to take some lessons with an English teacher, instead of watching Netflix shows. :)

vaol · 2018-09-12T08:36:43Z

I have just installed Semaphore v2.5.1 and I am still facing the same problem as reported in #198. I would need some user to only have the permission to run some templates, but not update or delete them.
Is it something possible to achieve ?

strangeman · 2018-09-12T08:53:54Z

@vaol please check user settings ('Admin user' checkbox):

vaol · 2018-09-12T08:57:23Z

Thank you for answering so quickly !
Here is my config :

And still I can delete a template when I am connected as user "demo"

strangeman · 2018-09-12T09:04:53Z

Ugh, sorry, I confused different roles mechanisms. launch-only users still not implemented. I made the PR some time ago, but it was not too good: #413
If you're ok with Go, you may adapt the code from that PR

strangeman added 2 commits July 26, 2017 15:55

add admin role, restrict users without it

05a1b3c

add admin rights for first user

59c10ac

twhiston suggested changes Feb 16, 2018

View reviewed changes

twhiston added 2.5.0 needs work labels Feb 16, 2018

twhiston mentioned this pull request Feb 16, 2018

All users have administrator privileges #198

Closed

strangeman added 2 commits February 17, 2018 09:46

minor fixes after code review

fe5dcdc

fix for semaphoreui#158

5e26bfb

strangeman mentioned this pull request Feb 16, 2018

Non admin user shouldn't delete deploy logs #158

Closed

twhiston suggested changes Feb 18, 2018

View reviewed changes

strangeman added 2 commits February 19, 2018 08:49

another minor fixes after review

910c8bc

rename migration asset

ec41a59

twhiston suggested changes Feb 19, 2018

View reviewed changes

another minor grammar fixes

10f2b4b

twhiston approved these changes Feb 21, 2018

View reviewed changes

twhiston merged commit 0fceedb into semaphoreui:develop Feb 27, 2018

twhiston removed the needs work label Feb 27, 2018

strangeman deleted the 198-nonpriv-user branch October 22, 2018 12:12

steadfasterX mentioned this pull request Mar 16, 2022

proposal: role based authorization #891

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add non-privileged user role #405

Add non-privileged user role #405

strangeman commented Jul 26, 2017

twhiston left a comment

twhiston Feb 16, 2018

strangeman Feb 16, 2018

twhiston Feb 16, 2018

strangeman Feb 16, 2018

twhiston Feb 16, 2018

strangeman Feb 16, 2018

twhiston commented Feb 16, 2018

strangeman commented Feb 16, 2018

twhiston Feb 18, 2018 •

edited

strangeman Feb 18, 2018

twhiston Feb 18, 2018

strangeman Feb 18, 2018

twhiston Feb 18, 2018

strangeman Feb 18, 2018

twhiston commented Feb 18, 2018

twhiston left a comment

twhiston Feb 19, 2018

twhiston Feb 19, 2018

twhiston Feb 19, 2018

twhiston Feb 19, 2018

twhiston Feb 19, 2018

strangeman commented Feb 20, 2018

vaol commented Sep 12, 2018

strangeman commented Sep 12, 2018

vaol commented Sep 12, 2018

strangeman commented Sep 12, 2018 •

edited

Add non-privileged user role #405

Add non-privileged user role #405

Conversation

strangeman commented Jul 26, 2017

twhiston left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twhiston commented Feb 16, 2018

strangeman commented Feb 16, 2018

twhiston Feb 18, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twhiston commented Feb 18, 2018

twhiston left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strangeman commented Feb 20, 2018

vaol commented Sep 12, 2018

strangeman commented Sep 12, 2018

vaol commented Sep 12, 2018

strangeman commented Sep 12, 2018 • edited

twhiston Feb 18, 2018 •

edited

strangeman commented Sep 12, 2018 •

edited