feat: tauri opener plugin support for deeplink auth #472
Conversation
WalkthroughThis PR implements signature-based authentication across multiple platforms, adds EVault pod-level logging/metrics methods, introduces deeplink-based login flows, centralizes key management with a fixed KEY_ID approach, enhances cryptographic logging, and updates state management to use Svelte's $state store pattern. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant LoginPage as Login Page
participant AuthAPI as Auth API
participant SigValidator as Signature Validator
participant Registry as Registry Service
Client->>LoginPage: Navigate with query params<br/>(ename, session, signature)
LoginPage->>LoginPage: Parse URL parameters
LoginPage->>LoginPage: Clear URL history
LoginPage->>AuthAPI: POST /api/auth<br/>(ename, session, signature, appVersion)
AuthAPI->>AuthAPI: Validate signature present
AuthAPI->>AuthAPI: Fetch PUBLIC_REGISTRY_URL config
AuthAPI->>SigValidator: verifySignature<br/>(ename, signature, session, registryUrl)
SigValidator->>Registry: Fetch public key for ename
Registry-->>SigValidator: Return public key
SigValidator->>SigValidator: Decode signature & public key
SigValidator->>SigValidator: Verify signature against session
SigValidator-->>AuthAPI: Return verification result<br/>{valid: boolean}
alt Signature Valid
AuthAPI->>AuthAPI: Lookup user by ename
AuthAPI->>AuthAPI: Generate auth token
AuthAPI-->>LoginPage: 200 OK<br/>{token, user}
LoginPage->>LoginPage: Store token & user in storage
LoginPage->>Client: Redirect to home/dashboard
else Signature Invalid
AuthAPI-->>LoginPage: 401 Unauthorized<br/>{error: "Invalid signature"}
LoginPage->>LoginPage: Display error message
LoginPage->>Client: Show retry button
end
sequenceDiagram
participant Wallet as eID Wallet
participant QRCode as QR Code Handler
participant KeyService as Key Service
participant CryptoMgr as Crypto Manager
participant DeepLink as Deep Link URL
Wallet->>Wallet: Scan QR code
QRCode->>KeyService: ensureKey("default", context)
KeyService->>CryptoMgr: Get/create key manager
CryptoMgr-->>KeyService: Return key manager
KeyService-->>QRCode: Key ready
QRCode->>KeyService: signPayload("default", context, payload)
KeyService->>CryptoMgr: getPublicKey()
CryptoMgr-->>KeyService: Return public key
KeyService->>CryptoMgr: sign(payload)
CryptoMgr-->>KeyService: Return signature (base64)
KeyService-->>QRCode: Return signature
QRCode->>DeepLink: Construct URL with<br/>ename, session, signature, appVersion
QRCode->>DeepLink: openUrl() via Tauri opener
Wallet->>Wallet: Open deep link in browser
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coodos
force-pushed
the
feat/tauri-opener-login
branch
from
9dd7bce to
8a8f8e9
Compare
coodos
changed the title
coodos
force-pushed
the
feat/tauri-opener-login
branch
from
7bc4a5f to
b02795f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
infrastructure/eid-wallet/src/lib/crypto/SoftwareKeyManager.ts (1)
118-199: Same concern: verbose cryptographic logging should be guarded in production.This is consistent with the logging pattern in
HardwareKeyManagerandKeyService. Notable additions here include logging the private key buffer length (line 149) and full public key in base64. Apply the same debug-mode guard across all key managers for consistency.platforms/evoting-api/src/controllers/AuthController.ts (1)
60-108: Code duplication: signature verification logic repeated across platforms.This signature verification implementation is nearly identical to:
platforms/blabsy-w3ds-auth-api/src/controllers/AuthController.ts(lines 59-108)platforms/pictique-api/src/controllers/AuthController.ts(lines 72-119)platforms/group-charter-manager-api/src/controllers/AuthController.ts(lines 61-108)Consider extracting this into a shared middleware or utility function:
Create a shared authentication middleware:
// shared/middleware/signatureAuth.ts import { Request, Response, NextFunction } from "express"; import { verifySignature } from "signature-validator"; import { isVersionValid } from "../utils/version"; const MIN_REQUIRED_VERSION = "0.4.0"; export async function validateSignatureAuth( req: Request, res: Response, next: NextFunction ) { const { ename, session, appVersion, signature } = req.body; // Validation logic if (!ename) { return res.status(400).json({ error: "ename is required" }); } if (!session) { return res.status(400).json({ error: "session is required" }); } if (!signature) { return res.status(400).json({ error: "signature is required" }); } // Version check if (!appVersion || !isVersionValid(appVersion, MIN_REQUIRED_VERSION)) { const errorMessage = { error: true, message: `Your eID Wallet app version is outdated. Please update to version ${MIN_REQUIRED_VERSION} or later.`, type: "version_mismatch" }; return res.status(400).json({ error: "App version too old", message: errorMessage.message }); } // Registry URL check const registryBaseUrl = process.env.PUBLIC_REGISTRY_URL; if (!registryBaseUrl) { console.error("PUBLIC_REGISTRY_URL not configured"); return res.status(500).json({ error: "Server configuration error" }); } // Signature verification const verificationResult = await verifySignature({ eName: ename, signature: signature, payload: session, registryBaseUrl: registryBaseUrl, }); if (!verificationResult.valid) { console.error("Signature validation failed:", verificationResult.error); return res.status(401).json({ error: "Invalid signature", message: verificationResult.error }); } // Attach validated data to request req.validatedAuth = { ename, session }; next(); }Then use it in your routes:
router.post('/api/auth', validateSignatureAuth, authController.login);infrastructure/signature-validator/src/index.ts (1)
109-143: Update JSDoc to document null return value.The function now returns
Promise<string | null>but the JSDoc comment on line 113 doesn't mention thatnullcan be returned when the public key is not found. This could confuse callers.Apply this diff to update the JSDoc:
/** * Retrieves the public key for a given eName * @param eName - The eName (W3ID) of the user * @param registryBaseUrl - Base URL of the registry service - * @returns The public key in multibase format + * @returns The public key in multibase format, or null if not found */
♻️ Duplicate comments (6)
platforms/dreamSync/client/src/pages/deeplink-login.tsx (1)
1-122: Duplicate implementation - see previous comments.This file is nearly identical to
platforms/eCurrency/client/src/pages/deeplink-login.tsxwith only platform-specific configuration differences. Please refer to the review comments on that file regarding:
- Complex URL parsing that could be simplified
- localStorage security considerations
- The need for code deduplication
platforms/evoting-api/src/controllers/AuthController.ts (1)
88-92: Environment variable validation should occur at startup.Same issue as in
platforms/blabsy-w3ds-auth-api/src/controllers/AuthController.ts. ThePUBLIC_REGISTRY_URLcheck during each request can cause authentication failures if misconfigured. See the previous comment on that file for a suggested startup validation approach.platforms/pictique-api/src/controllers/AuthController.ts (2)
72-119: Duplicate implementation - see previous comments.This signature verification implementation is identical to other platforms. Please refer to the review comment on
platforms/evoting-api/src/controllers/AuthController.ts(lines 60-108) for the suggested middleware approach to eliminate this duplication.
100-104: Environment variable validation should occur at startup.Same issue as in other AuthController files. See the comment on
platforms/blabsy-w3ds-auth-api/src/controllers/AuthController.ts(lines 89-93) for the suggested startup validation approach.platforms/group-charter-manager-api/src/controllers/AuthController.ts (2)
61-108: Duplicate implementation - see previous comments.This signature verification implementation is identical to other platforms. Please refer to the review comment on
platforms/evoting-api/src/controllers/AuthController.ts(lines 60-108) for the suggested middleware approach to eliminate this duplication.
89-93: Environment variable validation should occur at startup.Same issue as in other AuthController files. See the comment on
platforms/blabsy-w3ds-auth-api/src/controllers/AuthController.ts(lines 89-93) for the suggested startup validation approach.
🟠 Major comments (15)
platforms/pictique/src/routes/(auth)/auth/+page.svelte-38-70 (1)
38-70: Use apiClient for consistency and add parameter validation.The function uses
fetchdirectly while the existing code usesapiClient(line 93). This inconsistency bypasses any centralized error handling, interceptors, or configuration thatapiClientmay provide.Additionally:
- Parameters are not validated before sending (empty strings could be submitted).
- Only
version_mismatcherrors receive user-facing feedback; other authentication failures are silently logged.- No loading state is set, leaving users without feedback during the request.
Apply this diff to improve consistency and error handling:
async function handleAutoLogin( ename: string, session: string, signature: string, appVersion: string ) { + if (!ename?.trim() || !session?.trim() || !signature?.trim()) { + errorMessage = 'Invalid authentication parameters'; + return; + } + try { - const response = await fetch(`${PUBLIC_PICTIQUE_BASE_URL}/api/auth`, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ ename, session, signature, appVersion }) - }); - - if (response.ok) { - const data = await response.json(); + const { data } = await apiClient.post('/api/auth', { + ename, + session, + signature, + appVersion + }); + - if (data.token && data.user) { - setAuthId(data.user.id); - setAuthToken(data.token); - goto('/home'); - } - } else { - const errorData = await response.json(); - console.error('Login failed:', errorData); - if (errorData.error && errorData.type === 'version_mismatch') { - errorMessage = - errorData.message || - 'Your eID Wallet app version is outdated. Please update to continue.'; - } - } + if (data.token && data.user) { + setAuthId(data.user.id); + setAuthToken(data.token); + goto('/home'); + } else { + errorMessage = 'Authentication failed. Please try again.'; + } } catch (error) { - console.error('Login request failed:', error); + console.error('Login request failed:', error); + if (error?.response?.data?.type === 'version_mismatch') { + errorMessage = error.response.data.message || + 'Your eID Wallet app version is outdated. Please update to continue.'; + } else { + errorMessage = 'Authentication failed. Please try again.'; + } } }Committable suggestion skipped: line range outside the PR's diff.
platforms/pictique/src/routes/(auth)/deeplink-login/+page.svelte-50-82 (1)
50-82: Use apiClient for consistency and extract hardcoded version.Similar to the issue in the
/authroute, this code usesfetchdirectly instead of theapiClientutility (imported but not used). This creates:
- Inconsistent error handling across the application
- Bypassed centralized request configuration
- Duplicated HTTP logic
Additionally, the hardcoded
'0.4.0'default (line 55) should be extracted to a shared constant.Apply this diff:
+const DEFAULT_APP_VERSION = '0.4.0'; + async function handleDeeplinkLogin() { try { // ... parameter parsing ... if (!ename || !session || !signature) { error = 'Missing required authentication parameters'; isLoading = false; return; } // Clean up URL window.history.replaceState({}, '', window.location.pathname); - // Make POST request to login endpoint - const loginUrl = `${PUBLIC_PICTIQUE_BASE_URL}/api/auth`; - const requestBody = { ename, session, signature, appVersion: appVersion || '0.4.0' }; - - const response = await fetch(loginUrl, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(requestBody) - }); - - if (response.ok) { - const data = await response.json(); - if (data.token && data.user) { - setAuthId(data.user.id); - setAuthToken(data.token); - goto('/home'); - } else { - error = 'Invalid response from server'; - isLoading = false; - } - } else { - let errorData; - try { - errorData = await response.json(); - } catch (parseError) { - errorData = { error: `Server error: ${response.status}` }; - } - error = errorData.error || 'Authentication failed'; + const { data } = await apiClient.post('/api/auth', { + ename, + session, + signature, + appVersion: appVersion || DEFAULT_APP_VERSION + }); + + if (data.token && data.user) { + setAuthId(data.user.id); + setAuthToken(data.token); + goto('/home'); + } else { + error = 'Invalid response from server'; isLoading = false; } } catch (err) { console.error('Login request failed:', err); - error = 'Failed to connect to server'; + error = err?.response?.data?.error || 'Failed to connect to server'; isLoading = false; } }Committable suggestion skipped: line range outside the PR's diff.
infrastructure/eid-wallet/src/lib/crypto/HardwareKeyManager.ts-67-99 (1)
67-99: Same concern: verbose cryptographic logging should be guarded in production.This mirrors the logging added in
KeyService.signPayload. The full payload, hex representation, public key, and signature are all logged. Apply the same debug-mode guard here for consistency and to prevent sensitive data leakage.infrastructure/eid-wallet/src/lib/global/controllers/key.ts-148-186 (1)
148-186: Consider removing or guarding verbose cryptographic logging in production.The detailed logging of payloads (including hex representation), full public keys, and signatures is valuable for debugging but poses a security/privacy risk in production:
- Line 153: Full payload content logged
- Line 158: Payload hex representation logged
- Line 169: Full public key logged
- Line 182-184: Signature details logged
Consider wrapping these logs in a debug/development mode check or using a configurable log level to prevent sensitive data leakage in production builds.
+const DEBUG_CRYPTO = import.meta.env.DEV; // or a feature flag + async signPayload( keyId: string, context: KeyServiceContext, payload: string, ): Promise<string> { - console.log("=".repeat(70)); - console.log("🔐 [KeyService] signPayload called"); - console.log("=".repeat(70)); - console.log(`Key ID: ${keyId}`); - console.log(`Context: ${context}`); - console.log(`Payload: "${payload}"`); - console.log(`Payload length: ${payload.length} bytes`); - const payloadHex = Array.from(new TextEncoder().encode(payload)) - .map((b) => b.toString(16).padStart(2, "0")) - .join(""); - console.log(`Payload (hex): ${payloadHex}`); + if (DEBUG_CRYPTO) { + console.log("=".repeat(70)); + console.log("🔐 [KeyService] signPayload called"); + console.log(`Key ID: ${keyId}, Context: ${context}`); + console.log(`Payload length: ${payload.length} bytes`); + console.log("=".repeat(70)); + }Committable suggestion skipped: line range outside the PR's diff.
platforms/blabsy/src/components/login/login-main.tsx-111-148 (1)
111-148: Auto-login failure leaves user without QR fallback; harden error handling and fix hooks dependencyThe core auto-login POST +
signInWithCustomTokenflow is sound, but three important issues need attention:
Major: No fallback when auto-login fails.
On any failure (non-OK response that isn'tversion_mismatch, network error, JSON parse error),handleAutoLoginreturns silently without callinggetOfferData(). The component ends up withqrundefined and no error shown (unless it'sversion_mismatch). The UI then renders an empty state with no working login path.
Add a fallback to the QR flow for non-version_mismatcherrors:
- For missing
NEXT_PUBLIC_BASE_URL, set an error message and fall back togetOfferData()- For non-OK errors that aren't
version_mismatch, callawait getOfferData()after logging- In the catch block, call
await getOfferData()to ensure users always have a login optionNon-OK error parsing assumes JSON.
Line 136 callsresponse.json()without checkingContent-Typeor handling parse errors. If the server responds with non-JSON (HTML error page, plain text), the parse fails silently and the error is lost. Wrap the parse in a try-catch and check theContent-Typeheader first to avoid losing error context.Hooks dependency issue.
TheuseEffectat line 87 has an empty dependency array but referenceshandleAutoLogin(defined at line 111). This violatesreact-hooks/exhaustive-depsand relies on hoisting. WraphandleAutoLogininuseCallbackwith dependencies[signInWithCustomToken]and include it in theuseEffectdependency array.platforms/group-charter-manager/src/components/auth/login-screen.tsx-41-72 (1)
41-72: Auto-login failure leaves user without a working login path (no QR/init fallback)On non-OK responses that are not
version_mismatch, and on network errors,handleAutoLoginclears loading/auth states but never callsinitializeAuth(). Because the URL has already been cleaned (no query) andinitializeAuthonly runs in the initial effect when no params are present, the user ends up on a screen that talks about scanning a QR code but never shows one (noqrData), and they have no obvious way to retry except a manual refresh.This effectively blocks login in common failure modes (bad signature, expired session, transient backend error) and is confusing UX.
Consider falling back to the normal QR/SSE flow and surfacing a generic error message whenever auto-login fails for reasons other than
version_mismatch. For example:const handleAutoLogin = async (ename: string, session: string, signature: string, appVersion: string) => { setIsAuthenticating(true); try { const baseUrl = process.env.NEXT_PUBLIC_GROUP_CHARTER_BASE_URL || 'http://localhost:3001'; const response = await fetch(`${baseUrl}/api/auth`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ ename, session, signature, appVersion }) }); if (response.ok) { const data = await response.json(); if (data.token && data.user) { setAuthId(data.user.id); setAuthToken(data.token); window.location.reload(); } } else { const errorData = await response.json(); console.error('Login failed:', errorData); - if (errorData.error && errorData.type === 'version_mismatch') { - setErrorMessage(errorData.message || 'Your eID Wallet app version is outdated. Please update to continue.'); - } - setIsAuthenticating(false); - setIsLoading(false); + if (errorData.error && errorData.type === 'version_mismatch') { + setErrorMessage(errorData.message || 'Your eID Wallet app version is outdated. Please update to continue.'); + setIsAuthenticating(false); + setIsLoading(false); + return; + } + + // Fallback: show a generic error and start the QR-based auth flow + setErrorMessage(errorData.message || 'Auto-login failed. Please try again by scanning the QR code.'); + setIsAuthenticating(false); + setIsLoading(true); + await initializeAuth(); } } catch (error) { console.error('Login request failed:', error); - setIsAuthenticating(false); - setIsLoading(false); + setErrorMessage('Unable to reach the authentication service. Please try again by scanning the QR code.'); + setIsAuthenticating(false); + setIsLoading(true); + await initializeAuth(); } };This keeps the strict
version_mismatchbehavior, while ensuring other failures degrade gracefully to the existing QR-based auth experience instead of leaving the screen in a dead-end state.infrastructure/signature-validator/package.json-9-14 (1)
9-14: Reconsiderpostinstallbuild hook for published package consumptionThe new scripts/devDeps are coherent, but
postinstall: "npm run build"creates problems for this package if installed from npm registry:
- Registry installs: devDependencies (
typescript,tsx) are not installed for dependency packages, sotsc/tsxinpostinstallwill fail.- Security concern: Many npm clients and registries disable lifecycle scripts by default; arbitrary
postinstallexecution is a supply-chain risk surface.- Best practice: Build in CI and publish pre-compiled artifacts (
.d.ts+.js) to the registry; set"main"and"types"so consumers use the built output, not run build on install.Instead of
postinstallat publish time, use:
preparelifecycle script for git installs only (npm will install devDependencies duringprepare, sotscworks) and ensure CI publishes the compileddist/to the tarball viaprepack/prepublishOnly.- Add a
"files"field topackage.jsonto include compiled output and type declarations; exclude source/dev files.If workspace-only installs must trigger build, guard with an environment flag to skip when consumed from registry (e.g.,
SKIP_SIGNATURE_VALIDATOR_BUILD).platforms/eCurrency/client/src/components/auth/login-screen.tsx-70-98 (1)
70-98: Silent failure on auto-login does not inform the user.When
handleAutoLoginfails (lines 87-97), it only logs the error and resets state flags without settingerrorstate or falling back to the normal login flow. The user is left in an unclear state with no QR code and no error message.} else { const errorData = await response.json(); console.error('Login failed:', errorData); + setError(errorData.error || 'Auto-login failed. Please try again.'); setIsConnecting(false); setIsLoading(false); + // Fall back to normal login flow + getAuthOffer(); } } catch (error) { console.error('Login request failed:', error); + setError('Failed to connect to server'); setIsConnecting(false); setIsLoading(false); + getAuthOffer(); }platforms/eCurrency-api/src/controllers/AuthController.ts-60-60 (1)
60-60: Remove debug logging of sensitive authentication data.This console.log outputs the signature, ename, and session values, which are sensitive authentication parameters. This should be removed before merging to avoid leaking authentication data in production logs.
- console.log(signature, ename, session) -platforms/eCurrency/client/src/pages/deeplink-login.tsx-1-122 (1)
1-122: High code duplication across deeplink login components.This file is nearly identical to:
platforms/dreamSync/client/src/pages/deeplink-login.tsx- Similar patterns in
platforms/eVoting/src/app/(auth)/login/page.tsxThe only differences are environment variable names and localStorage keys. Consider extracting shared logic into a reusable hook or utility function.
Create a shared authentication utility:
// shared/auth/useDeeplinkAuth.ts export function useDeeplinkAuth(config: { apiBaseUrl: string; tokenKey: string; userKey: string; redirectPath: string; }) { const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState<string | null>(null); useEffect(() => { // Shared logic here }, []); return { isLoading, error }; }Then use it in each platform-specific component:
export default function DeeplinkLogin() { const { isLoading, error } = useDeeplinkAuth({ apiBaseUrl: import.meta.env.VITE_ECURRENCY_BASE_URL, tokenKey: 'ecurrency_token', userKey: 'ecurrency_user', redirectPath: '/dashboard' }); // Render logic }platforms/eVoting/src/app/(auth)/login/page.tsx-66-101 (1)
66-101: Code duplication: auto-login logic repeated across platforms.This
handleAutoLoginfunction is nearly identical to implementations in:
platforms/dreamSync/client/src/components/auth/login-screen.tsx(lines 56-87)platforms/eCurrency/client/src/pages/deeplink-login.tsx(lines 10-89)platforms/dreamSync/client/src/pages/deeplink-login.tsx(lines 10-89)Only the environment variable names and storage keys differ. Consider extracting this into a shared utility function to improve maintainability and reduce the risk of inconsistent behavior across platforms.
See the comment on
platforms/eCurrency/client/src/pages/deeplink-login.tsxfor a suggested shared implementation approach.platforms/blabsy-w3ds-auth-api/src/controllers/AuthController.ts-89-93 (1)
89-93: Environment variable validation should occur at startup.The
PUBLIC_REGISTRY_URLis checked during each login request, causing authentication to fail with a 500 error if misconfigured. This configuration error should be caught at application startup to fail fast and provide immediate feedback to operators.Consider adding startup validation in your application entry point:
// In your app initialization (e.g., index.ts or app.ts) function validateEnvironment() { const required = ['PUBLIC_REGISTRY_URL', 'PUBLIC_BLABSY_BASE_URL']; const missing = required.filter(key => !process.env[key]); if (missing.length > 0) { throw new Error(`Missing required environment variables: ${missing.join(', ')}`); } } validateEnvironment();platforms/dreamSync/client/src/components/auth/login-screen.tsx-56-87 (1)
56-87: Improve error handling to surface all authentication failures.Currently, only
version_mismatcherrors set theerrorMessagestate. Other authentication failures (invalid signature, network errors, etc.) are logged but not displayed to users, leaving them with a loading state or no feedback.Apply this diff to improve error handling:
} else { const errorData = await response.json(); console.error('Login failed:', errorData); if (errorData.error && errorData.type === 'version_mismatch') { setErrorMessage(errorData.message || 'Your eID Wallet app version is outdated. Please update to continue.'); + } else { + setErrorMessage(errorData.message || 'Authentication failed. Please try again.'); } setIsConnecting(false); setIsLoading(false); } } catch (error) { console.error('Login request failed:', error); + setErrorMessage('Failed to connect to the server. Please check your connection and try again.'); setIsConnecting(false); setIsLoading(false); }platforms/eCurrency/client/src/pages/deeplink-login.tsx-67-68 (1)
67-68: Use httpOnly cookies for authentication tokens instead of localStorage to mitigate XSS attacks.The current implementation stores auth tokens in
localStorage(deeplink-login.tsx lines 67-68, apiClient.ts line 15, auth-context.tsx), making them readable by any script on the origin. This violates OWASP security best practices.To address this:
- Migrate to httpOnly cookies: Modify the backend to set tokens via
Set-Cookieheaders withHttpOnly,Secure, andSameSite=Strictattributes instead of returning them in response bodies. The apiClient interceptor (line 15) will then read the cookie automatically with credentials.- Implement Content Security Policy (CSP): Add CSP headers to index.html to further restrict XSS attack surface (no external scripts, no unsafe-eval, etc.).
- Validate input handling: The deeplink-login.tsx already sanitizes URL parameters before use (lines 13-51), which is good; ensure this pattern is consistent across the application.
If backend changes are not immediately feasible, consider storing short-lived access tokens in memory and moving only refresh tokens to httpOnly cookies.
platforms/eVoting/src/components/auth/login-screen.tsx-15-35 (1)
15-35: Auto‑login failures never fall back to QR/SSE flow, and can leave the UI stuck in a “connecting” stateWhen deeplink params are present, this effect returns after calling
handleAutoLogin, sogetAuthOfferis never run on that mount (Line 31). InhandleAutoLogin, all non‑success paths simply clearisConnecting(Lines 71, 75) but never start the normal QR/SSE flow or reload without query params. As a result:
- If the auto‑login fails (network error, invalid signature, backend 4xx/5xx),
qrCodeandsessionIdremain empty, so the user just sees the “Loading QR Code…” skeleton forever, with no active request behind it.- If
response.okis true but the payload is malformed (notokenoruser), the code doesn’t resetisConnectingor provide any fallback, so the spinner can remain indefinitely with no way out except a manual hard refresh.This is a user‑visible functional issue for all deeplink failures.
Consider treating any non‑successful auto‑login path as “fall back to normal login”, e.g.:
- On failure (non‑OK or catch), either:
- Reload the page without query params (which you’re already removing via
replaceState), or- Explicitly trigger the existing
/api/auth/offer+ SSE flow.A minimal, localized option is to trigger a reload to the same path (now without search) in failure paths:
- } else { + } else { const errorData = await response.json(); console.error('Login failed:', errorData); if (errorData.error && errorData.type === 'version_mismatch') { setErrorMessage(errorData.message || 'Your eID Wallet app version is outdated. Please update to continue.'); } - setIsConnecting(false); + setIsConnecting(false); + // For non-version-mismatch errors, fall back to the normal QR/SSE flow + if (!errorData.error || errorData.type !== 'version_mismatch') { + window.location.href = window.location.pathname; + } } } catch (error) { console.error('Login request failed:', error); - setIsConnecting(false); + setIsConnecting(false); + // Network/unknown error: also fall back to QR/SSE flow + window.location.href = window.location.pathname; }This preserves the explicit version‑mismatch behavior while ensuring other failures don’t strand the user. Adjust the exact fallback (reload vs. calling the offer endpoint directly) to match your product expectations.
Also applies to: 58-76
🟡 Minor comments (4)
platforms/group-charter-manager/src/app/deeplink-login/page.tsx-55-57 (1)
55-57: Missing validation forbaseUrlenvironment variable.If
NEXT_PUBLIC_GROUP_CHARTER_BASE_URLis undefined,loginUrlwill beundefined/api/auth, causing a failed fetch. The eVoting implementation (lines 56-60) includes this check, but this file does not.// Make POST request to login endpoint const baseUrl = process.env.NEXT_PUBLIC_GROUP_CHARTER_BASE_URL; + if (!baseUrl) { + setError("Server configuration error"); + setIsLoading(false); + return; + } const loginUrl = `${baseUrl}/api/auth`;platforms/blabsy/src/pages/deeplink-login.tsx-77-84 (1)
77-84: Response validation differs intentionally due to Firebase custom token implementation.This implementation only checks for
data.token(line 79) because it relies on Firebase'ssignInWithCustomToken()to validate the token and extract user data implicitly. Other platforms check for bothdata.token && data.userbecause they use direct localStorage without Firebase validation. While this is an intentional design difference, consider whether explicit validation ofdata.useris needed for consistency and early error detection before passing to Firebase.platforms/dreamSync/client/src/components/auth/login-screen.tsx-29-29 (1)
29-29: Hardcoded default version may mask version requirements.The default
appVersionof'0.4.0'is applied when the parameter is missing. This could mask clients that don't send version information, potentially allowing outdated clients to authenticate. Consider requiring theappVersionparameter or explicitly documenting why this default is acceptable.If
appVersionshould be required, apply this diff:- handleAutoLogin(ename, session, signature, appVersion || '0.4.0'); + if (!appVersion) { + setErrorMessage('App version is required'); + setIsLoading(false); + return; + } + handleAutoLogin(ename, session, signature, appVersion);platforms/dreamSync/client/src/components/auth/login-screen.tsx-24-30 (1)
24-30: Add client-side validation for URL query parameters before auto-login.Query parameters (
ename,session,signature) are extracted and sent directly to the backend without validation. The current check at line 24 only verifies they are non-empty; it doesn't validate format, length, or content. Consider adding validation to fail fast on malformed inputs:
ename: Expected format and max length?session: Expected format and max length?signature: Expected format (hex string?) and length?appVersion: Should missing/invalid version error rather than default to '0.4.0'?Also, the generic catch block at line 82 doesn't surface auth-specific validation errors to the user—only
version_mismatcherrors have user-facing messages.
🧹 Nitpick comments (21)
platforms/eCurrency/client/src/lib/apiClient.ts (1)
3-3: Env-based baseURL aligns with fail-fast config; consider explicit assertion if you want stricter failureUsing
VITE_ECURRENCY_BASE_URLwith no fallback is a good move and matches the pattern of requiring critical URLs to be set via env. If you want truly fail‑fast behavior instead of axios silently using the current origin when the env is missing, you could add an explicit runtime check:-const baseURL = import.meta.env.VITE_ECURRENCY_BASE_URL; +const baseURL = import.meta.env.VITE_ECURRENCY_BASE_URL; + +if (!baseURL) { + throw new Error("VITE_ECURRENCY_BASE_URL is not defined"); +}This makes misconfiguration obvious during startup instead of manifesting as misrouted API calls. Based on learnings, this would mirror the intended behavior of other
VITE_*_BASE_URLvariables in the repo.infrastructure/control-panel/src/routes/api/evaults/[evaultId]/details/+server.ts (1)
24-26: Consider logging the underlying error in health check failure.The catch block silently converts all errors to "Unreachable" without logging details. For debugging, consider logging the caught error while preserving the user-facing response.
} catch { + console.debug(`Health check failed for eVault '${evaultId}':`, error); healthStatus = 'Unreachable'; }platforms/dreamsync-api/src/controllers/AuthController.ts (1)
140-143: Consider addingreturnfor consistency.The final response doesn't use
return, unlike other response statements in this function. While functionally correct (end of try block), addingreturnimproves consistency and guards against accidental code additions after the response.// Emit via SSE for backward compatibility this.eventEmitter.emit(session, data); // Return JSON response for direct POST requests - res.status(200).json(data); + return res.status(200).json(data);platforms/pictique/src/routes/(auth)/auth/+page.svelte (1)
76-90: Extract hardcoded appVersion default to a constant.The hardcoded
'0.4.0'default appears in multiple places (also in the new deeplink-login route). This duplication makes version updates error-prone.Additionally, consider adding a loading indicator during auto-login, as users currently receive no feedback that authentication is in progress.
Add a constant at the top of the file:
const DEFAULT_APP_VERSION = '0.4.0';Then update line 88:
-await handleAutoLogin(ename, session, signature, appVersion || '0.4.0'); +await handleAutoLogin(ename, session, signature, appVersion || DEFAULT_APP_VERSION);Consider setting a loading state before calling
handleAutoLogin:if (ename && session && signature) { // Clean up URL window.history.replaceState({}, '', window.location.pathname); + errorMessage = 'Authenticating...'; // Auto-submit login await handleAutoLogin(ename, session, signature, appVersion || DEFAULT_APP_VERSION); return; }infrastructure/eid-wallet/src/lib/global/controllers/evault.ts (1)
141-174: Reduce verbosity and avoid logging the full public key.The new banner-style logs are helpful during debugging, but logging the full
publicKeyin addition to a 60‑char prefix will spam logs and may not be desirable in production. Consider keeping only the truncated preview and gating the separator/banner logs behind a debug flag or environment check.- console.log( - `Public key retrieved: ${publicKey?.substring(0, 60)}...`, - ); - console.log(`Public key (full): ${publicKey}`); + console.log( + `Public key retrieved (first 60 chars): ${publicKey?.substring(0, 60)}...`, + );infrastructure/control-panel/src/lib/services/registry.ts (1)
18-137: Reconsider hard‑coded staging fallback and dev IP fallbacks inRegistryService.Using
env.PUBLIC_REGISTRY_URL || 'https://registry.staging.metastate.foundation'means a missing/misconfigured env will silently send traffic to staging, and the platform fallback returns fixed192.168.*dev URLs even when the real registry is down. It’s safer to fail fast (or at least log and throw) whenPUBLIC_REGISTRY_URLis unset, and optionally gate the dev IP fallbacks behind a dev flag.- constructor() { - this.baseUrl = env.PUBLIC_REGISTRY_URL || 'https://registry.staging.metastate.foundation'; - } + constructor() { + const url = env.PUBLIC_REGISTRY_URL; + if (!url) { + throw new Error('PUBLIC_REGISTRY_URL is required for RegistryService'); + } + this.baseUrl = url; + }Based on learnings, this matches the existing fail‑fast pattern for similar base URLs.
infrastructure/control-panel/src/routes/+page.svelte (1)
299-314: Uptime and IP are now permanently shown asN/A.Given the current
EVaultshape doesn’t expose uptime/IP, hard‑coding'N/A'is understandable, but it means these columns will never show real values even if new metrics become available. Consider wiring these to the new metrics/monitoring data once it exists, or leave a TODO so this isn’t forgotten.infrastructure/control-panel/src/lib/services/evaultService.ts (1)
117-154: New API methods look good; consider adding a type for metrics.The
getEVaultLogsByPodandgetEVaultMetricsmethods follow the established error handling pattern. However,getEVaultMetricsreturnsPromise<any>which provides no type safety.Consider defining an interface for the metrics response:
interface EVaultMetrics { resources?: { cpu?: string; memory?: string; note?: string }; status?: { podStatus?: string; podAge?: string; events?: string[]; conditions?: string[] }; logs?: { totalLines?: number; errorCount?: number; warningCount?: number; lastUpdate?: string }; } static async getEVaultMetrics(namespace: string, podName: string): Promise<EVaultMetrics> { // ... }infrastructure/eid-wallet/src/routes/(app)/scan-qr/scanLogic.ts (1)
287-297: Add error handling foropenUrland consider dynamic app version.Two concerns:
openUrlcan fail (e.g., no handler for URL scheme) but has no try/catchappVersionis hardcoded as"0.4.0"- consider sourcing from package.json or a config- loginUrl.searchParams.set("appVersion", "0.4.0"); - - console.log(`🔗 Opening login URL: ${loginUrl.toString()}`); - - // Open URL in browser using tauri opener - await openUrl(loginUrl.toString()); + // Consider: import { version } from '$app/environment' or package.json + loginUrl.searchParams.set("appVersion", "0.4.0"); + + console.log(`🔗 Opening login URL: ${loginUrl.toString()}`); + + // Open URL in browser using tauri opener + try { + await openUrl(loginUrl.toString()); + } catch (openError) { + console.error("Failed to open login URL:", openError); + throw new Error("Failed to open authentication page. Please try again."); + }platforms/blabsy/src/components/login/login-main.tsx (1)
86-109: Auto-login trigger is solid; reconsider hardcoded appVersion and broader URL cleanupThe auto-login detection and
windowguard look good, and cleaning the URL to remove sensitive query params is the right idea. A couple of refinements to consider:
Line [101]:
appVersion || '0.4.0'bakes a magic version into the UI. Prefer either:
- Sending
undefined/omitting the field so the backend owns the defaulting logic, or- Reading the default from a single shared constant/env so version bumps don’t require hunting through components.
Lines [98]–[99]:
replaceStatewith justwindow.location.pathnamedrops all query parameters and hash fragments, not justename/session/signature/appVersion. If this route can be visited with unrelated params (e.g. tracking, redirect hints), you may want to reconstruct the URL with only the auth params stripped instead of discarding everything.platforms/eReputation/client/src/components/auth/login-screen.tsx (1)
55-83: Consider consolidating auth client usage and hardening error handling
handleAutoLoginworks, but a few refinements would improve robustness and consistency:
- You already have
apiClientconfigured; using it here (instead of barefetch) would centralize base URL, error handling, and interceptors.- The non-OK branch assumes a JSON error body; if the server returns HTML/plain text,
response.json()will throw, and you’ll hit the generic catch. Wrapping the JSON parse in a try/catch (as you do in the newDeeplinkLoginpage) would keep error messaging more precise.- If you expect
VITE_EREPUTATION_BASE_URLto be sometimes unset, consider aligning the fallback here with the deeplink-login page (or vice versa) so both code paths behave identically in dev and staging.platforms/eReputation/client/src/pages/deeplink-login.tsx (1)
9-88: Deeplink flow is solid; align base URL handling and clean up unused hookThe deeplink parsing, validation, URL cleanup, and error handling look good and match the pattern used in other platforms.
A couple of small improvements:
const apiBaseUrl = import.meta.env.VITE_EREPUTATION_BASE_URL;will produce"undefined/api/auth"if the env var is missing. Inlogin-screen.tsxyou useimport.meta.env.VITE_EREPUTATION_BASE_URL || "http://localhost:8765". For consistency and safer local/dev behavior, consider using the same fallback here or defaulting to a relative/api/authwhen the env is absent.const { login } = useAuth();is currently unused. Either remove it to avoid dead code, or call it once the token/user are stored if your auth context expects an explicit login signal.platforms/group-charter-manager/src/app/deeplink-login/page.tsx (2)
14-14: Unused variable declaration.The
paramsvariable is declared at line 14 but only assigned at line 38. Consider removing the early declaration.- let params: URLSearchParams; let searchString = window.location.search;Then at line 38:
- params = new URLSearchParams(searchString); + const params = new URLSearchParams(searchString);
70-70: Inconsistent post-auth navigation.This component uses
window.location.reload()after successful authentication, while other platforms (eVoting, blabsy, dreamSync) usewindow.location.href = "/". Consider using consistent navigation for a unified user experience and to avoid unnecessary re-fetching of resources.platforms/eVoting/src/app/deeplink-login/page.tsx (1)
6-99: LGTM with minor note on code duplication.This implementation correctly validates the
baseUrlenvironment variable and handles all error cases appropriately. The authentication flow is well-structured with proper loading and error states.Note: This component is nearly identical to implementations in other platforms (group-charter-manager, blabsy, dreamSync, etc.). Consider extracting the shared deeplink login logic into a reusable package or shared utility to reduce maintenance burden across platforms.
platforms/eCurrency/client/src/components/auth/login-screen.tsx (1)
41-57: Remove debug logging statements before merging.Console logs with emoji prefixes (🔍, ✅, ❌) appear to be debug statements. Consider removing them or replacing with a proper logging utility that supports log levels for production.
platforms/eCurrency/client/src/App.tsx (1)
15-33: Redundant route definition for/deeplink-login.The early return at lines 18-24 handles
/deeplink-loginfor all cases (regardless of authentication state), making the route at line 33 unreachable. Consider removing the duplicate route from the authenticated Switch.return ( <Switch> - <Route path="/deeplink-login" component={DeeplinkLogin} /> <Route path="/dashboard" component={Dashboard} /> <Route path="/currencies" component={Currencies} /> <Route path="/currency/:currencyId" component={CurrencyDetail} /> <Route path="/" component={Dashboard} /> </Switch> );platforms/dreamSync/client/src/components/auth/login-screen.tsx (1)
17-54: Potential race condition between auto-login and normal flow.The
useEffecton lines 17-54 contains both the auto-login check and the normal QR code fetch. If the auto-login check passes, it returns early, but there's no dependency management to prevent the normal flow from also executing in edge cases. The logic appears correct, but the separation of concerns could be clearer.Consider splitting into two separate effects:
+ // Check for query parameters and auto-login useEffect(() => { const params = new URLSearchParams(window.location.search); const ename = params.get('ename'); const session = params.get('session'); const signature = params.get('signature'); const appVersion = params.get('appVersion'); if (ename && session && signature) { // Clean up URL window.history.replaceState({}, '', window.location.pathname); // Auto-submit login handleAutoLogin(ename, session, signature, appVersion || '0.4.0'); - return; } + }, []); - // If no query params, proceed with normal flow + // Fetch QR code for normal login flow + useEffect(() => { + const params = new URLSearchParams(window.location.search); + if (params.get('ename')) return; // Skip if auto-login params present + const getAuthOffer = async () => { // ... existing code }; getAuthOffer(); }, []);platforms/eCurrency/client/src/pages/deeplink-login.tsx (1)
12-38: Simplify URL parameter parsing logic.The URL parsing logic attempts multiple fallback strategies (search string, hash, full URL). This complexity may be unnecessary. Modern browsers reliably support
window.location.search. If you need hash-based routing support, document why and consider using a URL routing library.Simplify to:
- // Try parsing from search string first - let params: URLSearchParams; - let searchString = window.location.search; - - // If search is empty, try parsing from hash or full URL - if (!searchString || searchString === '') { - const hash = window.location.hash; - if (hash && hash.includes('?')) { - searchString = hash.substring(hash.indexOf('?')); - } else { - try { - const fullUrl = new URL(window.location.href); - searchString = fullUrl.search; - } catch (e) { - // Ignore parsing errors - } - } - } - - // Remove leading ? if present - if (searchString.startsWith('?')) { - searchString = searchString.substring(1); - } - - // Parse the search string - params = new URLSearchParams(searchString); + const params = new URLSearchParams(window.location.search);If hash-based routing is required, add a comment explaining the use case.
platforms/eVoting/src/components/auth/login-screen.tsx (2)
65-76: Surface a generic error message for non–version‑mismatch auto‑login failuresRight now only
version_mismatcherrors seterrorMessage(Lines 68‑70). For all other server errors (4xx/5xxwith a JSON body) and network exceptions in thecatchblock, the user loses the spinner (isConnectingbecomesfalse) but still sees the “Loading QR Code…” skeleton with no explanation.To avoid completely silent failures, consider setting a generic error message in both branches, something like:
} else { const errorData = await response.json(); console.error('Login failed:', errorData); if (errorData.error && errorData.type === 'version_mismatch') { setErrorMessage(errorData.message || 'Your eID Wallet app version is outdated. Please update to continue.'); + } else { + setErrorMessage('Auto login failed. Please try again or scan the QR code with your wallet.'); } setIsConnecting(false); } } catch (error) { console.error('Login request failed:', error); - setIsConnecting(false); + setIsConnecting(false); + setErrorMessage('Unable to reach the authentication service. Please try again or use the QR code flow.'); }This keeps the special handling for version mismatches while giving users a clear path when auto‑login fails for other reasons.
51-55: Deduplicate EVoting base URL usage for maintainabilityYou currently hard‑code the EVoting base URL twice:
- In
handleAutoLoginviaconst baseUrl = process.env.NEXT_PUBLIC_EVOTING_BASE_URL || "http://localhost:4000";(Lines 51‑52).- In the SSE
EventSourceURL (Lines 82‑84).To avoid drift between these two and make environment changes safer, consider extracting a shared constant at the top of the file and reusing it:
// Near the top of the file const EVOTING_BASE_URL = process.env.NEXT_PUBLIC_EVOTING_BASE_URL || "http://localhost:4000";Then:
- const baseUrl = process.env.NEXT_PUBLIC_EVOTING_BASE_URL || "http://localhost:4000"; - const response = await fetch(`${baseUrl}/api/auth`, { + const response = await fetch(`${EVOTING_BASE_URL}/api/auth`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ ename, session, signature, appVersion }) });and:
- const eventSource = new EventSource( - `${process.env.NEXT_PUBLIC_EVOTING_BASE_URL || "http://localhost:4000"}/api/auth/sessions/${sessionId}` - ); + const eventSource = new EventSource( + `${EVOTING_BASE_URL}/api/auth/sessions/${sessionId}` + );This makes configuration updates and future refactors less error‑prone.
Also applies to: 82-84
Description of change
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.