diff --git a/libs/auth/CLAUDE.md b/libs/auth/CLAUDE.md new file mode 100644 index 00000000000..9e049d02f1c --- /dev/null +++ b/libs/auth/CLAUDE.md @@ -0,0 +1,367 @@ +# Auth Components - Multi-Client Component and Service Design Guidelines + +This document outlines the architectural patterns and best practices for building components and services in the `@bitwarden/auth` library that work across web, browser extension, and desktop clients. + +## Core Principles + +### 1. Single Component, Multiple Clients + +- Build ONE Angular component for each page that works across all clients (web, extension, desktop) +- Use responsive design (Tailwind breakpoints) to handle different viewport sizes +- Avoid creating separate components per client + +### 2. Abstract Client-Specific Logic into Services + +- Move ALL client-specific logic out of components and into strongly typed, well-tested services +- Components should be presentational - controlled by services, not `@Input` properties +- Services should be injected via dependency injection with client-specific implementations + +### 3. Avoid Direct Client Type Checks + +**CODE SMELL - DO NOT DO THIS:** + +```typescript +// WRONG - checking clientType in component +if (this.platformUtilsService.getClientType() === ClientType.Extension) { + // extension-specific behavior +} +``` + +**CORRECT APPROACH:** + +```typescript +// RIGHT - delegate to service +this.componentService.handleClientSpecificBehavior?.(); +``` + +## Service Architecture Hierarchy + +### Service Types (in order of preference) + +1. **Domain-scoped service** (no client-specific behavior) + - Used when logic is identical across all clients + - Example: `DeviceTrustService`, `AuthRequestApiService` + - Inject the same instance across all clients + +2. **Client-specific, domain-scoped service** (reusable across components) + - Used when client-specific logic can be reused by multiple components + - Example: `BrowserEnvironmentService`, `PopupViewCacheService` + - Can have different implementations per client + +3. **Client-specific component service** (component-specific) + - Used ONLY for logic specific to a single component + - Naming convention: `{Client}{ComponentName}ComponentService` + - Example: `WebLoginComponentService`, `ExtensionTwoFactorAuthComponentService` + - Should only be consumed by the component it's designed for + +## Naming Conventions + +### Component Services + +Must use the `ComponentService` suffix for clarity: + +- GOOD: `WebLoginComponentService` +- GOOD: `ExtensionTwoFactorAuthComponentService` +- GOOD: `DesktopSetPasswordComponentService` +- BAD: `WebLoginService` (ambiguous - is it domain or component scoped?) + +### File Organization + +``` +libs/auth/src/angular/ + login/ + login.component.ts # Shared component + login-component.service.abstraction.ts # Service interface + default-login-component.service.ts # Default implementation + +apps/web/src/auth/ + login/ + web-login-component.service.ts # Web-specific implementation + +apps/browser/src/auth/popup/ + login/ + extension-login-component.service.ts # Extension-specific implementation +``` + +## Service Patterns + +### Pattern 1: Default Service + Optional Client Overrides + +Use when only ONE client needs custom behavior: + +```typescript +// Service abstraction with optional methods +export abstract class LoginComponentServiceAbstraction { + abstract performLogin(): Promise; + + // Optional - only extension implements this + handlePopupResize?(): void; +} + +// Default implementation (in jslib-services.module.ts) +export class DefaultLoginComponentService implements LoginComponentServiceAbstraction { + async performLogin(): Promise { + // Default behavior for web and desktop + } + // No handlePopupResize - it's optional +} + +// Extension-specific implementation (in browser services.module.ts) +export class ExtensionLoginComponentService extends DefaultLoginComponentService { + handlePopupResize(): void { + // Extension-only behavior + } +} + +// Component usage +export class LoginComponent { + constructor(private service: LoginComponentServiceAbstraction) {} + + ngOnInit() { + // Optional chaining makes it explicit this only runs on some clients + this.service.handlePopupResize?.(); + } +} +``` + +### Pattern 2: Base Service + All Clients Override + +Use when ALL clients need different implementations: + +```typescript +export abstract class LoginComponentServiceAbstraction { + abstract performLogin(): Promise; + abstract getRedirectUrl(): string; +} + +// Default base with shared logic +export class DefaultLoginComponentService { + protected sharedHelper(): void { + // Common logic all clients use + } +} + +// Each client extends and customizes +export class WebLoginComponentService + extends DefaultLoginComponentService + implements LoginComponentServiceAbstraction +{ + async performLogin(): Promise { + this.sharedHelper(); + // Web-specific behavior + } + + getRedirectUrl(): string { + return "/vault"; + } +} + +export class ExtensionLoginComponentService + extends DefaultLoginComponentService + implements LoginComponentServiceAbstraction +{ + async performLogin(): Promise { + this.sharedHelper(); + // Extension-specific behavior + } + + getRedirectUrl(): string { + return "popup/vault.html"; + } +} +``` + +### Pattern 3: Fully Independent Implementations + +Use when clients have completely different logic with no shared code: + +```typescript +export abstract class LoginComponentServiceAbstraction { + abstract performLogin(): Promise; +} + +// Each client has independent implementation +export class WebLoginComponentService implements LoginComponentServiceAbstraction { + async performLogin(): Promise { + // Completely different web logic + } +} + +export class ExtensionLoginComponentService implements LoginComponentServiceAbstraction { + async performLogin(): Promise { + // Completely different extension logic + } +} +``` + +## Component Best Practices + +### Keep Components Presentational + +Components should focus on presentation and delegate logic to services: + +**BAD - Logic in component:** + +```typescript +export class LoginComponent { + async submit() { + if (this.clientType === ClientType.Extension) { + // Check popup size + // Validate form differently + // Use different API endpoint + } else { + // Different behavior for web/desktop + } + } +} +``` + +**GOOD - Logic in service:** + +```typescript +export class LoginComponent { + constructor(private loginService: LoginComponentServiceAbstraction) {} + + async submit() { + await this.loginService.performLogin(); + } +} +``` + +### Use Optional Chaining for Client-Specific Features + +```typescript +export class TwoFactorComponent implements OnInit { + constructor(private service: TwoFactorComponentServiceAbstraction) {} + + ngOnInit() { + // Explicitly shows this only runs on some clients + this.service.setupBrowserExtensionAutofill?.(); + } +} +``` + +### Responsive Design Over Conditionals + +**BAD - Structural directives based on client:** + +```html +
Extension UI
+
Web/Desktop UI
+``` + +**GOOD - Responsive breakpoints:** + +```html +
Desktop/Web UI
+
Mobile/Extension UI
+``` + +**ACCEPTABLE - When performance matters:** + +```html + + +``` + +## Dependency Injection Configuration + +### Shared Default Service (jslib-services.module.ts) + +```typescript +@NgModule({ + providers: [ + { + provide: LoginComponentServiceAbstraction, + useClass: DefaultLoginComponentService, + }, + ], +}) +export class JsLibServicesModule {} +``` + +### Client-Specific Override (apps/browser/src/services.module.ts) + +```typescript +@NgModule({ + providers: [ + { + provide: LoginComponentServiceAbstraction, + useClass: ExtensionLoginComponentService, // Overrides default + }, + ], +}) +export class ServicesModule {} +``` + +## Testing Requirements + +### Component Services MUST Be Strongly Unit Tested + +Since components delegate to services, services must have comprehensive test coverage: + +```typescript +describe("ExtensionLoginComponentService", () => { + let service: ExtensionLoginComponentService; + + beforeEach(() => { + // Setup mocks + }); + + it("should handle popup resize correctly", () => { + service.handlePopupResize(); + // Verify behavior + }); + + it("should redirect to correct URL after login", () => { + expect(service.getRedirectUrl()).toBe("popup/vault.html"); + }); +}); +``` + +## Code Review Checklist + +When reviewing auth library code, check for: + +- [ ] Is there a single component shared across clients? (not separate components per client) +- [ ] Is client-specific logic abstracted into services? +- [ ] Are there any `clientType` checks in component `.ts` or `.html` files? +- [ ] Do service names follow `ComponentService` suffix convention? +- [ ] Are optional methods used for client-specific features? +- [ ] Is responsive design (Tailwind) used instead of conditional rendering? +- [ ] Are component services strongly unit tested? +- [ ] Is the service registered correctly in dependency injection? +- [ ] Could any component service logic be refactored into a domain-scoped service for reuse? + +## Migration Path for Existing Components + +When refactoring existing multi-client components: + +1. **Identify client-specific logic** - Look for `clientType` checks or conditional behavior +2. **Create service abstraction** - Define interface with optional methods where appropriate +3. **Implement default service** - Extract common logic +4. **Implement client overrides** - Only for clients that need custom behavior +5. **Update component** - Remove client checks, inject service, use optional chaining +6. **Add service tests** - Comprehensive unit tests for each implementation +7. **Register in DI** - Configure default in jslib, overrides in client services modules + +## Warning Signs (Code Smells) + +- Component checking `clientType` or `platformUtilsService.getClientType()` +- Service name without `ComponentService` suffix when it's component-specific +- Separate component files per client (e.g., `web-login.component.ts`, `extension-login.component.ts`) +- No-op methods in service instead of optional methods +- Component service with untested or poorly tested logic +- Unnecessary default service when only one client needs the feature + +## Goals + +By following these patterns, we achieve: + +- Write functionality once, benefit across all clients +- Maintainable, easy-to-understand components +- Strong separation of concerns (presentation vs. logic) +- Testable service layer with high coverage +- Clear dependency injection patterns +- Responsive design that scales across viewports