From 4f2e0bb3353e1d73ccf5f7b0437f1344d9262dc6 Mon Sep 17 00:00:00 2001 From: Ahmed Date: Mon, 27 Jan 2025 14:52:12 +0300 Subject: [PATCH 1/6] feat: add username param to AccountGetter --- app/services/account_getter.go | 31 ++++++++++++++++++++---- app/services/account_getter_test.go | 34 ++++++++++++++++++++++++--- app/services/totp_creator.go | 4 +++- app/services/totp_setter.go | 4 +++- server/handlers/get_account.go | 4 +++- server/handlers/get_oauth_accounts.go | 4 +++- 6 files changed, 69 insertions(+), 12 deletions(-) diff --git a/app/services/account_getter.go b/app/services/account_getter.go index e5403ab77..6ba83fb85 100644 --- a/app/services/account_getter.go +++ b/app/services/account_getter.go @@ -6,16 +6,37 @@ import ( "github.com/pkg/errors" ) -func AccountGetter(store data.AccountStore, accountID int) (*models.Account, error) { - account, err := store.Find(accountID) - if err != nil { - return nil, errors.Wrap(err, "Find") +type AccountGetterParams struct { + AccountID *int + Username *string +} + +func AccountGetter(store data.AccountStore, params AccountGetterParams) (*models.Account, error) { + var account *models.Account + + if params.AccountID != nil { + ac, err := store.Find(*params.AccountID) + if err != nil { + return nil, errors.Wrap(err, "Find") + } + + account = ac } + + if params.Username != nil && params.AccountID == nil { + ac, err := store.FindByUsername(*params.Username) + if err != nil { + return nil, errors.Wrap(err, "FindByUsername") + } + + account = ac + } + if account == nil { return nil, FieldErrors{{"account", ErrNotFound}} } - oauthAccounts, err := store.GetOauthAccounts(accountID) + oauthAccounts, err := store.GetOauthAccounts(account.ID) if err != nil { return nil, errors.Wrap(err, "GetOauthAccounts") } diff --git a/app/services/account_getter_test.go b/app/services/account_getter_test.go index 8bec14a33..3d18f287d 100644 --- a/app/services/account_getter_test.go +++ b/app/services/account_getter_test.go @@ -11,9 +11,29 @@ import ( func TestAccountGetter(t *testing.T) { + t.Run("get username", func(t *testing.T) { + accountStore := mock.NewAccountStore() + acc, err := accountStore.Create("user@keratin.tech", []byte("password")) + require.NoError(t, err) + + accountID := acc.ID + + account, err := services.AccountGetter(accountStore, services.AccountGetterParams{ + Username: &acc.Username, + }) + require.NoError(t, err) + + require.Equal(t, accountID, account.ID) + }) + t.Run("get non existing account", func(t *testing.T) { accountStore := mock.NewAccountStore() - account, err := services.AccountGetter(accountStore, 9999) + + accountID := 9999 + + account, err := services.AccountGetter(accountStore, services.AccountGetterParams{ + AccountID: &accountID, + }) require.NotNil(t, err) require.Nil(t, account) @@ -24,7 +44,11 @@ func TestAccountGetter(t *testing.T) { acc, err := accountStore.Create("user@keratin.tech", []byte("password")) require.NoError(t, err) - account, err := services.AccountGetter(accountStore, acc.ID) + accountID := acc.ID + + account, err := services.AccountGetter(accountStore, services.AccountGetterParams{ + AccountID: &accountID, + }) require.NoError(t, err) require.Equal(t, 0, len(account.OauthAccounts)) @@ -41,7 +65,11 @@ func TestAccountGetter(t *testing.T) { err = accountStore.AddOauthAccount(acc.ID, "trial", "ID2", "email2", "TOKEN2") require.NoError(t, err) - account, err := services.AccountGetter(accountStore, acc.ID) + accountID := acc.ID + + account, err := services.AccountGetter(accountStore, services.AccountGetterParams{ + AccountID: &accountID, + }) require.NoError(t, err) oAccounts := account.OauthAccounts diff --git a/app/services/totp_creator.go b/app/services/totp_creator.go index cdc1fa4d9..f0ce99243 100644 --- a/app/services/totp_creator.go +++ b/app/services/totp_creator.go @@ -12,7 +12,9 @@ var ErrExistingTOTPSecret = errors.New("a OTP secret has already been establishe // TOTPCreator handles the creation and storage of new OTP tokens func TOTPCreator(accountStore data.AccountStore, totpCache data.TOTPCache, accountID int, audience *route.Domain) (*otp.Key, error) { - account, err := AccountGetter(accountStore, accountID) + account, err := AccountGetter(accountStore, AccountGetterParams{ + AccountID: &accountID, + }) if err != nil { return nil, err } diff --git a/app/services/totp_setter.go b/app/services/totp_setter.go index c7eac8f52..7e6aac1e5 100644 --- a/app/services/totp_setter.go +++ b/app/services/totp_setter.go @@ -14,7 +14,9 @@ func TOTPSetter(accountStore data.AccountStore, totpCache data.TOTPCache, cfg *a return FieldErrors{{"otp", ErrInvalidOrExpired}} } - account, err := AccountGetter(accountStore, accountID) + account, err := AccountGetter(accountStore, AccountGetterParams{ + AccountID: &accountID, + }) if err != nil { return err } diff --git a/server/handlers/get_account.go b/server/handlers/get_account.go index a47c87786..0880f3ac2 100644 --- a/server/handlers/get_account.go +++ b/server/handlers/get_account.go @@ -17,7 +17,9 @@ func GetAccount(app *app.App) http.HandlerFunc { return } - account, err := services.AccountGetter(app.AccountStore, id) + account, err := services.AccountGetter(app.AccountStore, services.AccountGetterParams{ + AccountID: &id, + }) if err != nil { if _, ok := err.(services.FieldErrors); ok { WriteNotFound(w, "account") diff --git a/server/handlers/get_oauth_accounts.go b/server/handlers/get_oauth_accounts.go index 31eead8e8..741afb48c 100644 --- a/server/handlers/get_oauth_accounts.go +++ b/server/handlers/get_oauth_accounts.go @@ -16,7 +16,9 @@ func GetOauthAccounts(app *app.App) http.HandlerFunc { return } - account, err := services.AccountGetter(app.AccountStore, accountID) + account, err := services.AccountGetter(app.AccountStore, services.AccountGetterParams{ + AccountID: &accountID, + }) if err != nil { WriteErrors(w, err) return From b02a7117bdc8078115a116b0b793dece43d39b59 Mon Sep 17 00:00:00 2001 From: Ahmed Date: Mon, 27 Jan 2025 15:00:25 +0300 Subject: [PATCH 2/6] feat: use username param in get account --- server/handlers/get_account.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/server/handlers/get_account.go b/server/handlers/get_account.go index 0880f3ac2..02844f8c1 100644 --- a/server/handlers/get_account.go +++ b/server/handlers/get_account.go @@ -11,14 +11,27 @@ import ( func GetAccount(app *app.App) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + var paramID *int + var paramUsername *string + id, err := strconv.Atoi(mux.Vars(r)["id"]) - if err != nil { + if err == nil { + paramID = &id + } + + username := mux.Vars(r)["username"] + if username != "" { + paramUsername = &username + } + + if paramID == nil && paramUsername == nil { WriteNotFound(w, "account") return } account, err := services.AccountGetter(app.AccountStore, services.AccountGetterParams{ - AccountID: &id, + AccountID: paramID, + Username: paramUsername, }) if err != nil { if _, ok := err.(services.FieldErrors); ok { From 457bd6f8bacce7dfcbab6fe334ffa37d3b7cf2f2 Mon Sep 17 00:00:00 2001 From: Ahmed Date: Mon, 27 Jan 2025 16:41:03 +0300 Subject: [PATCH 3/6] fix: account route for username --- server/handlers/get_account.go | 20 ++++++++------------ server/private_routes.go | 2 +- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/server/handlers/get_account.go b/server/handlers/get_account.go index 02844f8c1..9aa74260f 100644 --- a/server/handlers/get_account.go +++ b/server/handlers/get_account.go @@ -14,19 +14,16 @@ func GetAccount(app *app.App) http.HandlerFunc { var paramID *int var paramUsername *string - id, err := strconv.Atoi(mux.Vars(r)["id"]) - if err == nil { - paramID = &id - } - - username := mux.Vars(r)["username"] - if username != "" { - paramUsername = &username + idOrUsername := mux.Vars(r)["id"] + if idOrUsername == "" { + return } - if paramID == nil && paramUsername == nil { - WriteNotFound(w, "account") - return + id, err := strconv.Atoi(idOrUsername) + if err != nil { + paramUsername = &idOrUsername + } else { + paramID = &id } account, err := services.AccountGetter(app.AccountStore, services.AccountGetterParams{ @@ -35,7 +32,6 @@ func GetAccount(app *app.App) http.HandlerFunc { }) if err != nil { if _, ok := err.(services.FieldErrors); ok { - WriteNotFound(w, "account") return } diff --git a/server/private_routes.go b/server/private_routes.go index f394b359b..aa24024e1 100644 --- a/server/private_routes.go +++ b/server/private_routes.go @@ -32,7 +32,7 @@ func PrivateRoutes(app *app.App) []*route.HandledRoute { SecuredWith(authentication). Handle(handlers.PostAccountsImport(app)), - route.Get("/accounts/{id:[0-9]+}"). + route.Get("/accounts/{id}"). SecuredWith(authentication). Handle(handlers.GetAccount(app)), From 927c32d2ad3a4bd23bc7c68ed1d10371f99998f8 Mon Sep 17 00:00:00 2001 From: Ahmed Date: Mon, 27 Jan 2025 16:41:12 +0300 Subject: [PATCH 4/6] test: add test for get account route --- server/handlers/get_account_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/server/handlers/get_account_test.go b/server/handlers/get_account_test.go index cf0668eec..247e7ed97 100644 --- a/server/handlers/get_account_test.go +++ b/server/handlers/get_account_test.go @@ -3,6 +3,7 @@ package handlers_test import ( "fmt" "net/http" + "net/url" "sort" "testing" "time" @@ -52,6 +53,28 @@ func TestGetAccount(t *testing.T) { assertGetAccountResponse(t, res, account, oauthAccounts) }) + + t.Run("valid account username", func(t *testing.T) { + account, err := app.AccountStore.Create("unlocked@test.com", []byte("bar")) + require.NoError(t, err) + + err = app.AccountStore.AddOauthAccount(account.ID, "test", "ID1", "email", "TOKEN1") + require.NoError(t, err) + + err = app.AccountStore.AddOauthAccount(account.ID, "trial", "ID2", "email", "TOKEN2") + require.NoError(t, err) + + oauthAccounts, err := app.AccountStore.GetOauthAccounts(account.ID) + require.NoError(t, err) + + username := url.QueryEscape(account.Username) + fmt.Println(username) + res, err := client.Get(fmt.Sprintf("/accounts/%v", username)) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + + assertGetAccountResponse(t, res, account, oauthAccounts) + }) } func assertGetAccountResponse(t *testing.T, res *http.Response, acc *models.Account, oAccs []*models.OauthAccount) { From 354c64792ac4bf8e4eaf0a7d31571dc8599de347 Mon Sep 17 00:00:00 2001 From: Ahmed Date: Mon, 27 Jan 2025 17:07:41 +0300 Subject: [PATCH 5/6] test: remove encoding for @ in test file --- server/handlers/get_account_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server/handlers/get_account_test.go b/server/handlers/get_account_test.go index 247e7ed97..9e52dea38 100644 --- a/server/handlers/get_account_test.go +++ b/server/handlers/get_account_test.go @@ -3,7 +3,6 @@ package handlers_test import ( "fmt" "net/http" - "net/url" "sort" "testing" "time" @@ -67,9 +66,7 @@ func TestGetAccount(t *testing.T) { oauthAccounts, err := app.AccountStore.GetOauthAccounts(account.ID) require.NoError(t, err) - username := url.QueryEscape(account.Username) - fmt.Println(username) - res, err := client.Get(fmt.Sprintf("/accounts/%v", username)) + res, err := client.Get(fmt.Sprintf("/accounts/%v", account.Username)) require.NoError(t, err) assert.Equal(t, http.StatusOK, res.StatusCode) From 14d9b9981baf23b43339790353a89d6d4c57ba9d Mon Sep 17 00:00:00 2001 From: Ahmed Date: Wed, 5 Feb 2025 13:50:43 +0300 Subject: [PATCH 6/6] fix: re-add not found error --- server/handlers/get_account.go | 1 + 1 file changed, 1 insertion(+) diff --git a/server/handlers/get_account.go b/server/handlers/get_account.go index 9aa74260f..b41599f10 100644 --- a/server/handlers/get_account.go +++ b/server/handlers/get_account.go @@ -32,6 +32,7 @@ func GetAccount(app *app.App) http.HandlerFunc { }) if err != nil { if _, ok := err.(services.FieldErrors); ok { + WriteNotFound(w, "account") return }