diff --git a/Featurecreep.md b/Featurecreep.md index 9bb4fea1a5..7df06b27cb 100644 --- a/Featurecreep.md +++ b/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) -* [ ] When giving a team or user access to a list/namespace, they should be reffered to by uuid, not numeric id -* [ ] Adding users to a team should also use uuid +* [x] When giving a 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 * [ ] 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 diff --git a/pkg/models/list_users.go b/pkg/models/list_users.go index 4f35f41aeb..8327382b54 100644 --- a/pkg/models/list_users.go +++ b/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. - UserID int64 `xorm:"int(11) not null INDEX" json:"userID" param:"user"` + // The username. + 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. diff --git a/pkg/models/list_users_create.go b/pkg/models/list_users_create.go index fc141eda7a..2b9e6a2b8e 100644 --- a/pkg/models/list_users_create.go +++ b/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 diff --git a/pkg/models/list_users_delete.go b/pkg/models/list_users_delete.go index a44a81c224..d8e64926ef 100644 --- a/pkg/models/list_users_delete.go +++ b/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). diff --git a/pkg/models/list_users_test.go b/pkg/models/list_users_test.go index 79684ead96..2d3097d879 100644 --- a/pkg/models/list_users_test.go +++ b/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, - ListID: 2, + Username: "user1", + ListID: 2, }, }, { name: "ListUsers Create for duplicate", fields: fields{ - UserID: 1, - ListID: 2, + Username: "user1", + 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, - ListID: 2, - Right: 500, + Username: "user1", + ListID: 2, + 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, - ListID: 2000, + Username: "user1", + 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, - ListID: 2, + Username: "user500", + 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, - ListID: 1, + Username: "user1", + 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, - ListID: 2, + Username: "user1000", + 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, - ListID: 4, + Username: "user1", + ListID: 4, }, wantErr: true, errType: IsErrUserDoesNotHaveAccessToList, @@ -326,8 +328,8 @@ func TestListUser_Delete(t *testing.T) { { name: "Try deleting normally", fields: fields{ - UserID: 1, - ListID: 3, + Username: "user1", + 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, diff --git a/pkg/models/namespace_users.go b/pkg/models/namespace_users.go index f07e2da7bd..f94cdcb8c8 100644 --- a/pkg/models/namespace_users.go +++ b/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. - UserID int64 `xorm:"int(11) not null INDEX" json:"userID" param:"user"` + // The username. + 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. diff --git a/pkg/models/namespace_users_create.go b/pkg/models/namespace_users_create.go index d0daa6e4dd..8953058bb1 100644 --- a/pkg/models/namespace_users_create.go +++ b/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 diff --git a/pkg/models/namespace_users_delete.go b/pkg/models/namespace_users_delete.go index 9995eaa91f..12f81b521b 100644 --- a/pkg/models/namespace_users_delete.go +++ b/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). diff --git a/pkg/models/namespace_users_test.go b/pkg/models/namespace_users_test.go index d241a11888..29c65a8dcf 100644 --- a/pkg/models/namespace_users_test.go +++ b/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, diff --git a/pkg/models/team_members_create.go b/pkg/models/team_members_create.go index 9e5ce958b0..811729cc56 100644 --- a/pkg/models/team_members_create.go +++ b/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). diff --git a/pkg/models/team_members_delete.go b/pkg/models/team_members_delete.go index 749c9cfef7..824307740e 100644 --- a/pkg/models/team_members_delete.go +++ b/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 } diff --git a/pkg/models/team_members_test.go b/pkg/models/team_members_test.go index 90ce1c3466..c9fd303903 100644 --- a/pkg/models/team_members_test.go +++ b/pkg/models/team_members_test.go @@ -25,8 +25,8 @@ func TestTeamMember_Create(t *testing.T) { // Dummy team member dummyteammember := TeamMember{ - TeamID: 1, - UserID: 3, + TeamID: 1, + 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)) diff --git a/pkg/models/teams.go b/pkg/models/teams.go index 83f680ccef..965334c8a2 100644 --- a/pkg/models/teams.go +++ b/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. - UserID int64 `xorm:"int(11) not null INDEX" json:"userID" param:"user"` + // The username of the member. We use this to prevent automated user id entering. + 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"` diff --git a/pkg/models/teams_create.go b/pkg/models/teams_create.go index 09c2f5c29e..774beef4c1 100644 --- a/pkg/models/teams_create.go +++ b/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 } diff --git a/pkg/models/user.go b/pkg/models/user.go index 9b657dd6ed..55f570245c 100644 --- a/pkg/models/user.go +++ b/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) diff --git a/pkg/models/user_add_update.go b/pkg/models/user_add_update.go index 2c71a5b6bc..03d61072e2 100644 --- a/pkg/models/user_add_update.go +++ b/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 diff --git a/pkg/swagger/docs.go b/pkg/swagger/docs.go index d49f709487..6730454850 100644 --- a/pkg/swagger/docs.go +++ b/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": { - "description": "The user id.", - "type": "integer" + "username": { + "description": "The username.", + "type": "string" } } }, @@ -4054,8 +4054,8 @@ var doc = `{ "type": "integer" }, "userID": { - "description": "The user id.", - "type": "integer" + "description": "The username.", + "type": "string" } } }, @@ -4203,9 +4203,9 @@ var doc = `{ "description": "The unique, numeric id of this team member relation.", "type": "integer" }, - "userID": { - "description": "The id of the member.", - "type": "integer" + "username": { + "description": "The username of the member. We use this to prevent automated user id entering.", + "type": "string" } } }, diff --git a/pkg/swagger/swagger.json b/pkg/swagger/swagger.json index ca3c8f4219..7a700ec98a 100644 --- a/pkg/swagger/swagger.json +++ b/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": { - "description": "The user id.", - "type": "integer" + "username": { + "description": "The username.", + "type": "string" } } }, @@ -4040,8 +4040,8 @@ "type": "integer" }, "userID": { - "description": "The user id.", - "type": "integer" + "description": "The username.", + "type": "string" } } }, @@ -4189,9 +4189,9 @@ "description": "The unique, numeric id of this team member relation.", "type": "integer" }, - "userID": { - "description": "The id of the member.", - "type": "integer" + "username": { + "description": "The username of the member. We use this to prevent automated user id entering.", + "type": "string" } } }, diff --git a/pkg/swagger/swagger.yaml b/pkg/swagger/swagger.yaml index 8c680dd3fd..bd464c261c 100644 --- a/pkg/swagger/swagger.yaml +++ b/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: - description: The user id. - type: integer + username: + description: The username. + type: string type: object models.Message: properties: @@ -369,8 +369,8 @@ definitions: change this value. type: integer userID: - description: The user id. - type: integer + description: The username. + 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: - description: The id of the member. - type: integer + username: + description: The username of the member. We use this to prevent automated + 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: '' + 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. + # Authorization + **JWT-Auth:** Main authorization method, used for most of the requests. Needs ` + "`" + `Authorization: Bearer ` + "`" + `-header to authenticate successfully. + + **BasicAuth:** Only used when requesting tasks via caldav. + license: name: GPLv3 url: http://code.vikunja.io/api/src/branch/master/LICENSE