Use the username instead of a user when adding a user to a team or giving it rights (#76)

Featurecreep.md

@@ -215,9 +215,8 @@ Sorry for some of them being in German, I'll tranlate them at some point.
 * [x] Fix lint errors
 * [x] Add settings for max open/idle connections and max connection lifetime
 * [x] Reminders should use an extra table so we can make reverse lookups aka "give me all tasks with reminders in this period" which we'll need for things like email reminders notifications
-* [ ] Teams and users should also have uuids (for users these can be the username)
+* [x] When giving a user access to a list/namespace, they should be reffered to by uuid, not numeric id
-* [ ] When giving a team or user access to a list/namespace, they should be reffered to by uuid, not numeric id
+* [x] Adding users to a team should also use uuid
-* [ ] Adding users to a team should also use uuid
 * [ ] Check if the team/user really exist before updating them on lists/namespaces
 * [ ] Check if the email is properly obfuscated everywhere -> alter GetUser() and add a new method GetUserWithEmail
 

pkg/models/list_users.go

@@ -22,8 +22,10 @@ import "code.vikunja.io/web"
 type ListUser struct {
 	// The unique, numeric id of this list <-> user relation.
 	ID int64 `xorm:"int(11) autoincr not null unique pk" json:"id" param:"namespace"`
-	// The user id.
+	// The username.
-	UserID int64 `xorm:"int(11) not null INDEX" json:"userID" param:"user"`
+	Username string `xorm:"-" json:"username" param:"user"`
+	// Used internally to reference the user
+	UserID int64 `xorm:"int(11) not null INDEX" json:"-"`
 	// The list id.
 	ListID int64 `xorm:"int(11) not null INDEX" json:"-" param:"list"`
 	// The right this user has. 0 = Read only, 1 = Read & Write, 2 = Admin. See the docs for more details.

pkg/models/list_users_create.go

@@ -47,9 +47,11 @@ func (lu *ListUser) Create(a web.Auth) (err error) {
 	}
 
 	// Check if the user exists
-	if _, err = GetUserByID(lu.UserID); err != nil {
+	user, err := GetUserByUsername(lu.Username)
+	if err != nil {
 		return err
 	}
+	lu.UserID = user.ID
 
 	// Check if the user already has access or is owner of that list
 	// We explicitly DONT check for teams here

pkg/models/list_users_delete.go

@@ -34,10 +34,11 @@ import _ "code.vikunja.io/web" // For swaggerdocs generation
 func (lu *ListUser) Delete() (err error) {
 
 	// Check if the user exists
-	_, err = GetUserByID(lu.UserID)
+	user, err := GetUserByUsername(lu.Username)
 	if err != nil {
 		return
 	}
+	lu.UserID = user.ID
 
 	// Check if the user has access to the list
 	has, err := x.Where("user_id = ? AND list_id = ?", lu.UserID, lu.ListID).

pkg/models/list_users_test.go

@@ -29,6 +29,7 @@ func TestListUser_Create(t *testing.T) {
 	type fields struct {
 		ID       int64
 		UserID   int64
+		Username string
 		ListID   int64
 		Right    Right
 		Created  int64
@@ -49,15 +50,15 @@ func TestListUser_Create(t *testing.T) {
 		{
 			name: "ListUsers Create normally",
 			fields: fields{
-				UserID: 1,
+				Username: "user1",
-				ListID: 2,
+				ListID:   2,
 			},
 		},
 		{
 			name: "ListUsers Create for duplicate",
 			fields: fields{
-				UserID: 1,
+				Username: "user1",
-				ListID: 2,
+				ListID:   2,
 			},
 			wantErr: true,
 			errType: IsErrUserAlreadyHasAccess,
@@ -65,9 +66,9 @@ func TestListUser_Create(t *testing.T) {
 		{
 			name: "ListUsers Create with invalid right",
 			fields: fields{
-				UserID: 1,
+				Username: "user1",
-				ListID: 2,
+				ListID:   2,
-				Right:  500,
+				Right:    500,
 			},
 			wantErr: true,
 			errType: IsErrInvalidRight,
@@ -75,8 +76,8 @@ func TestListUser_Create(t *testing.T) {
 		{
 			name: "ListUsers Create with inexisting list",
 			fields: fields{
-				UserID: 1,
+				Username: "user1",
-				ListID: 2000,
+				ListID:   2000,
 			},
 			wantErr: true,
 			errType: IsErrListDoesNotExist,
@@ -84,8 +85,8 @@ func TestListUser_Create(t *testing.T) {
 		{
 			name: "ListUsers Create with inexisting user",
 			fields: fields{
-				UserID: 500,
+				Username: "user500",
-				ListID: 2,
+				ListID:   2,
 			},
 			wantErr: true,
 			errType: IsErrUserDoesNotExist,
@@ -93,8 +94,8 @@ func TestListUser_Create(t *testing.T) {
 		{
 			name: "ListUsers Create with the owner as shared user",
 			fields: fields{
-				UserID: 1,
+				Username: "user1",
-				ListID: 1,
+				ListID:   1,
 			},
 			wantErr: true,
 			errType: IsErrUserAlreadyHasAccess,
@@ -105,6 +106,7 @@ func TestListUser_Create(t *testing.T) {
 			ul := &ListUser{
 				ID:       tt.fields.ID,
 				UserID:   tt.fields.UserID,
+				Username: tt.fields.Username,
 				ListID:   tt.fields.ListID,
 				Right:    tt.fields.Right,
 				Created:  tt.fields.Created,
@@ -291,7 +293,7 @@ func TestListUser_Update(t *testing.T) {
 func TestListUser_Delete(t *testing.T) {
 	type fields struct {
 		ID       int64
-		UserID   int64
+		Username string
 		ListID   int64
 		Right    Right
 		Created  int64
@@ -308,8 +310,8 @@ func TestListUser_Delete(t *testing.T) {
 		{
 			name: "Try deleting some unexistant user",
 			fields: fields{
-				UserID: 1000,
+				Username: "user1000",
-				ListID: 2,
+				ListID:   2,
 			},
 			wantErr: true,
 			errType: IsErrUserDoesNotExist,
@@ -317,8 +319,8 @@ func TestListUser_Delete(t *testing.T) {
 		{
 			name: "Try deleting a user which does not has access but exists",
 			fields: fields{
-				UserID: 1,
+				Username: "user1",
-				ListID: 4,
+				ListID:   4,
 			},
 			wantErr: true,
 			errType: IsErrUserDoesNotHaveAccessToList,
@@ -326,8 +328,8 @@ func TestListUser_Delete(t *testing.T) {
 		{
 			name: "Try deleting normally",
 			fields: fields{
-				UserID: 1,
+				Username: "user1",
-				ListID: 3,
+				ListID:   3,
 			},
 		},
 	}
@@ -335,7 +337,7 @@ func TestListUser_Delete(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			lu := &ListUser{
 				ID:       tt.fields.ID,
-				UserID:   tt.fields.UserID,
+				Username: tt.fields.Username,
 				ListID:   tt.fields.ListID,
 				Right:    tt.fields.Right,
 				Created:  tt.fields.Created,

pkg/models/namespace_users.go

@@ -22,8 +22,9 @@ import "code.vikunja.io/web"
 type NamespaceUser struct {
 	// The unique, numeric id of this namespace <-> user relation.
 	ID int64 `xorm:"int(11) autoincr not null unique pk" json:"id" param:"namespace"`
-	// The user id.
+	// The username.
-	UserID int64 `xorm:"int(11) not null INDEX" json:"userID" param:"user"`
+	Username string `xorm:"-" json:"userID" param:"user"`
+	UserID   int64  `xorm:"int(11) not null INDEX" json:"-"`
 	// The namespace id
 	NamespaceID int64 `xorm:"int(11) not null INDEX" json:"-" param:"namespace"`
 	// The right this user has. 0 = Read only, 1 = Read & Write, 2 = Admin. See the docs for more details.

pkg/models/namespace_users_create.go

@@ -49,9 +49,11 @@ func (nu *NamespaceUser) Create(a web.Auth) (err error) {
 	}
 
 	// Check if the user exists
-	if _, err = GetUserByID(nu.UserID); err != nil {
+	user, err := GetUserByUsername(nu.Username)
+	if err != nil {
 		return err
 	}
+	nu.UserID = user.ID
 
 	// Check if the user already has access or is owner of that namespace
 	// We explicitly DO NOT check for teams here

pkg/models/namespace_users_delete.go

@@ -34,10 +34,11 @@ import _ "code.vikunja.io/web" // For swaggerdocs generation
 func (nu *NamespaceUser) Delete() (err error) {
 
 	// Check if the user exists
-	_, err = GetUserByID(nu.UserID)
+	user, err := GetUserByUsername(nu.Username)
 	if err != nil {
 		return
 	}
+	nu.UserID = user.ID
 
 	// Check if the user has access to the namespace
 	has, err := x.Where("user_id = ? AND namespace_id = ?", nu.UserID, nu.NamespaceID).

pkg/models/namespace_users_test.go

@@ -29,7 +29,7 @@ import (
 func TestNamespaceUser_Create(t *testing.T) {
 	type fields struct {
 		ID          int64
-		UserID      int64
+		Username    string
 		NamespaceID int64
 		Right       Right
 		Created     int64
@@ -50,14 +50,14 @@ func TestNamespaceUser_Create(t *testing.T) {
 		{
 			name: "NamespaceUsers Create normally",
 			fields: fields{
-				UserID:      1,
+				Username:    "user1",
 				NamespaceID: 2,
 			},
 		},
 		{
 			name: "NamespaceUsers Create for duplicate",
 			fields: fields{
-				UserID:      1,
+				Username:    "user1",
 				NamespaceID: 2,
 			},
 			wantErr: true,
@@ -66,7 +66,7 @@ func TestNamespaceUser_Create(t *testing.T) {
 		{
 			name: "NamespaceUsers Create with invalid right",
 			fields: fields{
-				UserID:      1,
+				Username:    "user1",
 				NamespaceID: 2,
 				Right:       500,
 			},
@@ -76,7 +76,7 @@ func TestNamespaceUser_Create(t *testing.T) {
 		{
 			name: "NamespaceUsers Create with inexisting list",
 			fields: fields{
-				UserID:      1,
+				Username:    "user1",
 				NamespaceID: 2000,
 			},
 			wantErr: true,
@@ -85,7 +85,7 @@ func TestNamespaceUser_Create(t *testing.T) {
 		{
 			name: "NamespaceUsers Create with inexisting user",
 			fields: fields{
-				UserID:      500,
+				Username:    "user500",
 				NamespaceID: 2,
 			},
 			wantErr: true,
@@ -94,7 +94,7 @@ func TestNamespaceUser_Create(t *testing.T) {
 		{
 			name: "NamespaceUsers Create with the owner as shared user",
 			fields: fields{
-				UserID:      1,
+				Username:    "user1",
 				NamespaceID: 1,
 			},
 			wantErr: true,
@@ -105,7 +105,7 @@ func TestNamespaceUser_Create(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			un := &NamespaceUser{
 				ID:          tt.fields.ID,
-				UserID:      tt.fields.UserID,
+				Username:    tt.fields.Username,
 				NamespaceID: tt.fields.NamespaceID,
 				Right:       tt.fields.Right,
 				Created:     tt.fields.Created,
@@ -293,7 +293,7 @@ func TestNamespaceUser_Update(t *testing.T) {
 func TestNamespaceUser_Delete(t *testing.T) {
 	type fields struct {
 		ID          int64
-		UserID      int64
+		Username    string
 		NamespaceID int64
 		Right       Right
 		Created     int64
@@ -310,7 +310,7 @@ func TestNamespaceUser_Delete(t *testing.T) {
 		{
 			name: "Try deleting some unexistant user",
 			fields: fields{
-				UserID:      1000,
+				Username:    "user1000",
 				NamespaceID: 2,
 			},
 			wantErr: true,
@@ -319,7 +319,7 @@ func TestNamespaceUser_Delete(t *testing.T) {
 		{
 			name: "Try deleting a user which does not has access but exists",
 			fields: fields{
-				UserID:      1,
+				Username:    "user1",
 				NamespaceID: 4,
 			},
 			wantErr: true,
@@ -328,7 +328,7 @@ func TestNamespaceUser_Delete(t *testing.T) {
 		{
 			name: "Try deleting normally",
 			fields: fields{
-				UserID:      1,
+				Username:    "user1",
 				NamespaceID: 3,
 			},
 		},
@@ -337,7 +337,7 @@ func TestNamespaceUser_Delete(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			nu := &NamespaceUser{
 				ID:          tt.fields.ID,
-				UserID:      tt.fields.UserID,
+				Username:    tt.fields.Username,
 				NamespaceID: tt.fields.NamespaceID,
 				Right:       tt.fields.Right,
 				Created:     tt.fields.Created,

pkg/models/team_members_create.go

@@ -41,10 +41,11 @@ func (tm *TeamMember) Create(a web.Auth) (err error) {
 	}
 
 	// Check if the user exists
-	_, err = GetUserByID(tm.UserID)
+	user, err := GetUserByUsername(tm.Username)
 	if err != nil {
 		return
 	}
+	tm.UserID = user.ID
 
 	// Check if that user is already part of the team
 	exists, err := x.Where("team_id = ? AND user_id = ?", tm.TeamID, tm.UserID).

pkg/models/team_members_delete.go

@@ -37,6 +37,13 @@ func (tm *TeamMember) Delete() (err error) {
 		return ErrCannotDeleteLastTeamMember{tm.TeamID, tm.UserID}
 	}
 
+	// Find the numeric user id
+	user, err := GetUserByUsername(tm.Username)
+	if err != nil {
+		return
+	}
+	tm.UserID = user.ID
+
 	_, err = x.Where("team_id = ? AND user_id = ?", tm.TeamID, tm.UserID).Delete(&TeamMember{})
 	return
 }

pkg/models/team_members_test.go

@@ -25,8 +25,8 @@ func TestTeamMember_Create(t *testing.T) {
 
 	// Dummy team member
 	dummyteammember := TeamMember{
-		TeamID: 1,
+		TeamID:   1,
-		UserID: 3,
+		Username: "user3",
 	}
 
 	// Doer
@@ -57,24 +57,24 @@ func TestTeamMember_Create(t *testing.T) {
 	assert.NoError(t, err)
 
 	// Delete the other one
-	tm := TeamMember{TeamID: 1, UserID: 2}
+	tm := TeamMember{TeamID: 1, Username: "user2"}
 	err = tm.Delete()
 	assert.NoError(t, err)
 
 	// Try deleting the last one
-	tm = TeamMember{TeamID: 1, UserID: 1}
+	tm = TeamMember{TeamID: 1, Username: "user1"}
 	err = tm.Delete()
 	assert.Error(t, err)
 	assert.True(t, IsErrCannotDeleteLastTeamMember(err))
 
 	// Try inserting a user which does not exist
-	dummyteammember.UserID = 9484
+	dummyteammember.Username = "user9484"
 	err = dummyteammember.Create(&doer)
 	assert.Error(t, err)
 	assert.True(t, IsErrUserDoesNotExist(err))
 
 	// Try adding a user to a team which does not exist
-	tm = TeamMember{TeamID: 94824, UserID: 1}
+	tm = TeamMember{TeamID: 94824, Username: "user1"}
 	err = tm.Create(&doer)
 	assert.Error(t, err)
 	assert.True(t, IsErrTeamDoesNotExist(err))

pkg/models/teams.go

@@ -66,8 +66,10 @@ type TeamMember struct {
 	ID int64 `xorm:"int(11) autoincr not null unique pk" json:"id"`
 	// The team id.
 	TeamID int64 `xorm:"int(11) not null INDEX" json:"-" param:"team"`
-	// The id of the member.
+	// The username of the member. We use this to prevent automated user id entering.
-	UserID int64 `xorm:"int(11) not null INDEX" json:"userID" param:"user"`
+	Username string `xorm:"-" json:"username" param:"user"`
+	// Used under the hood to manage team members
+	UserID int64 `xorm:"int(11) not null INDEX" json:"-"`
 	// Whether or not the member is an admin of the team. See the docs for more about what a team admin can do
 	Admin bool `xorm:"tinyint(1) INDEX null" json:"admin"`
 

pkg/models/teams_create.go

@@ -53,7 +53,7 @@ func (t *Team) Create(a web.Auth) (err error) {
 	}
 
 	// Insert the current user as member and admin
-	tm := TeamMember{TeamID: t.ID, UserID: doer.ID, Admin: true}
+	tm := TeamMember{TeamID: t.ID, Username: doer.Username, Admin: true}
 	if err = tm.Create(doer); err != nil {
 		return err
 	}

pkg/models/user.go

@@ -119,6 +119,15 @@ func GetUserByID(id int64) (user User, err error) {
 	return GetUser(User{ID: id})
 }
 
+// GetUserByUsername gets a user from its user name. This is an extra function to be able to add an extra error check.
+func GetUserByUsername(username string) (user User, err error) {
+	if username == "" {
+		return User{}, ErrUserDoesNotExist{}
+	}
+
+	return GetUser(User{Username: username})
+}
+
 // GetUser gets a user object
 func GetUser(user User) (userOut User, err error) {
 	userOut = user
@@ -139,7 +148,7 @@ func CheckUserCredentials(u *UserLogin) (User, error) {
 	}
 
 	// Check if the user exists
-	user, err := GetUser(User{Username: u.Username})
+	user, err := GetUserByUsername(u.Username)
 	if err != nil {
 		// hashing the password takes a long time, so we hash something to not make it clear if the username was wrong
 		bcrypt.GenerateFromPassword([]byte(u.Username), 14)

pkg/models/user_add_update.go

@@ -36,7 +36,7 @@ func CreateUser(user User) (newUser User, err error) {
 
 	// Check if the user already existst with that username
 	exists := true
-	existingUser, err := GetUser(User{Username: newUser.Username})
+	existingUser, err := GetUserByUsername(newUser.Username)
 	if err != nil {
 		if IsErrUserDoesNotExist(err) {
 			exists = false

pkg/swagger/docs.go

@@ -1,6 +1,6 @@
 // GENERATED BY THE COMMAND ABOVE; DO NOT EDIT
 // This file was generated by swaggo/swag at
-// 2019-05-22 19:24:37.734465408 +0200 CEST m=+0.660846954
+// 2019-05-25 11:35:41.214134069 +0200 CEST m=+0.124896065
 
 package swagger
 
@@ -14,7 +14,7 @@ import (
 var doc = `{
     "swagger": "2.0",
     "info": {
-        "description": "\u003c!-- ReDoc-Inject: \u003csecurity-definitions\u003e --\u003e",
+        "description": "This is the documentation for the [Vikunja](http://vikunja.io) API. Vikunja is a cross-plattform Todo-application with a lot of features, such as sharing lists with users or teams. \u003c!-- ReDoc-Inject: \u003csecurity-definitions\u003e --\u003e\n# Authorization\n**JWT-Auth:** Main authorization method, used for most of the requests. Needs ` + "`" + `Authorization: Bearer \u003cjwt-token\u003e` + "`" + `-header to authenticate successfully.\n\n**BasicAuth:** Only used when requesting tasks via caldav.\n\u003c!-- ReDoc-Inject: \u003csecurity-definitions\u003e --\u003e",
         "title": "Vikunja API",
         "contact": {
             "name": "General Vikunja contact",
@@ -3984,9 +3984,9 @@ var doc = `{
                     "description": "A unix timestamp when this relation was last updated. You cannot change this value.",
                     "type": "integer"
                 },
-                "userID": {
+                "username": {
-                    "description": "The user id.",
+                    "description": "The username.",
-                    "type": "integer"
+                    "type": "string"
                 }
             }
         },
@@ -4054,8 +4054,8 @@ var doc = `{
                     "type": "integer"
                 },
                 "userID": {
-                    "description": "The user id.",
+                    "description": "The username.",
-                    "type": "integer"
+                    "type": "string"
                 }
             }
         },
@@ -4203,9 +4203,9 @@ var doc = `{
                     "description": "The unique, numeric id of this team member relation.",
                     "type": "integer"
                 },
-                "userID": {
+                "username": {
-                    "description": "The id of the member.",
+                    "description": "The username of the member. We use this to prevent automated user id entering.",
-                    "type": "integer"
+                    "type": "string"
                 }
             }
         },

pkg/swagger/swagger.json

@@ -1,7 +1,7 @@
 {
     "swagger": "2.0",
     "info": {
-        "description": "\u003c!-- ReDoc-Inject: \u003csecurity-definitions\u003e --\u003e",
+        "description": "This is the documentation for the [Vikunja](http://vikunja.io) API. Vikunja is a cross-plattform Todo-application with a lot of features, such as sharing lists with users or teams. \u003c!-- ReDoc-Inject: \u003csecurity-definitions\u003e --\u003e\n# Authorization\n**JWT-Auth:** Main authorization method, used for most of the requests. Needs ` + \"`\" + `Authorization: Bearer \u003cjwt-token\u003e` + \"`\" + `-header to authenticate successfully.\n\n**BasicAuth:** Only used when requesting tasks via caldav.\n\u003c!-- ReDoc-Inject: \u003csecurity-definitions\u003e --\u003e",
         "title": "Vikunja API",
         "contact": {
             "name": "General Vikunja contact",
@@ -3970,9 +3970,9 @@
                     "description": "A unix timestamp when this relation was last updated. You cannot change this value.",
                     "type": "integer"
                 },
-                "userID": {
+                "username": {
-                    "description": "The user id.",
+                    "description": "The username.",
-                    "type": "integer"
+                    "type": "string"
                 }
             }
         },
@@ -4040,8 +4040,8 @@
                     "type": "integer"
                 },
                 "userID": {
-                    "description": "The user id.",
+                    "description": "The username.",
-                    "type": "integer"
+                    "type": "string"
                 }
             }
         },
@@ -4189,9 +4189,9 @@
                     "description": "The unique, numeric id of this team member relation.",
                     "type": "integer"
                 },
-                "userID": {
+                "username": {
-                    "description": "The id of the member.",
+                    "description": "The username of the member. We use this to prevent automated user id entering.",
-                    "type": "integer"
+                    "type": "string"
                 }
             }
         },

pkg/swagger/swagger.yaml

@@ -312,9 +312,9 @@ definitions:
         description: A unix timestamp when this relation was last updated. You cannot
           change this value.
         type: integer
-      userID:
+      username:
-        description: The user id.
+        description: The username.
-        type: integer
+        type: string
     type: object
   models.Message:
     properties:
@@ -369,8 +369,8 @@ definitions:
           change this value.
         type: integer
       userID:
-        description: The user id.
+        description: The username.
-        type: integer
+        type: string
     type: object
   models.NamespaceWithLists:
     properties:
@@ -487,9 +487,10 @@ definitions:
       id:
         description: The unique, numeric id of this team member relation.
         type: integer
-      userID:
+      username:
-        description: The id of the member.
+        description: The username of the member. We use this to prevent automated
-        type: integer
+          user id entering.
+        type: string
     type: object
   models.TeamNamespace:
     properties:
@@ -651,7 +652,13 @@ info:
     email: hello@vikunja.io
     name: General Vikunja contact
     url: http://vikunja.io/en/contact/
-  description: '<!-- ReDoc-Inject: <security-definitions> -->'
+  description: |-
+    This is the documentation for the [Vikunja](http://vikunja.io) API. Vikunja is a cross-plattform Todo-application with a lot of features, such as sharing lists with users or teams. <!-- ReDoc-Inject: <security-definitions> -->
+    # Authorization
+    **JWT-Auth:** Main authorization method, used for most of the requests. Needs ` + "`" + `Authorization: Bearer <jwt-token>` + "`" + `-header to authenticate successfully.
+
+    **BasicAuth:** Only used when requesting tasks via caldav.
+    <!-- ReDoc-Inject: <security-definitions> -->
   license:
     name: GPLv3
     url: http://code.vikunja.io/api/src/branch/master/LICENSE