Feat/selfhosted setup page#1556
Conversation
- Add SelfhostedService with signals for configuration state management - Add ConfigurationGuard to redirect /login to /setup when not configured - Add SetupGuard to protect /setup route (only accessible when not configured) - Add SetupComponent with email/password form matching login page style - Update app-routing.module.ts with /setup route and guards Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace HttpClient with native fetch API in SelfhostedService - Convert Observable methods to async/await Promises - Update guards to use async functions - Update SetupComponent to use async/await - Update tests to work with Promise-based methods Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR implements a self-hosted setup flow for Rocketadmin, enabling initial configuration and admin account creation for self-hosted deployments.
Changes:
- Adds a new SelfhostedService for managing configuration state and initial user creation
- Implements two route guards (setupGuard and configurationGuard) to control access to setup and login pages based on configuration status
- Creates a new SetupComponent with a form for creating the initial admin account
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/app/services/selfhosted.service.ts | New service managing self-hosted configuration state and API calls for checking configuration and creating initial user |
| frontend/src/app/guards/setup.guard.ts | Guard protecting /setup route, ensuring it's only accessible in unconfigured self-hosted instances |
| frontend/src/app/guards/configuration.guard.ts | Guard protecting /login route, redirecting to /setup if self-hosted instance is not configured |
| frontend/src/app/components/setup/setup.component.ts | Component handling the initial admin account creation form |
| frontend/src/app/components/setup/setup.component.spec.ts | Test suite for SetupComponent |
| frontend/src/app/components/setup/setup.component.html | Template for the setup page with email and password inputs |
| frontend/src/app/components/setup/setup.component.css | Styling for the setup page with responsive design |
| frontend/src/app/app-routing.module.ts | Adds /setup route and applies configurationGuard to /login route |
Comments suppressed due to low confidence (2)
frontend/src/app/components/setup/setup.component.spec.ts:93
- The test suite is missing coverage for error scenarios in the createAdminAccount method. While there's a test for successful account creation, there should also be a test verifying that when createInitialUser() throws an error, the submitting state is properly reset to false. This ensures the UI doesn't get stuck in a "Creating..." state when errors occur.
it('should create admin account and navigate to login on success', async () => {
const testable = component as SetupComponentTestable;
const fakeCreateInitialUser = vi.spyOn(selfhostedService, 'createInitialUser').mockResolvedValue({ success: true });
const fakeNavigate = vi.spyOn(router, 'navigate').mockResolvedValue(true);
testable.email.set('admin@example.com');
testable.password.set('SecurePass123');
await component.createAdminAccount();
expect(fakeCreateInitialUser).toHaveBeenCalledWith({
email: 'admin@example.com',
password: 'SecurePass123',
});
expect(testable.submitting()).toBe(false);
expect(fakeNavigate).toHaveBeenCalledWith(['/login']);
});
frontend/src/app/services/selfhosted.service.ts:91
- The selfhosted.service.ts file lacks test coverage. All other services in this codebase have corresponding .spec.ts files with comprehensive test suites (e.g., auth.service.spec.ts, notifications.service.spec.ts, company.service.spec.ts). This service should follow the same pattern with tests covering:
- checkConfiguration() success and failure scenarios
- createInitialUser() success and error handling
- resetConfigurationState() functionality
- isSelfHosted computed value behavior
import { computed, Injectable, inject, signal } from '@angular/core';
import { environment } from 'src/environments/environment';
import { AlertActionType, AlertType } from '../models/alert';
import { NotificationsService } from './notifications.service';
export interface IsConfiguredResponse {
isConfigured: boolean;
}
export interface CreateInitialUserRequest {
email: string;
password: string;
}
export interface CreateInitialUserResponse {
success: boolean;
}
@Injectable({
providedIn: 'root',
})
export class SelfhostedService {
private _notifications = inject(NotificationsService);
private _isConfigured = signal<boolean | null>(null);
private _isCheckingConfiguration = signal<boolean>(false);
public readonly isConfigured = this._isConfigured.asReadonly();
public readonly isCheckingConfiguration = this._isCheckingConfiguration.asReadonly();
public readonly isSelfHosted = computed(() => !(environment as any).saas);
async checkConfiguration(): Promise<IsConfiguredResponse> {
this._isCheckingConfiguration.set(true);
try {
const response = await fetch('/api/selfhosted/is-configured');
if (!response.ok) {
throw new Error(`HTTP error: ${response.status}`);
}
const data: IsConfiguredResponse = await response.json();
this._isConfigured.set(data.isConfigured);
this._isCheckingConfiguration.set(false);
return data;
} catch (err) {
console.error('Failed to check configuration:', err);
this._isCheckingConfiguration.set(false);
// If the endpoint fails, assume configured to avoid blocking login
this._isConfigured.set(true);
return { isConfigured: true };
}
}
async createInitialUser(userData: CreateInitialUserRequest): Promise<CreateInitialUserResponse> {
try {
const response = await fetch('/api/selfhosted/initial-user', {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify(userData),
});
if (!response.ok) {
const errorData = await response.json().catch(() => ({}));
throw { error: errorData, message: `HTTP error: ${response.status}` };
}
const data: CreateInitialUserResponse = await response.json();
this._notifications.showSuccessSnackbar('Admin account created successfully.');
this._isConfigured.set(true);
return data;
} catch (err: any) {
console.error('Failed to create initial user:', err);
this._notifications.showAlert(
AlertType.Error,
{ abstract: err.error?.message || err.message, details: err.error?.originalMessage },
[
{
type: AlertActionType.Button,
caption: 'Dismiss',
action: () => this._notifications.dismissAlert(),
},
],
);
throw err;
}
}
resetConfigurationState(): void {
this._isConfigured.set(null);
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // If we already know the configuration state, use it | ||
| const currentState = selfhostedService.isConfigured(); | ||
| if (currentState !== null) { | ||
| if (currentState) { | ||
| // Already configured, redirect to login | ||
| return of(router.createUrlTree(['/login'])); | ||
| return router.createUrlTree(['/login']); | ||
| } else { | ||
| // Not configured, allow access to setup | ||
| return of(true); | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| // Check configuration from the server | ||
| return selfhostedService.checkConfiguration().pipe( | ||
| map((response) => { | ||
| if (response.isConfigured) { | ||
| // Already configured, redirect to login | ||
| return router.createUrlTree(['/login']); | ||
| } else { | ||
| // Not configured, allow access to setup | ||
| return true; | ||
| } | ||
| }), | ||
| ); | ||
| const response = await selfhostedService.checkConfiguration(); |
There was a problem hiding this comment.
There's a potential race condition if a user opens multiple tabs or if the guard is triggered multiple times simultaneously. Both guards cache the configuration state and make API calls if the state is null. However, there's no mechanism to prevent multiple concurrent API calls to checkConfiguration() when the state is null. Consider implementing request deduplication or a loading flag to ensure only one API call is made at a time.
| const response = await fetch('/api/selfhosted/is-configured'); | ||
| if (!response.ok) { | ||
| throw new Error(`HTTP error: ${response.status}`); | ||
| } | ||
| const data: IsConfiguredResponse = await response.json(); | ||
| this._isConfigured.set(data.isConfigured); | ||
| this._isCheckingConfiguration.set(false); | ||
| return data; | ||
| } catch (err) { | ||
| console.error('Failed to check configuration:', err); | ||
| this._isCheckingConfiguration.set(false); | ||
| // If the endpoint fails, assume configured to avoid blocking login | ||
| this._isConfigured.set(true); | ||
| return { isConfigured: true }; | ||
| } | ||
| } | ||
|
|
||
| createInitialUser(userData: CreateInitialUserRequest): Observable<CreateInitialUserResponse> { | ||
| return this._http.post<CreateInitialUserResponse>('/selfhosted/initial-user', userData).pipe( | ||
| map((res) => { | ||
| this._notifications.showSuccessSnackbar('Admin account created successfully.'); | ||
| this._isConfigured.set(true); | ||
| return res; | ||
| }), | ||
| catchError((err) => { | ||
| console.error('Failed to create initial user:', err); | ||
| this._notifications.showAlert( | ||
| AlertType.Error, | ||
| { abstract: err.error?.message || err.message, details: err.error?.originalMessage }, | ||
| [ | ||
| { | ||
| type: AlertActionType.Button, | ||
| caption: 'Dismiss', | ||
| action: () => this._notifications.dismissAlert(), | ||
| }, | ||
| ], | ||
| ); | ||
| return EMPTY; | ||
| }), | ||
| ); | ||
| async createInitialUser(userData: CreateInitialUserRequest): Promise<CreateInitialUserResponse> { | ||
| try { | ||
| const response = await fetch('/api/selfhosted/initial-user', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify(userData), | ||
| }); |
There was a problem hiding this comment.
The selfhosted.service uses the native fetch API instead of Angular's HttpClient, which is inconsistent with the rest of the codebase. All other services in this application (auth.service.ts, company.service.ts, connections.service.ts, etc.) use HttpClient for HTTP requests. Using HttpClient provides benefits such as:
- Automatic integration with Angular's HttpInterceptor (including token.interceptor.ts)
- Better testability with HttpClientTestingModule
- RxJS observable integration
- Consistent error handling patterns
Consider refactoring to use HttpClient to maintain consistency with the established patterns in the codebase.
| import { inject } from '@angular/core'; | ||
| import { CanActivateFn, Router } from '@angular/router'; | ||
| import { map, of } from 'rxjs'; | ||
| import { SelfhostedService } from '../services/selfhosted.service'; | ||
|
|
||
| /** | ||
| * Guard that protects the /login route. | ||
| * In self-hosted mode, redirects to /setup if the app is not configured. | ||
| * In SaaS mode, allows access immediately. | ||
| */ | ||
| export const configurationGuard: CanActivateFn = () => { | ||
| export const configurationGuard: CanActivateFn = async () => { | ||
| const selfhostedService = inject(SelfhostedService); | ||
| const router = inject(Router); | ||
|
|
||
| // In SaaS mode, always allow access to login | ||
| if (!selfhostedService.isSelfHosted()) { | ||
| return of(true); | ||
| return true; | ||
| } | ||
|
|
||
| // If we already know the configuration state, use it | ||
| const currentState = selfhostedService.isConfigured(); | ||
| if (currentState !== null) { | ||
| if (currentState) { | ||
| return of(true); | ||
| return true; | ||
| } else { | ||
| return of(router.createUrlTree(['/setup'])); | ||
| return router.createUrlTree(['/setup']); | ||
| } | ||
| } | ||
|
|
||
| // Check configuration from the server | ||
| return selfhostedService.checkConfiguration().pipe( | ||
| map((response) => { | ||
| if (response.isConfigured) { | ||
| return true; | ||
| } else { | ||
| return router.createUrlTree(['/setup']); | ||
| } | ||
| }), | ||
| ); | ||
| const response = await selfhostedService.checkConfiguration(); | ||
| if (response.isConfigured) { | ||
| return true; | ||
| } else { | ||
| return router.createUrlTree(['/setup']); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The guards setup.guard.ts and configuration.guard.ts lack test coverage. The codebase has a test file for auth.guard.ts (auth.guard.spec.ts), indicating that guards should have test coverage. These guards should be tested to ensure proper routing behavior in different scenarios (SaaS mode, self-hosted configured, self-hosted unconfigured).
| // If we already know the configuration state, use it | ||
| const currentState = selfhostedService.isConfigured(); | ||
| if (currentState !== null) { | ||
| if (currentState) { | ||
| return of(true); | ||
| return true; | ||
| } else { | ||
| return of(router.createUrlTree(['/setup'])); | ||
| return router.createUrlTree(['/setup']); | ||
| } | ||
| } | ||
|
|
||
| // Check configuration from the server | ||
| return selfhostedService.checkConfiguration().pipe( | ||
| map((response) => { | ||
| if (response.isConfigured) { | ||
| return true; | ||
| } else { | ||
| return router.createUrlTree(['/setup']); | ||
| } | ||
| }), | ||
| ); | ||
| const response = await selfhostedService.checkConfiguration(); |
There was a problem hiding this comment.
There's a potential race condition if a user opens multiple tabs or if the guard is triggered multiple times simultaneously. Both guards cache the configuration state and make API calls if the state is null. However, there's no mechanism to prevent multiple concurrent API calls to checkConfiguration() when the state is null. Consider implementing request deduplication or a loading flag to ensure only one API call is made at a time.
| } | ||
|
|
||
| createAdminAccount(): void { | ||
| async createAdminAccount(): Promise<void> { |
There was a problem hiding this comment.
The password field validation is delegated entirely to the UserPasswordComponent, but there's no explicit validation in the setup component itself. While the submit button checks for empty values (!this.email() || !this.password()), it doesn't verify password strength requirements. Consider whether password strength validation should be enforced at the component level in addition to any validation performed by UserPasswordComponent, or add a comment clarifying that password validation is handled by the child component.
| async createAdminAccount(): Promise<void> { | |
| async createAdminAccount(): Promise<void> { | |
| // Password strength and detailed validation are handled by UserPasswordComponent | |
| // and on the server side; here we only ensure that values are present. |
| } catch (err) { | ||
| console.error('Failed to check configuration:', err); | ||
| this._isCheckingConfiguration.set(false); | ||
| // If the endpoint fails, assume configured to avoid blocking login | ||
| this._isConfigured.set(true); | ||
| return { isConfigured: true }; | ||
| } |
There was a problem hiding this comment.
In the error catch block, the service assumes configured state (line 47) when the configuration check fails. This is a fail-open approach that could allow access to the login page even when the backend is unreachable or has an issue. The comment on line 46 says this is to "avoid blocking login", but in a self-hosted setup scenario, this could lead to a confusing user experience where the login page is accessible but the actual login might fail. Consider whether this fallback behavior is appropriate, or if it should fail more gracefully (e.g., show a specific error page or retry logic).
No description provided.