From 287314f01644e42ddb2ce1b1115bd14f2f0c1768 Mon Sep 17 00:00:00 2001 From: Elias Schneider Date: Fri, 27 Jun 2025 23:38:02 +0200 Subject: [PATCH] feat: improve initial admin creation workflow --- .../internal/controller/e2etest_controller.go | 9 ++- .../internal/controller/user_controller.go | 23 ++++-- backend/internal/service/user_service.go | 22 +++--- frontend/messages/en.json | 7 +- frontend/src/lib/services/user-service.ts | 5 ++ frontend/src/lib/utils/redirection-util.ts | 2 +- frontend/src/routes/login/setup/+page.svelte | 49 ------------- frontend/src/routes/setup/+page.ts | 5 ++ .../routes/signup/add-passkey/+page.svelte | 22 ++++-- frontend/src/routes/signup/setup/+page.svelte | 70 +++++++++++++++++++ tests/specs/account-settings.spec.ts | 2 +- tests/specs/application-configuration.spec.ts | 2 +- tests/specs/ldap.spec.ts | 2 +- tests/specs/oidc-client-settings.spec.ts | 2 +- tests/specs/oidc.spec.ts | 15 +--- tests/specs/one-time-access-token.spec.ts | 2 +- tests/specs/user-group.spec.ts | 2 +- tests/specs/user-settings.spec.ts | 2 +- tests/specs/user-signup.spec.ts | 37 +++++++++- tests/utils/cleanup.util.ts | 8 ++- 20 files changed, 180 insertions(+), 108 deletions(-) delete mode 100644 frontend/src/routes/login/setup/+page.svelte create mode 100644 frontend/src/routes/setup/+page.ts create mode 100644 frontend/src/routes/signup/setup/+page.svelte diff --git a/backend/internal/controller/e2etest_controller.go b/backend/internal/controller/e2etest_controller.go index 801f6c40..4e5e791c 100644 --- a/backend/internal/controller/e2etest_controller.go +++ b/backend/internal/controller/e2etest_controller.go @@ -33,6 +33,7 @@ func (tc *TestController) resetAndSeedHandler(c *gin.Context) { } skipLdap := c.Query("skip-ldap") == "true" + skipSeed := c.Query("skip-seed") == "true" if err := tc.TestService.ResetDatabase(); err != nil { _ = c.Error(err) @@ -44,9 +45,11 @@ func (tc *TestController) resetAndSeedHandler(c *gin.Context) { return } - if err := tc.TestService.SeedDatabase(baseURL); err != nil { - _ = c.Error(err) - return + if !skipSeed { + if err := tc.TestService.SeedDatabase(baseURL); err != nil { + _ = c.Error(err) + return + } } if err := tc.TestService.ResetAppConfig(c.Request.Context()); err != nil { diff --git a/backend/internal/controller/user_controller.go b/backend/internal/controller/user_controller.go index 5df0e9d2..311bf0e9 100644 --- a/backend/internal/controller/user_controller.go +++ b/backend/internal/controller/user_controller.go @@ -44,7 +44,6 @@ func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMi group.POST("/users/:id/one-time-access-token", authMiddleware.Add(), uc.createAdminOneTimeAccessTokenHandler) group.POST("/users/:id/one-time-access-email", authMiddleware.Add(), uc.RequestOneTimeAccessEmailAsAdminHandler) group.POST("/one-time-access-token/:token", rateLimitMiddleware.Add(rate.Every(10*time.Second), 5), uc.exchangeOneTimeAccessTokenHandler) - group.POST("/one-time-access-token/setup", uc.getSetupAccessTokenHandler) group.POST("/one-time-access-email", rateLimitMiddleware.Add(rate.Every(10*time.Minute), 3), uc.RequestOneTimeAccessEmailAsUnauthenticatedUserHandler) group.DELETE("/users/:id/profile-picture", authMiddleware.Add(), uc.resetUserProfilePictureHandler) @@ -54,6 +53,7 @@ func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMi group.GET("/signup-tokens", authMiddleware.Add(), uc.listSignupTokensHandler) group.DELETE("/signup-tokens/:id", authMiddleware.Add(), uc.deleteSignupTokenHandler) group.POST("/signup", rateLimitMiddleware.Add(rate.Every(1*time.Minute), 10), uc.signupHandler) + group.POST("/signup/setup", uc.signUpInitialAdmin) } @@ -446,14 +446,23 @@ func (uc *UserController) exchangeOneTimeAccessTokenHandler(c *gin.Context) { c.JSON(http.StatusOK, userDto) } -// getSetupAccessTokenHandler godoc -// @Summary Setup initial admin -// @Description Generate setup access token for initial admin user configuration +// signUpInitialAdmin godoc +// @Summary Sign up initial admin user +// @Description Sign up and generate setup access token for initial admin user // @Tags Users +// @Accept json +// @Produce json +// @Param body body dto.SignUpDto true "User information" // @Success 200 {object} dto.UserDto -// @Router /api/one-time-access-token/setup [post] -func (uc *UserController) getSetupAccessTokenHandler(c *gin.Context) { - user, token, err := uc.userService.SetupInitialAdmin(c.Request.Context()) +// @Router /api/signup/setup [post] +func (uc *UserController) signUpInitialAdmin(c *gin.Context) { + var input dto.SignUpDto + if err := c.ShouldBindJSON(&input); err != nil { + _ = c.Error(err) + return + } + + user, token, err := uc.userService.SignUpInitialAdmin(c.Request.Context(), input) if err != nil { _ = c.Error(err) return diff --git a/backend/internal/service/user_service.go b/backend/internal/service/user_service.go index e0b261b3..e3f52a0b 100644 --- a/backend/internal/service/user_service.go +++ b/backend/internal/service/user_service.go @@ -529,7 +529,7 @@ func (s *UserService) UpdateUserGroups(ctx context.Context, id string, userGroup return user, nil } -func (s *UserService) SetupInitialAdmin(ctx context.Context) (model.User, string, error) { +func (s *UserService) SignUpInitialAdmin(ctx context.Context, signUpData dto.SignUpDto) (model.User, string, error) { tx := s.db.Begin() defer func() { tx.Rollback() @@ -539,25 +539,19 @@ func (s *UserService) SetupInitialAdmin(ctx context.Context) (model.User, string if err := tx.WithContext(ctx).Model(&model.User{}).Count(&userCount).Error; err != nil { return model.User{}, "", err } - if userCount > 1 { + if userCount != 0 { return model.User{}, "", &common.SetupAlreadyCompletedError{} } - user := model.User{ - FirstName: "Admin", - LastName: "Admin", - Username: "admin", - Email: "admin@admin.com", + userToCreate := dto.UserCreateDto{ + FirstName: signUpData.FirstName, + LastName: signUpData.LastName, + Username: signUpData.Username, + Email: signUpData.Email, IsAdmin: true, } - if err := tx.WithContext(ctx).Model(&model.User{}).Preload("Credentials").FirstOrCreate(&user).Error; err != nil { - return model.User{}, "", err - } - - if len(user.Credentials) > 0 { - return model.User{}, "", &common.SetupAlreadyCompletedError{} - } + user, err := s.createUserInternal(ctx, userToCreate, false, tx) token, err := s.jwtService.GenerateAccessToken(user) if err != nil { diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 3629e91b..4337bf38 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -67,9 +67,7 @@ "please_try_to_sign_in_again": "Please try to sign in again.", "authenticate_with_passkey_to_access_account": "Authenticate yourself with your passkey to access your account.", "authenticate": "Authenticate", - "appname_setup": "{appName} Setup", "please_try_again": "Please try again.", - "you_are_about_to_sign_in_to_the_initial_admin_account": "You're about to sign in to the initial admin account. Anyone with this link can access the account until a passkey is added. Please set up a passkey as soon as possible to prevent unauthorized access.", "continue": "Continue", "alternative_sign_in": "Alternative Sign In", "if_you_do_not_have_access_to_your_passkey_you_can_sign_in_using_one_of_the_following_methods": "If you don't have access to your passkey, you can sign in using one of the following methods.", @@ -391,6 +389,7 @@ "go_to_login": "Go to login", "signup_to_appname": "Sign Up to {appName}", "create_your_account_to_get_started": "Create your account to get started.", + "initial_account_creation_description": "Please create your account to get started. You will be able to set up a passkey later.", "setup_your_passkey": "Set up your passkey", "create_a_passkey_to_securely_access_your_account": "Create a passkey to securely access your account. This will be your primary way to sign in.", "skip_for_now": "Skip for now", @@ -417,5 +416,7 @@ "signup_with_token_description": "Users can only sign up using a valid signup token created by an administrator.", "signup_open": "Open Signup", "signup_open_description": "Anyone can create a new account without restrictions.", - "of": "of" + "of": "of", + "skip_passkey_setup": "Skip Passkey Setup", + "skip_passkey_setup_description": "It's highly recommended to set up a passkey because without one, you will be locked out of your account as soon as the session expires." } diff --git a/frontend/src/lib/services/user-service.ts b/frontend/src/lib/services/user-service.ts index eae55f08..3ac705df 100644 --- a/frontend/src/lib/services/user-service.ts +++ b/frontend/src/lib/services/user-service.ts @@ -114,6 +114,11 @@ export default class UserService extends APIService { return res.data as User; } + async signupInitialUser(data: UserSignUp) { + const res = await this.api.post(`/signup/setup`, data); + return res.data as User; + } + async listSignupTokens(options?: SearchPaginationSortRequest) { const res = await this.api.get('/signup-tokens', { params: options diff --git a/frontend/src/lib/utils/redirection-util.ts b/frontend/src/lib/utils/redirection-util.ts index 072c4101..4d411cc6 100644 --- a/frontend/src/lib/utils/redirection-util.ts +++ b/frontend/src/lib/utils/redirection-util.ts @@ -12,7 +12,7 @@ export function getAuthRedirectPath(path: string, user: User | null) { path == '/lc' || path.startsWith('/lc/') || path == '/signup' || - path.startsWith('/signup/') || + path == '/signup/setup' || path.startsWith('/st/'); const isPublicPath = ['/authorize', '/device', '/health', '/healthz'].includes(path); const isAdminPath = path == '/settings/admin' || path.startsWith('/settings/admin/'); diff --git a/frontend/src/routes/login/setup/+page.svelte b/frontend/src/routes/login/setup/+page.svelte deleted file mode 100644 index 99be9119..00000000 --- a/frontend/src/routes/login/setup/+page.svelte +++ /dev/null @@ -1,49 +0,0 @@ - - - -
- -
-

- {m.appname_setup({ appName: $appConfigStore.appName })} -

- {#if error} -

- {error}. {m.please_try_again()} -

- {:else} -

- {m.you_are_about_to_sign_in_to_the_initial_admin_account()} -

- - {/if} - diff --git a/frontend/src/routes/setup/+page.ts b/frontend/src/routes/setup/+page.ts new file mode 100644 index 00000000..903cd74a --- /dev/null +++ b/frontend/src/routes/setup/+page.ts @@ -0,0 +1,5 @@ +import { redirect } from '@sveltejs/kit'; +import type { PageLoad } from './$types'; + +// Alias for /signup/setup +export const load: PageLoad = async () => redirect(307, '/signup/setup'); diff --git a/frontend/src/routes/signup/add-passkey/+page.svelte b/frontend/src/routes/signup/add-passkey/+page.svelte index e9d04eb6..f66b96f7 100644 --- a/frontend/src/routes/signup/add-passkey/+page.svelte +++ b/frontend/src/routes/signup/add-passkey/+page.svelte @@ -1,5 +1,6 @@ @@ -66,12 +81,7 @@ {/if}

- +
+ diff --git a/tests/specs/account-settings.spec.ts b/tests/specs/account-settings.spec.ts index 0c6fd25a..b851633e 100644 --- a/tests/specs/account-settings.spec.ts +++ b/tests/specs/account-settings.spec.ts @@ -4,7 +4,7 @@ import authUtil from '../utils/auth.util'; import { cleanupBackend } from '../utils/cleanup.util'; import passkeyUtil from '../utils/passkey.util'; -test.beforeEach(cleanupBackend); +test.beforeEach(() => cleanupBackend()); test('Update account details', async ({ page }) => { await page.goto('/settings/account'); diff --git a/tests/specs/application-configuration.spec.ts b/tests/specs/application-configuration.spec.ts index f8c828b4..0edc6831 100644 --- a/tests/specs/application-configuration.spec.ts +++ b/tests/specs/application-configuration.spec.ts @@ -1,7 +1,7 @@ import test, { expect } from '@playwright/test'; import { cleanupBackend } from '../utils/cleanup.util'; -test.beforeEach(cleanupBackend); +test.beforeEach(() => cleanupBackend()); test('Update general configuration', async ({ page }) => { await page.goto('/settings/admin/application-configuration'); diff --git a/tests/specs/ldap.spec.ts b/tests/specs/ldap.spec.ts index 0925ecd3..2fa30127 100644 --- a/tests/specs/ldap.spec.ts +++ b/tests/specs/ldap.spec.ts @@ -1,7 +1,7 @@ import test, { expect } from '@playwright/test'; import { cleanupBackend } from '../utils/cleanup.util'; -test.beforeEach(cleanupBackend); +test.beforeEach(() => cleanupBackend()); test.describe('LDAP Integration', () => { test.skip( diff --git a/tests/specs/oidc-client-settings.spec.ts b/tests/specs/oidc-client-settings.spec.ts index 4bbff886..0ff6c4e5 100644 --- a/tests/specs/oidc-client-settings.spec.ts +++ b/tests/specs/oidc-client-settings.spec.ts @@ -2,7 +2,7 @@ import test, { expect } from '@playwright/test'; import { oidcClients } from '../data'; import { cleanupBackend } from '../utils/cleanup.util'; -test.beforeEach(cleanupBackend); +test.beforeEach(() => cleanupBackend()); test('Create OIDC client', async ({ page }) => { await page.goto('/settings/admin/oidc-clients'); diff --git a/tests/specs/oidc.spec.ts b/tests/specs/oidc.spec.ts index 6cb958b8..3a584327 100644 --- a/tests/specs/oidc.spec.ts +++ b/tests/specs/oidc.spec.ts @@ -5,7 +5,7 @@ import { generateIdToken, generateOauthAccessToken } from '../utils/jwt.util'; import * as oidcUtil from '../utils/oidc.util'; import passkeyUtil from '../utils/passkey.util'; -test.beforeEach(cleanupBackend); +test.beforeEach(() => cleanupBackend()); test('Authorize existing client', async ({ page }) => { const oidcClient = oidcClients.nextcloud; @@ -189,19 +189,6 @@ test('Refresh token fails when used for the wrong client', async ({ request }) = }) .then((r) => r.text()); - // Perform the exchange - const refreshResponse = await request.post('/api/oidc/token', { - headers: { - 'Content-Type': 'application/x-www-form-urlencoded' - }, - form: { - grant_type: 'refresh_token', - client_id: clientId, - refresh_token: refreshToken, - client_secret: clientSecret - } - }); - expect(refreshResponse.status()).toBe(400); }); diff --git a/tests/specs/one-time-access-token.spec.ts b/tests/specs/one-time-access-token.spec.ts index 5ebf217b..5e039a09 100644 --- a/tests/specs/one-time-access-token.spec.ts +++ b/tests/specs/one-time-access-token.spec.ts @@ -2,7 +2,7 @@ import test, { expect } from '@playwright/test'; import { oneTimeAccessTokens } from '../data'; import { cleanupBackend } from '../utils/cleanup.util'; -test.beforeEach(cleanupBackend); +test.beforeEach(() => cleanupBackend()); // Disable authentication for these tests test.use({ storageState: { cookies: [], origins: [] } }); diff --git a/tests/specs/user-group.spec.ts b/tests/specs/user-group.spec.ts index 8bc11192..688b1b4b 100644 --- a/tests/specs/user-group.spec.ts +++ b/tests/specs/user-group.spec.ts @@ -2,7 +2,7 @@ import test, { expect } from '@playwright/test'; import { userGroups, users } from '../data'; import { cleanupBackend } from '../utils/cleanup.util'; -test.beforeEach(cleanupBackend); +test.beforeEach(() => cleanupBackend()); test('Create user group', async ({ page }) => { await page.goto('/settings/admin/user-groups'); diff --git a/tests/specs/user-settings.spec.ts b/tests/specs/user-settings.spec.ts index 1b2df8d7..2e35468c 100644 --- a/tests/specs/user-settings.spec.ts +++ b/tests/specs/user-settings.spec.ts @@ -2,7 +2,7 @@ import test, { expect } from '@playwright/test'; import { userGroups, users } from '../data'; import { cleanupBackend } from '../utils/cleanup.util'; -test.beforeEach(cleanupBackend); +test.beforeEach(() => cleanupBackend()); test('Create user', async ({ page }) => { const user = users.steve; diff --git a/tests/specs/user-signup.spec.ts b/tests/specs/user-signup.spec.ts index 9560a44e..2514f327 100644 --- a/tests/specs/user-signup.spec.ts +++ b/tests/specs/user-signup.spec.ts @@ -1,9 +1,9 @@ import test, { expect } from '@playwright/test'; +import { signupTokens, users } from 'data'; import { cleanupBackend } from '../utils/cleanup.util'; import passkeyUtil from '../utils/passkey.util'; -import { users, signupTokens } from 'data'; -test.beforeEach(cleanupBackend); +test.beforeEach(() => cleanupBackend()); test.describe('User Signup', () => { async function setSignupMode(page: any, mode: 'Disabled' | 'Signup with token' | 'Open Signup') { @@ -177,3 +177,36 @@ test.describe('User Signup', () => { await expect(page.getByText('Token is invalid or expired.')).toBeVisible(); }); }); + +test.describe('Initial User Signup', () => { + test.beforeEach(async ({ page }) => { + await page.context().clearCookies(); + }); + test('Initial Signup - success flow', async ({ page }) => { + await cleanupBackend(true); + await page.goto('/setup'); + + await page.getByLabel('First name').fill('Jane'); + await page.getByLabel('Last name').fill('Smith'); + await page.getByLabel('Username').fill('janesmith'); + await page.getByLabel('Email').fill('jane.smith@test.com'); + + await page.getByRole('button', { name: 'Sign Up' }).click(); + + await page.waitForURL('/signup/add-passkey'); + await expect(page.getByText('Set up your passkey')).toBeVisible(); + }); + + test('Initial Signup - setup already completed', async ({ page }) => { + await page.goto('/setup'); + + await page.getByLabel('First name').fill('Test'); + await page.getByLabel('Last name').fill('User'); + await page.getByLabel('Username').fill('testuser123'); + await page.getByLabel('Email').fill(users.tim.email); + + await page.getByRole('button', { name: 'Sign Up' }).click(); + + await expect(page.getByText('Setup already completed')).toBeVisible(); + }); +}); diff --git a/tests/utils/cleanup.util.ts b/tests/utils/cleanup.util.ts index f9e25c7f..3e82b0b7 100644 --- a/tests/utils/cleanup.util.ts +++ b/tests/utils/cleanup.util.ts @@ -1,12 +1,16 @@ import playwrightConfig from '../playwright.config'; -export async function cleanupBackend() { +export async function cleanupBackend(skipSeed = false) { const url = new URL('/api/test/reset', playwrightConfig.use!.baseURL); - if (process.env.SKIP_LDAP_TESTS === 'true') { + if (process.env.SKIP_LDAP_TESTS === 'true' || skipSeed) { url.searchParams.append('skip-ldap', 'true'); } + if (skipSeed) { + url.searchParams.append('skip-seed', 'true'); + } + const response = await fetch(url, { method: 'POST' });