From b52f6db76bf33b74e2b776be3381758f037b63aa Mon Sep 17 00:00:00 2001 From: Frederic Jahn Date: Thu, 26 Jan 2023 11:27:44 +0100 Subject: [PATCH 1/6] feat: add query params to search users --- backend/dto/admin/email.go | 28 +++++++++++++ backend/dto/admin/user.go | 35 ++++++++++++++++ backend/handler/user_admin.go | 58 +++++++++++++++++++++------ backend/persistence/user_persister.go | 44 +++++++++++++++++--- backend/server/admin_router.go | 4 +- 5 files changed, 150 insertions(+), 19 deletions(-) create mode 100644 backend/dto/admin/email.go create mode 100644 backend/dto/admin/user.go diff --git a/backend/dto/admin/email.go b/backend/dto/admin/email.go new file mode 100644 index 000000000..96335279a --- /dev/null +++ b/backend/dto/admin/email.go @@ -0,0 +1,28 @@ +package admin + +import ( + "github.com/gofrs/uuid" + "github.com/teamhanko/hanko/backend/persistence/models" + "time" +) + +type Email struct { + ID uuid.UUID `json:"id"` + Address string `json:"address"` + IsVerified bool `json:"is_verified"` + IsPrimary bool `json:"is_primary"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +// FromEmailModel Converts the DB model to a DTO object +func FromEmailModel(email *models.Email) *Email { + return &Email{ + ID: email.ID, + Address: email.Address, + IsVerified: email.Verified, + IsPrimary: email.IsPrimary(), + CreatedAt: email.CreatedAt, + UpdatedAt: email.UpdatedAt, + } +} diff --git a/backend/dto/admin/user.go b/backend/dto/admin/user.go new file mode 100644 index 000000000..06b675c15 --- /dev/null +++ b/backend/dto/admin/user.go @@ -0,0 +1,35 @@ +package admin + +import ( + "github.com/gofrs/uuid" + "github.com/teamhanko/hanko/backend/dto" + "github.com/teamhanko/hanko/backend/persistence/models" + "time" +) + +type User struct { + ID uuid.UUID `json:"id"` + WebauthnCredentials []dto.WebauthnCredentialResponse `json:"webauthn_credentials,omitempty"` + Emails []Email `json:"emails,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +// FromUserModel Converts the DB model to a DTO object +func FromUserModel(model models.User) User { + credentials := make([]dto.WebauthnCredentialResponse, len(model.WebauthnCredentials)) + for i := range model.WebauthnCredentials { + credentials[i] = *dto.FromWebauthnCredentialModel(&model.WebauthnCredentials[i]) + } + emails := make([]Email, len(model.Emails)) + for i := range model.Emails { + emails[i] = *FromEmailModel(&model.Emails[i]) + } + return User{ + ID: model.ID, + WebauthnCredentials: credentials, + Emails: emails, + CreatedAt: model.CreatedAt, + UpdatedAt: model.UpdatedAt, + } +} diff --git a/backend/handler/user_admin.go b/backend/handler/user_admin.go index f693235a4..d75c7d8da 100644 --- a/backend/handler/user_admin.go +++ b/backend/handler/user_admin.go @@ -5,11 +5,13 @@ import ( "github.com/gofrs/uuid" "github.com/labstack/echo/v4" "github.com/teamhanko/hanko/backend/dto" + "github.com/teamhanko/hanko/backend/dto/admin" "github.com/teamhanko/hanko/backend/pagination" "github.com/teamhanko/hanko/backend/persistence" "net/http" "net/url" "strconv" + "strings" ) type UserHandlerAdmin struct { @@ -41,18 +43,15 @@ func (h *UserHandlerAdmin) Delete(c echo.Context) error { return fmt.Errorf("failed to delete user: %w", err) } - return c.JSON(http.StatusNoContent, nil) -} - -type UserPatchRequest struct { - UserId string `param:"id" validate:"required,uuid4"` - Email string `json:"email" validate:"omitempty,email"` - Verified *bool `json:"verified"` + return c.NoContent(http.StatusNoContent) } type UserListRequest struct { - PerPage int `query:"per_page"` - Page int `query:"page"` + PerPage int `query:"per_page"` + Page int `query:"page"` + Q string `query:"q"` + Email string `query:"email"` + UserId string `query:"user_id"` } func (h *UserHandlerAdmin) List(c echo.Context) error { @@ -70,12 +69,23 @@ func (h *UserHandlerAdmin) List(c echo.Context) error { request.PerPage = 20 } - users, err := h.persister.GetUserPersister().List(request.Page, request.PerPage) + userId := uuid.Nil + if request.UserId != "" { + userId, err = uuid.FromString(request.UserId) + if err != nil { + return dto.NewHTTPError(http.StatusBadRequest, "failed to parse user_id as uuid").SetInternal(err) + } + } + + email := strings.ToLower(request.Email) + q := strings.ToLower(request.Q) + + users, err := h.persister.GetUserPersister().List(request.Page, request.PerPage, q, userId, email) if err != nil { return fmt.Errorf("failed to get list of users: %w", err) } - userCount, err := h.persister.GetUserPersister().Count() + userCount, err := h.persister.GetUserPersister().Count(q, userId, email) if err != nil { return fmt.Errorf("failed to get total count of users: %w", err) } @@ -85,5 +95,29 @@ func (h *UserHandlerAdmin) List(c echo.Context) error { c.Response().Header().Set("Link", pagination.CreateHeader(u, userCount, request.Page, request.PerPage)) c.Response().Header().Set("X-Total-Count", strconv.FormatInt(int64(userCount), 10)) - return c.JSON(http.StatusOK, users) + l := make([]admin.User, len(users)) + for i := range users { + l[i] = admin.FromUserModel(users[i]) + } + + return c.JSON(http.StatusOK, l) +} + +func (h *UserHandlerAdmin) Get(c echo.Context) error { + userId, err := uuid.FromString(c.Param("id")) + if err != nil { + return dto.NewHTTPError(http.StatusBadRequest, "failed to parse userId as uuid").SetInternal(err) + } + + p := h.persister.GetUserPersister() + user, err := p.Get(userId) + if err != nil { + return fmt.Errorf("failed to get user: %w", err) + } + + if user == nil { + return dto.NewHTTPError(http.StatusNotFound, "user not found") + } + + return c.JSON(http.StatusOK, admin.FromUserModel(*user)) } diff --git a/backend/persistence/user_persister.go b/backend/persistence/user_persister.go index d586e829f..cf2c838de 100644 --- a/backend/persistence/user_persister.go +++ b/backend/persistence/user_persister.go @@ -14,8 +14,8 @@ type UserPersister interface { Create(models.User) error Update(models.User) error Delete(models.User) error - List(page int, perPage int) ([]models.User, error) - Count() (int, error) + List(page int, perPage int, q string, userId uuid.UUID, email string) ([]models.User, error) + Count(q string, userId uuid.UUID, email string) (int, error) } type userPersister struct { @@ -87,10 +87,18 @@ func (p *userPersister) Delete(user models.User) error { return nil } -func (p *userPersister) List(page int, perPage int) ([]models.User, error) { +func (p *userPersister) List(page int, perPage int, q string, userId uuid.UUID, email string) ([]models.User, error) { users := []models.User{} - err := p.db.Q().Paginate(page, perPage).All(&users) + query := p.db. + Q(). + EagerPreload("Emails", "Emails.PrimaryEmail", "WebauthnCredentials"). + LeftJoin("emails", "emails.user_id = users.id") + query = p.addQueryParamsToSqlQuery(query, q, userId, email) + err := query.GroupBy("users.id"). + Paginate(page, perPage). + All(&users) + if err != nil && errors.Is(err, sql.ErrNoRows) { return users, nil } @@ -101,11 +109,35 @@ func (p *userPersister) List(page int, perPage int) ([]models.User, error) { return users, nil } -func (p *userPersister) Count() (int, error) { - count, err := p.db.Count(&models.User{}) +func (p *userPersister) Count(q string, userId uuid.UUID, email string) (int, error) { + query := p.db. + Q(). + LeftJoin("emails", "emails.user_id = users.id") + query = p.addQueryParamsToSqlQuery(query, q, userId, email) + count, err := query.GroupBy("users.id"). + Count(&models.User{}) if err != nil { return 0, fmt.Errorf("failed to get user count: %w", err) } return count, nil } + +func (p *userPersister) addQueryParamsToSqlQuery(query *pop.Query, q string, userId uuid.UUID, email string) *pop.Query { + if q != "" { + switch p.db.Dialect.Name() { + case "postgres", "cockroach": + query = query.Where("emails.address LIKE ? OR users.id::text LIKE ?", "%"+q+"%", "%"+q+"%") + case "mysql", "mariadb": + query = query.Where("emails.address LIKE ? OR users.id LIKE ?", "%"+q+"%", "%"+q+"%") + } + } + if email != "" { + query = query.Where("emails.address = ?", email) + } + if !userId.IsNil() { + query = query.Where("users.id = ?", userId) + } + + return query +} diff --git a/backend/server/admin_router.go b/backend/server/admin_router.go index f5baab655..822cde695 100644 --- a/backend/server/admin_router.go +++ b/backend/server/admin_router.go @@ -13,6 +13,7 @@ func NewAdminRouter(persister persistence.Persister) *echo.Echo { e := echo.New() e.HideBanner = true + e.HTTPErrorHandler = dto.NewHTTPErrorHandler(dto.HTTPErrorHandlerConfig{Debug: false, Logger: e.Logger}) e.Use(middleware.RequestID()) e.Use(hankoMiddleware.GetLoggerMiddleware()) @@ -27,8 +28,9 @@ func NewAdminRouter(persister persistence.Persister) *echo.Echo { userHandler := handler.NewUserHandlerAdmin(persister) user := e.Group("/users") - user.DELETE("/:id", userHandler.Delete) user.GET("", userHandler.List) + user.GET("/:id", userHandler.Get) + user.DELETE("/:id", userHandler.Delete) auditLogHandler := handler.NewAuditLogHandler(persister) From 01e67a0c539341759e4a402172e1aee8f2f41ad6 Mon Sep 17 00:00:00 2001 From: Frederic Jahn Date: Thu, 26 Jan 2023 11:31:52 +0100 Subject: [PATCH 2/6] test: fix tests --- backend/test/user_persister.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/test/user_persister.go b/backend/test/user_persister.go index c9c8fb49e..67b10846e 100644 --- a/backend/test/user_persister.go +++ b/backend/test/user_persister.go @@ -53,7 +53,7 @@ func (p *userPersister) Delete(user models.User) error { return nil } -func (p *userPersister) List(page int, perPage int) ([]models.User, error) { +func (p *userPersister) List(page int, perPage int, q string, userId uuid.UUID, email string) ([]models.User, error) { if len(p.users) == 0 { return p.users, nil } @@ -81,6 +81,6 @@ func (p *userPersister) List(page int, perPage int) ([]models.User, error) { return result[page-1], nil } -func (p *userPersister) Count() (int, error) { +func (p *userPersister) Count(q string, userId uuid.UUID, email string) (int, error) { return len(p.users), nil } From dcd42946c9b772158f5f80557b7a4462928304b2 Mon Sep 17 00:00:00 2001 From: Frederic Jahn Date: Thu, 26 Jan 2023 12:04:27 +0100 Subject: [PATCH 3/6] docs: update admin spec --- docs/static/spec/admin.yaml | 151 ++++++++++++++++++++++++++++++++---- 1 file changed, 136 insertions(+), 15 deletions(-) diff --git a/docs/static/spec/admin.yaml b/docs/static/spec/admin.yaml index 3ad9e964f..4fb5f156e 100644 --- a/docs/static/spec/admin.yaml +++ b/docs/static/spec/admin.yaml @@ -1,4 +1,3 @@ - openapi: 3.0.0 info: version: '0.3.0' @@ -31,6 +30,39 @@ paths: operationId: listUsers tags: - User Management + parameters: + - in: query + name: page + schema: + type: integer + default: 1 + description: The page which should be returned + - in: query + name: per_page + schema: + type: integer + default: 20 + description: The number of returned items + - in: query + name: q + schema: + type: string + example: example.com + description: Only users are included when the search string matches values in email or user_id + - in: query + name: user_id + schema: + allOf: + - $ref: '#/components/schemas/UUID4' + example: c339547d-e17d-4ba7-8a1d-b3d5a4d17c1c + description: Only users with the specified user_id are included + - in: query + name: email + schema: + type: string + format: email + example: example@example.com + description: Only users with the specified email are included responses: '200': description: 'Details about users' @@ -49,6 +81,8 @@ paths: $ref: '#/components/headers/Link' '500': $ref: '#/components/responses/InternalServerError' + '400': + $ref: '#/components/responses/BadRequest' /users/{id}: delete: summary: 'Delete a user by ID' @@ -69,6 +103,31 @@ paths: $ref: '#/components/responses/NotFound' '500': $ref: '#/components/responses/InternalServerError' + get: + summary: 'Get a user by ID' + operationId: getUser + tags: + - User Management + parameters: + - name: id + in: path + description: ID of the user + required: false + schema: + $ref: '#/components/schemas/UUID4' + responses: + '200': + description: 'Details about the user' + content: + application/json: + schema: + $ref: '#/components/schemas/User' + '400': + $ref: '#/components/responses/BadRequest' + '404': + $ref: '#/components/responses/NotFound' + '500': + $ref: '#/components/responses/InternalServerError' /audit_logs: get: summary: 'Get a list of audit logs' @@ -191,15 +250,15 @@ components: schemas: User: type: object + required: + - id + - created_at + - updated_at properties: id: description: The ID of the user allOf: - $ref: '#/components/schemas/UUID4' - email: - description: The email address of the user - type: string - format: email created_at: description: Time of creation of the the user type: string @@ -208,20 +267,82 @@ components: description: Time of last update of the user type: string format: date-time - verified: - description: Indicates whether the user's email address was verified - type: boolean webauthn_credentials: description: List of registered Webauthn credentials type: array items: - type: object - properties: - id: - description: The ID of the Webauthn credential - type: string - format: base64url - example: Meprtysj5ZZrTlg0qiLbsZ168OtQMeGVAikVy2n1hvvG... + $ref: '#/components/schemas/WebAuthnCredential' + emails: + description: List of emails associated to the user + type: array + items: + $ref: '#/components/schemas/Email' + WebAuthnCredential: + type: object + required: + - id + - public_key + - attestation_type + - aaguid + - created_at + properties: + id: + description: The ID of the credential + allOf: + - $ref: '#/components/schemas/UUID4' + name: + description: A name that the user choose + type: string + public_key: + description: The public key of the credential + type: string + attestation_type: + description: The attestation type the credential was registered with + type: string + aaguid: + description: The AAGUID of the authenticator the credentials was created on + type: string + transports: + description: The ways the authenticator is connected + type: array + items: + type: string + created_at: + description: Time of creation of the credential + type: string + format: date-time + Email: + type: object + required: + - id + - address + - is_verified + - is_primary + - created_at + - updated_at + properties: + id: + description: The ID of the email + allOf: + - $ref: '#/components/schemas/UUID4' + address: + description: The email address + type: string + format: email + is_verified: + description: Indicated the email has been verified. + type: boolean + is_primary: + description: Indicates it's the primary email address. + type: boolean + created_at: + description: Time of creation of the email + type: string + format: date-time + updated_at: + description: Time of last update of the email + type: string + format: date-time AuditLog: type: object required: From 112602eaefa9f767e260a197a56b0b6c6c15d4fa Mon Sep 17 00:00:00 2001 From: Frederic Jahn Date: Thu, 26 Jan 2023 16:38:03 +0100 Subject: [PATCH 4/6] feat: remove unnecessary query param --- backend/handler/user_admin.go | 6 ++---- backend/persistence/user_persister.go | 24 ++++++++---------------- backend/test/user_persister.go | 4 ++-- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/backend/handler/user_admin.go b/backend/handler/user_admin.go index d75c7d8da..025459ca7 100644 --- a/backend/handler/user_admin.go +++ b/backend/handler/user_admin.go @@ -49,7 +49,6 @@ func (h *UserHandlerAdmin) Delete(c echo.Context) error { type UserListRequest struct { PerPage int `query:"per_page"` Page int `query:"page"` - Q string `query:"q"` Email string `query:"email"` UserId string `query:"user_id"` } @@ -78,14 +77,13 @@ func (h *UserHandlerAdmin) List(c echo.Context) error { } email := strings.ToLower(request.Email) - q := strings.ToLower(request.Q) - users, err := h.persister.GetUserPersister().List(request.Page, request.PerPage, q, userId, email) + users, err := h.persister.GetUserPersister().List(request.Page, request.PerPage, userId, email) if err != nil { return fmt.Errorf("failed to get list of users: %w", err) } - userCount, err := h.persister.GetUserPersister().Count(q, userId, email) + userCount, err := h.persister.GetUserPersister().Count(userId, email) if err != nil { return fmt.Errorf("failed to get total count of users: %w", err) } diff --git a/backend/persistence/user_persister.go b/backend/persistence/user_persister.go index cf2c838de..af8ba0567 100644 --- a/backend/persistence/user_persister.go +++ b/backend/persistence/user_persister.go @@ -14,8 +14,8 @@ type UserPersister interface { Create(models.User) error Update(models.User) error Delete(models.User) error - List(page int, perPage int, q string, userId uuid.UUID, email string) ([]models.User, error) - Count(q string, userId uuid.UUID, email string) (int, error) + List(page int, perPage int, userId uuid.UUID, email string) ([]models.User, error) + Count(userId uuid.UUID, email string) (int, error) } type userPersister struct { @@ -87,14 +87,14 @@ func (p *userPersister) Delete(user models.User) error { return nil } -func (p *userPersister) List(page int, perPage int, q string, userId uuid.UUID, email string) ([]models.User, error) { +func (p *userPersister) List(page int, perPage int, userId uuid.UUID, email string) ([]models.User, error) { users := []models.User{} query := p.db. Q(). EagerPreload("Emails", "Emails.PrimaryEmail", "WebauthnCredentials"). LeftJoin("emails", "emails.user_id = users.id") - query = p.addQueryParamsToSqlQuery(query, q, userId, email) + query = p.addQueryParamsToSqlQuery(query, userId, email) err := query.GroupBy("users.id"). Paginate(page, perPage). All(&users) @@ -109,11 +109,11 @@ func (p *userPersister) List(page int, perPage int, q string, userId uuid.UUID, return users, nil } -func (p *userPersister) Count(q string, userId uuid.UUID, email string) (int, error) { +func (p *userPersister) Count(userId uuid.UUID, email string) (int, error) { query := p.db. Q(). LeftJoin("emails", "emails.user_id = users.id") - query = p.addQueryParamsToSqlQuery(query, q, userId, email) + query = p.addQueryParamsToSqlQuery(query, userId, email) count, err := query.GroupBy("users.id"). Count(&models.User{}) if err != nil { @@ -123,17 +123,9 @@ func (p *userPersister) Count(q string, userId uuid.UUID, email string) (int, er return count, nil } -func (p *userPersister) addQueryParamsToSqlQuery(query *pop.Query, q string, userId uuid.UUID, email string) *pop.Query { - if q != "" { - switch p.db.Dialect.Name() { - case "postgres", "cockroach": - query = query.Where("emails.address LIKE ? OR users.id::text LIKE ?", "%"+q+"%", "%"+q+"%") - case "mysql", "mariadb": - query = query.Where("emails.address LIKE ? OR users.id LIKE ?", "%"+q+"%", "%"+q+"%") - } - } +func (p *userPersister) addQueryParamsToSqlQuery(query *pop.Query, userId uuid.UUID, email string) *pop.Query { if email != "" { - query = query.Where("emails.address = ?", email) + query = query.Where("emails.address LIKE ?", "%"+email+"%") } if !userId.IsNil() { query = query.Where("users.id = ?", userId) diff --git a/backend/test/user_persister.go b/backend/test/user_persister.go index 67b10846e..e4cc6f492 100644 --- a/backend/test/user_persister.go +++ b/backend/test/user_persister.go @@ -53,7 +53,7 @@ func (p *userPersister) Delete(user models.User) error { return nil } -func (p *userPersister) List(page int, perPage int, q string, userId uuid.UUID, email string) ([]models.User, error) { +func (p *userPersister) List(page int, perPage int, userId uuid.UUID, email string) ([]models.User, error) { if len(p.users) == 0 { return p.users, nil } @@ -81,6 +81,6 @@ func (p *userPersister) List(page int, perPage int, q string, userId uuid.UUID, return result[page-1], nil } -func (p *userPersister) Count(q string, userId uuid.UUID, email string) (int, error) { +func (p *userPersister) Count(userId uuid.UUID, email string) (int, error) { return len(p.users), nil } From 7f031eccf045ec5bf72ff216968bac63b2120a6c Mon Sep 17 00:00:00 2001 From: Frederic Jahn Date: Thu, 26 Jan 2023 17:07:17 +0100 Subject: [PATCH 5/6] docs: remove removed query from admin spec --- docs/static/spec/admin.yaml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docs/static/spec/admin.yaml b/docs/static/spec/admin.yaml index 4fb5f156e..b4007d6d5 100644 --- a/docs/static/spec/admin.yaml +++ b/docs/static/spec/admin.yaml @@ -43,12 +43,6 @@ paths: type: integer default: 20 description: The number of returned items - - in: query - name: q - schema: - type: string - example: example.com - description: Only users are included when the search string matches values in email or user_id - in: query name: user_id schema: From 1a4f2bf81beaf59c2eb43311e2574cc1ebc90f17 Mon Sep 17 00:00:00 2001 From: Frederic Jahn Date: Thu, 26 Jan 2023 17:08:26 +0100 Subject: [PATCH 6/6] docs: fix admin spec --- docs/static/spec/admin.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/static/spec/admin.yaml b/docs/static/spec/admin.yaml index b4007d6d5..aa92060a8 100644 --- a/docs/static/spec/admin.yaml +++ b/docs/static/spec/admin.yaml @@ -106,7 +106,7 @@ paths: - name: id in: path description: ID of the user - required: false + required: true schema: $ref: '#/components/schemas/UUID4' responses: