diff --git a/Featurecreep.md b/Featurecreep.md index 39a0f2e1e9b..0a0660d17c0 100644 --- a/Featurecreep.md +++ b/Featurecreep.md @@ -183,7 +183,7 @@ Sorry for some of them being in German, I'll tranlate them at some point. * [x] Refactor config handling: Custom type "key" or so which holds the viper const and then mixins on that type to get the values from viper * [x] Less files, but with some kind of logic * [x] Have extra functions for logging to call so it is possible to call `log.Info` instead of `log.Log.Info` -* [ ] `GetUserByID` and the likes should return pointers +* [x] `GetUserByID` and the likes should return pointers * [ ] `ListTask` should be just `Task` ### Linters diff --git a/pkg/models/label.go b/pkg/models/label.go index 70c34df4add..7c8b3aadbb4 100644 --- a/pkg/models/label.go +++ b/pkg/models/label.go @@ -173,7 +173,7 @@ func (l *Label) ReadOne() (err error) { return err } - l.CreatedBy = &user + l.CreatedBy = user return } diff --git a/pkg/models/label_task.go b/pkg/models/label_task.go index 9af2d2207c0..9494dfa0060 100644 --- a/pkg/models/label_task.go +++ b/pkg/models/label_task.go @@ -179,6 +179,11 @@ func getLabelsByTaskIDs(opts *LabelByTaskIDsOptions) (ls []*labelWithTaskID, err return nil, err } + // Obfuscate all user emails + for _, u := range users { + u.Email = "" + } + // Put it all together for in, l := range labels { labels[in].CreatedBy = users[l.CreatedByID] diff --git a/pkg/models/list.go b/pkg/models/list.go index 81a8c218903..41fc16afd54 100644 --- a/pkg/models/list.go +++ b/pkg/models/list.go @@ -33,7 +33,7 @@ type List struct { NamespaceID int64 `xorm:"int(11) INDEX not null" json:"-" param:"namespace"` // The user who created this list. - Owner User `xorm:"-" json:"owner" valid:"-"` + Owner *User `xorm:"-" json:"owner" valid:"-"` // An array of tasks which belong to the list. Tasks []*ListTask `xorm:"-" json:"tasks"` @@ -224,7 +224,7 @@ func AddListDetails(lists []*List) (err error) { // Owner for _, owner := range owners { if list.OwnerID == owner.ID { - lists[in].Owner = *owner + lists[in].Owner = owner break } } @@ -326,7 +326,7 @@ func (l *List) Create(a web.Auth) (err error) { } l.OwnerID = doer.ID - l.Owner = *doer + l.Owner = doer l.ID = 0 // Otherwise only the first time a new list would be created return CreateOrUpdateList(l) diff --git a/pkg/models/list_create_test.go b/pkg/models/list_create_test.go index 95520804b3f..89f4dbaa08c 100644 --- a/pkg/models/list_create_test.go +++ b/pkg/models/list_create_test.go @@ -37,16 +37,16 @@ func TestList_Create(t *testing.T) { } // Check if the user can create - allowed, _ := dummylist.CanCreate(&doer) + allowed, _ := dummylist.CanCreate(doer) assert.True(t, allowed) // Create it - err = dummylist.Create(&doer) + err = dummylist.Create(doer) assert.NoError(t, err) // Get the list newdummy := List{ID: dummylist.ID} - canRead, err := newdummy.CanRead(&doer) + canRead, err := newdummy.CanRead(doer) assert.NoError(t, err) assert.True(t, canRead) err = newdummy.ReadOne() @@ -56,18 +56,18 @@ func TestList_Create(t *testing.T) { assert.Equal(t, dummylist.OwnerID, doer.ID) // Check if the user can see it - allowed, _ = dummylist.CanRead(&doer) + allowed, _ = dummylist.CanRead(doer) assert.True(t, allowed) // Try updating a list - allowed, _ = dummylist.CanUpdate(&doer) + allowed, _ = dummylist.CanUpdate(doer) assert.True(t, allowed) dummylist.Description = "Lorem Ipsum dolor sit amet." err = dummylist.Update() assert.NoError(t, err) // Delete it - allowed, _ = dummylist.CanDelete(&doer) + allowed, _ = dummylist.CanDelete(doer) assert.True(t, allowed) err = dummylist.Delete() @@ -85,7 +85,7 @@ func TestList_Create(t *testing.T) { NamespaceID: 876694, } - err = list3.Create(&doer) + err = list3.Create(doer) assert.Error(t, err) assert.True(t, IsErrNamespaceDoesNotExist(err)) diff --git a/pkg/models/list_read_test.go b/pkg/models/list_read_test.go index a61c1f17d78..c796085f0c9 100644 --- a/pkg/models/list_read_test.go +++ b/pkg/models/list_read_test.go @@ -36,7 +36,7 @@ func TestList_ReadAll(t *testing.T) { assert.NoError(t, err) lists2 := List{} - lists3, err := lists2.ReadAll("", &u, 1) + lists3, err := lists2.ReadAll("", u, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(lists3).Kind(), reflect.Slice) s := reflect.ValueOf(lists3) diff --git a/pkg/models/list_task_assignees.go b/pkg/models/list_task_assignees.go index f6bb227c57a..c873364b0a5 100644 --- a/pkg/models/list_task_assignees.go +++ b/pkg/models/list_task_assignees.go @@ -193,7 +193,7 @@ func (t *ListTask) addNewAssigneeByID(newAssigneeID int64, list *List) (err erro if err != nil { return err } - canRead, err := list.CanRead(&newAssignee) + canRead, err := list.CanRead(newAssignee) if err != nil { return err } diff --git a/pkg/models/list_task_readall_test.go b/pkg/models/list_task_readall_test.go index 2931a781288..04f81f090ef 100644 --- a/pkg/models/list_task_readall_test.go +++ b/pkg/models/list_task_readall_test.go @@ -16,20 +16,20 @@ import ( ) func sortTasksForTesting(by SortBy) (tasks []*ListTask) { - user1 := User{ + user1 := &User{ ID: 1, Username: "user1", Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", IsActive: true, AvatarURL: "111d68d06e2d317b5a59c2c6c5bad808", // hash for "" } - user2 := User{ + user2 := &User{ ID: 2, Username: "user2", Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", AvatarURL: "ab53a2911ddf9b4817ac01ddcd3d975f", // hash for "" } - user6 := User{ + user6 := &User{ ID: 6, Username: "user6", Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", @@ -50,7 +50,7 @@ func sortTasksForTesting(by SortBy) (tasks []*ListTask) { ID: 4, Title: "Label #4 - visible via other task", CreatedByID: 2, - CreatedBy: &user2, + CreatedBy: user2, Updated: 0, Created: 0, }, @@ -301,8 +301,8 @@ func sortTasksForTesting(by SortBy) (tasks []*ListTask) { CreatedBy: user1, ListID: 1, Assignees: []*User{ - &user1, - &user2, + user1, + user2, }, Created: 1543626724, Updated: 1543626724, @@ -345,7 +345,7 @@ func TestListTask_ReadAll(t *testing.T) { assert.NoError(t, LoadFixtures()) // Dummy users - user1 := User{ + user1 := &User{ ID: 1, Username: "user1", Password: "$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.", @@ -371,7 +371,7 @@ func TestListTask_ReadAll(t *testing.T) { Subtasks []*ListTask Created int64 Updated int64 - CreatedBy User + CreatedBy *User CRUDable web.CRUDable Rights web.Rights } diff --git a/pkg/models/list_tasks.go b/pkg/models/list_tasks.go index 04e6ebe4146..89f700de72c 100644 --- a/pkg/models/list_tasks.go +++ b/pkg/models/list_tasks.go @@ -77,7 +77,7 @@ type ListTask struct { Updated int64 `xorm:"updated not null" json:"updated"` // The user who initially created the task. - CreatedBy User `xorm:"-" json:"createdBy" valid:"-"` + CreatedBy *User `xorm:"-" json:"createdBy" valid:"-"` web.CRUDable `xorm:"-" json:"-"` web.Rights `xorm:"-" json:"-"` @@ -372,6 +372,7 @@ func addMoreInfoToTasks(taskMap map[int64]*ListTask) (tasks []*ListTask, err err // Put the assignees in the task map for _, a := range taskAssignees { if a != nil { + a.Email = "" // Obfuscate the email taskMap[a.TaskID].Assignees = append(taskMap[a.TaskID].Assignees, &a.User) } } @@ -395,6 +396,11 @@ func addMoreInfoToTasks(taskMap map[int64]*ListTask) (tasks []*ListTask, err err return } + // Obfuscate all user emails + for _, u := range users { + u.Email = "" + } + // Get all reminders and put them in a map to have it easier later reminders := []*TaskReminder{} err = x.Table("task_reminders").In("task_id", taskIDs).Find(&reminders) @@ -411,7 +417,7 @@ func addMoreInfoToTasks(taskMap map[int64]*ListTask) (tasks []*ListTask, err err for _, task := range taskMap { // Make created by user objects - taskMap[task.ID].CreatedBy = *users[task.CreatedByID] + taskMap[task.ID].CreatedBy = users[task.CreatedByID] // Add the reminders taskMap[task.ID].RemindersUnix = taskRemindersUnix[task.ID] diff --git a/pkg/models/list_tasks_test.go b/pkg/models/list_tasks_test.go index 1ac80c248b6..e086dbd9a0a 100644 --- a/pkg/models/list_tasks_test.go +++ b/pkg/models/list_tasks_test.go @@ -35,15 +35,15 @@ func TestListTask_Create(t *testing.T) { doer, err := GetUserByID(1) assert.NoError(t, err) - allowed, _ := listtask.CanCreate(&doer) + allowed, _ := listtask.CanCreate(doer) assert.True(t, allowed) - err = listtask.Create(&doer) + err = listtask.Create(doer) assert.NoError(t, err) // Update it listtask.Text = "Test34" - allowed, _ = listtask.CanUpdate(&doer) + allowed, _ = listtask.CanUpdate(doer) assert.True(t, allowed) err = listtask.Update() assert.NoError(t, err) @@ -54,7 +54,7 @@ func TestListTask_Create(t *testing.T) { assert.Equal(t, li.Text, "Test34") // Delete the task - allowed, _ = listtask.CanDelete(&doer) + allowed, _ = listtask.CanDelete(doer) assert.True(t, allowed) err = listtask.Delete() assert.NoError(t, err) @@ -67,14 +67,14 @@ func TestListTask_Create(t *testing.T) { // Try adding a list task with an empty text listtask.Text = "" - err = listtask.Create(&doer) + err = listtask.Create(doer) assert.Error(t, err) assert.True(t, IsErrListTaskCannotBeEmpty(err)) // Try adding one to a nonexistant list listtask.ListID = 99993939 listtask.Text = "Lorem Ipsum" - err = listtask.Create(&doer) + err = listtask.Create(doer) assert.Error(t, err) assert.True(t, IsErrListDoesNotExist(err)) diff --git a/pkg/models/list_users.go b/pkg/models/list_users.go index 8e530c37bb8..571e4d3fcd8 100644 --- a/pkg/models/list_users.go +++ b/pkg/models/list_users.go @@ -186,6 +186,11 @@ func (lu *ListUser) ReadAll(search string, a web.Auth, page int) (interface{}, e Where("users.username LIKE ?", "%"+search+"%"). Find(&all) + // Obfuscate all user emails + for _, u := range all { + u.Email = "" + } + return all, err } diff --git a/pkg/models/namespace.go b/pkg/models/namespace.go index 1ef0cf38177..91b7754c4f0 100644 --- a/pkg/models/namespace.go +++ b/pkg/models/namespace.go @@ -34,7 +34,7 @@ type Namespace struct { OwnerID int64 `xorm:"int(11) not null INDEX" json:"-"` // The user who owns this namespace - Owner User `xorm:"-" json:"owner" valid:"-"` + Owner *User `xorm:"-" json:"owner" valid:"-"` // A unix timestamp when this namespace was created. You cannot change this value. Created int64 `xorm:"created not null" json:"created"` @@ -148,7 +148,7 @@ func (n *Namespace) ReadAll(search string, a web.Auth, page int) (interface{}, e // Create our pseudo-namespace to hold the shared lists // We want this one at the beginning, which is why we create it here pseudonamespace := PseudoNamespace - pseudonamespace.Owner = *doer + pseudonamespace.Owner = doer all = append(all, &NamespaceWithLists{ pseudonamespace, []*List{}, @@ -238,7 +238,7 @@ func (n *Namespace) ReadAll(search string, a web.Auth, page int) (interface{}, e // Users for _, u := range users { if n.OwnerID == u.ID { - all[i].Owner = *u + all[i].Owner = u break } } diff --git a/pkg/models/namespace_test.go b/pkg/models/namespace_test.go index 74ae224eff4..118a76a1000 100644 --- a/pkg/models/namespace_test.go +++ b/pkg/models/namespace_test.go @@ -37,13 +37,13 @@ func TestNamespace_Create(t *testing.T) { assert.NoError(t, err) // Try creating it - allowed, _ := dummynamespace.CanCreate(&doer) + allowed, _ := dummynamespace.CanCreate(doer) assert.True(t, allowed) - err = dummynamespace.Create(&doer) + err = dummynamespace.Create(doer) assert.NoError(t, err) // check if it really exists - allowed, err = dummynamespace.CanRead(&doer) + allowed, err = dummynamespace.CanRead(doer) assert.NoError(t, err) assert.True(t, allowed) err = dummynamespace.ReadOne() @@ -52,7 +52,7 @@ func TestNamespace_Create(t *testing.T) { // Try creating one without a name n2 := Namespace{} - err = n2.Create(&doer) + err = n2.Create(doer) assert.Error(t, err) assert.True(t, IsErrNamespaceNameCannotBeEmpty(err)) @@ -64,7 +64,7 @@ func TestNamespace_Create(t *testing.T) { assert.True(t, IsErrUserDoesNotExist(err)) // Update it - allowed, err = dummynamespace.CanUpdate(&doer) + allowed, err = dummynamespace.CanUpdate(doer) assert.NoError(t, err) assert.True(t, allowed) dummynamespace.Description = "Dolor sit amet." @@ -74,7 +74,7 @@ func TestNamespace_Create(t *testing.T) { // Check if it was updated assert.Equal(t, "Dolor sit amet.", dummynamespace.Description) // Get it and check it again - allowed, err = dummynamespace.CanRead(&doer) + allowed, err = dummynamespace.CanRead(doer) assert.NoError(t, err) assert.True(t, allowed) err = dummynamespace.ReadOne() @@ -100,7 +100,7 @@ func TestNamespace_Create(t *testing.T) { assert.True(t, IsErrNamespaceDoesNotExist(err)) // Delete it - allowed, err = dummynamespace.CanDelete(&doer) + allowed, err = dummynamespace.CanDelete(doer) assert.NoError(t, err) assert.True(t, allowed) err = dummynamespace.Delete() @@ -112,13 +112,13 @@ func TestNamespace_Create(t *testing.T) { assert.True(t, IsErrNamespaceDoesNotExist(err)) // Check if it was successfully deleted - allowed, err = dummynamespace.CanRead(&doer) + allowed, err = dummynamespace.CanRead(doer) assert.False(t, allowed) assert.Error(t, err) assert.True(t, IsErrNamespaceDoesNotExist(err)) // Get all namespaces of a user - nsps, err := n.ReadAll("", &doer, 1) + nsps, err := n.ReadAll("", doer, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(nsps).Kind(), reflect.Slice) s := reflect.ValueOf(nsps) diff --git a/pkg/models/namespace_users.go b/pkg/models/namespace_users.go index fd67ca57a36..228ec5021fd 100644 --- a/pkg/models/namespace_users.go +++ b/pkg/models/namespace_users.go @@ -172,6 +172,11 @@ func (nu *NamespaceUser) ReadAll(search string, a web.Auth, page int) (interface Where("users.username LIKE ?", "%"+search+"%"). Find(&all) + // Obfuscate all user emails + for _, u := range all { + u.Email = "" + } + return all, err } diff --git a/pkg/models/team_list_test.go b/pkg/models/team_list_test.go index 16d0f6d35ae..b7c50633ee6 100644 --- a/pkg/models/team_list_test.go +++ b/pkg/models/team_list_test.go @@ -37,64 +37,64 @@ func TestTeamList(t *testing.T) { assert.NoError(t, err) // Check normal creation - allowed, _ := tl.CanCreate(&u) + allowed, _ := tl.CanCreate(u) assert.True(t, allowed) - err = tl.Create(&u) + err = tl.Create(u) assert.NoError(t, err) // Check again - err = tl.Create(&u) + err = tl.Create(u) assert.Error(t, err) assert.True(t, IsErrTeamAlreadyHasAccess(err)) // Check with wrong rights tl2 := tl tl2.Right = RightUnknown - err = tl2.Create(&u) + err = tl2.Create(u) assert.Error(t, err) assert.True(t, IsErrInvalidRight(err)) // Check with inexistant team tl3 := tl tl3.TeamID = 3253 - err = tl3.Create(&u) + err = tl3.Create(u) assert.Error(t, err) assert.True(t, IsErrTeamDoesNotExist(err)) // Check with inexistant list tl4 := tl tl4.ListID = 3252 - err = tl4.Create(&u) + err = tl4.Create(u) assert.Error(t, err) assert.True(t, IsErrListDoesNotExist(err)) // Test Read all - teams, err := tl.ReadAll("", &u, 1) + teams, err := tl.ReadAll("", u, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(teams).Kind(), reflect.Slice) s := reflect.ValueOf(teams) assert.Equal(t, s.Len(), 1) // Test Read all for nonexistant list - _, err = tl4.ReadAll("", &u, 1) + _, err = tl4.ReadAll("", u, 1) assert.Error(t, err) assert.True(t, IsErrListDoesNotExist(err)) // Test Read all for a list where the user is owner of the namespace this list belongs to tl5 := tl tl5.ListID = 2 - _, err = tl5.ReadAll("", &u, 1) + _, err = tl5.ReadAll("", u, 1) assert.NoError(t, err) // Test read all for a list where the user not has access tl6 := tl tl6.ListID = 5 - _, err = tl6.ReadAll("", &u, 1) + _, err = tl6.ReadAll("", u, 1) assert.Error(t, err) assert.True(t, IsErrNeedToHaveListReadAccess(err)) // Delete - allowed, _ = tl.CanDelete(&u) + allowed, _ = tl.CanDelete(u) assert.True(t, allowed) err = tl.Delete() assert.NoError(t, err) diff --git a/pkg/models/team_members_test.go b/pkg/models/team_members_test.go index c9fd303903f..8715d237901 100644 --- a/pkg/models/team_members_test.go +++ b/pkg/models/team_members_test.go @@ -34,9 +34,9 @@ func TestTeamMember_Create(t *testing.T) { assert.NoError(t, err) // Insert a new team member - allowed, _ := dummyteammember.CanCreate(&doer) + allowed, _ := dummyteammember.CanCreate(doer) assert.True(t, allowed) - err = dummyteammember.Create(&doer) + err = dummyteammember.Create(doer) assert.NoError(t, err) // Check he's in there @@ -46,12 +46,12 @@ func TestTeamMember_Create(t *testing.T) { assert.Equal(t, 3, len(team.Members)) // Try inserting a user twice - err = dummyteammember.Create(&doer) + err = dummyteammember.Create(doer) assert.Error(t, err) assert.True(t, IsErrUserIsMemberOfTeam(err)) // Delete it - allowed, _ = dummyteammember.CanDelete(&doer) + allowed, _ = dummyteammember.CanDelete(doer) assert.True(t, allowed) err = dummyteammember.Delete() assert.NoError(t, err) @@ -69,13 +69,13 @@ func TestTeamMember_Create(t *testing.T) { // Try inserting a user which does not exist dummyteammember.Username = "user9484" - err = dummyteammember.Create(&doer) + 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, Username: "user1"} - err = tm.Create(&doer) + err = tm.Create(doer) assert.Error(t, err) assert.True(t, IsErrTeamDoesNotExist(err)) } diff --git a/pkg/models/team_namespace_test.go b/pkg/models/team_namespace_test.go index 512ee639bab..fe4024f661c 100644 --- a/pkg/models/team_namespace_test.go +++ b/pkg/models/team_namespace_test.go @@ -36,46 +36,46 @@ func TestTeamNamespace(t *testing.T) { assert.NoError(t, err) // Test normal creation - allowed, _ := tn.CanCreate(&dummyuser) + allowed, _ := tn.CanCreate(dummyuser) assert.True(t, allowed) - err = tn.Create(&dummyuser) + err = tn.Create(dummyuser) assert.NoError(t, err) // Test again (should fail) - err = tn.Create(&dummyuser) + err = tn.Create(dummyuser) assert.Error(t, err) assert.True(t, IsErrTeamAlreadyHasAccess(err)) // Test with invalid team right tn2 := tn tn2.Right = RightUnknown - err = tn2.Create(&dummyuser) + err = tn2.Create(dummyuser) assert.Error(t, err) assert.True(t, IsErrInvalidRight(err)) // Check with inexistant team tn3 := tn tn3.TeamID = 324 - err = tn3.Create(&dummyuser) + err = tn3.Create(dummyuser) assert.Error(t, err) assert.True(t, IsErrTeamDoesNotExist(err)) // Check with a namespace which does not exist tn4 := tn tn4.NamespaceID = 423 - err = tn4.Create(&dummyuser) + err = tn4.Create(dummyuser) assert.Error(t, err) assert.True(t, IsErrNamespaceDoesNotExist(err)) // Check readall - teams, err := tn.ReadAll("", &dummyuser, 1) + teams, err := tn.ReadAll("", dummyuser, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(teams).Kind(), reflect.Slice) s := reflect.ValueOf(teams) assert.Equal(t, s.Len(), 1) // Check readall for a nonexistant namespace - _, err = tn4.ReadAll("", &dummyuser, 1) + _, err = tn4.ReadAll("", dummyuser, 1) assert.Error(t, err) assert.True(t, IsErrNamespaceDoesNotExist(err)) @@ -86,7 +86,7 @@ func TestTeamNamespace(t *testing.T) { assert.True(t, IsErrNeedToHaveNamespaceReadAccess(err)) // Delete it - allowed, _ = tn.CanDelete(&dummyuser) + allowed, _ = tn.CanDelete(dummyuser) assert.True(t, allowed) err = tn.Delete() assert.NoError(t, err) diff --git a/pkg/models/teams.go b/pkg/models/teams.go index 182603ea073..0d9c77e5a48 100644 --- a/pkg/models/teams.go +++ b/pkg/models/teams.go @@ -32,7 +32,7 @@ type Team struct { CreatedByID int64 `xorm:"int(11) not null INDEX" json:"-"` // The user who created this team. - CreatedBy User `xorm:"-" json:"createdBy"` + CreatedBy *User `xorm:"-" json:"createdBy"` // An array of all members in this team. Members []*TeamUser `xorm:"-" json:"members"` @@ -178,7 +178,7 @@ func (t *Team) Create(a web.Auth) (err error) { } t.CreatedByID = doer.ID - t.CreatedBy = *doer + t.CreatedBy = doer _, err = x.Insert(t) if err != nil { diff --git a/pkg/models/teams_rights_test.go b/pkg/models/teams_rights_test.go index 940a7990991..cb2f463c55e 100644 --- a/pkg/models/teams_rights_test.go +++ b/pkg/models/teams_rights_test.go @@ -28,7 +28,7 @@ func TestTeam_CanDoSomething(t *testing.T) { Name string Description string CreatedByID int64 - CreatedBy User + CreatedBy *User Members []*TeamUser Created int64 Updated int64 diff --git a/pkg/models/teams_test.go b/pkg/models/teams_test.go index 5ef6061bae4..fb96e085e98 100644 --- a/pkg/models/teams_test.go +++ b/pkg/models/teams_test.go @@ -34,9 +34,9 @@ func TestTeam_Create(t *testing.T) { assert.NoError(t, err) // Insert it - allowed, _ := dummyteam.CanCreate(&doer) + allowed, _ := dummyteam.CanCreate(doer) assert.True(t, allowed) - err = dummyteam.Create(&doer) + err = dummyteam.Create(doer) assert.NoError(t, err) // Check if it was inserted and we're admin @@ -46,7 +46,7 @@ func TestTeam_Create(t *testing.T) { assert.Equal(t, 1, len(tm.Members)) assert.Equal(t, doer.ID, tm.Members[0].User.ID) assert.True(t, tm.Members[0].Admin) - allowed, _ = dummyteam.CanRead(&doer) + allowed, _ = dummyteam.CanRead(doer) assert.True(t, allowed) // Try getting a team with an ID < 0 @@ -55,7 +55,7 @@ func TestTeam_Create(t *testing.T) { assert.True(t, IsErrTeamDoesNotExist(err)) // Get all teams the user is part of - ts, err := tm.ReadAll("", &doer, 1) + ts, err := tm.ReadAll("", doer, 1) assert.NoError(t, err) assert.Equal(t, reflect.TypeOf(ts).Kind(), reflect.Slice) s := reflect.ValueOf(ts) @@ -63,12 +63,12 @@ func TestTeam_Create(t *testing.T) { // Check inserting it with an empty name dummyteam.Name = "" - err = dummyteam.Create(&doer) + err = dummyteam.Create(doer) assert.Error(t, err) assert.True(t, IsErrTeamNameCannotBeEmpty(err)) // update it (still no name, should fail) - allowed, _ = dummyteam.CanUpdate(&doer) + allowed, _ = dummyteam.CanUpdate(doer) assert.True(t, allowed) err = dummyteam.Update() assert.Error(t, err) @@ -80,14 +80,14 @@ func TestTeam_Create(t *testing.T) { assert.NoError(t, err) // Delete it - allowed, err = dummyteam.CanDelete(&doer) + allowed, err = dummyteam.CanDelete(doer) assert.NoError(t, err) assert.True(t, allowed) err = dummyteam.Delete() assert.NoError(t, err) // Try deleting a (now) nonexistant team - allowed, err = dummyteam.CanDelete(&doer) + allowed, err = dummyteam.CanDelete(doer) assert.False(t, allowed) assert.Error(t, err) assert.True(t, IsErrTeamDoesNotExist(err)) diff --git a/pkg/models/user.go b/pkg/models/user.go index 559554fdbbf..7bfa8b5bb8f 100644 --- a/pkg/models/user.go +++ b/pkg/models/user.go @@ -65,7 +65,6 @@ type User struct { // AfterLoad is used to delete all emails to not have them leaked to the user func (u *User) AfterLoad() { u.AvatarURL = utils.Md5String(u.Email) - u.Email = "" } // GetID implements the Auth interface @@ -99,8 +98,8 @@ type APIUserPassword struct { } // APIFormat formats an API User into a normal user struct -func (apiUser *APIUserPassword) APIFormat() User { - return User{ +func (apiUser *APIUserPassword) APIFormat() *User { + return &User{ ID: apiUser.ID, Username: apiUser.Username, Password: apiUser.Password, @@ -109,41 +108,56 @@ func (apiUser *APIUserPassword) APIFormat() User { } // GetUserByID gets informations about a user by its ID -func GetUserByID(id int64) (user User, err error) { +func GetUserByID(id int64) (user *User, err error) { // Apparently xorm does otherwise look for all users but return only one, which leads to returing one even if the ID is 0 if id < 1 { - return User{}, ErrUserDoesNotExist{} + return &User{}, ErrUserDoesNotExist{} } - return GetUser(User{ID: id}) + 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) { +func GetUserByUsername(username string) (user *User, err error) { if username == "" { - return User{}, ErrUserDoesNotExist{} + return &User{}, ErrUserDoesNotExist{} } - return GetUser(User{Username: username}) + return GetUser(&User{Username: username}) } // GetUser gets a user object -func GetUser(user User) (userOut User, err error) { - userOut = user - exists, err := x.Get(&userOut) +func GetUser(user *User) (userOut *User, err error) { + return getUser(user, false) +} + +// GetUserWithEmail returns a user object with email +func GetUserWithEmail(user *User) (userOut *User, err error) { + return getUser(user, true) +} + +// getUser is a small helper function to avoid having duplicated code for almost the same use case +func getUser(user *User, withEmail bool) (userOut *User, err error) { + userOut = &User{} // To prevent a panic if user is nil + *userOut = *user + exists, err := x.Get(userOut) if !exists { - return User{}, ErrUserDoesNotExist{UserID: user.ID} + return &User{}, ErrUserDoesNotExist{UserID: user.ID} + } + + if !withEmail { + userOut.Email = "" } return userOut, err } // CheckUserCredentials checks user credentials -func CheckUserCredentials(u *UserLogin) (User, error) { +func CheckUserCredentials(u *UserLogin) (*User, error) { // Check if we have any credentials if u.Password == "" || u.Username == "" { - return User{}, ErrNoUsernamePassword{} + return &User{}, ErrNoUsernamePassword{} } // Check if the user exists @@ -151,21 +165,21 @@ func CheckUserCredentials(u *UserLogin) (User, error) { 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) - return User{}, ErrWrongUsernameOrPassword{} + return &User{}, ErrWrongUsernameOrPassword{} } // User is invalid if it needs to verify its email address if !user.IsActive { - return User{}, ErrEmailNotConfirmed{UserID: user.ID} + return &User{}, ErrEmailNotConfirmed{UserID: user.ID} } // Check the users password err = bcrypt.CompareHashAndPassword([]byte(user.Password), []byte(u.Password)) if err != nil { if err == bcrypt.ErrMismatchedHashAndPassword { - return User{}, ErrWrongUsernameOrPassword{} + return &User{}, ErrWrongUsernameOrPassword{} } - return User{}, err + return &User{}, err } return user, nil @@ -216,47 +230,47 @@ func UpdateActiveUsersFromContext(c echo.Context) (err error) { } // CreateUser creates a new user and inserts it into the database -func CreateUser(user User) (newUser User, err error) { +func CreateUser(user *User) (newUser *User, err error) { newUser = user // Check if we have all needed informations if newUser.Password == "" || newUser.Username == "" || newUser.Email == "" { - return User{}, ErrNoUsernamePassword{} + return &User{}, ErrNoUsernamePassword{} } // Check if the user already existst with that username exists := true - existingUser, err := GetUserByUsername(newUser.Username) + _, err = GetUserByUsername(newUser.Username) if err != nil { if IsErrUserDoesNotExist(err) { exists = false } else { - return User{}, err + return &User{}, err } } if exists { - return User{}, ErrUsernameExists{newUser.ID, newUser.Username} + return &User{}, ErrUsernameExists{newUser.ID, newUser.Username} } // Check if the user already existst with that email exists = true - existingUser, err = GetUser(User{Email: newUser.Email}) + _, err = GetUser(&User{Email: newUser.Email}) if err != nil { if IsErrUserDoesNotExist(err) { exists = false } else { - return User{}, err + return &User{}, err } } if exists { - return User{}, ErrUserEmailExists{existingUser.ID, existingUser.Email} + return &User{}, ErrUserEmailExists{newUser.ID, newUser.Email} } // Hash the password newUser.Password, err = hashPassword(user.Password) if err != nil { - return User{}, err + return &User{}, err } newUser.IsActive = true @@ -270,7 +284,7 @@ func CreateUser(user User) (newUser User, err error) { // Insert it _, err = x.Insert(newUser) if err != nil { - return User{}, err + return &User{}, err } // Update the metrics @@ -279,14 +293,14 @@ func CreateUser(user User) (newUser User, err error) { // Get the full new User newUserOut, err := GetUser(newUser) if err != nil { - return User{}, err + return &User{}, err } // Create the user's namespace newN := &Namespace{Name: newUserOut.Username, Description: newUserOut.Username + "'s namespace.", Owner: newUserOut} - err = newN.Create(&newUserOut) + err = newN.Create(newUserOut) if err != nil { - return User{}, err + return &User{}, err } // Dont send a mail if we're testing @@ -311,12 +325,12 @@ func hashPassword(password string) (string, error) { } // UpdateUser updates a user -func UpdateUser(user User) (updatedUser User, err error) { +func UpdateUser(user *User) (updatedUser *User, err error) { // Check if it exists theUser, err := GetUserByID(user.ID) if err != nil { - return User{}, err + return &User{}, err } // Check if we have at least a username @@ -330,13 +344,13 @@ func UpdateUser(user User) (updatedUser User, err error) { // Update it _, err = x.Id(user.ID).Update(user) if err != nil { - return User{}, err + return &User{}, err } // Get the newly updated user updatedUser, err = GetUserByID(user.ID) if err != nil { - return User{}, err + return &User{}, err } return updatedUser, err diff --git a/pkg/models/user_password_reset.go b/pkg/models/user_password_reset.go index 90047253823..fc1c39814a8 100644 --- a/pkg/models/user_password_reset.go +++ b/pkg/models/user_password_reset.go @@ -88,7 +88,7 @@ func RequestUserPasswordResetToken(tr *PasswordTokenRequest) (err error) { } // Check if the user exists - user, err := GetUser(User{Email: tr.Email}) + user, err := GetUserWithEmail(&User{Email: tr.Email}) if err != nil { return } @@ -97,7 +97,7 @@ func RequestUserPasswordResetToken(tr *PasswordTokenRequest) (err error) { user.PasswordResetToken = utils.MakeRandomString(400) // Save it - _, err = x.Where("id = ?", user.ID).Update(&user) + _, err = x.Where("id = ?", user.ID).Update(user) if err != nil { return } diff --git a/pkg/models/user_test.go b/pkg/models/user_test.go index b530f5b712d..95b8ec3b75b 100644 --- a/pkg/models/user_test.go +++ b/pkg/models/user_test.go @@ -31,7 +31,7 @@ func TestCreateUser(t *testing.T) { assert.NoError(t, err) // Our dummy user for testing - dummyuser := User{ + dummyuser := &User{ Username: "testuu", Password: "1234", Email: "noone@example.com", @@ -42,7 +42,7 @@ func TestCreateUser(t *testing.T) { assert.NoError(t, err) // Create a second new user - _, err = CreateUser(User{Username: dummyuser.Username + "2", Email: dummyuser.Email + "m", Password: dummyuser.Password}) + _, err = CreateUser(&User{Username: dummyuser.Username + "2", Email: dummyuser.Email + "m", Password: dummyuser.Password}) assert.NoError(t, err) // Check if it fails to create the same user again @@ -50,17 +50,17 @@ func TestCreateUser(t *testing.T) { assert.Error(t, err) // Check if it fails to create a user with just the same username - _, err = CreateUser(User{Username: dummyuser.Username, Password: "12345", Email: "email@example.com"}) + _, err = CreateUser(&User{Username: dummyuser.Username, Password: "12345", Email: "email@example.com"}) assert.Error(t, err) assert.True(t, IsErrUsernameExists(err)) // Check if it fails to create one with the same email - _, err = CreateUser(User{Username: "noone", Password: "1234", Email: dummyuser.Email}) + _, err = CreateUser(&User{Username: "noone", Password: "1234", Email: dummyuser.Email}) assert.Error(t, err) assert.True(t, IsErrUserEmailExists(err)) // Check if it fails to create a user without password and username - _, err = CreateUser(User{}) + _, err = CreateUser(&User{}) assert.Error(t, err) assert.True(t, IsErrNoUsernamePassword(err)) @@ -78,14 +78,14 @@ func TestCreateUser(t *testing.T) { assert.True(t, IsErrUserDoesNotExist(err)) // Check the user credentials with an unverified email - user, err := CheckUserCredentials(&UserLogin{"user5", "1234"}) + _, err = CheckUserCredentials(&UserLogin{"user5", "1234"}) assert.Error(t, err) assert.True(t, IsErrEmailNotConfirmed(err)) // Update everything and check again _, err = x.Cols("is_active").Where("true").Update(User{IsActive: true}) assert.NoError(t, err) - user, err = CheckUserCredentials(&UserLogin{"testuu", "1234"}) + user, err := CheckUserCredentials(&UserLogin{"testuu", "1234"}) assert.NoError(t, err) assert.Equal(t, "testuu", user.Username) @@ -100,23 +100,23 @@ func TestCreateUser(t *testing.T) { assert.True(t, IsErrWrongUsernameOrPassword(err)) // Update the user - uuser, err := UpdateUser(User{ID: theuser.ID, Password: "444444"}) + uuser, err := UpdateUser(&User{ID: theuser.ID, Password: "444444"}) assert.NoError(t, err) assert.Equal(t, theuser.Password, uuser.Password) // Password should not change assert.Equal(t, theuser.Username, uuser.Username) // Username should not change either // Try updating one which does not exist - _, err = UpdateUser(User{ID: 99999, Username: "dg"}) + _, err = UpdateUser(&User{ID: 99999, Username: "dg"}) assert.Error(t, err) assert.True(t, IsErrUserDoesNotExist(err)) // Update a users password newpassword := "55555" - err = UpdateUserPassword(&theuser, newpassword) + err = UpdateUserPassword(theuser, newpassword) assert.NoError(t, err) // Check if it was changed - user, err = CheckUserCredentials(&UserLogin{theuser.Username, newpassword}) + _, err = CheckUserCredentials(&UserLogin{theuser.Username, newpassword}) assert.NoError(t, err) // Check if the searchterm works @@ -134,11 +134,11 @@ func TestCreateUser(t *testing.T) { assert.True(t, IsErrUserDoesNotExist(err)) // Delete it - err = DeleteUserByID(theuser.ID, &doer) + err = DeleteUserByID(theuser.ID, doer) assert.NoError(t, err) // Try deleting one with ID = 0 - err = DeleteUserByID(0, &doer) + err = DeleteUserByID(0, doer) assert.Error(t, err) assert.True(t, IsErrIDCannotBeZero(err)) } diff --git a/pkg/models/users_list.go b/pkg/models/users_list.go index db13ff3965b..333957008ea 100644 --- a/pkg/models/users_list.go +++ b/pkg/models/users_list.go @@ -119,5 +119,11 @@ func ListUsersFromList(l *List, search string) (users []*User, err error) { GroupBy("id"). OrderBy("id"). Find(&users) + + // Obfuscate all user emails + for _, u := range users { + u.Email = "" + } + return } diff --git a/pkg/routes/api/v1/login.go b/pkg/routes/api/v1/login.go index 4753d6498a0..0db483b6f41 100644 --- a/pkg/routes/api/v1/login.go +++ b/pkg/routes/api/v1/login.go @@ -55,7 +55,7 @@ func Login(c echo.Context) error { } // Create token - t, err := CreateNewJWTTokenForUser(&user) + t, err := CreateNewJWTTokenForUser(user) if err != nil { return err }