From 33cceeafa8710362fe3896c8bfd9871c0c006e97 Mon Sep 17 00:00:00 2001 From: jose_d Date: Sun, 12 Apr 2026 18:29:42 +0200 Subject: [PATCH] feat: add ability to revoke passkeys of users as admin (#1386) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jose-d <7630424+jose-d@users.noreply.github.com> Co-authored-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com> Co-authored-by: Elias Schneider --- .../internal/bootstrap/router_bootstrap.go | 2 +- .../controller/audit_log_controller.go | 2 + .../internal/controller/user_controller.go | 61 ++++++++++++++++++- .../controller/webauthn_controller.go | 2 +- backend/internal/dto/audit_log_dto.go | 17 +++--- backend/internal/service/webauthn_service.go | 28 ++++++--- frontend/messages/cs.json | 1 + frontend/messages/en.json | 5 +- frontend/messages/it.json | 1 + .../src/lib/components/audit-log-list.svelte | 6 ++ .../src/lib/components/passkey-row.svelte | 38 ++++++------ frontend/src/lib/services/user-service.ts | 10 +++ frontend/src/lib/types/audit-log.type.ts | 1 + .../settings/admin/users/[id]/+page.svelte | 21 ++++++- .../routes/settings/admin/users/[id]/+page.ts | 8 ++- .../users/[id]/admin-passkey-list.svelte | 57 +++++++++++++++++ tests/specs/user-settings.spec.ts | 45 +++++++++++++- 17 files changed, 265 insertions(+), 40 deletions(-) create mode 100644 frontend/src/routes/settings/admin/users/[id]/admin-passkey-list.svelte diff --git a/backend/internal/bootstrap/router_bootstrap.go b/backend/internal/bootstrap/router_bootstrap.go index 1396e222..213c7d0e 100644 --- a/backend/internal/bootstrap/router_bootstrap.go +++ b/backend/internal/bootstrap/router_bootstrap.go @@ -83,7 +83,7 @@ func initRouter(db *gorm.DB, svc *services) (utils.Service, error) { controller.NewApiKeyController(apiGroup, authMiddleware, svc.apiKeyService) controller.NewWebauthnController(apiGroup, authMiddleware, middleware.NewRateLimitMiddleware(), svc.webauthnService, svc.appConfigService) controller.NewOidcController(apiGroup, authMiddleware, fileSizeLimitMiddleware, svc.oidcService, svc.jwtService) - controller.NewUserController(apiGroup, authMiddleware, middleware.NewRateLimitMiddleware(), svc.userService, svc.oneTimeAccessService, svc.appConfigService) + controller.NewUserController(apiGroup, authMiddleware, middleware.NewRateLimitMiddleware(), svc.userService, svc.oneTimeAccessService, svc.webauthnService, svc.appConfigService) controller.NewAppConfigController(apiGroup, authMiddleware, svc.appConfigService, svc.emailService, svc.ldapService) controller.NewAppImagesController(apiGroup, authMiddleware, svc.appImagesService) controller.NewAuditLogController(apiGroup, svc.auditLogService, authMiddleware) diff --git a/backend/internal/controller/audit_log_controller.go b/backend/internal/controller/audit_log_controller.go index 2126dfb3..3eccac4b 100644 --- a/backend/internal/controller/audit_log_controller.go +++ b/backend/internal/controller/audit_log_controller.go @@ -63,6 +63,7 @@ func (alc *AuditLogController) listAuditLogsForUserHandler(c *gin.Context) { // Add device information to the logs for i, logsDto := range logsDtos { logsDto.Device = alc.auditLogService.DeviceStringFromUserAgent(logs[i].UserAgent) + logsDto.ActorUsername = logsDto.Data["actorUsername"] logsDtos[i] = logsDto } @@ -101,6 +102,7 @@ func (alc *AuditLogController) listAllAuditLogsHandler(c *gin.Context) { for i, logsDto := range logsDtos { logsDto.Device = alc.auditLogService.DeviceStringFromUserAgent(logs[i].UserAgent) logsDto.Username = logs[i].User.Username + logsDto.ActorUsername = logsDto.Data["actorUsername"] logsDtos[i] = logsDto } diff --git a/backend/internal/controller/user_controller.go b/backend/internal/controller/user_controller.go index b38eeabc..38241975 100644 --- a/backend/internal/controller/user_controller.go +++ b/backend/internal/controller/user_controller.go @@ -21,10 +21,11 @@ const defaultOneTimeAccessTokenDuration = 15 * time.Minute // @Summary User management controller // @Description Initializes all user-related API endpoints // @Tags Users -func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMiddleware, rateLimitMiddleware *middleware.RateLimitMiddleware, userService *service.UserService, oneTimeAccessService *service.OneTimeAccessService, appConfigService *service.AppConfigService) { +func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMiddleware, rateLimitMiddleware *middleware.RateLimitMiddleware, userService *service.UserService, oneTimeAccessService *service.OneTimeAccessService, webAuthnService *service.WebAuthnService, appConfigService *service.AppConfigService) { uc := UserController{ userService: userService, oneTimeAccessService: oneTimeAccessService, + webAuthnService: webAuthnService, appConfigService: appConfigService, } @@ -34,8 +35,10 @@ func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMi group.POST("/users", authMiddleware.Add(), uc.createUserHandler) group.PUT("/users/:id", authMiddleware.Add(), uc.updateUserHandler) group.GET("/users/:id/groups", authMiddleware.Add(), uc.getUserGroupsHandler) + group.GET("/users/:id/webauthn-credentials", authMiddleware.Add(), uc.listUserWebauthnCredentialsHandler) group.PUT("/users/me", authMiddleware.WithAdminNotRequired().Add(), uc.updateCurrentUserHandler) group.DELETE("/users/:id", authMiddleware.Add(), uc.deleteUserHandler) + group.DELETE("/users/:id/webauthn-credentials/:credentialId", authMiddleware.Add(), uc.deleteUserWebauthnCredentialHandler) group.PUT("/users/:id/user-groups", authMiddleware.Add(), uc.updateUserGroups) @@ -60,6 +63,7 @@ func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMi type UserController struct { userService *service.UserService oneTimeAccessService *service.OneTimeAccessService + webAuthnService *service.WebAuthnService appConfigService *service.AppConfigService } @@ -87,6 +91,36 @@ func (uc *UserController) getUserGroupsHandler(c *gin.Context) { c.JSON(http.StatusOK, groupsDto) } +// listUserWebauthnCredentialsHandler godoc +// @Summary List user passkeys +// @Description Retrieve all WebAuthn credentials for a specific user +// @Tags Users +// @Param id path string true "User ID" +// @Success 200 {array} dto.WebauthnCredentialDto +// @Router /api/users/{id}/webauthn-credentials [get] +func (uc *UserController) listUserWebauthnCredentialsHandler(c *gin.Context) { + userID := c.Param("id") + + if _, err := uc.userService.GetUser(c.Request.Context(), userID); err != nil { + _ = c.Error(err) + return + } + + credentials, err := uc.webAuthnService.ListCredentials(c.Request.Context(), userID) + if err != nil { + _ = c.Error(err) + return + } + + var credentialDtos []dto.WebauthnCredentialDto + if err := dto.MapStructList(credentials, &credentialDtos); err != nil { + _ = c.Error(err) + return + } + + c.JSON(http.StatusOK, credentialDtos) +} + // listUsersHandler godoc // @Summary List users // @Description Get a paginated list of users with optional search and sorting @@ -181,6 +215,31 @@ func (uc *UserController) deleteUserHandler(c *gin.Context) { c.Status(http.StatusNoContent) } +// deleteUserWebauthnCredentialHandler godoc +// @Summary Delete user passkey +// @Description Delete a specific WebAuthn credential for a user +// @Tags Users +// @Param id path string true "User ID" +// @Param credentialId path string true "Credential ID" +// @Success 204 "No Content" +// @Router /api/users/{id}/webauthn-credentials/{credentialId} [delete] +func (uc *UserController) deleteUserWebauthnCredentialHandler(c *gin.Context) { + err := uc.webAuthnService.DeleteCredential( + c.Request.Context(), + c.Param("id"), + c.Param("credentialId"), + c.ClientIP(), + c.Request.UserAgent(), + c.GetString("userID"), + ) + if err != nil { + _ = c.Error(err) + return + } + + c.Status(http.StatusNoContent) +} + // createUserHandler godoc // @Summary Create user // @Description Create a new user diff --git a/backend/internal/controller/webauthn_controller.go b/backend/internal/controller/webauthn_controller.go index 7dee5602..7fee14be 100644 --- a/backend/internal/controller/webauthn_controller.go +++ b/backend/internal/controller/webauthn_controller.go @@ -137,7 +137,7 @@ func (wc *WebauthnController) deleteCredentialHandler(c *gin.Context) { clientIP := c.ClientIP() userAgent := c.Request.UserAgent() - err := wc.webAuthnService.DeleteCredential(c.Request.Context(), userID, credentialID, clientIP, userAgent) + err := wc.webAuthnService.DeleteCredential(c.Request.Context(), userID, credentialID, clientIP, userAgent, userID) if err != nil { _ = c.Error(err) return diff --git a/backend/internal/dto/audit_log_dto.go b/backend/internal/dto/audit_log_dto.go index 9ef7779d..edef87ff 100644 --- a/backend/internal/dto/audit_log_dto.go +++ b/backend/internal/dto/audit_log_dto.go @@ -8,12 +8,13 @@ type AuditLogDto struct { ID string `json:"id"` CreatedAt datatype.DateTime `json:"createdAt"` - Event string `json:"event"` - IpAddress string `json:"ipAddress"` - Country string `json:"country"` - City string `json:"city"` - Device string `json:"device"` - UserID string `json:"userID"` - Username string `json:"username"` - Data map[string]string `json:"data"` + Event string `json:"event"` + IpAddress string `json:"ipAddress"` + Country string `json:"country"` + City string `json:"city"` + Device string `json:"device"` + UserID string `json:"userID"` + Username string `json:"username"` + ActorUsername string `json:"actorUsername"` + Data map[string]string `json:"data"` } diff --git a/backend/internal/service/webauthn_service.go b/backend/internal/service/webauthn_service.go index 79aa2e5e..1aaa65e3 100644 --- a/backend/internal/service/webauthn_service.go +++ b/backend/internal/service/webauthn_service.go @@ -293,26 +293,40 @@ func (s *WebAuthnService) ListCredentials(ctx context.Context, userID string) ([ return credentials, nil } -func (s *WebAuthnService) DeleteCredential(ctx context.Context, userID string, credentialID string, ipAddress string, userAgent string) error { +func (s *WebAuthnService) DeleteCredential(ctx context.Context, userID string, credentialID string, ipAddress string, userAgent string, actorUserID string) error { tx := s.db.Begin() defer func() { tx.Rollback() }() credential := &model.WebauthnCredential{} - err := tx. + result := tx. WithContext(ctx). Clauses(clause.Returning{}). - Delete(credential, "id = ? AND user_id = ?", credentialID, userID). - Error - if err != nil { - return fmt.Errorf("failed to delete record: %w", err) + Delete(credential, "id = ? AND user_id = ?", credentialID, userID) + if result.Error != nil { + return fmt.Errorf("failed to delete record: %w", result.Error) + } + if result.RowsAffected == 0 { + return gorm.ErrRecordNotFound } auditLogData := model.AuditLogData{"credentialID": hex.EncodeToString(credential.CredentialID), "passkeyName": credential.Name} + if actorUserID != "" && actorUserID != userID { + var actor model.User + err := tx. + WithContext(ctx). + First(&actor, "id = ?", actorUserID). + Error + if err != nil { + return fmt.Errorf("failed to load actor user: %w", err) + } + auditLogData["actorUserID"] = actorUserID + auditLogData["actorUsername"] = actor.Username + } s.auditLogService.Create(ctx, model.AuditLogEventPasskeyRemoved, ipAddress, userAgent, userID, auditLogData, tx) - err = tx.Commit().Error + err := tx.Commit().Error if err != nil { return fmt.Errorf("failed to commit transaction: %w", err) } diff --git a/frontend/messages/cs.json b/frontend/messages/cs.json index 643e230c..0ea73dcf 100644 --- a/frontend/messages/cs.json +++ b/frontend/messages/cs.json @@ -117,6 +117,7 @@ "account_details": "Podrobnosti účtu", "passkeys": "Přístupové klíče", "manage_your_passkeys_that_you_can_use_to_authenticate_yourself": "Spravujte své přístupové klíče, které můžete použít pro ověření.", + "manage_this_users_passkeys": "Přehled a správa přístupových klíčů tohoto uživatele.", "add_passkey": "Přidat přístupový klíč", "create_a_one_time_login_code_to_sign_in_from_a_different_device_without_a_passkey": "Vytvořte jednorázový přihlašovací kód pro přihlášení z jiného zařízení bez přístupového klíče.", "create": "Vytvořit", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index e89938b8..1d958e12 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -106,6 +106,7 @@ "ip_address": "IP Address", "device": "Device", "client": "Client", + "actor": "Actor", "unknown": "Unknown", "account_details_updated_successfully": "Account details updated successfully", "profile_picture_updated_successfully": "Profile picture updated successfully. It may take a few minutes to update.", @@ -117,6 +118,7 @@ "account_details": "Account Details", "passkeys": "Passkeys", "manage_your_passkeys_that_you_can_use_to_authenticate_yourself": "Manage your passkeys that you can use to authenticate yourself.", + "manage_this_users_passkeys": "Manage this user's passkeys.", "add_passkey": "Add Passkey", "create_a_one_time_login_code_to_sign_in_from_a_different_device_without_a_passkey": "Create a one-time login code to sign in from a different device without a passkey.", "create": "Create", @@ -521,5 +523,6 @@ "mark_as_verified": "Mark as verified", "email_verification_sent": "Verification email sent successfully.", "emails_verified_by_default": "Emails verified by default", - "emails_verified_by_default_description": "When enabled, users' email addresses will be marked as verified by default upon signup or when their email address is changed." + "emails_verified_by_default_description": "When enabled, users' email addresses will be marked as verified by default upon signup or when their email address is changed.", + "user_has_no_passkeys_yet": "This user has no passkeys yet." } diff --git a/frontend/messages/it.json b/frontend/messages/it.json index 8d35ba97..5577bace 100644 --- a/frontend/messages/it.json +++ b/frontend/messages/it.json @@ -106,6 +106,7 @@ "ip_address": "Indirizzo IP", "device": "Dispositivo", "client": "Client", + "actor": "Autore", "unknown": "Sconosciuto", "account_details_updated_successfully": "Dettagli dell'account aggiornati con successo", "profile_picture_updated_successfully": "Immagine del profilo aggiornata con successo. Potrebbero essere necessari alcuni minuti per l'aggiornamento.", diff --git a/frontend/src/lib/components/audit-log-list.svelte b/frontend/src/lib/components/audit-log-list.svelte index d555374a..1f33d6d4 100644 --- a/frontend/src/lib/components/audit-log-list.svelte +++ b/frontend/src/lib/components/audit-log-list.svelte @@ -32,6 +32,12 @@ hidden: !isAdmin, value: (item) => item.username ?? m.unknown() }, + { + label: m.actor(), + key: 'actorUsername', + hidden: !isAdmin, + value: (item) => item.actorUsername ?? m.unknown() + }, { label: m.event(), column: 'event', diff --git a/frontend/src/lib/components/passkey-row.svelte b/frontend/src/lib/components/passkey-row.svelte index 7080a898..77dde69c 100644 --- a/frontend/src/lib/components/passkey-row.svelte +++ b/frontend/src/lib/components/passkey-row.svelte @@ -9,12 +9,14 @@ icon, onRename, onDelete, + showRenameAction = true, label, description }: { icon: typeof IconType; - onRename: () => void; + onRename?: () => void; onDelete: () => void; + showRenameAction?: boolean; description?: string; label?: string; } = $props(); @@ -36,22 +38,24 @@ {/if} - - - - - - {m.rename()} - - + {#if showRenameAction && onRename} + + + + + + {m.rename()} + + + {/if} diff --git a/frontend/src/lib/services/user-service.ts b/frontend/src/lib/services/user-service.ts index cbf68241..c7d69274 100644 --- a/frontend/src/lib/services/user-service.ts +++ b/frontend/src/lib/services/user-service.ts @@ -1,5 +1,6 @@ import userStore from '$lib/stores/user-store'; import type { ListRequestOptions, Paginated } from '$lib/types/list-request.type'; +import type { Passkey } from '$lib/types/passkey.type'; import type { SignupToken } from '$lib/types/signup-token.type'; import type { UserGroup } from '$lib/types/user-group.type'; import type { AccountUpdate, User, UserCreate, UserSignUp } from '$lib/types/user.type'; @@ -33,6 +34,11 @@ export default class UserService extends APIService { return res.data as UserGroup[]; }; + listUserPasskeys = async (userId: string) => { + const res = await this.api.get(`/users/${userId}/webauthn-credentials`); + return res.data as Passkey[]; + }; + update = async (id: string, user: UserCreate) => { const res = await this.api.put(`/users/${id}`, user); return res.data as User; @@ -47,6 +53,10 @@ export default class UserService extends APIService { await this.api.delete(`/users/${id}`); }; + removeUserPasskey = async (userId: string, passkeyId: string) => { + await this.api.delete(`/users/${userId}/webauthn-credentials/${passkeyId}`); + }; + updateProfilePicture = async (userId: string, image: File) => { const formData = new FormData(); formData.append('file', image!); diff --git a/frontend/src/lib/types/audit-log.type.ts b/frontend/src/lib/types/audit-log.type.ts index 99358b83..d3524c61 100644 --- a/frontend/src/lib/types/audit-log.type.ts +++ b/frontend/src/lib/types/audit-log.type.ts @@ -7,6 +7,7 @@ export type AuditLog = { device: string; userId: string; username?: string; + actorUsername?: string; createdAt: string; data: any; }; diff --git a/frontend/src/routes/settings/admin/users/[id]/+page.svelte b/frontend/src/routes/settings/admin/users/[id]/+page.svelte index 3a13b971..551dd995 100644 --- a/frontend/src/routes/settings/admin/users/[id]/+page.svelte +++ b/frontend/src/routes/settings/admin/users/[id]/+page.svelte @@ -5,23 +5,27 @@ import Badge from '$lib/components/ui/badge/badge.svelte'; import { Button } from '$lib/components/ui/button'; import * as Card from '$lib/components/ui/card'; + import * as Item from '$lib/components/ui/item/index.js'; import UserGroupSelection from '$lib/components/user-group-selection.svelte'; import { m } from '$lib/paraglide/messages'; import CustomClaimService from '$lib/services/custom-claim-service'; import UserService from '$lib/services/user-service'; import appConfigStore from '$lib/stores/application-configuration-store'; + import type { Passkey } from '$lib/types/passkey.type'; import type { UserCreate } from '$lib/types/user.type'; import { axiosErrorToast } from '$lib/utils/error-util'; - import { LucideChevronLeft } from '@lucide/svelte'; + import { KeyRound, LucideChevronLeft } from '@lucide/svelte'; import { toast } from 'svelte-sonner'; import { backNavigate } from '../navigate-back-util'; import UserForm from '../user-form.svelte'; + import AdminPasskeyList from './admin-passkey-list.svelte'; let { data } = $props(); let user = $state({ ...data.user, userGroupIds: data.user.userGroups.map((g) => g.id) }); + let passkeys: Passkey[] = $state(data.passkeys); const userService = new UserService(); const customClaimService = new CustomClaimService(); @@ -128,6 +132,21 @@ + + + + + + + {m.passkeys()} + {passkeys.length > 0 ? m.manage_this_users_passkeys() : m.user_has_no_passkeys_yet()} + + + {#if passkeys.length > 0} + + {/if} + + { const userService = new UserService(); - const user = await userService.get(params.id); + const [user, passkeys] = await Promise.all([ + userService.get(params.id), + userService.listUserPasskeys(params.id) + ]); return { - user + user, + passkeys }; }; diff --git a/frontend/src/routes/settings/admin/users/[id]/admin-passkey-list.svelte b/frontend/src/routes/settings/admin/users/[id]/admin-passkey-list.svelte new file mode 100644 index 00000000..d160ab89 --- /dev/null +++ b/frontend/src/routes/settings/admin/users/[id]/admin-passkey-list.svelte @@ -0,0 +1,57 @@ + + + + {#each [...passkeys].sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()) as passkey} + deletePasskey(passkey)} + /> + {/each} + diff --git a/tests/specs/user-settings.spec.ts b/tests/specs/user-settings.spec.ts index 42cedd0c..84cd1d87 100644 --- a/tests/specs/user-settings.spec.ts +++ b/tests/specs/user-settings.spec.ts @@ -1,5 +1,6 @@ import test, { expect } from '@playwright/test'; import { userGroups, users } from '../data'; +import authUtil from '../utils/auth.util'; import { cleanupBackend } from '../utils/cleanup.util'; test.beforeEach(async () => await cleanupBackend()); @@ -101,7 +102,7 @@ test('Delete user', async ({ page }) => { .getByRole('button') .click(); await page.getByRole('menuitem', { name: 'Delete' }).click(); - await page.getByRole('button', { name: 'Delete' }).click(); + await page.getByRole('alertdialog').getByRole('button', { name: 'Delete' }).click(); await expect(page.locator('[data-type="success"]')).toHaveText('User deleted successfully'); await expect( @@ -251,3 +252,45 @@ test('Update user group assignments', async ({ page }) => { page.getByRole('row', { name: userGroups.developers.name }).getByRole('checkbox') ).toHaveAttribute('data-state', 'unchecked'); }); + +test('Admin can view another user passkeys', async ({ page }) => { + await page.goto(`/settings/admin/users/${users.craig.id}`); + + await expect(page.getByText('Passkey 2')).toBeVisible(); + await expect(page.getByText(/Added on/)).toBeVisible(); +}); + +test('Admin can delete another user passkey and audit log is created', async ({ page }) => { + await page.goto(`/settings/admin/users/${users.craig.id}`); + + await page.locator('[data-slot="item"]').filter({ hasText: 'Passkey 2' }).getByLabel('Delete').click(); + await page.getByRole('alertdialog').getByRole('button', { name: 'Delete' }).click(); + + await expect(page.locator('[data-type="success"]')).toHaveText('Passkey deleted successfully'); + await expect(page.getByText('Passkey 2')).not.toBeVisible(); + + const auditLogResponse = await page.request.get('/api/audit-logs/all'); + expect(auditLogResponse.ok()).toBeTruthy(); + + const auditLogs = await auditLogResponse.json(); + expect( + auditLogs.data.some( + (log: { event: string; userID: string; data?: { passkeyName?: string } }) => + log.event === 'PASSKEY_REMOVED' && + log.userID === users.craig.id && + log.data?.passkeyName === 'Passkey 2' + ) + ).toBeTruthy(); +}); + +test('Non-admin cannot use admin passkey endpoints', async ({ page }) => { + await authUtil.changeUser(page, 'craig'); + + const listResponse = await page.request.get(`/api/users/${users.tim.id}/webauthn-credentials`); + expect(listResponse.status()).toBe(403); + + const deleteResponse = await page.request.delete( + `/api/users/${users.tim.id}/webauthn-credentials/test-passkey-id` + ); + expect(deleteResponse.status()).toBe(403); +});