local-it/vikunja-api
0
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

fix: don't allow setting a list namespace to 0

Browse source 
See https://github.com/go-vikunja/app/issues/13
This commit is contained in:
kolaente 2022-10-01 15:02:13 +02:00
parent 374a0f9ce3
commit 96ed1e33e3
No known key found for this signature in database
GPG key ID: F40E70337AB24C9B
7 changed files with 59 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
docs/content/doc/usage/errors.md
View file

@ -54,13 +54,14 @@ This document describes the different errors Vikunja can return.
| ErrorCode | HTTP Status Code | Description |
|-----------|------------------|-------------------------------------------------------------------------------------------------------------------------------|
| 3001 | 404 | The list does not exist. |
| 3004 | 403 | The user needs to have read permissions on that list to perform that action. |
| 3005 | 400 | The list title cannot be empty. |
| 3006 | 404 | The list share does not exist. |
| 3007 | 400 | A list with this identifier already exists. |
| 3008 | 412 | The list is archived and can therefore only be accessed read only. This is also true for all tasks associated with this list. |
| 3009 | 412 | The list cannot belong to a dynamically generated namespace like "Favorites". |
| 3001 | 404 | The list does not exist. |
| 3004 | 403 | The user needs to have read permissions on that list to perform that action. |
| 3005 | 400 | The list title cannot be empty. |
| 3006 | 404 | The list share does not exist. |
| 3007 | 400 | A list with this identifier already exists. |
| 3008 | 412 | The list is archived and can therefore only be accessed read only. This is also true for all tasks associated with this list. |
| 3009 | 412 | The list cannot belong to a dynamically generated namespace like "Favorites". |
| 3010 | 412 | The list must belong to a namespace. |
## Task

2
pkg/integrations/archived_test.go
View file

@ -227,7 +227,7 @@ func TestArchived(t *testing.T) {
assertHandlerErrorCode(t, err, models.ErrCodeListIsArchived)
})
t.Run("unarchivable", func(t *testing.T) {
rec, err := testListHandler.testUpdateWithUser(nil, map[string]string{"list": "22"}, `{"title":"LoremIpsum","is_archived":false}`)
rec, err := testListHandler.testUpdateWithUser(nil, map[string]string{"list": "22"}, `{"title":"LoremIpsum","is_archived":false,"namespace_id":1}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"is_archived":false`)
})

4
pkg/integrations/link_sharing_test.go
View file

@ -232,12 +232,12 @@ func TestLinkSharing(t *testing.T) {
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
})
t.Run("Shared write", func(t *testing.T) {
rec, err := testHandlerListWrite.testUpdateWithLinkShare(nil, map[string]string{"list": "2"}, `{"title":"TestLoremIpsum"}`)
rec, err := testHandlerListWrite.testUpdateWithLinkShare(nil, map[string]string{"list": "2"}, `{"title":"TestLoremIpsum","namespace_id":1}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
})
t.Run("Shared admin", func(t *testing.T) {
rec, err := testHandlerListAdmin.testUpdateWithLinkShare(nil, map[string]string{"list": "3"}, `{"title":"TestLoremIpsum"}`)
rec, err := testHandlerListAdmin.testUpdateWithLinkShare(nil, map[string]string{"list": "3"}, `{"title":"TestLoremIpsum","namespace_id":2}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
})

20
pkg/integrations/list_test.go
View file

@ -171,7 +171,7 @@ func TestList(t *testing.T) {
t.Run("Update", func(t *testing.T) {
t.Run("Normal", func(t *testing.T) {
// Check the list was loaded successfully afterwards, see testReadOneWithUser
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "1"}, `{"title":"TestLoremIpsum"}`)
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "1"}, `{"title":"TestLoremIpsum","namespace_id":1}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
// The description should not be updated but returned correctly
@ -183,7 +183,7 @@ func TestList(t *testing.T) {
assertHandlerErrorCode(t, err, models.ErrCodeListDoesNotExist)
})
t.Run("Normal with updating the description", func(t *testing.T) {
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "1"}, `{"title":"TestLoremIpsum","description":"Lorem Ipsum dolor sit amet"}`)
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "1"}, `{"title":"TestLoremIpsum","description":"Lorem Ipsum dolor sit amet","namespace_id":1}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
assert.Contains(t, rec.Body.String(), `"description":"Lorem Ipsum dolor sit amet`)
@ -211,12 +211,12 @@ func TestList(t *testing.T) {
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
})
t.Run("Shared Via Team write", func(t *testing.T) {
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "7"}, `{"title":"TestLoremIpsum"}`)
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "7"}, `{"title":"TestLoremIpsum","namespace_id":6}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
})
t.Run("Shared Via Team admin", func(t *testing.T) {
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "8"}, `{"title":"TestLoremIpsum"}`)
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "8"}, `{"title":"TestLoremIpsum","namespace_id":6}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
})
@ -227,12 +227,12 @@ func TestList(t *testing.T) {
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
})
t.Run("Shared Via User write", func(t *testing.T) {
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "10"}, `{"title":"TestLoremIpsum"}`)
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "10"}, `{"title":"TestLoremIpsum","namespace_id":6}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
})
t.Run("Shared Via User admin", func(t *testing.T) {
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "11"}, `{"title":"TestLoremIpsum"}`)
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "11"}, `{"title":"TestLoremIpsum","namespace_id":6}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
})
@ -243,12 +243,12 @@ func TestList(t *testing.T) {
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
})
t.Run("Shared Via NamespaceTeam write", func(t *testing.T) {
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "13"}, `{"title":"TestLoremIpsum"}`)
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "13"}, `{"title":"TestLoremIpsum","namespace_id":8}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
})
t.Run("Shared Via NamespaceTeam admin", func(t *testing.T) {
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "14"}, `{"title":"TestLoremIpsum"}`)
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "14"}, `{"title":"TestLoremIpsum","namespace_id":9}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
})
@ -259,12 +259,12 @@ func TestList(t *testing.T) {
assert.Contains(t, err.(*echo.HTTPError).Message, `Forbidden`)
})
t.Run("Shared Via NamespaceUser write", func(t *testing.T) {
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "16"}, `{"title":"TestLoremIpsum"}`)
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "16"}, `{"title":"TestLoremIpsum","namespace_id":11}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
})
t.Run("Shared Via NamespaceUser admin", func(t *testing.T) {
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "17"}, `{"title":"TestLoremIpsum"}`)
rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"list": "17"}, `{"title":"TestLoremIpsum","namespace_id":12}`)
assert.NoError(t, err)
assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`)
})

28
pkg/models/error.go
View file

@ -283,6 +283,34 @@ func (err *ErrListCannotBelongToAPseudoNamespace) HTTPError() web.HTTPError {
}
}
// ErrListMustBelongToANamespace represents an error where a list must belong to a namespace
type ErrListMustBelongToANamespace struct {
ListID int64
NamespaceID int64
}
// IsErrListMustBelongToANamespace checks if an error is a list must belong to a namespace error.
func IsErrListMustBelongToANamespace(err error) bool {
_, ok := err.(*ErrListMustBelongToANamespace)
return ok
}
func (err *ErrListMustBelongToANamespace) Error() string {
return fmt.Sprintf("List must belong to a namespace [ListID: %d, NamespaceID: %d]", err.ListID, err.NamespaceID)
}
// ErrCodeListMustBelongToANamespace holds the unique world-error code of this error
const ErrCodeListMustBelongToANamespace = 3010
// HTTPError holds the http error description
func (err *ErrListMustBelongToANamespace) HTTPError() web.HTTPError {
return web.HTTPError{
HTTPCode: http.StatusPreconditionFailed,
Code: ErrCodeListMustBelongToANamespace,
Message: "This list must belong to a namespace.",
}
}
// ================
// List task errors
// ================

7
pkg/models/list.go
View file

@ -640,6 +640,13 @@ func UpdateList(s *xorm.Session, list *List, auth web.Auth, updateListBackground
return
}
if list.NamespaceID == 0 {
return &ErrListMustBelongToANamespace{
ListID: list.ID,
NamespaceID: list.NamespaceID,
}
}
// We need to specify the cols we want to update here to be able to un-archive lists
colsToUpdate := []string{
"title",

5
pkg/models/list_test.go
View file

@ -140,8 +140,9 @@ func TestList_CreateOrUpdate(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
list := List{
ID: 99999999,
Title: "test",
ID: 99999999,
Title: "test",
NamespaceID: 1,
}
err := list.Update(s, usr)
assert.Error(t, err)