-
Notifications
You must be signed in to change notification settings - Fork 19
Add multi-tenancy support for WebAuthn authentication #993
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add multi-tenancy support for WebAuthn authentication #993
Conversation
- Add tenant utilities (src/lib/tenant.ts) for tenant ID storage and API path building
- Add TenantContext (src/lib/TenantContext.tsx) for URL-based tenant extraction
- Update API to use tenant-scoped registration endpoints (/t/{tenantId}/user/register-webauthn-*)
- Login remains global (tenant-discovering from passkey userHandle)
- Add tenant-scoped routes (/:tenantId/*) to App.jsx
- Pass tenant context to signup flow in Login.tsx
- Backward compatible with single-tenant deployments
Implements frontend support for go-wallet-backend ADR 011 multi-tenancy
src/lib/tenant.ts
Outdated
| */ | ||
| export function buildTenantApiPath(tenantId: string, basePath: string, useApiPrefix: boolean = true): string { | ||
| const cleanPath = basePath.startsWith('/') ? basePath : `/${basePath}`; | ||
| if (useApiPrefix) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it could be nice to use a ternary here as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ternary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Jesse is talking about the multiple return statements. Me and Pascal also thought about this but thought of a different approach, perhaps constructing the url out of the different path elements and then returning. For example:
export function buildTenantApiPath(tenantId: string, basePath: string, useApiPrefix: boolean = true): string {
const pathParts = [
tenantId,
...basePath.split('/').filter(String),
];
if (useApiPrefix) pathParts.unshift('t');
return `/${pathParts.join('/')}`;
}
src/lib/tenant.ts
Outdated
| * | ||
| * @param tenantId - The tenant ID to scope to | ||
| * @param basePath - The base path (e.g., '/user/register-webauthn-begin') | ||
| * @param useApiPrefix - Whether to use /t/ prefix (for public tenant routes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the parameter useApiPrefix does not seem to be used anywhere yet. I do not understand the distinction between public and private tenant routes.
smncd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying this out with the go-wallet-backend, things seem to work, except for when I add a tenant and try to use it to sign up, e.g. http://localhost:3000/test/login. This gives me a "verification failed" error. from the webauthn finish endpoint. This is only the case when the tenant is specified in the url.
src/App.jsx
Outdated
| * Routes that require authentication. | ||
| * These are the same for both global and tenant-scoped contexts. | ||
| */ | ||
| const AuthenticatedRoutes = () => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The routes here look to be replicated further down for the non-tenant "fallbacks".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could consider exploring react-router's data mode as a replacement for the current react-router-dom, their loader and action capabilities could be interesting for taking things out of JSX.
https://reactrouter.com/start/modes#data
src/lib/TenantContext.tsx
Outdated
| isMultiTenant: !!effectiveTenantId, | ||
| switchTenant, | ||
| clearTenant, | ||
| }), [effectiveTenantId]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint says: missing dependency: 'switchTenant'
|
I got past the issue with the registering/login, but it doesn't seem to pick up the tenant from the passkey as intended. This would need to take place before the webauthn-finish request is made to the backend, no? |
- Move TenantContext.tsx to src/context/ per project conventions - Fix ESLint missing switchTenant dependency using useCallback - Remove unused useApiPrefix parameter from buildTenantApiPath - Fix route duplication by reusing authenticatedRoutes() helper - Update all import paths after moving TenantContext
|
Pushed a bunch of fixes to the comments just now. The issue identified by @smncd was likely due to a bug on the backend. This was just fixed in go-wallet-backend#main. Please check if it looks better now! |
|
I pushed some more fixes to the backend that clarified the security model: only users with no prefix or "default" are allowed to login to the default tenant. |
|
The e2e tests have also been updated to cover this. |
|
I think we need some guidance on how to properly test this @leifj, I'm still running into the same "verification failed" error when signing up when a tenant is specified in the url. It would also be useful to hear more how this is meant to work so we can better test the PR. |
| // This ensures the passkey's userHandle encodes the tenant for proper isolation | ||
| const storedTenant = tenantId || getStoredTenant(); | ||
| const registerBeginPath = storedTenant | ||
| ? buildTenantApiPath(storedTenant, '/user/register-webauthn-begin') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would put the tenant section at a higher level for all api requests to contain the tenant in their URLs, it would help to factorize the paths creation. For instance, login begin/finish paths do not contain the tenant. Or I may be wrong and tenants may be filled in only for registration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumping this, the wallet needs to check if the tenant it's on exists and is the "correct one". I'm also in favour of requiring the tenant ID in all api routes in the backend, this will make the backend <-> frontend relationship easier to follow.
After successful WebAuthn login, navigate to the correct path based on
the tenant returned by the backend:
- Non-default tenants: /{tenantId}/
- Default tenant: /
This ensures users with tenant-scoped passkeys are directed to their
tenant's home page rather than always going to the root path.
Added helpers to tenant.ts:
- DEFAULT_TENANT_ID constant ('default')
- isDefaultTenant() - checks if tenant is the default
- buildTenantRoutePath() - builds frontend route path for tenant
- Redirect authenticated users to correct tenant path on URL mismatch
- Scenario 1: Default tenant user at /default/* → redirect to /
- Scenario 2: Non-default tenant user at / → redirect to /{tenantId}/
- Scenario 3: User at wrong tenant /B/ → redirect to /{correctTenantId}/
- Scenario 4: Default tenant user at /other-tenant/ → redirect to /
This ensures:
- Users always see URLs matching their authenticated tenant
- Default tenant users never see 'default' in the URL path
- Cross-tenant URL access is prevented for authenticated users
When an authenticated user navigates to a different tenant's URL path
(e.g., default tenant user accessing /{tenantId}/), the TenantProvider
was incorrectly overwriting their stored tenant ID with the URL tenant.
This fix ensures that:
1. If a user is already authenticated (has stored tenant), keep it
2. Only sync URL tenant to storage for unauthenticated users
3. PrivateRoute will redirect authenticated users to their correct tenant
This completes the tenant isolation by preventing navigation-based
tenant switching for authenticated users.
- Add buildPath helper to TenantContext for building tenant-aware paths - Update Sidebar.jsx and BottomNav.jsx navigation items to use buildPath - Update Home.jsx navigation to use buildPath for add, pending, and credential pages - Update Credential.jsx to use buildPath for history and details navigation - Update CredentialLayout.jsx breadcrumb link to use buildPath - Update HistoryList.jsx mobile navigation to use buildPath - Update PinInput.jsx cancel navigation to use buildPath - Update NotFound.jsx home button to use buildPath - Update LoginState.jsx redirects and cancel button to use buildPath - Update OpenID4VCI.ts transaction completion redirect to use buildPath - Update PrivateRoute.tsx to preserve tenant path in login redirects This ensures non-default tenant users stay on their tenant-scoped URLs without unnecessary redirects to the default tenant paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making progress! Looking better, but one issue is now that on the /settings page (with or without tenantid in the path), I get these fatal errors, and no rendering of the settings page.
It seems like this is due to the backend returning the following from http://localhost:8080/user/session/account-info in the go-wallet-backend:
{
"uuid": "<REDACTED>",
"displayName": "<REDACTED>",
"hasPassword": false,
"settings": {},
"webauthnCredentials": [
{
"id": "<REDACTED>",
"credentialId": null, // <-- the frontend requires this to be typeof BufferSource
"prfCapable": false,
"createTime": "2026-01-20T15:02:53.402824954+01:00"
}
]
}The frontend expects this to be a BufferSource. The failure originates here:
Lines 8 to 14 in 711d944
| export function toU8(b: BufferSource) { | |
| if (b instanceof ArrayBuffer) { | |
| return new Uint8Array(b); | |
| } else { | |
| return new Uint8Array(b.buffer); | |
| } | |
| } |
| export function useTenant(): TenantContextValue { | ||
| const context = useContext(TenantContext); | ||
| if (!context) { | ||
| // Return a default context for components outside TenantProvider | ||
| // This allows the app to work in single-tenant mode | ||
| const storedTenant = getStoredTenant(); | ||
| return { | ||
| tenantId: storedTenant, | ||
| isMultiTenant: false, | ||
| switchTenant: () => { | ||
| console.warn('switchTenant called outside TenantProvider'); | ||
| }, | ||
| clearTenant: clearStoredTenant, | ||
| buildPath: (subPath?: string) => buildTenantRoutePath(storedTenant, subPath), | ||
| }; | ||
| } | ||
| return context; | ||
| } | ||
|
|
||
| /** | ||
| * Hook to get tenant ID, throwing if not available. | ||
| * Use this when tenant is required (e.g., in tenant-scoped routes). | ||
| */ | ||
| export function useRequiredTenant(): string { | ||
| const { tenantId } = useTenant(); | ||
| if (!tenantId) { | ||
| throw new Error('Tenant ID is required but not available. Ensure this component is within a tenant-scoped route.'); | ||
| } | ||
| return tenantId; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd argue that both of these hooks should be in a separate file in src/hooks, say src/hooks/tenants.ts.
(for reference, currently most contexts are used in components without a "wrapper hook", like useContext(CredentialsContext). I think this should be standardised in a wrapper hook for consistency, but those are a later worry)
smncd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've now tested the PR with the node wallet-backend and it looks good! Some things I noticed is referenced in the review comments but to summarize:
If I provide a tenant ID in the url path on login or registration, this sticks, even if the tenant ID in question doesn't exist. This can be solved by some of the suggestions I made, as well as Pascal's suggestion of enforcing that all backend api paths should have the tenant ID in them.
| // This ensures the passkey's userHandle encodes the tenant for proper isolation | ||
| const storedTenant = tenantId || getStoredTenant(); | ||
| const registerBeginPath = storedTenant | ||
| ? buildTenantApiPath(storedTenant, '/user/register-webauthn-begin') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumping this, the wallet needs to check if the tenant it's on exists and is the "correct one". I'm also in favour of requiring the tenant ID in all api routes in the backend, this will make the backend <-> frontend relationship easier to follow.
src/api/index.ts
Outdated
| if (finishResp?.data?.tenantId) { | ||
| setStoredTenant(finishResp.data.tenantId); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (finishResp?.data?.tenantId) { | |
| setStoredTenant(finishResp.data.tenantId); | |
| } | |
| setStoredTenant(finishResp.data.tenantId ?? "default"); |
If we don't set a stored tenant ID but a tenant ID is present in the URL, the URL tenant ID will be used, regardless if it's valid or not. If there is no tenant ID present in the response from the backend, is there a reason we shouldn't assume that we're on the "default" tenant?
| if (finishResp?.data?.tenantId) { | ||
| setStoredTenant(finishResp.data.tenantId); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (finishResp?.data?.tenantId) { | |
| setStoredTenant(finishResp.data.tenantId); | |
| } | |
| setStoredTenant(finishResp.data.tenantId ?? "default"); |
Same as above.
| } else { | ||
| return <Navigate to={`/login${window.location.search}`} replace />; | ||
| return <Navigate to={`${loginTenantPath}/login${window.location.search}`} replace />; | ||
| } | ||
| } | ||
|
|
||
| // Enforce tenant-aware routing for authenticated users | ||
| if (tenantRedirectPath) { | ||
| return <Navigate to={tenantRedirectPath} replace />; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When SWITCHING tenants, rather than do a React style navigation, we should always do a normal navigation reloading the full page. We potentially want to use index.html to store tenant specific config in the future, and this does not get loaded if we don't do a normal navigation event.
When a user selects a passkey for login, the tenant ID is now extracted
from the passkey's userHandle (format: tenantId:userId) and used to
determine the correct backend API path.
This ensures that regardless of which login page the user starts from
(/login or /{tenant}/login), the correct tenant-scoped backend endpoint
is used based on the passkey's embedded tenant.
Changes:
- Add extractTenantFromUserHandle() to parse tenant from userHandle
- Add buildLoginFinishPath() to determine correct login-finish API path
- Modify loginWebauthn() to use tenant-aware path based on passkey
- Store extracted tenant in session on successful login
- Add comprehensive unit tests for tenant extraction functions
The login-webauthn-begin endpoint must also be tenant-scoped for tenant-scoped users. The tenant is now extracted from the cached user's userHandleB64u (if available) before calling login-webauthn-begin. Changes: - Add buildLoginBeginPath() function for login-begin path - Extract tenant from cachedUser.userHandleB64u for begin call - Both begin and finish now use correct tenant-scoped paths - Add tests for buildLoginBeginPath()
When a user has no cached user in local storage and logs in via a global
/login path with a non-default tenant passkey, the frontend now:
1. Detects the tenant mismatch after passkey selection
2. Returns a new error type 'tenantDiscovered' with the discovered tenantId
3. Callers (Login.tsx, LoginState.jsx, SyncPopup.jsx) handle this by
redirecting to /{tenantId}/login
This ensures the second login attempt uses the correct tenant-scoped
begin/finish endpoints, which is required because the backend validates
that the challenge's tenant matches the request tenant.
The UX tradeoff is that first-time logins from /login with non-default
tenant passkeys require two passkey prompts (one to discover the tenant,
one to complete login). This is unavoidable without backend changes.
Issue 1: Cached user with tenant failed with 'Session was not initiated as a client-side discoverable login' error. Fix: For tenant-scoped login, do NOT include allowCredentials in the WebAuthn request. The backend's BeginTenantLogin/FinishTenantLogin use discoverable credential flow (BeginDiscoverableLogin/ValidateDiscoverableLogin). We can still include PRF extension inputs (evalByCredential) without allowCredentials - the browser will show discoverable credential picker and evaluate PRF for the selected credential. Issue 2: After tenant discovery redirect, login did not auto-retry. Fix: Add ?autoRetry=true query param when redirecting to tenant login. The WebauthnSignupLogin component now has an effect that detects this param and automatically triggers login, providing seamless UX where the user only needs to authenticate once with their passkey.
With the backend fix that removes UserID from FinishTenantLogin session data, we can now include allowCredentials for tenant-scoped login as well. This restores better UX by filtering the browser's credential picker to show only matching passkeys, rather than showing all discoverable credentials.
- Pass tenantId from URL path context to loginWebauthn() in all login components
- Handle tenant discovery redirect (409) by navigating to /{tenantId}/login?autoRetry=true
- Support auto-retry login after tenant discovery redirect
- Update Login.tsx, LoginState.jsx, and SyncPopup.jsx with tenant context
Implements frontend support for go-wallet-backend ADR 011 multi-tenancy