Skip to content

Commit 1ea0809

Browse files
authored
Merge pull request #27 from HarperFast/issue-26
disambiguate session provider
2 parents 996da05 + 8631fc3 commit 1ea0809

File tree

7 files changed

+44
-17
lines changed

7 files changed

+44
-17
lines changed

docs/multi-tenant-sso.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ registerHooks({
373373
id: user.email,
374374
name: user.name,
375375
role: user.role || 'user',
376-
tenant: session.oauth.provider,
376+
tenant: session.oauth.providerConfigId, // Config ID = tenant ID
377377
});
378378
}
379379

src/index.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -182,41 +182,42 @@ export async function handleApplication(scope: Scope): Promise<void> {
182182
return next(request);
183183
}
184184

185-
// Get the provider for this OAuth session
186-
const providerName = request.session.oauth.provider;
187-
let providerData = providers[providerName];
185+
// Get the provider config ID for this OAuth session
186+
// Use providerConfigId (new) or fall back to provider (old) for backwards compatibility
187+
const providerConfigId = request.session.oauth.providerConfigId || request.session.oauth.provider;
188+
let providerData = providers[providerConfigId];
188189

189190
// If provider not found in registry, try to resolve via hook (dynamic providers)
190191
if (!providerData && hookManager?.hasHook('onResolveProvider')) {
191192
try {
192-
logger?.debug?.(`Provider "${providerName}" not in registry, attempting dynamic resolution`);
193+
logger?.debug?.(`Provider config "${providerConfigId}" not in registry, attempting dynamic resolution`);
193194

194-
const hookConfig = await hookManager.callResolveProvider(providerName, logger);
195+
const hookConfig = await hookManager.callResolveProvider(providerConfigId, logger);
195196

196197
if (hookConfig) {
197198
// Hook resolved provider - build full config and register dynamically
198199
const { OAuthProvider } = await import('./lib/OAuthProvider.ts');
199200
const { buildProviderConfig } = await import('./lib/config.ts');
200201

201202
// Build full provider config (handles Okta/Azure/Auth0 domain configuration)
202-
const config = buildProviderConfig(hookConfig, providerName, pluginDefaults);
203+
const config = buildProviderConfig(hookConfig, providerConfigId, pluginDefaults);
203204
const provider = new OAuthProvider(config, logger);
204205

205-
providers[providerName] = {
206+
providers[providerConfigId] = {
206207
provider,
207208
config,
208209
};
209210

210-
providerData = providers[providerName];
211-
logger?.info?.(`Dynamically registered provider for session validation: ${providerName}`);
211+
providerData = providers[providerConfigId];
212+
logger?.info?.(`Dynamically registered provider for session validation: ${providerConfigId}`);
212213
}
213214
} catch (error) {
214-
logger?.error?.(`Error resolving provider ${providerName} for session:`, (error as Error).message);
215+
logger?.error?.(`Error resolving provider ${providerConfigId} for session:`, (error as Error).message);
215216
}
216217
}
217218

218219
if (!providerData) {
219-
logger?.warn?.(`OAuth provider '${providerName}' not found, logging out user`);
220+
logger?.warn?.(`OAuth provider config '${providerConfigId}' not found, logging out user`);
220221
// Provider no longer exists - complete logout
221222
await clearOAuthSession(request.session, logger);
222223
return next(request);
@@ -229,7 +230,7 @@ export async function handleApplication(scope: Scope): Promise<void> {
229230
// Session is no longer valid (already cleaned up by validator)
230231
logger?.debug?.(`OAuth session invalidated: ${validation.error}`);
231232
} else if (validation.refreshed) {
232-
logger?.debug?.(`OAuth token auto-refreshed for ${providerName}`);
233+
logger?.debug?.(`OAuth token auto-refreshed for ${providerConfigId}`);
233234
}
234235

235236
// Continue with the request (session updated if refreshed)

src/lib/handlers.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,13 +219,13 @@ export async function handleCallback(
219219
// Leave expiresAt and refreshThreshold undefined so middleware doesn't try to refresh
220220

221221
// Prepare session data
222-
// Store providerName (registry key) not config.provider (provider type)
223-
// This ensures session validation can look up the correct provider
224222
const sessionData: any = {
225223
user: hookData?.user || user.username, // Use hook's user if provided, otherwise OAuth username
226224
oauthUser: user, // Store full OAuth user object separately
227225
oauth: {
228-
provider: providerName, // Store registry key (e.g., 'harperdb') not provider type (e.g., 'okta')
226+
provider: providerName, // Config key (backwards compatible - e.g., 'my-custom-github', 'production-okta')
227+
providerConfigId: providerName, // Config key/ID (clearer naming for new code)
228+
providerType: config.provider, // Provider type (e.g., 'github', 'okta')
229229
accessToken: tokenResponse.access_token,
230230
refreshToken: tokenResponse.refresh_token,
231231
expiresAt,
@@ -358,6 +358,8 @@ export async function handleUserInfo(request: Request, tokenRefreshed = false):
358358
oauth: oauthMetadata
359359
? {
360360
provider: oauthMetadata.provider,
361+
providerConfigId: oauthMetadata.providerConfigId,
362+
providerType: oauthMetadata.providerType,
361363
expiresAt: oauthMetadata.expiresAt,
362364
refreshThreshold: oauthMetadata.refreshThreshold,
363365
lastRefreshed: oauthMetadata.lastRefreshed,

src/lib/sessionValidator.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,8 @@ export async function validateAndRefreshSession(
151151
// Update session with new tokens and metadata
152152
const updatedMetadata: OAuthSessionMetadata = {
153153
provider: oauthMetadata.provider,
154+
providerConfigId: oauthMetadata.providerConfigId,
155+
providerType: oauthMetadata.providerType,
154156
accessToken: tokenResponse.access_token,
155157
refreshToken: tokenResponse.refresh_token || oauthMetadata.refreshToken, // Keep existing if not provided
156158
expiresAt: newExpiresAt,

src/types.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,8 +315,15 @@ export interface Request extends IncomingMessage {
315315
* Token and expiration data stored in session for automatic refresh
316316
*/
317317
export interface OAuthSessionMetadata {
318-
/** OAuth provider name (e.g., 'github', 'google') */
318+
/**
319+
* Provider configuration ID/key from config (e.g., 'my-custom-github', 'production-okta').
320+
* @deprecated Use `providerConfigId` instead. This field is maintained for backwards compatibility only.
321+
*/
319322
provider: string;
323+
/** Provider configuration ID/key from config (e.g., 'my-custom-github', 'production-okta'). */
324+
providerConfigId: string;
325+
/** OAuth provider type (e.g., 'github', 'google', 'okta') */
326+
providerType: string;
320327
/** Current access token */
321328
accessToken: string;
322329
/** Refresh token for obtaining new access tokens */

test/lib/handlers.test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,9 @@ describe('OAuth Handlers', () => {
367367
// Token data is now stored in oauth object
368368
assert.ok(mockRequest.session.oauth);
369369
assert.equal(mockRequest.session.oauth.accessToken, 'access-token-123');
370+
assert.equal(mockRequest.session.oauth.provider, 'test-provider', 'provider (config key) should be set');
371+
assert.equal(mockRequest.session.oauth.providerConfigId, 'test-provider', 'providerConfigId should be set');
372+
assert.equal(mockRequest.session.oauth.providerType, 'test', 'providerType should match config.provider');
370373
});
371374

372375
it('should handle tokens without expiration (GitHub style)', async () => {

test/lib/sessionValidator.test.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ test('should preserve provider field during token refresh', async () => {
131131
const session = createMockSession({
132132
oauth: {
133133
provider: 'google',
134+
providerConfigId: 'google',
135+
providerType: 'google',
134136
accessToken: 'old_token',
135137
refreshToken: 'old_refresh',
136138
expiresAt: Date.now() - 1000, // Already expired
@@ -146,6 +148,16 @@ test('should preserve provider field during token refresh', async () => {
146148
// BUG: This assertion will FAIL before the fix!
147149
// The provider field gets lost during session.update()
148150
assert.strictEqual(session.oauth.provider, 'google', 'provider field should be preserved after token refresh');
151+
assert.strictEqual(
152+
session.oauth.providerConfigId,
153+
'google',
154+
'providerConfigId field should be preserved after token refresh'
155+
);
156+
assert.strictEqual(
157+
session.oauth.providerType,
158+
'google',
159+
'providerType field should be preserved after token refresh'
160+
);
149161
});
150162

151163
test('should preserve custom session fields from hooks during refresh', async () => {

0 commit comments

Comments
 (0)