From b3c604fd2f5e04e90fcb7640d13da28bc0cb4287 Mon Sep 17 00:00:00 2001 From: konrad Date: Sun, 11 Apr 2021 13:17:50 +0000 Subject: [PATCH] Add link share password authentication (#831) Reviewed-on: https://kolaente.dev/vikunja/api/pulls/831 Co-authored-by: konrad Co-committed-by: konrad --- docs/content/doc/usage/errors.md | 7 + pkg/db/fixtures/link_shares.yml | 9 ++ pkg/integrations/integrations.go | 4 +- pkg/integrations/link_sharing_auth_test.go | 60 ++++++++ pkg/integrations/login_test.go | 10 +- pkg/integrations/register_test.go | 14 +- pkg/integrations/user_confirm_email_test.go | 8 +- .../user_password_request_token_test.go | 8 +- pkg/integrations/user_password_reset_test.go | 8 +- pkg/migration/20210411113105.go | 50 +++++++ pkg/models/error.go | 58 ++++++++ pkg/models/link_sharing.go | 39 ++++- pkg/models/link_sharing_test.go | 140 ++++++++++++++++++ pkg/routes/api/v1/link_sharing_auth.go | 26 +++- pkg/swagger/docs.go | 23 ++- pkg/swagger/swagger.json | 23 ++- pkg/swagger/swagger.yaml | 16 +- pkg/user/user.go | 2 +- pkg/user/user_create.go | 4 +- pkg/user/user_password_reset.go | 2 +- 20 files changed, 471 insertions(+), 40 deletions(-) create mode 100644 pkg/integrations/link_sharing_auth_test.go create mode 100644 pkg/migration/20210411113105.go create mode 100644 pkg/models/link_sharing_test.go diff --git a/docs/content/doc/usage/errors.md b/docs/content/doc/usage/errors.md index 9b072cd0d..9219bf9b2 100644 --- a/docs/content/doc/usage/errors.md +++ b/docs/content/doc/usage/errors.md @@ -150,3 +150,10 @@ This document describes the different errors Vikunja can return. |-----------|------------------|-------------| | 12001 | 412 | The subscription entity type is invalid. | | 12002 | 412 | The user is already subscribed to the entity itself or a parent entity. | + +## Link Shares + +| ErrorCode | HTTP Status Code | Description | +|-----------|------------------|-------------| +| 13001 | 412 | This link share requires a password for authentication, but none was provided. | +| 13002 | 403 | The provided link share password was invalid. | diff --git a/pkg/db/fixtures/link_shares.yml b/pkg/db/fixtures/link_shares.yml index fb756cfa7..e0d19984d 100644 --- a/pkg/db/fixtures/link_shares.yml +++ b/pkg/db/fixtures/link_shares.yml @@ -22,3 +22,12 @@ shared_by_id: 1 created: 2018-12-01 15:13:12 updated: 2018-12-02 15:13:12 +- id: 4 + hash: testWithPassword + list_id: 1 + right: 0 + password: '$2a$14$dcadBoMBL9jQoOcZK8Fju.cy0Ptx2oZECkKLnaa8ekRoTFe1w7To.' # 1234 + sharing_type: 2 + shared_by_id: 1 + created: 2018-12-01 15:13:12 + updated: 2018-12-02 15:13:12 diff --git a/pkg/integrations/integrations.go b/pkg/integrations/integrations.go index ad4f4fbc4..f9c8a69ea 100644 --- a/pkg/integrations/integrations.go +++ b/pkg/integrations/integrations.go @@ -114,8 +114,8 @@ func bootstrapTestRequest(t *testing.T, method string, payload string, queryPara return } -func newTestRequest(t *testing.T, method string, handler func(ctx echo.Context) error, payload string) (rec *httptest.ResponseRecorder, err error) { - c, rec := bootstrapTestRequest(t, method, payload, nil) +func newTestRequest(t *testing.T, method string, handler func(ctx echo.Context) error, payload string, queryParams url.Values, urlParams map[string]string) (rec *httptest.ResponseRecorder, err error) { + rec, c := testRequestSetup(t, method, payload, queryParams, urlParams) err = handler(c) return } diff --git a/pkg/integrations/link_sharing_auth_test.go b/pkg/integrations/link_sharing_auth_test.go new file mode 100644 index 000000000..53d694fba --- /dev/null +++ b/pkg/integrations/link_sharing_auth_test.go @@ -0,0 +1,60 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-2021 Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public Licensee as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public Licensee for more details. +// +// You should have received a copy of the GNU Affero General Public Licensee +// along with this program. If not, see . + +package integrations + +import ( + "net/http" + "testing" + + "code.vikunja.io/api/pkg/models" + apiv1 "code.vikunja.io/api/pkg/routes/api/v1" + "github.com/stretchr/testify/assert" +) + +func TestLinkSharingAuth(t *testing.T) { + t.Run("Without Password", func(t *testing.T) { + rec, err := newTestRequest(t, http.MethodPost, apiv1.AuthenticateLinkShare, ``, nil, map[string]string{"share": "test"}) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), `"token":"`) + assert.Contains(t, rec.Body.String(), `"list_id":1`) + }) + t.Run("Without Password, Password Provided", func(t *testing.T) { + rec, err := newTestRequest(t, http.MethodPost, apiv1.AuthenticateLinkShare, `{"password":"somethingsomething"}`, nil, map[string]string{"share": "test"}) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), `"token":"`) + assert.Contains(t, rec.Body.String(), `"list_id":1`) + }) + t.Run("With Password, No Password Provided", func(t *testing.T) { + _, err := newTestRequest(t, http.MethodPost, apiv1.AuthenticateLinkShare, ``, nil, map[string]string{"share": "testWithPassword"}) + assert.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeLinkSharePasswordRequired) + }) + t.Run("With Password, Password Provided", func(t *testing.T) { + rec, err := newTestRequest(t, http.MethodPost, apiv1.AuthenticateLinkShare, `{"password":"1234"}`, nil, map[string]string{"share": "testWithPassword"}) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), `"token":"`) + assert.Contains(t, rec.Body.String(), `"list_id":1`) + }) + t.Run("With Wrong Password", func(t *testing.T) { + _, err := newTestRequest(t, http.MethodPost, apiv1.AuthenticateLinkShare, `{"password":"someWrongPassword"}`, nil, map[string]string{"share": "testWithPassword"}) + assert.Error(t, err) + assertHandlerErrorCode(t, err, models.ErrCodeLinkSharePasswordInvalid) + }) +} diff --git a/pkg/integrations/login_test.go b/pkg/integrations/login_test.go index 34ad3206c..8d1ffa1a7 100644 --- a/pkg/integrations/login_test.go +++ b/pkg/integrations/login_test.go @@ -30,12 +30,12 @@ func TestLogin(t *testing.T) { rec, err := newTestRequest(t, http.MethodPost, apiv1.Login, `{ "username": "user1", "password": "1234" -}`) +}`, nil, nil) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), "token") }) t.Run("Empty payload", func(t *testing.T) { - _, err := newTestRequest(t, http.MethodPost, apiv1.Login, `{}`) + _, err := newTestRequest(t, http.MethodPost, apiv1.Login, `{}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeNoUsernamePassword) }) @@ -43,7 +43,7 @@ func TestLogin(t *testing.T) { _, err := newTestRequest(t, http.MethodPost, apiv1.Login, `{ "username": "userWichDoesNotExist", "password": "1234" -}`) +}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeWrongUsernameOrPassword) }) @@ -51,7 +51,7 @@ func TestLogin(t *testing.T) { _, err := newTestRequest(t, http.MethodPost, apiv1.Login, `{ "username": "user1", "password": "wrong" -}`) +}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeWrongUsernameOrPassword) }) @@ -59,7 +59,7 @@ func TestLogin(t *testing.T) { _, err := newTestRequest(t, http.MethodPost, apiv1.Login, `{ "username": "user5", "password": "1234" -}`) +}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeEmailNotConfirmed) }) diff --git a/pkg/integrations/register_test.go b/pkg/integrations/register_test.go index 191c51498..774b662a7 100644 --- a/pkg/integrations/register_test.go +++ b/pkg/integrations/register_test.go @@ -31,12 +31,12 @@ func TestRegister(t *testing.T) { "username": "newUser", "password": "1234", "email": "email@example.com" -}`) +}`, nil, nil) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `"username":"newUser"`) }) t.Run("Empty payload", func(t *testing.T) { - _, err := newTestRequest(t, http.MethodPost, apiv1.RegisterUser, `{}`) + _, err := newTestRequest(t, http.MethodPost, apiv1.RegisterUser, `{}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeNoUsernamePassword) }) @@ -45,7 +45,7 @@ func TestRegister(t *testing.T) { "username": "", "password": "1234", "email": "email@example.com" -}`) +}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeNoUsernamePassword) }) @@ -54,7 +54,7 @@ func TestRegister(t *testing.T) { "username": "newUser", "password": "", "email": "email@example.com" -}`) +}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeNoUsernamePassword) }) @@ -63,7 +63,7 @@ func TestRegister(t *testing.T) { "username": "newUser", "password": "1234", "email": "" -}`) +}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeNoUsernamePassword) }) @@ -72,7 +72,7 @@ func TestRegister(t *testing.T) { "username": "user1", "password": "1234", "email": "email@example.com" -}`) +}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrorCodeUsernameExists) }) @@ -81,7 +81,7 @@ func TestRegister(t *testing.T) { "username": "newUser", "password": "1234", "email": "user1@example.com" -}`) +}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrorCodeUserEmailExists) }) diff --git a/pkg/integrations/user_confirm_email_test.go b/pkg/integrations/user_confirm_email_test.go index 00bfd7932..0fb65c019 100644 --- a/pkg/integrations/user_confirm_email_test.go +++ b/pkg/integrations/user_confirm_email_test.go @@ -28,23 +28,23 @@ import ( func TestUserConfirmEmail(t *testing.T) { t.Run("Normal test", func(t *testing.T) { - rec, err := newTestRequest(t, http.MethodPost, apiv1.UserConfirmEmail, `{"token": "tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Ael"}`) + rec, err := newTestRequest(t, http.MethodPost, apiv1.UserConfirmEmail, `{"token": "tiepiQueed8ahc7zeeFe1eveiy4Ein8osooxegiephauph2Ael"}`, nil, nil) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `The email was confirmed successfully.`) }) t.Run("Empty payload", func(t *testing.T) { - _, err := newTestRequest(t, http.MethodPost, apiv1.UserConfirmEmail, `{}`) + _, err := newTestRequest(t, http.MethodPost, apiv1.UserConfirmEmail, `{}`, nil, nil) assert.Error(t, err) assert.Equal(t, http.StatusPreconditionFailed, err.(*echo.HTTPError).Code) assertHandlerErrorCode(t, err, user.ErrCodeInvalidEmailConfirmToken) }) t.Run("Empty token", func(t *testing.T) { - _, err := newTestRequest(t, http.MethodPost, apiv1.UserConfirmEmail, `{"token": ""}`) + _, err := newTestRequest(t, http.MethodPost, apiv1.UserConfirmEmail, `{"token": ""}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeInvalidEmailConfirmToken) }) t.Run("Invalid token", func(t *testing.T) { - _, err := newTestRequest(t, http.MethodPost, apiv1.UserConfirmEmail, `{"token": "invalidToken"}`) + _, err := newTestRequest(t, http.MethodPost, apiv1.UserConfirmEmail, `{"token": "invalidToken"}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeInvalidEmailConfirmToken) }) diff --git a/pkg/integrations/user_password_request_token_test.go b/pkg/integrations/user_password_request_token_test.go index 59ee3a02d..5d5535e45 100644 --- a/pkg/integrations/user_password_request_token_test.go +++ b/pkg/integrations/user_password_request_token_test.go @@ -28,22 +28,22 @@ import ( func TestUserRequestResetPasswordToken(t *testing.T) { t.Run("Normal requesting a password reset token", func(t *testing.T) { - rec, err := newTestRequest(t, http.MethodPost, apiv1.UserRequestResetPasswordToken, `{"email": "user1@example.com"}`) + rec, err := newTestRequest(t, http.MethodPost, apiv1.UserRequestResetPasswordToken, `{"email": "user1@example.com"}`, nil, nil) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `Token was sent.`) }) t.Run("Empty payload", func(t *testing.T) { - _, err := newTestRequest(t, http.MethodPost, apiv1.UserRequestResetPasswordToken, `{}`) + _, err := newTestRequest(t, http.MethodPost, apiv1.UserRequestResetPasswordToken, `{}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeNoUsernamePassword) }) t.Run("Invalid email address", func(t *testing.T) { - _, err := newTestRequest(t, http.MethodPost, apiv1.UserRequestResetPasswordToken, `{"email": "user1example.com"}`) + _, err := newTestRequest(t, http.MethodPost, apiv1.UserRequestResetPasswordToken, `{"email": "user1example.com"}`, nil, nil) assert.Error(t, err) assert.Equal(t, http.StatusBadRequest, err.(*echo.HTTPError).Code) }) t.Run("No user with that email address", func(t *testing.T) { - _, err := newTestRequest(t, http.MethodPost, apiv1.UserRequestResetPasswordToken, `{"email": "user1000@example.com"}`) + _, err := newTestRequest(t, http.MethodPost, apiv1.UserRequestResetPasswordToken, `{"email": "user1000@example.com"}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeUserDoesNotExist) }) diff --git a/pkg/integrations/user_password_reset_test.go b/pkg/integrations/user_password_reset_test.go index ad50a5f14..f3e593d1c 100644 --- a/pkg/integrations/user_password_reset_test.go +++ b/pkg/integrations/user_password_reset_test.go @@ -31,12 +31,12 @@ func TestUserPasswordReset(t *testing.T) { rec, err := newTestRequest(t, http.MethodPost, apiv1.UserResetPassword, `{ "new_password": "1234", "token": "passwordresettesttoken" -}`) +}`, nil, nil) assert.NoError(t, err) assert.Contains(t, rec.Body.String(), `The password was updated successfully.`) }) t.Run("Empty payload", func(t *testing.T) { - _, err := newTestRequest(t, http.MethodPost, apiv1.UserResetPassword, `{}`) + _, err := newTestRequest(t, http.MethodPost, apiv1.UserResetPassword, `{}`, nil, nil) assert.Error(t, err) assert.Equal(t, http.StatusBadRequest, err.(*echo.HTTPError).Code) }) @@ -44,7 +44,7 @@ func TestUserPasswordReset(t *testing.T) { _, err := newTestRequest(t, http.MethodPost, apiv1.UserResetPassword, `{ "new_password": "", "token": "passwordresettesttoken" -}`) +}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeNoUsernamePassword) }) @@ -52,7 +52,7 @@ func TestUserPasswordReset(t *testing.T) { _, err := newTestRequest(t, http.MethodPost, apiv1.UserResetPassword, `{ "new_password": "1234", "token": "invalidtoken" -}`) +}`, nil, nil) assert.Error(t, err) assertHandlerErrorCode(t, err, user.ErrCodeInvalidPasswordResetToken) }) diff --git a/pkg/migration/20210411113105.go b/pkg/migration/20210411113105.go new file mode 100644 index 000000000..96d9ee4cf --- /dev/null +++ b/pkg/migration/20210411113105.go @@ -0,0 +1,50 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-2021 Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public Licensee as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public Licensee for more details. +// +// You should have received a copy of the GNU Affero General Public Licensee +// along with this program. If not, see . + +package migration + +import ( + "src.techknowlogick.com/xormigrate" + "xorm.io/xorm" +) + +type linkShares20210411113105 struct { + Password string `xorm:"text null"` + SharingType int `xorm:"bigint INDEX not null default 0"` +} + +func (linkShares20210411113105) TableName() string { + return "link_shares" +} + +func init() { + migrations = append(migrations, &xormigrate.Migration{ + ID: "20210411113105", + Description: "Add password field to link shares", + Migrate: func(tx *xorm.Engine) error { + + // Make all existing share links type 1 (no password) + if _, err := tx.Update(&linkShares20210411113105{SharingType: 1}); err != nil { + return err + } + + return tx.Sync2(linkShares20210411113105{}) + }, + Rollback: func(tx *xorm.Engine) error { + return nil + }, + }) +} diff --git a/pkg/models/error.go b/pkg/models/error.go index 81e481536..8f97d24c2 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -1512,3 +1512,61 @@ func (err ErrSubscriptionAlreadyExists) HTTPError() web.HTTPError { Message: "You're already subscribed.", } } + +// ================= +// Link Share errors +// ================= + +// ErrLinkSharePasswordRequired represents an error where a link share authentication requires a password and none was provided +type ErrLinkSharePasswordRequired struct { + ShareID int64 +} + +// IsErrLinkSharePasswordRequired checks if an error is ErrLinkSharePasswordRequired. +func IsErrLinkSharePasswordRequired(err error) bool { + _, ok := err.(*ErrLinkSharePasswordRequired) + return ok +} + +func (err *ErrLinkSharePasswordRequired) Error() string { + return fmt.Sprintf("Link Share requires a password for authentication [ShareID: %d]", err.ShareID) +} + +// ErrCodeLinkSharePasswordRequired holds the unique world-error code of this error +const ErrCodeLinkSharePasswordRequired = 13001 + +// HTTPError holds the http error description +func (err ErrLinkSharePasswordRequired) HTTPError() web.HTTPError { + return web.HTTPError{ + HTTPCode: http.StatusPreconditionFailed, + Code: ErrCodeLinkSharePasswordRequired, + Message: "This link share requires a password for authentication, but none was provided.", + } +} + +// ErrLinkSharePasswordInvalid represents an error where a subscription entity type is unknown +type ErrLinkSharePasswordInvalid struct { + ShareID int64 +} + +// IsErrLinkSharePasswordInvalid checks if an error is ErrLinkSharePasswordInvalid. +func IsErrLinkSharePasswordInvalid(err error) bool { + _, ok := err.(*ErrLinkSharePasswordInvalid) + return ok +} + +func (err *ErrLinkSharePasswordInvalid) Error() string { + return fmt.Sprintf("Provided Link Share password did not match the saved one [ShareID: %d]", err.ShareID) +} + +// ErrCodeLinkSharePasswordInvalid holds the unique world-error code of this error +const ErrCodeLinkSharePasswordInvalid = 13002 + +// HTTPError holds the http error description +func (err ErrLinkSharePasswordInvalid) HTTPError() web.HTTPError { + return web.HTTPError{ + HTTPCode: http.StatusForbidden, + Code: ErrCodeLinkSharePasswordInvalid, + Message: "The provided link share password is invalid.", + } +} diff --git a/pkg/models/link_sharing.go b/pkg/models/link_sharing.go index e8835e2d6..73a531e91 100644 --- a/pkg/models/link_sharing.go +++ b/pkg/models/link_sharing.go @@ -17,8 +17,11 @@ package models import ( + "errors" "time" + "golang.org/x/crypto/bcrypt" + "code.vikunja.io/api/pkg/user" "code.vikunja.io/api/pkg/utils" "code.vikunja.io/web" @@ -49,9 +52,12 @@ type LinkSharing struct { // The right this list is shared with. 0 = Read only, 1 = Read & Write, 2 = Admin. See the docs for more details. Right Right `xorm:"bigint INDEX not null default 0" json:"right" valid:"length(0|2)" maximum:"2" default:"0"` - // The kind of this link. 0 = undefined, 1 = without password, 2 = with password (currently not implemented). + // The kind of this link. 0 = undefined, 1 = without password, 2 = with password. SharingType SharingType `xorm:"bigint INDEX not null default 0" json:"sharing_type" valid:"length(0|2)" maximum:"2" default:"0"` + // The password of this link share. You can only set it, not retrieve it after the link share has been created. + Password string `xorm:"text null" json:"password"` + // The user who shared this list SharedBy *user.User `xorm:"-" json:"shared_by"` SharedByID int64 `xorm:"bigint INDEX not null" json:"-"` @@ -129,7 +135,19 @@ func (share *LinkSharing) Create(s *xorm.Session, a web.Auth) (err error) { share.SharedByID = a.GetID() share.Hash = utils.MakeRandomString(40) + + if share.Password != "" { + share.SharingType = SharingTypeWithPassword + share.Password, err = user.HashPassword(share.Password) + if err != nil { + return + } + } else { + share.SharingType = SharingTypeWithoutPassword + } + _, err = s.Insert(share) + share.Password = "" share.SharedBy, _ = user.GetFromAuth(a) return } @@ -156,6 +174,7 @@ func (share *LinkSharing) ReadOne(s *xorm.Session, a web.Auth) (err error) { if !exists { return ErrListShareDoesNotExist{ID: share.ID, Hash: share.Hash} } + share.Password = "" return } @@ -212,6 +231,7 @@ func (share *LinkSharing) ReadAll(s *xorm.Session, a web.Auth, search string, pa for _, s := range shares { s.SharedBy = users[s.SharedByID] + s.Password = "" } // Total count @@ -287,3 +307,20 @@ func GetLinkSharesByIDs(s *xorm.Session, ids []int64) (shares map[int64]*LinkSha err = s.In("id", ids).Find(&shares) return } + +// VerifyLinkSharePassword checks if a password of a link share matches a provided one. +func VerifyLinkSharePassword(share *LinkSharing, password string) (err error) { + if password == "" { + return &ErrLinkSharePasswordRequired{ShareID: share.ID} + } + + err = bcrypt.CompareHashAndPassword([]byte(share.Password), []byte(password)) + if err != nil { + if errors.Is(err, bcrypt.ErrMismatchedHashAndPassword) { + return &ErrLinkSharePasswordInvalid{ShareID: share.ID} + } + return err + } + + return nil +} diff --git a/pkg/models/link_sharing_test.go b/pkg/models/link_sharing_test.go new file mode 100644 index 000000000..1e8917de7 --- /dev/null +++ b/pkg/models/link_sharing_test.go @@ -0,0 +1,140 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-2021 Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public Licensee as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public Licensee for more details. +// +// You should have received a copy of the GNU Affero General Public Licensee +// along with this program. If not, see . + +package models + +import ( + "testing" + + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/user" + "github.com/stretchr/testify/assert" +) + +func TestLinkSharing_Create(t *testing.T) { + doer := &user.User{ID: 1} + + t.Run("normal", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + share := &LinkSharing{ + ListID: 1, + Right: RightRead, + } + err := share.Create(s, doer) + + assert.NoError(t, err) + assert.NotEmpty(t, share.Hash) + assert.NotEmpty(t, share.ID) + assert.Equal(t, SharingTypeWithoutPassword, share.SharingType) + db.AssertExists(t, "link_shares", map[string]interface{}{ + "id": share.ID, + }, false) + }) + t.Run("invalid right", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + share := &LinkSharing{ + ListID: 1, + Right: Right(123), + } + err := share.Create(s, doer) + + assert.Error(t, err) + assert.True(t, IsErrInvalidRight(err)) + }) + t.Run("password should be hashed", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + share := &LinkSharing{ + ListID: 1, + Right: RightRead, + Password: "somePassword", + } + err := share.Create(s, doer) + + assert.NoError(t, err) + assert.NotEmpty(t, share.Hash) + assert.NotEmpty(t, share.ID) + assert.Empty(t, share.Password) + db.AssertExists(t, "link_shares", map[string]interface{}{ + "id": share.ID, + "sharing_type": SharingTypeWithPassword, + }, false) + }) +} + +func TestLinkSharing_ReadAll(t *testing.T) { + doer := &user.User{ID: 1} + + t.Run("all no password", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + share := &LinkSharing{ + ListID: 1, + } + all, _, _, err := share.ReadAll(s, doer, "", 1, -1) + shares := all.([]*LinkSharing) + + assert.NoError(t, err) + assert.Len(t, shares, 2) + for _, sharing := range shares { + assert.Empty(t, sharing.Password) + } + }) +} + +func TestLinkSharing_ReadOne(t *testing.T) { + doer := &user.User{ID: 1} + + t.Run("normal", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + share := &LinkSharing{ + ID: 1, + } + err := share.ReadOne(s, doer) + + assert.NoError(t, err) + assert.NotEmpty(t, share.Hash) + assert.Equal(t, SharingTypeWithoutPassword, share.SharingType) + }) + t.Run("with password", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + s := db.NewSession() + defer s.Close() + + share := &LinkSharing{ + ID: 4, + } + err := share.ReadOne(s, doer) + + assert.NoError(t, err) + assert.NotEmpty(t, share.Hash) + assert.Equal(t, SharingTypeWithPassword, share.SharingType) + assert.Empty(t, share.Password) + }) +} diff --git a/pkg/routes/api/v1/link_sharing_auth.go b/pkg/routes/api/v1/link_sharing_auth.go index 8a009f317..2cc73629c 100644 --- a/pkg/routes/api/v1/link_sharing_auth.go +++ b/pkg/routes/api/v1/link_sharing_auth.go @@ -34,32 +34,44 @@ type LinkShareToken struct { ListID int64 `json:"list_id"` } +// LinkShareAuth represents everything required to authenticate a link share +type LinkShareAuth struct { + Hash string `param:"share" json:"-"` + Password string `json:"password"` +} + // AuthenticateLinkShare gives a jwt auth token for valid share hashes // @Summary Get an auth token for a share // @Description Get a jwt auth token for a shared list from a share hash. // @tags sharing // @Accept json // @Produce json +// @Param password body v1.LinkShareAuth true "The password for link shares which require one." // @Param share path string true "The share hash" // @Success 200 {object} auth.Token "The valid jwt auth token." // @Failure 400 {object} web.HTTPError "Invalid link share object provided." // @Failure 500 {object} models.Message "Internal error" // @Router /shares/{share}/auth [post] func AuthenticateLinkShare(c echo.Context) error { - hash := c.Param("share") + sh := &LinkShareAuth{} + err := c.Bind(sh) + if err != nil { + return handler.HandleHTTPError(err, c) + } s := db.NewSession() defer s.Close() - share, err := models.GetLinkShareByHash(s, hash) + share, err := models.GetLinkShareByHash(s, sh.Hash) if err != nil { - _ = s.Rollback() return handler.HandleHTTPError(err, c) } - if err := s.Commit(); err != nil { - _ = s.Rollback() - return handler.HandleHTTPError(err, c) + if share.SharingType == models.SharingTypeWithPassword { + err := models.VerifyLinkSharePassword(share, sh.Password) + if err != nil { + return handler.HandleHTTPError(err, c) + } } t, err := auth.NewLinkShareJWTAuthtoken(share) @@ -67,6 +79,8 @@ func AuthenticateLinkShare(c echo.Context) error { return handler.HandleHTTPError(err, c) } + share.Password = "" + return c.JSON(http.StatusOK, LinkShareToken{ Token: auth.Token{Token: t}, LinkSharing: share, diff --git a/pkg/swagger/docs.go b/pkg/swagger/docs.go index 6f4005a67..01e0cfe6b 100644 --- a/pkg/swagger/docs.go +++ b/pkg/swagger/docs.go @@ -4141,6 +4141,15 @@ var doc = `{ ], "summary": "Get an auth token for a share", "parameters": [ + { + "description": "The password for link shares which require one.", + "name": "password", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/v1.LinkShareAuth" + } + }, { "type": "string", "description": "The share hash", @@ -7493,6 +7502,10 @@ var doc = `{ "description": "The name of this link share. All actions someone takes while being authenticated with that link will appear with that name.", "type": "string" }, + "password": { + "description": "The password of this link share. You can only set it, not retrieve it after the link share has been created.", + "type": "string" + }, "right": { "description": "The right this list is shared with. 0 = Read only, 1 = Read \u0026 Write, 2 = Admin. See the docs for more details.", "type": "integer", @@ -7504,7 +7517,7 @@ var doc = `{ "$ref": "#/definitions/user.User" }, "sharing_type": { - "description": "The kind of this link. 0 = undefined, 1 = without password, 2 = with password (currently not implemented).", + "description": "The kind of this link. 0 = undefined, 1 = without password, 2 = with password.", "type": "integer", "default": 0, "maximum": 2 @@ -8511,6 +8524,14 @@ var doc = `{ } } }, + "v1.LinkShareAuth": { + "type": "object", + "properties": { + "password": { + "type": "string" + } + } + }, "v1.UserAvatarProvider": { "type": "object", "properties": { diff --git a/pkg/swagger/swagger.json b/pkg/swagger/swagger.json index f5c42c025..de2de1374 100644 --- a/pkg/swagger/swagger.json +++ b/pkg/swagger/swagger.json @@ -4124,6 +4124,15 @@ ], "summary": "Get an auth token for a share", "parameters": [ + { + "description": "The password for link shares which require one.", + "name": "password", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/v1.LinkShareAuth" + } + }, { "type": "string", "description": "The share hash", @@ -7476,6 +7485,10 @@ "description": "The name of this link share. All actions someone takes while being authenticated with that link will appear with that name.", "type": "string" }, + "password": { + "description": "The password of this link share. You can only set it, not retrieve it after the link share has been created.", + "type": "string" + }, "right": { "description": "The right this list is shared with. 0 = Read only, 1 = Read \u0026 Write, 2 = Admin. See the docs for more details.", "type": "integer", @@ -7487,7 +7500,7 @@ "$ref": "#/definitions/user.User" }, "sharing_type": { - "description": "The kind of this link. 0 = undefined, 1 = without password, 2 = with password (currently not implemented).", + "description": "The kind of this link. 0 = undefined, 1 = without password, 2 = with password.", "type": "integer", "default": 0, "maximum": 2 @@ -8494,6 +8507,14 @@ } } }, + "v1.LinkShareAuth": { + "type": "object", + "properties": { + "password": { + "type": "string" + } + } + }, "v1.UserAvatarProvider": { "type": "object", "properties": { diff --git a/pkg/swagger/swagger.yaml b/pkg/swagger/swagger.yaml index 26b9da991..defd34ea7 100644 --- a/pkg/swagger/swagger.yaml +++ b/pkg/swagger/swagger.yaml @@ -313,6 +313,9 @@ definitions: name: description: The name of this link share. All actions someone takes while being authenticated with that link will appear with that name. type: string + password: + description: The password of this link share. You can only set it, not retrieve it after the link share has been created. + type: string right: default: 0 description: The right this list is shared with. 0 = Read only, 1 = Read & Write, 2 = Admin. See the docs for more details. @@ -323,7 +326,7 @@ definitions: description: The user who shared this list sharing_type: default: 0 - description: The kind of this link. 0 = undefined, 1 = without password, 2 = with password (currently not implemented). + description: The kind of this link. 0 = undefined, 1 = without password, 2 = with password. maximum: 2 type: integer updated: @@ -1064,6 +1067,11 @@ definitions: minLength: 1 type: string type: object + v1.LinkShareAuth: + properties: + password: + type: string + type: object v1.UserAvatarProvider: properties: avatar_provider: @@ -3850,6 +3858,12 @@ paths: - application/json description: Get a jwt auth token for a shared list from a share hash. parameters: + - description: The password for link shares which require one. + in: body + name: password + required: true + schema: + $ref: '#/definitions/v1.LinkShareAuth' - description: The share hash in: path name: share diff --git a/pkg/user/user.go b/pkg/user/user.go index de7fd8547..527cf352c 100644 --- a/pkg/user/user.go +++ b/pkg/user/user.go @@ -400,7 +400,7 @@ func UpdateUserPassword(s *xorm.Session, user *User, newPassword string) (err er } // Hash the new password and set it - hashed, err := hashPassword(newPassword) + hashed, err := HashPassword(newPassword) if err != nil { return err } diff --git a/pkg/user/user_create.go b/pkg/user/user_create.go index f3472b271..d9ac03523 100644 --- a/pkg/user/user_create.go +++ b/pkg/user/user_create.go @@ -48,7 +48,7 @@ func CreateUser(s *xorm.Session, user *User) (newUser *User, err error) { if user.Issuer == issuerLocal { // Hash the password - user.Password, err = hashPassword(user.Password) + user.Password, err = HashPassword(user.Password) if err != nil { return nil, err } @@ -98,7 +98,7 @@ func CreateUser(s *xorm.Session, user *User) (newUser *User, err error) { } // HashPassword hashes a password -func hashPassword(password string) (string, error) { +func HashPassword(password string) (string, error) { bytes, err := bcrypt.GenerateFromPassword([]byte(password), 11) return string(bytes), err } diff --git a/pkg/user/user_password_reset.go b/pkg/user/user_password_reset.go index 4860e549a..debdc2837 100644 --- a/pkg/user/user_password_reset.go +++ b/pkg/user/user_password_reset.go @@ -57,7 +57,7 @@ func ResetPassword(s *xorm.Session, reset *PasswordReset) (err error) { } // Hash the password - user.Password, err = hashPassword(reset.NewPassword) + user.Password, err = HashPassword(reset.NewPassword) if err != nil { return }