[SBA-06] Create initial UnifiedCredentialStore with password auth

[jamaljsr]

Summary

This PR introduces the UnifiedCredentialStore with a strategy-based authentication architecture, laying the foundation for upcoming passkey and session features. The key goal is to move away from the monolithic credential handling in LncCredentialStore toward a pluggable system where different authentication methods (password, passkey, session) can be added without modifying core code.

Why a strategy pattern? As we add passkey and session-based auth, we need a clean way to support multiple unlock methods. The strategy pattern lets each auth method encapsulate its own encryption/decryption logic while the UnifiedCredentialStore provides a consistent interface to the rest of the library.

Why CredentialOrchestrator? The orchestrator serves as the bridge between LNC and the credential stores. It decides which store to use based on configuration and provides high-level methods (unlock, persistWithPassword, getAuthenticationInfo) that work regardless of which underlying store is active. This keeps LNC focused on connection management while the orchestrator handles auth complexity.

The PR also updates the passkeys-demo app to use the new unified store, enabling end-to-end testing of password authentication with the new architecture.

Screenshots

Technical Notes

• CredentialCache: An in-memory key-value store for credentials during a session. Separating cache from persistence allows credentials to be populated during connection and persisted afterward, which is important for the pairing flow where credentials arrive before the user sets a password.

• AuthStrategy interface: Defines the contract for authentication methods. Each strategy handles its own unlock(), credential get/set, and storage checks. Currently only PasswordStrategy is implemented; PasskeyStrategy and SessionStrategy will follow in later PRs.

• StrategyManager: Manages strategy registration and selection. It answers questions like "which strategies are available?" and "which should be preferred?" This becomes important when multiple auth methods are registered.

• UnifiedCredentialStore: Implements the same CredentialStore interface as the legacy store, so existing code continues to work. Internally delegates to the active strategy for encryption/persistence.

• Backward compatibility: Setting useUnifiedStore: true in config opts into the new system. Without this flag, LNC continues using LncCredentialStore exactly as before. This is a temporary flag that will be removed in a future PR. I just added it here to be able to test the new functionality using the demo app.

Steps to Test

1. Run the unit tests to verify the new components:

   yarn test:coverage
   

2. Build the library to ensure the production bundle compiles:

   yarn build
   

3. Test password auth with the demo app:

   cd demos/passkeys-demo
   yarn start
   

   • update the useLNC hook to use new LNC{ serverHost: 'localhost:11110' }) for regtest testing
   • Enter a pairing phrase and password, click Submit
   • After connecting, credentials are persisted with password encryption
   • Reload the page—you should see the "Login" button
   • Enter your password and submit to reconnect

Related Issues & Pull Requests

• Session-Based Authentication & Passkeys #128

Depends on:

• [SBA-05] Introduce new password encryption abstractions #137

[Copilot]

Pull request overview

This PR introduces a strategy-based authentication architecture by implementing the UnifiedCredentialStore with support for password authentication. The changes lay the foundation for future passkey and session authentication features while maintaining backward compatibility with the existing LncCredentialStore.

Key Changes:

• Implements strategy pattern for authentication with AuthStrategy interface and PasswordStrategy implementation
• Adds CredentialOrchestrator to manage credential store lifecycle and bridge between LNC and stores
• Introduces CredentialCache for efficient in-memory credential management during sessions

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
lib/stores/authStrategy.ts	Defines the AuthStrategy interface for pluggable authentication methods
lib/stores/credentialCache.ts	Implements in-memory credential caching with comprehensive helper methods
lib/stores/credentialCache.test.ts	Comprehensive test coverage for credential cache functionality
lib/stores/passwordStrategy.ts	Implements password-based authentication strategy using encryption service and repository
lib/stores/passwordStrategy.test.ts	Test coverage for password strategy implementation
lib/stores/strategyManager.ts	Manages registration, lookup, and coordination of authentication strategies
lib/stores/strategyManager.test.ts	Test coverage for strategy manager functionality
lib/stores/unifiedCredentialStore.ts	Main unified credential store implementing strategy-based authentication
lib/stores/unifiedCredentialStore.test.ts	Comprehensive test suite for unified credential store
lib/credentialOrchestrator.ts	Orchestrates credential management between LNC and credential stores
lib/credentialOrchestrator.test.ts	Test coverage for orchestrator including legacy and unified store paths
lib/lnc.ts	Updates LNC class to use orchestrator and adds new authentication methods
lib/lnc.test.ts	Adds integration tests for orchestrator and new authentication methods
lib/types/lnc.ts	Adds useUnifiedStore config flag and documents temporary nature
lib/index.ts	Exports new types and orchestrator for public API
demos/passkeys-demo/src/hooks/useLNC.ts	Updates demo to use new unified store authentication methods

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lightninglabs-deploy]

@jamaljsr, remember to re-request review from reviewers when ready

[jbrill]

Starting to dive into this PR, and the architecture is becoming clearer, but I'm concerned we might be over-abstracting.

Some points made by Claude:

The isUnlocked problem - this appears in 4 different interfaces, with each layer mostly delegating to the one below:

EncryptionService.isUnlocked      // actual logic
CredentialRepository.isUnlocked   // delegates to encryption
AuthStrategy.isUnlocked           // delegates to repository  
UnifiedCredentialStore._isUnlocked // duplicates state!

The unlock() call stack - 6 layers deep:

LNC → Orchestrator → UnifiedCredentialStore → Strategy → Repository → EncryptionService

I'd love to hop on a call to understand the reasoning behind each layer. A few specific questions:

1. Why separate EncryptionService from CredentialRepository? The encryption service's hasStoredData() always returns false because "storage is at the repository layer" - if they're this tightly coupled, should they be one class?

2. Do we need StrategyManager now? With only password auth, UnifiedCredentialStore could hold the strategy directly and we add the manager when passkey/session land.

3. Why does UnifiedCredentialStore track _isUnlocked separately instead of delegating to strategy.isUnlocked?

Possible simplification (2 interfaces + 3 classes instead of 4 + 6):

interface CredentialStore { /* existing */ }

interface AuthStrategy {
  readonly method: UnlockMethod;
  isUnlocked: boolean;
  hasStoredCredentials: boolean;
  unlock(options: UnlockOptions): Promise<boolean>;
  getCredential(key: string): Promise<string | undefined>;
  setCredential(key: string, value: string): Promise<void>;
  clear(): void;
}

// Single class handles encryption + localStorage
class PasswordStrategy implements AuthStrategy { }

// Uses strategy directly, no manager
class UnifiedCredentialStore implements CredentialStore {
  private strategy: AuthStrategy;
}

I may be missing context on why the extra layers are needed - happy to discuss!

[jbrill]

These are re-used/duplicated in loadCredentialsToCache. I think a better place might be a const declared at the top of the file?

[jbrill]

Do we definitely want to throw here?

[jbrill]

nit get supportedUnlockMethods

[jbrill]

Nit: getter

[jbrill]

Maybe instead I think this comment could be modified from enhanced to something a bit clearer for future reviewers(?) just a thought

[jbrill]

Would it be better to return | undefined instead of blank strings in these methods?

[jbrill]

This seems weird to me. Do we need to implement all methods in the interface?

[jbrill]

nit: getter