Skip to content

Commit

Permalink
Addressing comments
Browse files Browse the repository at this point in the history
  • Loading branch information
diegosperes committed Apr 5, 2024
1 parent fc71db4 commit ea18267
Show file tree
Hide file tree
Showing 14 changed files with 118 additions and 144 deletions.
9 changes: 5 additions & 4 deletions app/models/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ type Account struct {
RequireNewPassword bool `db:"require_new_password"`
PasswordChangedAt time.Time `db:"password_changed_at"`
TOTPSecret sql.NullString `db:"totp_secret"`
LastLoginAt *time.Time `db:"last_login_at"`
CreatedAt time.Time `db:"created_at"`
UpdatedAt time.Time `db:"updated_at"`
DeletedAt *time.Time `db:"deleted_at"`
OauthAccounts []*OauthAccount
LastLoginAt *time.Time `db:"last_login_at"`
CreatedAt time.Time `db:"created_at"`
UpdatedAt time.Time `db:"updated_at"`
DeletedAt *time.Time `db:"deleted_at"`
}

func (a Account) Archived() bool {
Expand Down
42 changes: 42 additions & 0 deletions app/services/account_getter.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,47 @@
package services

import (
"time"

"github.com/keratin/authn-server/app/data"
"github.com/keratin/authn-server/app/models"
"github.com/pkg/errors"
)

func AccountToJson(account *models.Account) map[string]interface{} {
formattedLastLogin := ""
if account.LastLoginAt != nil {
formattedLastLogin = account.LastLoginAt.Format(time.RFC3339)
}

formattedPasswordChangedAt := ""
if !account.PasswordChangedAt.IsZero() {
formattedPasswordChangedAt = account.PasswordChangedAt.Format(time.RFC3339)
}

oAccountsMapped := []map[string]interface{}{}
for _, oAccount := range account.OauthAccounts {
oAccountsMapped = append(
oAccountsMapped,
map[string]interface{}{
"provider": oAccount.Provider,
"provider_account_id": oAccount.ProviderID,
"email": oAccount.Email,
},
)
}

return map[string]interface{}{
"id": account.ID,
"username": account.Username,
"oauth_accounts": oAccountsMapped,
"last_login_at": formattedLastLogin,
"password_changed_at": formattedPasswordChangedAt,
"locked": account.Locked,
"deleted": account.DeletedAt != nil,
}
}

func AccountGetter(store data.AccountStore, accountID int) (*models.Account, error) {
account, err := store.Find(accountID)
if err != nil {
Expand All @@ -15,5 +51,11 @@ func AccountGetter(store data.AccountStore, accountID int) (*models.Account, err
return nil, FieldErrors{{"account", ErrNotFound}}
}

oauthAccounts, err := store.GetOauthAccounts(accountID)
if err != nil {
return nil, errors.Wrap(err, "GetOauthAccounts")
}

account.OauthAccounts = oauthAccounts
return account, nil
}
53 changes: 53 additions & 0 deletions app/services/account_getter_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package services_test

import (
"sort"
"testing"

"github.com/keratin/authn-server/app/data/mock"
"github.com/keratin/authn-server/app/services"
"github.com/stretchr/testify/require"
)

func TestAccountGetter(t *testing.T) {
t.Run("returns empty map when no oauth accounts", func(t *testing.T) {
accountStore := mock.NewAccountStore()
acc, err := accountStore.Create("user@keratin.tech", []byte("password"))
require.NoError(t, err)

account, err := services.AccountGetter(accountStore, acc.ID)
require.NoError(t, err)

require.Equal(t, 0, len(account.OauthAccounts))
})

t.Run("returns oauth accounts for different providers", func(t *testing.T) {
accountStore := mock.NewAccountStore()
acc, err := accountStore.Create("user@keratin.tech", []byte("password"))
require.NoError(t, err)

err = accountStore.AddOauthAccount(acc.ID, "test", "ID1", "email1", "TOKEN1")
require.NoError(t, err)

err = accountStore.AddOauthAccount(acc.ID, "trial", "ID2", "email2", "TOKEN2")
require.NoError(t, err)

account, err := services.AccountGetter(accountStore, acc.ID)
require.NoError(t, err)

oAccounts := account.OauthAccounts

sort.Slice(oAccounts, func(i, j int) bool {
return oAccounts[i].ProviderID < oAccounts[j].ProviderID
})

require.Equal(t, 2, len(oAccounts))
require.Equal(t, "test", oAccounts[0].Provider)
require.Equal(t, "ID1", oAccounts[0].ProviderID)
require.Equal(t, "email1", oAccounts[0].Email)

require.Equal(t, "trial", oAccounts[1].Provider)
require.Equal(t, "ID2", oAccounts[1].ProviderID)
require.Equal(t, "email2", oAccounts[1].Email)
})
}
28 changes: 0 additions & 28 deletions app/services/account_oauth_getter.go

This file was deleted.

59 changes: 0 additions & 59 deletions app/services/account_oauth_getter_test.go

This file was deleted.

1 change: 0 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ services:
- "${REDIS_PORT:-8701}:6379"

mysql:
platform: linux/x86_64
image: mysql:5.7
platform: linux/amd64
ports:
Expand Down
2 changes: 1 addition & 1 deletion docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ If the OAuth process failed, the redirect will have `status=failed` appended to

Visibility: Public

`GET /oauth/info`
`GET /oauth/accounts`

Returns relevant oauth information for the current session.

Expand Down
13 changes: 2 additions & 11 deletions server/handlers/delete_account_oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/gorilla/mux"
"github.com/keratin/authn-server/app"
"github.com/keratin/authn-server/app/services"
"github.com/keratin/authn-server/lib/parse"
)

func DeleteAccountOauth(app *app.App) http.HandlerFunc {
Expand All @@ -27,22 +26,14 @@ func DeleteAccountOauth(app *app.App) http.HandlerFunc {
return "", false
}

provider := mux.Vars(r)["name"]
accountID, err := strconv.Atoi(mux.Vars(r)["id"])
if err != nil {
WriteNotFound(w, "account")
return
}

var payload struct {
OauthProviders []string `json:"oauth_providers"`
}

if err := parse.Payload(r, &payload); err != nil {
WriteErrors(w, err)
return
}

err = services.AccountOauthEnder(app.AccountStore, accountID, payload.OauthProviders)
err = services.AccountOauthEnder(app.AccountStore, accountID, []string{provider})
if err != nil {
app.Logger.WithError(err).Error("AccountOauthEnder")

Expand Down
11 changes: 5 additions & 6 deletions server/handlers/delete_account_oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,16 @@ func TestDeleteAccountOauth(t *testing.T) {
err = app.AccountStore.AddOauthAccount(account.ID, "test", "DELETEDID", "email", "TOKEN")
require.NoError(t, err)

payload := map[string]interface{}{"oauth_providers": []string{"test"}}
res, err := client.DeleteJSON("/accounts/1/oauth", payload)
url := fmt.Sprintf("/accounts/%d/oauth/%s", account.ID, "test")
res, err := client.Delete(url)
require.NoError(t, err)

require.Equal(t, http.StatusOK, res.StatusCode)
require.Equal(t, []byte{}, test.ReadBody(res))
})

t.Run("user does not exist", func(t *testing.T) {
payload := map[string]interface{}{"oauth_providers": []string{"test"}}
res, err := client.DeleteJSON("/accounts/9999/oauth", payload)
res, err := client.Delete("/accounts/9999/oauth/test")
require.NoError(t, err)

require.Equal(t, http.StatusNotFound, res.StatusCode)
Expand All @@ -56,8 +55,8 @@ func TestDeleteAccountOauth(t *testing.T) {
err = app.AccountStore.AddOauthAccount(account.ID, "test", "DELETEDID3", "email", "TOKEN")
require.NoError(t, err)

payload := map[string]interface{}{"oauth_providers": []string{"test"}}
res, err := client.DeleteJSON(fmt.Sprintf("/accounts/%d/oauth", account.ID), payload)
url := fmt.Sprintf("/accounts/%d/oauth/%s", account.ID, "test")
res, err := client.Delete(url)
require.NoError(t, err)

require.Equal(t, http.StatusUnprocessableEntity, res.StatusCode)
Expand Down
27 changes: 1 addition & 26 deletions server/handlers/get_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package handlers
import (
"net/http"
"strconv"
"time"

"github.com/gorilla/mux"
"github.com/keratin/authn-server/app"
Expand All @@ -28,30 +27,6 @@ func GetAccount(app *app.App) http.HandlerFunc {
panic(err)
}

oautAccounts, err := services.AccountOauthGetter(app.AccountStore, id)
if err != nil {
WriteErrors(w, err)
return
}

formattedLastLogin := ""
if account.LastLoginAt != nil {
formattedLastLogin = account.LastLoginAt.Format(time.RFC3339)
}

formattedPasswordChangedAt := ""
if !account.PasswordChangedAt.IsZero() {
formattedPasswordChangedAt = account.PasswordChangedAt.Format(time.RFC3339)
}

WriteData(w, http.StatusOK, map[string]interface{}{
"id": account.ID,
"username": account.Username,
"oauth_accounts": oautAccounts,
"last_login_at": formattedLastLogin,
"password_changed_at": formattedPasswordChangedAt,
"locked": account.Locked,
"deleted": account.DeletedAt != nil,
})
WriteData(w, http.StatusOK, services.AccountToJson(account))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,21 @@ import (
"github.com/keratin/authn-server/server/sessions"
)

func GetOauthInfo(app *app.App) http.HandlerFunc {
func GetOauthAccounts(app *app.App) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
accountID := sessions.GetAccountID(r)
if accountID == 0 {
w.WriteHeader(http.StatusUnauthorized)
return
}

oAccounts, err := services.AccountOauthGetter(app.AccountStore, accountID)
account, err := services.AccountGetter(app.AccountStore, accountID)
if err != nil {
WriteErrors(w, err)
return
}

WriteData(w, http.StatusOK, oAccounts)
accountData := services.AccountToJson(account)
WriteData(w, http.StatusOK, accountData["oauth_accounts"])
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestGetOauthInfo(t *testing.T) {
})

t.Run("unauthorized", func(t *testing.T) {
res, err := client.Get("/oauth/info")
res, err := client.Get("/oauth/accounts")
require.NoError(t, err)

require.Equal(t, http.StatusUnauthorized, res.StatusCode)
Expand All @@ -38,7 +38,7 @@ func TestGetOauthInfo(t *testing.T) {

session := test.CreateSession(app.RefreshTokenStore, app.Config, account.ID)

res, err := client.WithCookie(session).Get("/oauth/info")
res, err := client.WithCookie(session).Get("/oauth/accounts")
require.NoError(t, err)

require.Equal(t, http.StatusOK, res.StatusCode)
Expand Down
2 changes: 1 addition & 1 deletion server/private_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func PrivateRoutes(app *app.App) []*route.HandledRoute {
SecuredWith(authentication).
Handle(handlers.DeleteAccount(app)),

route.Delete("/accounts/{id:[0-9]+}/oauth").
route.Delete("/accounts/{id:[0-9]+}/oauth/{name}").
SecuredWith(authentication).
Handle(handlers.DeleteAccountOauth(app)),
)
Expand Down

0 comments on commit ea18267

Please sign in to comment.