Use the auth methods to get IDs to avoid unneeded casts

This commit is contained in:
kolaente 2019-06-28 10:21:48 +02:00
parent fc3c5f2187
commit f1d21ea52b
Signed by untrusted user: konrad
GPG Key ID: F40E70337AB24C9B
25 changed files with 65 additions and 146 deletions

2
go.mod
View File

@ -18,7 +18,7 @@ module code.vikunja.io/api
require (
cloud.google.com/go v0.34.0 // indirect
code.vikunja.io/web v0.0.0-20190628071027-b5c16e24b0a7
code.vikunja.io/web v0.0.0-20190628075253-b457b5a1a332
github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a
github.com/beevik/etree v1.1.0 // indirect

2
go.sum
View File

@ -5,6 +5,8 @@ code.vikunja.io/web v0.0.0-20190507193736-edb39812af9c h1:L8aPCsaLQe9qytRavkRqip
code.vikunja.io/web v0.0.0-20190507193736-edb39812af9c/go.mod h1:9dOotUqYZJhDhimNh4Xo4e2i+8cR+qPFEQNCUzaplsI=
code.vikunja.io/web v0.0.0-20190628071027-b5c16e24b0a7 h1:P9ncMaJE7RbYqBXF9lwT0hab7EPwuHOPslz3k1VxFs8=
code.vikunja.io/web v0.0.0-20190628071027-b5c16e24b0a7/go.mod h1:cuP1/ieGWAZzgQGw+QPt6Y5F0fVb/8Ol5NV4QSezGdo=
code.vikunja.io/web v0.0.0-20190628075253-b457b5a1a332 h1:gXxyLkjhgN+vqrLvPyqyScyG5fbu44FJp61TvntWM24=
code.vikunja.io/web v0.0.0-20190628075253-b457b5a1a332/go.mod h1:cuP1/ieGWAZzgQGw+QPt6Y5F0fVb/8Ol5NV4QSezGdo=
github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/PuerkitoBio/purell v1.1.0 h1:rmGxhojJlM0tuKtfdvliR84CFHljx9ag64t2xmVkjK4=

View File

@ -34,10 +34,8 @@ import (
// @Failure 500 {object} models.Message "Internal error"
// @Router /labels [get]
func (l *Label) ReadAll(search string, a web.Auth, page int) (ls interface{}, err error) {
u, err := getUserWithError(a)
if err != nil {
return nil, err
}
u := &User{ID: a.GetID()}
// Get all tasks
taskIDs, err := getUserTaskIDs(u)

View File

@ -43,20 +43,17 @@ func (l *Label) CanCreate(a web.Auth) (bool, error) {
}
func (l *Label) isLabelOwner(a web.Auth) (bool, error) {
u := getUserForRights(a)
lorig, err := getLabelByIDSimple(l.ID)
if err != nil {
return false, err
}
return lorig.CreatedByID == u.ID, nil
return lorig.CreatedByID == a.GetID(), nil
}
// Helper method to check if a user can see a specific label
func (l *Label) hasAccessToLabel(a web.Auth) (bool, error) {
u := getUserForRights(a)
// Get all tasks
taskIDs, err := getUserTaskIDs(u)
taskIDs, err := getUserTaskIDs(&User{ID: a.GetID()})
if err != nil {
return false, err
}
@ -66,7 +63,7 @@ func (l *Label) hasAccessToLabel(a web.Auth) (bool, error) {
has, err := x.Table("labels").
Select("labels.*").
Join("LEFT", "label_task", "label_task.label_id = labels.id").
Where("label_task.label_id != null OR labels.created_by_id = ?", u.ID).
Where("label_task.label_id != null OR labels.created_by_id = ?", a.GetID()).
Or(builder.In("label_task.task_id", taskIDs)).
And("labels.id = ?", l.ID).
Exist(&labels)

View File

@ -108,11 +108,6 @@ func (lt *LabelTask) Create(a web.Auth) (err error) {
// @Failure 500 {object} models.Message "Internal error"
// @Router /tasks/{task}/labels [get]
func (lt *LabelTask) ReadAll(search string, a web.Auth, page int) (labels interface{}, err error) {
u, err := getUserWithError(a)
if err != nil {
return nil, err
}
// Check if the user has the right to see the task
task := ListTask{ID: lt.TaskID}
canRead, err := task.CanRead(a)
@ -120,11 +115,11 @@ func (lt *LabelTask) ReadAll(search string, a web.Auth, page int) (labels interf
return nil, err
}
if !canRead {
return nil, ErrNoRightToSeeTask{lt.TaskID, u.ID}
return nil, ErrNoRightToSeeTask{lt.TaskID, a.GetID()}
}
return getLabelsByTaskIDs(&LabelByTaskIDsOptions{
User: u,
User: &User{ID: a.GetID()},
Search: search,
Page: page,
TaskIDs: []int64{lt.TaskID},

View File

@ -96,7 +96,10 @@ func TestLabel_ReadAll(t *testing.T) {
},
},
{
name: "invalid user",
name: "invalid user",
args: args{
a: &User{ID: -1},
},
wantErr: true,
},
}

View File

@ -84,12 +84,7 @@ func GetListsByNamespaceID(nID int64, doer *User) (lists []*List, err error) {
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists [get]
func (l *List) ReadAll(search string, a web.Auth, page int) (interface{}, error) {
u, err := getUserWithError(a)
if err != nil {
return nil, err
}
lists, err := getRawListsForUser(search, u, page)
lists, err := getRawListsForUser(search, &User{ID: a.GetID()}, page)
if err != nil {
return nil, err
}

View File

@ -106,14 +106,8 @@ func (l *List) Create(a web.Auth) (err error) {
return err
}
// Check rights
u, err := GetUserByID(doer.ID)
if err != nil {
return
}
l.OwnerID = u.ID
l.Owner.ID = u.ID
l.OwnerID = doer.ID
l.Owner = *doer
l.ID = 0 // Otherwise only the first time a new list would be created
return CreateOrUpdateList(l)

View File

@ -31,27 +31,24 @@ func (l *List) CanWrite(a web.Auth) (bool, error) {
return false, err
}
user := getUserForRights(a)
// Check if the user is either owner or can write to the list
if originalList.isOwner(user) {
if originalList.isOwner(&User{ID: a.GetID()}) {
return true, nil
}
return originalList.checkRight(user, RightWrite, RightAdmin)
return originalList.checkRight(a, RightWrite, RightAdmin)
}
// CanRead checks if a user has read access to a list
func (l *List) CanRead(a web.Auth) (bool, error) {
user := getUserForRights(a)
// Check if the user is either owner or can read
if err := l.GetSimpleByID(); err != nil {
return false, err
}
if l.isOwner(user) {
if l.isOwner(&User{ID: a.GetID()}) {
return true, nil
}
return l.checkRight(user, RightRead, RightWrite, RightAdmin)
return l.checkRight(a, RightRead, RightWrite, RightAdmin)
}
// CanUpdate checks if the user can update a list
@ -73,8 +70,6 @@ func (l *List) CanCreate(a web.Auth) (bool, error) {
// IsAdmin returns whether the user has admin rights on the list or not
func (l *List) IsAdmin(a web.Auth) (bool, error) {
user := getUserForRights(a)
originalList := &List{ID: l.ID}
err := originalList.GetSimpleByID()
if err != nil {
@ -84,10 +79,10 @@ func (l *List) IsAdmin(a web.Auth) (bool, error) {
// Check all the things
// Check if the user is either owner or can write to the list
// Owners are always admins
if originalList.isOwner(user) {
if originalList.isOwner(&User{ID: a.GetID()}) {
return true, nil
}
return originalList.checkRight(user, RightAdmin)
return originalList.checkRight(a, RightAdmin)
}
// Little helper function to check if a user is list owner
@ -96,7 +91,7 @@ func (l *List) isOwner(u *User) bool {
}
// Checks n different rights for any given user
func (l *List) checkRight(user *User, rights ...Right) (bool, error) {
func (l *List) checkRight(a web.Auth, rights ...Right) (bool, error) {
/*
The following loop creates an sql condition like this one:
@ -113,24 +108,24 @@ func (l *List) checkRight(user *User, rights ...Right) (bool, error) {
// User conditions
// If the list was shared directly with the user and the user has the right
conds = append(conds, builder.And(
builder.Eq{"ul.user_id": user.ID},
builder.Eq{"ul.user_id": a.GetID()},
builder.Eq{"ul.right": r},
))
// If the namespace this list belongs to was shared directly with the user and the user has the right
conds = append(conds, builder.And(
builder.Eq{"un.user_id": user.ID},
builder.Eq{"un.user_id": a.GetID()},
builder.Eq{"un.right": r},
))
// Team rights
// If the list was shared directly with the team and the team has the right
conds = append(conds, builder.And(
builder.Eq{"tm2.user_id": user.ID},
builder.Eq{"tm2.user_id": a.GetID()},
builder.Eq{"tl.right": r},
))
// If the namespace this list belongs to was shared directly with the team and the team has the right
conds = append(conds, builder.And(
builder.Eq{"tm.user_id": user.ID},
builder.Eq{"tm.user_id": a.GetID()},
builder.Eq{"tn.right": r},
))
}

View File

@ -40,11 +40,6 @@ const (
// @Failure 500 {object} models.Message "Internal error"
// @Router /tasks/all [get]
func (t *ListTask) ReadAll(search string, a web.Auth, page int) (interface{}, error) {
u, err := getUserWithError(a)
if err != nil {
return nil, err
}
var sortby SortBy
switch t.Sorting {
case "priority":
@ -63,7 +58,7 @@ func (t *ListTask) ReadAll(search string, a web.Auth, page int) (interface{}, er
sortby = SortTasksByUnsorted
}
return GetTasksByUser(search, u, page, sortby, time.Unix(t.StartDateSortUnix, 0), time.Unix(t.EndDateSortUnix, 0))
return GetTasksByUser(search, &User{ID: a.GetID()}, page, sortby, time.Unix(t.StartDateSortUnix, 0), time.Unix(t.EndDateSortUnix, 0))
}
//GetTasksByUser returns all tasks for a user

View File

@ -39,10 +39,6 @@ import (
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists/{id} [put]
func (t *ListTask) Create(a web.Auth) (err error) {
doer, err := getUserWithError(a)
if err != nil {
return err
}
t.ID = 0
@ -57,7 +53,7 @@ func (t *ListTask) Create(a web.Auth) (err error) {
return
}
u, err := GetUserByID(doer.ID)
u, err := GetUserByID(a.GetID())
if err != nil {
return err
}

View File

@ -32,11 +32,9 @@ func (t *ListTask) CanUpdate(a web.Auth) (bool, error) {
// CanCreate determines if a user has the right to create a list task
func (t *ListTask) CanCreate(a web.Auth) (bool, error) {
doer := getUserForRights(a)
// A user can do a task if he has write acces to its list
l := &List{ID: t.ListID}
return l.CanWrite(doer)
return l.CanWrite(a)
}
// CanRead determines if a user can read a task
@ -55,8 +53,6 @@ func (t *ListTask) CanRead(a web.Auth) (canRead bool, err error) {
// Helper function to check if a user can do stuff on a list task
func (t *ListTask) canDoListTask(a web.Auth) (bool, error) {
doer := getUserForRights(a)
// Get the task
lI, err := GetTaskByIDSimple(t.ID)
if err != nil {
@ -65,5 +61,5 @@ func (t *ListTask) canDoListTask(a web.Auth) (bool, error) {
// A user can do a task if he has write acces to its list
l := &List{ID: lI.ListID}
return l.CanWrite(doer)
return l.CanWrite(a)
}

View File

@ -33,19 +33,14 @@ import "code.vikunja.io/web"
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists/{id}/users [get]
func (lu *ListUser) ReadAll(search string, a web.Auth, page int) (interface{}, error) {
u, err := getUserWithError(a)
if err != nil {
return nil, err
}
// Check if the user has access to the list
l := &List{ID: lu.ListID}
canRead, err := l.CanRead(u)
canRead, err := l.CanRead(a)
if err != nil {
return nil, err
}
if !canRead {
return nil, ErrNeedToHaveListReadAccess{UserID: u.ID, ListID: lu.ListID}
return nil, ErrNeedToHaveListReadAccess{UserID: a.GetID(), ListID: lu.ListID}
}
// Get all users

View File

@ -35,19 +35,14 @@ import (
// @Failure 500 {object} models.Message "Internal error"
// @Router /namespaces [put]
func (n *Namespace) Create(a web.Auth) (err error) {
doer, err := getUserWithError(a)
if err != nil {
return err
}
// Check if we have at least a name
if n.Name == "" {
return ErrNamespaceNameCannotBeEmpty{NamespaceID: 0, UserID: doer.ID}
return ErrNamespaceNameCannotBeEmpty{NamespaceID: 0, UserID: a.GetID()}
}
n.ID = 0 // This would otherwise prevent the creation of new lists after one was created
// Check if the User exists
n.Owner, err = GetUserByID(doer.ID)
n.Owner, err = GetUserByID(a.GetID())
if err != nil {
return
}

View File

@ -60,8 +60,7 @@ func (n *Namespace) checkRight(a web.Auth, rights ...Right) (bool, error) {
return false, err
}
user := getUserForRights(a)
if user.ID == n.OwnerID {
if a.GetID() == n.OwnerID {
return true, nil
}
@ -78,19 +77,19 @@ func (n *Namespace) checkRight(a web.Auth, rights ...Right) (bool, error) {
*/
var conds []builder.Cond
conds = append(conds, builder.Eq{"namespaces.owner_id": user.ID})
conds = append(conds, builder.Eq{"namespaces.owner_id": a.GetID()})
for _, r := range rights {
// User conditions
// If the namespace was shared directly with the user and the user has the right
conds = append(conds, builder.And(
builder.Eq{"users_namespace.user_id": user.ID},
builder.Eq{"users_namespace.user_id": a.GetID()},
builder.Eq{"users_namespace.right": r},
))
// Team rights
// If the namespace was shared directly with the team and the team has the right
conds = append(conds, builder.And(
builder.Eq{"team_members.user_id": user.ID},
builder.Eq{"team_members.user_id": a.GetID()},
builder.Eq{"team_namespaces.right": r},
))
}

View File

@ -33,14 +33,9 @@ import "code.vikunja.io/web"
// @Failure 500 {object} models.Message "Internal error"
// @Router /namespaces/{id}/users [get]
func (nu *NamespaceUser) ReadAll(search string, a web.Auth, page int) (interface{}, error) {
u, err := getUserWithError(a)
if err != nil {
return nil, err
}
// Check if the user has access to the namespace
l := Namespace{ID: nu.NamespaceID}
canRead, err := l.CanRead(u)
canRead, err := l.CanRead(a)
if err != nil {
return nil, err
}

View File

@ -33,19 +33,14 @@ import "code.vikunja.io/web"
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists/{id}/teams [get]
func (tl *TeamList) ReadAll(search string, a web.Auth, page int) (interface{}, error) {
u, err := getUserWithError(a)
if err != nil {
return nil, err
}
// Check if the user can read the namespace
l := &List{ID: tl.ListID}
canRead, err := l.CanRead(u)
canRead, err := l.CanRead(a)
if err != nil {
return nil, err
}
if !canRead {
return nil, ErrNeedToHaveListReadAccess{ListID: tl.ListID, UserID: u.ID}
return nil, ErrNeedToHaveListReadAccess{ListID: tl.ListID, UserID: a.GetID()}
}
// Get the teams

View File

@ -22,24 +22,20 @@ import (
// CanCreate checks if the user can create a team <-> list relation
func (tl *TeamList) CanCreate(a web.Auth) (bool, error) {
u := getUserForRights(a)
l := List{ID: tl.ListID}
return l.IsAdmin(u)
return tl.canDoTeamList(a)
}
// CanDelete checks if the user can delete a team <-> list relation
func (tl *TeamList) CanDelete(a web.Auth) (bool, error) {
user := getUserForRights(a)
l := List{ID: tl.ListID}
return l.IsAdmin(user)
return tl.canDoTeamList(a)
}
// CanUpdate checks if the user can update a team <-> list relation
func (tl *TeamList) CanUpdate(a web.Auth) (bool, error) {
user := getUserForRights(a)
l := List{ID: tl.ListID}
return l.IsAdmin(user)
return tl.canDoTeamList(a)
}
func (tl *TeamList) canDoTeamList(a web.Auth) (bool, error) {
l := List{ID: tl.ListID}
return l.IsAdmin(a)
}

View File

@ -32,10 +32,8 @@ func (tm *TeamMember) CanDelete(a web.Auth) (bool, error) {
// IsAdmin checks if the user is team admin
func (tm *TeamMember) IsAdmin(a web.Auth) (bool, error) {
u := getUserForRights(a)
// A user can add a member to a team if he is admin of that team
exists, err := x.Where("user_id = ? AND team_id = ? AND admin = ?", u.ID, tm.TeamID, true).
exists, err := x.Where("user_id = ? AND team_id = ? AND admin = ?", a.GetID(), tm.TeamID, true).
Get(&TeamMember{})
return exists, err
}

View File

@ -33,19 +33,14 @@ import "code.vikunja.io/web"
// @Failure 500 {object} models.Message "Internal error"
// @Router /namespaces/{id}/teams [get]
func (tn *TeamNamespace) ReadAll(search string, a web.Auth, page int) (interface{}, error) {
user, err := getUserWithError(a)
if err != nil {
return nil, err
}
// Check if the user can read the namespace
n := Namespace{ID: tn.NamespaceID}
canRead, err := n.CanRead(user)
canRead, err := n.CanRead(a)
if err != nil {
return nil, err
}
if !canRead {
return nil, ErrNeedToHaveNamespaceReadAccess{NamespaceID: tn.NamespaceID, UserID: user.ID}
return nil, ErrNeedToHaveNamespaceReadAccess{NamespaceID: tn.NamespaceID, UserID: a.GetID()}
}
// Get the teams

View File

@ -139,16 +139,11 @@ func (t *Team) ReadOne() (err error) {
// @Failure 500 {object} models.Message "Internal error"
// @Router /teams [get]
func (t *Team) ReadAll(search string, a web.Auth, page int) (interface{}, error) {
user, err := getUserWithError(a)
if err != nil {
return nil, err
}
all := []*Team{}
err = x.Select("teams.*").
err := x.Select("teams.*").
Table("teams").
Join("INNER", "team_members", "team_members.team_id = teams.id").
Where("team_members.user_id = ?", user.ID).
Where("team_members.user_id = ?", a.GetID()).
Limit(getLimitFromPageIndex(page)).
Where("teams.name LIKE ?", "%"+search+"%").
Find(&all)

View File

@ -38,8 +38,6 @@ func (t *Team) CanDelete(a web.Auth) (bool, error) {
// IsAdmin returns true when the user is admin of a team
func (t *Team) IsAdmin(a web.Auth) (bool, error) {
u := getUserForRights(a)
// Check if the team exists to be able to return a proper error message if not
_, err := GetTeamByID(t.ID)
if err != nil {
@ -47,17 +45,15 @@ func (t *Team) IsAdmin(a web.Auth) (bool, error) {
}
return x.Where("team_id = ?", t.ID).
And("user_id = ?", u.ID).
And("user_id = ?", a.GetID()).
And("admin = ?", true).
Get(&TeamMember{})
}
// CanRead returns true if the user has read access to the team
func (t *Team) CanRead(a web.Auth) (bool, error) {
user := getUserForRights(a)
// Check if the user is in the team
return x.Where("team_id = ?", t.ID).
And("user_id = ?", user.ID).
And("user_id = ?", a.GetID()).
Get(&TeamMember{})
}

View File

@ -17,7 +17,6 @@
package models
import (
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/api/pkg/metrics"
"code.vikunja.io/api/pkg/utils"
"code.vikunja.io/web"
@ -67,22 +66,16 @@ func (u *User) AfterLoad() {
u.Email = ""
}
// AuthDummy implements the auth of the crud handler
func (User) AuthDummy() {}
// GetID implements the Auth interface
func (u *User) GetID() int64 {
return u.ID
}
// TableName returns the table name for users
func (User) TableName() string {
return "users"
}
func getUserForRights(a web.Auth) *User {
u, err := getUserWithError(a)
if err != nil {
log.Log.Error(err.Error())
}
return u
}
func getUserWithError(a web.Auth) (*User, error) {
u, is := a.(*User)
if !is {

3
vendor/code.vikunja.io/web/web.go generated vendored
View File

@ -50,7 +50,8 @@ type HTTPError struct {
// Auth defines the authentication interface used to get some auth thing
type Auth interface {
AuthDummy()
// Most of the time, we need an ID from the auth object only. Having this method saves the need to cast it.
GetID() int64
}
// Authprovider is a holder for the implementation of an authprovider by the application

2
vendor/modules.txt vendored
View File

@ -1,4 +1,4 @@
# code.vikunja.io/web v0.0.0-20190628071027-b5c16e24b0a7
# code.vikunja.io/web v0.0.0-20190628075253-b457b5a1a332
code.vikunja.io/web
code.vikunja.io/web/handler
# github.com/BurntSushi/toml v0.3.1