From 37e811d455dbc97254da50fdb5c7a72a543165b9 Mon Sep 17 00:00:00 2001 From: Dina Berry Date: Wed, 8 Apr 2026 08:55:40 -0700 Subject: [PATCH 1/2] fix(sdk): Teams adapter token security + migration guide --- .changeset/teams-adapter-security.md | 5 + docs/migration/teams-async-adapter.md | 129 +++++++ .../squad-sdk/src/platform/comms-teams.ts | 218 +++++++++-- packages/squad-sdk/src/platform/types.ts | 7 + test/comms-teams-security.test.ts | 363 ++++++++++++++++++ 5 files changed, 682 insertions(+), 40 deletions(-) create mode 100644 .changeset/teams-adapter-security.md create mode 100644 docs/migration/teams-async-adapter.md create mode 100644 test/comms-teams-security.test.ts diff --git a/.changeset/teams-adapter-security.md b/.changeset/teams-adapter-security.md new file mode 100644 index 000000000..99ce0319e --- /dev/null +++ b/.changeset/teams-adapter-security.md @@ -0,0 +1,5 @@ +--- +'@bradygaster/squad-sdk': patch +--- + +Teams adapter token security: tenant-scoped token cache (keyed by tenant ID hash), explicit revoke() for logout, 15-minute device-code timeout guard, stale token cleanup on permanent auth errors, per-instance user ID cache. Migration guide for async createCommunicationAdapter change. diff --git a/docs/migration/teams-async-adapter.md b/docs/migration/teams-async-adapter.md new file mode 100644 index 000000000..b3ebefc49 --- /dev/null +++ b/docs/migration/teams-async-adapter.md @@ -0,0 +1,129 @@ +# Migration Guide: Teams Adapter — Async Factory + Token Security + +> **Applies to:** Squad SDK ≥ v0.10.0 (PR #768) + +## Breaking Change: `createCommunicationAdapter` is now async + +PR #768 changed `createCommunicationAdapter` from a synchronous function to an async function that returns `Promise`. + +### Why + +The Teams adapter (`teams-graph` channel) requires interactive OAuth authentication (browser PKCE or device-code flow). These operations are inherently asynchronous. Making the factory async ensures all adapters — including those requiring network auth — can be created through the same interface. + +### Before (v0.9.x — synchronous) + +```typescript +import { createCommunicationAdapter } from '@bradygaster/squad-sdk'; + +const adapter = createCommunicationAdapter(repoRoot); +await adapter.postUpdate({ title: 'Hello', body: 'World' }); +``` + +### After (v0.10.x — async) + +```typescript +import { createCommunicationAdapter } from '@bradygaster/squad-sdk'; + +const adapter = await createCommunicationAdapter(repoRoot); +await adapter.postUpdate({ title: 'Hello', body: 'World' }); +``` + +### Migration steps + +1. Add `await` before every `createCommunicationAdapter()` call +2. Ensure the calling function is `async` +3. If the adapter is created at module top-level, wrap it in an async IIFE or move it into an `async` init function + +**Find affected code:** + +```bash +# Find all call sites in your codebase +grep -rn "createCommunicationAdapter" --include="*.ts" --include="*.js" +``` + +**Common patterns:** + +```typescript +// ❌ Module-level (breaks) +const adapter = createCommunicationAdapter(root); + +// ✅ Module-level (works) +let adapter: CommunicationAdapter; +async function init() { + adapter = await createCommunicationAdapter(root); +} + +// ✅ Inside an async function (simplest fix) +async function setup() { + const adapter = await createCommunicationAdapter(root); + // ... +} +``` + +--- + +## New: Token Security Improvements + +These changes ship alongside the async migration and require no code changes — they're internal to the Teams adapter. + +### 1. Identity-scoped token cache + +**Before:** All tenants shared a single token file (`~/.squad/teams-tokens.json`). In multi-tenant environments, one tenant's token could be served to another. + +**After:** Tokens are stored per-identity at `~/.squad/teams-tokens-{hash}.json`. The hash is derived from both the configured `tenantId` and `clientId`, preventing cross-tenant and cross-app token reuse. The actual authenticated identity (`tid` and `oid` from the JWT) is stored as metadata for audit. Legacy token files are automatically migrated on first use and then deleted. + +**Configuration:** Set `tenantId` in your `.squad/config.json` to explicitly scope tokens: + +```json +{ + "communications": { + "channel": "teams-graph", + "adapterConfig": { + "teams-graph": { + "tenantId": "contoso.onmicrosoft.com", + "recipientUpn": "bradyg@contoso.com" + } + } + } +} +``` + +> **Note:** When using the default multi-tenant authority (`organizations`), all users on the same OS account share one cache file per `clientId`. If you switch Microsoft accounts, call `logout()` or wait for token expiry. For true per-account isolation, configure an explicit `tenantId`. + +### 2. Explicit logout (`logout()`) + +The Teams adapter now exposes `logout()` for explicit credential cleanup: + +```typescript +const adapter = await createCommunicationAdapter(root); + +// ... use the adapter ... + +// Logout: clears in-memory tokens + deletes cached token file +if (adapter.logout) { + await adapter.logout(); +} +``` + +This is a local credential purge — it does not revoke server-side tokens (not supported for public-client AAD flows). The `CommunicationAdapter` interface now includes an optional `logout?(): Promise` method. + +### 3. Device-code timeout guard + +**Before:** The device-code auth flow timeout was server-controlled (could be arbitrarily long). + +**After:** A 15-minute maximum timeout is enforced client-side. The poll interval is also clamped to 2–30 seconds to prevent both rapid polling and excessively slow polling from malformed server responses. + +### 4. Stale token cleanup + +**Before:** On token refresh failure, stale tokens remained on disk and would be reloaded on the next attempt. + +**After:** On permanent auth errors (`invalid_grant`, `interaction_required`, `consent_required`), the stale token file is deleted before re-authenticating. Transient failures (network errors, 5xx) preserve the token for retry. + +--- + +## Checklist + +- [ ] Updated all `createCommunicationAdapter()` calls to use `await` +- [ ] Verified calling functions are `async` +- [ ] Set explicit `tenantId` in config (recommended for multi-tenant) +- [ ] Tested auth flow still works after upgrade diff --git a/packages/squad-sdk/src/platform/comms-teams.ts b/packages/squad-sdk/src/platform/comms-teams.ts index 608edaf6e..9cad475de 100644 --- a/packages/squad-sdk/src/platform/comms-teams.ts +++ b/packages/squad-sdk/src/platform/comms-teams.ts @@ -4,14 +4,14 @@ * Auth priority: cached token → refresh → browser PKCE → device code fallback. * Uses the Microsoft Graph PowerShell first-party client ID by default (works * in every Microsoft tenant with no custom Entra registration required). - * Tokens stored in ~/.squad/teams-tokens.json. + * Tokens stored per-identity in ~/.squad/teams-tokens-{hash(tenantId:clientId)}.json. * * @module platform/comms-teams */ import { join } from 'node:path'; import { homedir, platform } from 'node:os'; -import { existsSync, readFileSync, writeFileSync, mkdirSync, chmodSync } from 'node:fs'; +import { existsSync, readFileSync, writeFileSync, mkdirSync, chmodSync, unlinkSync } from 'node:fs'; import { createServer } from 'node:http'; import { randomBytes, createHash } from 'node:crypto'; import { execFile } from 'node:child_process'; @@ -38,44 +38,129 @@ export interface TeamsCommsConfig { teamId?: string; } -// ─── Token Storage ─────────────────────────────────────────────────── +// ─── Token Storage (identity-scoped) ───────────────────────────────── interface StoredTokens { accessToken: string; refreshToken: string; expiresAt: number; // epoch ms + /** Configured tenant authority — validated on load to prevent cross-config reuse */ + configTenantId?: string; + /** Client ID (app registration) — validated on load */ + clientId?: string; + /** Actual tenant GUID from JWT `tid` claim — tracks real authenticated identity */ + authenticatedTenantId?: string; + /** Actual user object ID from JWT `oid` claim — tracks real authenticated identity */ + authenticatedUserId?: string; } const SQUAD_DIR = join(homedir(), '.squad'); -const TOKEN_PATH = join(SQUAD_DIR, 'teams-tokens.json'); +/** Legacy single-file path (pre-tenant-scoped) — migrated away on first use */ +const LEGACY_TOKEN_PATH = join(SQUAD_DIR, 'teams-tokens.json'); + +/** + * Derive a safe, collision-resistant filename for a token cache entry. + * Uses SHA-256 hash of `tenantId + clientId` to avoid path traversal, + * special character issues, and to provide collision-resistant separation for different app registrations. Uses 16 hex chars (~64 bits) of SHA-256 — sufficient for practical uniqueness across tenant/app combinations. + */ +function getTokenPath(tenantId: string, clientId: string): string { + const hash = createHash('sha256').update(`${tenantId}:${clientId}`).digest('hex').slice(0, 16); + return join(SQUAD_DIR, `teams-tokens-${hash}.json`); +} -function loadTokens(): StoredTokens | null { +function loadTokens(tenantId: string, clientId: string): StoredTokens | null { + const tokenPath = getTokenPath(tenantId, clientId); try { - if (!existsSync(TOKEN_PATH)) return null; - const raw = readFileSync(TOKEN_PATH, 'utf-8'); - return JSON.parse(raw) as StoredTokens; + if (!existsSync(tokenPath)) return null; + const raw = readFileSync(tokenPath, 'utf-8'); + const parsed = JSON.parse(raw) as StoredTokens; + // Validate minimum required shape + if (typeof parsed.accessToken !== 'string' || typeof parsed.expiresAt !== 'number') return null; + // Reject tokens from a different config (stale/corrupted cache) + if (parsed.configTenantId && parsed.configTenantId !== tenantId) return null; + if (parsed.clientId && parsed.clientId !== clientId) return null; + return parsed; } catch { return null; } } // Security: tokens stored with 0o600 permissions (owner-only read/write). -// Consider OS keychain integration for production use. -function saveTokens(tokens: StoredTokens): void { +function saveTokens(tenantId: string, clientId: string, tokens: StoredTokens): void { + const tokenPath = getTokenPath(tenantId, clientId); if (!existsSync(SQUAD_DIR)) { mkdirSync(SQUAD_DIR, { recursive: true, mode: 0o700 }); } - writeFileSync(TOKEN_PATH, JSON.stringify(tokens, null, 2), { encoding: 'utf-8', mode: 0o600 }); + const jwtClaims = extractJwtClaims(tokens.accessToken); + const withMeta: StoredTokens = { + ...tokens, + configTenantId: tenantId, + clientId, + authenticatedTenantId: jwtClaims.tid, + authenticatedUserId: jwtClaims.oid, + }; + writeFileSync(tokenPath, JSON.stringify(withMeta, null, 2), { encoding: 'utf-8', mode: 0o600 }); // Ensure permissions are correct even if file already existed if (platform() === 'win32') { - execFile('icacls', [TOKEN_PATH, '/inheritance:r', '/grant:r', `${process.env.USERNAME ?? 'CURRENT_USER'}:(R,W)`], () => {}); + execFile('icacls', [tokenPath, '/inheritance:r', '/grant:r', `${process.env.USERNAME ?? 'CURRENT_USER'}:(R,W)`], () => {}); } else { chmodSync(SQUAD_DIR, 0o700); - chmodSync(TOKEN_PATH, 0o600); + chmodSync(tokenPath, 0o600); } } +/** + * Remove cached tokens from disk for a specific config. + * Used on permanent auth errors and explicit logout. + */ +function clearTokens(tenantId: string, clientId: string): void { + const tokenPath = getTokenPath(tenantId, clientId); + try { + if (existsSync(tokenPath)) unlinkSync(tokenPath); + } catch { /* best-effort cleanup */ } +} + +/** + * Migrate legacy single-file token cache to identity-scoped storage. + * Moves tokens from `teams-tokens.json` → `teams-tokens-{hash}.json`, + * then deletes the legacy file. + */ +function migrateLegacyTokens(tenantId: string, clientId: string): void { + try { + if (!existsSync(LEGACY_TOKEN_PATH)) return; + const raw = readFileSync(LEGACY_TOKEN_PATH, 'utf-8'); + const tokens = JSON.parse(raw) as StoredTokens; + if (tokens.accessToken) { + saveTokens(tenantId, clientId, tokens); + } + unlinkSync(LEGACY_TOKEN_PATH); + } catch { /* best-effort migration */ } +} + +/** + * Extract `tid` (tenant GUID) and `oid` (user object ID) from a JWT access token. + * Best-effort: returns empty object if the token can't be decoded. + * Does NOT verify the signature — the token was just received over TLS from Microsoft. + */ +function extractJwtClaims(accessToken: string): { tid?: string; oid?: string } { + try { + const parts = accessToken.split('.'); + if (parts.length !== 3 || !parts[1]) return {}; + const payload = parts[1].replace(/-/g, '+').replace(/_/g, '/'); + const decoded = JSON.parse(Buffer.from(payload, 'base64').toString('utf-8')) as Record; + return { + tid: typeof decoded.tid === 'string' ? decoded.tid : undefined, + oid: typeof decoded.oid === 'string' ? decoded.oid : undefined, + }; + } catch { + return {}; + } +} + +/** Errors that indicate a permanently invalid refresh token (do not retry) */ +const PERMANENT_AUTH_ERRORS = ['invalid_grant', 'interaction_required', 'consent_required', 'invalid_client']; + // ─── Graph Helpers ─────────────────────────────────────────────────── const GRAPH_BASE = 'https://graph.microsoft.com/v1.0'; @@ -225,7 +310,7 @@ async function startBrowserAuthFlow(tenantId: string, clientId: string): Promise throw new Error(`Token exchange failed: ${data.error} — ${data.error_description}`); } const tokens = parseTokens(data); - saveTokens(tokens); + saveTokens(tenantId, clientId, tokens); res.writeHead(200, { 'Content-Type': 'text/html' }); res.end(SUCCESS_HTML); @@ -289,6 +374,13 @@ interface DeviceCodeResponse { message: string; } +/** Maximum time allowed for device-code auth flow (15 minutes) */ +const DEVICE_CODE_TIMEOUT_MS = 15 * 60 * 1000; +/** Minimum poll interval for device-code flow (2 seconds) */ +const DEVICE_CODE_MIN_POLL_MS = 2_000; +/** Maximum poll interval for device-code flow (30 seconds) */ +const DEVICE_CODE_MAX_POLL_MS = 30_000; + async function startDeviceCodeFlow(tenantId: string, clientId: string): Promise { const deviceCodeUrl = `https://login.microsoftonline.com/${tenantId}/oauth2/v2.0/devicecode`; const tokenUrl = `https://login.microsoftonline.com/${tenantId}/oauth2/v2.0/token`; @@ -309,8 +401,11 @@ async function startDeviceCodeFlow(tenantId: string, clientId: string): Promise< console.log(`\n🔐 Teams authentication required`); console.log(` ${dcData.message}\n`); - const pollInterval = (dcData.interval || 5) * 1000; - const deadline = Date.now() + dcData.expires_in * 1000; + // Clamp poll interval and timeout to sane bounds + const rawInterval = (dcData.interval || 5) * 1000; + const pollInterval = Math.max(DEVICE_CODE_MIN_POLL_MS, Math.min(rawInterval, DEVICE_CODE_MAX_POLL_MS)); + const serverTimeout = dcData.expires_in > 0 ? dcData.expires_in * 1000 : DEVICE_CODE_TIMEOUT_MS; + const deadline = Date.now() + Math.min(serverTimeout, DEVICE_CODE_TIMEOUT_MS); while (Date.now() < deadline) { await new Promise((r) => setTimeout(r, pollInterval)); @@ -329,7 +424,7 @@ async function startDeviceCodeFlow(tenantId: string, clientId: string): Promise< if (tokenData.access_token) { const tokens = parseTokens(tokenData); - saveTokens(tokens); + saveTokens(tenantId, clientId, tokens); console.log(`✅ Teams authentication successful — tokens saved\n`); return tokens; } @@ -369,7 +464,10 @@ async function refreshAccessToken( const data = (await res.json()) as TokenResponse; if (!data.access_token) { - throw new Error(`Token refresh failed: ${data.error} — ${data.error_description}`); + const err = new Error(`Token refresh failed: ${data.error} — ${data.error_description}`); + // Attach error code for callers to distinguish permanent vs transient failures + (err as Error & { authError?: string }).authError = data.error ?? 'unknown'; + throw err; } const tokens: StoredTokens = { @@ -377,26 +475,10 @@ async function refreshAccessToken( refreshToken: data.refresh_token ?? refreshToken, expiresAt: Date.now() + data.expires_in * 1000, }; - saveTokens(tokens); + saveTokens(tenantId, clientId, tokens); return tokens; } -// ─── User ID Cache ─────────────────────────────────────────────────── - -let cachedUserId: string | null = null; - -async function getMyUserId(accessToken: string): Promise { - if (cachedUserId) return cachedUserId; - try { - const me = (await graphFetch(`${GRAPH_BASE}/me`, accessToken)) as { id: string }; - cachedUserId = me.id; - return cachedUserId; - } catch (err) { - console.warn(`⚠️ Teams /me fetch failed: ${(err as Error).message}`); - return null; - } -} - // ─── Adapter ───────────────────────────────────────────────────────── export class TeamsCommunicationAdapter implements CommunicationAdapter { @@ -406,11 +488,21 @@ export class TeamsCommunicationAdapter implements CommunicationAdapter { private resolvedChatId: string | null; private readonly clientId: string; private readonly tenantId: string; + /** Per-instance user ID cache — cleared on every token change to prevent cross-account leaks */ + private cachedUserId: string | null = null; constructor(private readonly config: TeamsCommsConfig) { this.resolvedChatId = config.chatId ?? null; this.clientId = config.clientId ?? DEFAULT_CLIENT_ID; this.tenantId = config.tenantId ?? DEFAULT_TENANT_ID; + // One-time migration from legacy single-file token storage + migrateLegacyTokens(this.tenantId, this.clientId); + } + + /** Reset all identity-sensitive caches. Called on every token change. */ + private resetIdentityCaches(): void { + this.cachedUserId = null; + this.resolvedChatId = this.config.chatId ?? null; } /** @@ -419,7 +511,12 @@ export class TeamsCommunicationAdapter implements CommunicationAdapter { */ private async ensureAuthenticated(): Promise { if (!this.tokens) { - this.tokens = loadTokens(); + this.tokens = loadTokens(this.tenantId, this.clientId); + if (this.tokens) { + // Loaded from disk — reset identity caches since this may be + // a different session or the identity may have changed + this.resetIdentityCaches(); + } } // Valid token — return it @@ -435,15 +532,26 @@ export class TeamsCommunicationAdapter implements CommunicationAdapter { this.clientId, this.tokens.refreshToken, ); + this.resetIdentityCaches(); return this.tokens.accessToken; - } catch { - console.warn('⚠️ Token refresh failed — re-authenticating...'); + } catch (err) { + const authError = (err as Error & { authError?: string }).authError ?? ''; + if (PERMANENT_AUTH_ERRORS.includes(authError)) { + // Permanent failure — clear stale tokens so they're not reloaded + clearTokens(this.tenantId, this.clientId); + this.tokens = null; + this.resetIdentityCaches(); + console.warn(`⚠️ Token refresh permanently failed (${authError}) — re-authenticating...`); + } else { + console.warn('⚠️ Token refresh failed (transient) — re-authenticating...'); + } } } // Try browser auth code flow with PKCE first try { this.tokens = await startBrowserAuthFlow(this.tenantId, this.clientId); + this.resetIdentityCaches(); console.log('✅ Teams authentication successful — tokens saved'); return this.tokens.accessToken; } catch { @@ -452,9 +560,34 @@ export class TeamsCommunicationAdapter implements CommunicationAdapter { // Fallback — device code flow (works in headless/SSH environments) this.tokens = await startDeviceCodeFlow(this.tenantId, this.clientId); + this.resetIdentityCaches(); return this.tokens.accessToken; } + /** + * Logout: clear cached credentials (memory + disk) for this adapter's config. + * This is a local credential purge — does not call Microsoft's revocation endpoint + * (public-client refresh tokens cannot be reliably revoked server-side). + */ + async logout(): Promise { + clearTokens(this.tenantId, this.clientId); + this.tokens = null; + this.resetIdentityCaches(); + } + + /** Resolve the current user's Graph ID, cached per auth session. */ + private async getMyUserId(accessToken: string): Promise { + if (this.cachedUserId) return this.cachedUserId; + try { + const me = (await graphFetch(`${GRAPH_BASE}/me`, accessToken)) as { id: string }; + this.cachedUserId = me.id; + return this.cachedUserId; + } catch (err) { + console.warn(`⚠️ Teams /me fetch failed: ${(err as Error).message}`); + return null; + } + } + /** * Find or create a 1:1 chat with the recipient. */ @@ -592,7 +725,7 @@ export class TeamsCommunicationAdapter implements CommunicationAdapter { return []; } - const myId = await getMyUserId(accessToken); + const myId = await this.getMyUserId(accessToken); return data.value .filter((m) => { @@ -640,4 +773,9 @@ function stripHtml(html: string): string { return html.replace(/<[^>]+>/g, '').replace(/ /g, ' ').replace(/&/g, '&').replace(/</g, '<').replace(/>/g, '>').replace(/"/g, '"').trim(); } -export { escapeHtml, stripHtml, formatTeamsMessage, parseTokens, base64url, validateGraphId }; +export { + escapeHtml, stripHtml, formatTeamsMessage, parseTokens, base64url, validateGraphId, + getTokenPath, clearTokens, loadTokens, saveTokens, migrateLegacyTokens, extractJwtClaims, + DEVICE_CODE_TIMEOUT_MS, DEVICE_CODE_MIN_POLL_MS, DEVICE_CODE_MAX_POLL_MS, + LEGACY_TOKEN_PATH, PERMANENT_AUTH_ERRORS, +}; diff --git a/packages/squad-sdk/src/platform/types.ts b/packages/squad-sdk/src/platform/types.ts index 39d7599d4..ae33ea0d2 100644 --- a/packages/squad-sdk/src/platform/types.ts +++ b/packages/squad-sdk/src/platform/types.ts @@ -140,4 +140,11 @@ export interface CommunicationAdapter { * Returns undefined if the channel has no web UI (e.g., file-log). */ getNotificationUrl(threadId: string): string | undefined; + + /** + * Logout: clear cached credentials for this adapter. + * Local credential purge — does not revoke server-side tokens. + * Optional — not all adapters require authentication. + */ + logout?(): Promise; } diff --git a/test/comms-teams-security.test.ts b/test/comms-teams-security.test.ts new file mode 100644 index 000000000..1c464aba5 --- /dev/null +++ b/test/comms-teams-security.test.ts @@ -0,0 +1,363 @@ +/** + * Security tests for Teams adapter token management. + * Covers: identity-scoped storage, logout, device-code timeout guards, + * stale token cleanup, legacy migration, JWT claim extraction, + * traversal protection, and identity cache resets. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { createHash } from 'node:crypto'; +import { + getTokenPath, + clearTokens, + loadTokens, + saveTokens, + migrateLegacyTokens, + extractJwtClaims, + TeamsCommunicationAdapter, + DEVICE_CODE_TIMEOUT_MS, + DEVICE_CODE_MIN_POLL_MS, + DEVICE_CODE_MAX_POLL_MS, + LEGACY_TOKEN_PATH, + PERMANENT_AUTH_ERRORS, +} from '../packages/squad-sdk/src/platform/comms-teams.js'; + +// Default client ID from the source (Microsoft Graph PowerShell) +const DEFAULT_CLIENT_ID = '14d82eec-204b-4c2f-b7e8-296a70dab67e'; + +// ─── Mock node:fs so we never touch the real filesystem ────────────── + +const mockFiles = new Map(); + +vi.mock('node:fs', async () => { + return { + existsSync: (p: string) => mockFiles.has(p), + readFileSync: (p: string) => { + const content = mockFiles.get(p); + if (content === undefined) throw new Error(`ENOENT: ${p}`); + return content; + }, + writeFileSync: (p: string, data: string) => { + mockFiles.set(p, data); + }, + mkdirSync: () => {}, + chmodSync: () => {}, + unlinkSync: (p: string) => { + mockFiles.delete(p); + }, + }; +}); + +// Prevent icacls / chmod side effects +vi.mock('node:child_process', () => ({ + execFile: (_cmd: string, _args: string[], _cb: unknown) => {}, +})); + +beforeEach(() => { + mockFiles.clear(); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +/** Build a fake JWT with the given payload claims for testing. */ +function fakeJwt(claims: Record): string { + const header = Buffer.from(JSON.stringify({ alg: 'RS256', typ: 'JWT' })).toString('base64url'); + const payload = Buffer.from(JSON.stringify(claims)).toString('base64url'); + const signature = 'fake-sig'; + return `${header}.${payload}.${signature}`; +} + +// ─── getTokenPath — cache key design ───────────────────────────────── + +describe('getTokenPath — identity-scoped file paths', () => { + it('includes clientId in hash — different apps get different paths', () => { + const pathA = getTokenPath('organizations', 'client-aaa'); + const pathB = getTokenPath('organizations', 'client-bbb'); + expect(pathA).not.toBe(pathB); + }); + + it('includes tenantId in hash — different tenants get different paths', () => { + const pathA = getTokenPath('contoso.com', DEFAULT_CLIENT_ID); + const pathB = getTokenPath('fabrikam.com', DEFAULT_CLIENT_ID); + expect(pathA).not.toBe(pathB); + }); + + it('same config produces same path (deterministic)', () => { + expect(getTokenPath('t', 'c')).toBe(getTokenPath('t', 'c')); + }); + + it('stays under the ~/.squad directory', () => { + const p = getTokenPath('organizations', DEFAULT_CLIENT_ID); + expect(p).toContain('.squad'); + expect(p).toContain('teams-tokens-'); + }); + + it('traversal chars cannot escape ~/.squad', () => { + const p = getTokenPath('../../etc/passwd', '../../../root/.ssh/id_rsa'); + expect(p).not.toContain('..'); + expect(p).not.toContain('etc'); + expect(p).not.toContain('passwd'); + expect(p).not.toContain('root'); + expect(p).toContain('teams-tokens-'); + }); + + it('hash matches expected SHA-256(tenantId:clientId) prefix', () => { + const hash = createHash('sha256').update('test-tenant:test-client').digest('hex').slice(0, 16); + const p = getTokenPath('test-tenant', 'test-client'); + expect(p).toContain(`teams-tokens-${hash}.json`); + }); + + it('default "organizations" authority gets a valid unique path', () => { + const p = getTokenPath('organizations', DEFAULT_CLIENT_ID); + expect(p).toMatch(/teams-tokens-[a-f0-9]{16}\.json$/); + }); +}); + +// ─── Token storage: load / save / clear ────────────────────────────── + +describe('identity-scoped token CRUD', () => { + const tenantA = 'contoso.com'; + const tenantB = 'fabrikam.com'; + const client = DEFAULT_CLIENT_ID; + + const tokenA = { + accessToken: fakeJwt({ tid: 'contoso-guid', oid: 'user-a' }), + refreshToken: 'rt-contoso', + expiresAt: Date.now() + 3600_000, + }; + const tokenB = { + accessToken: fakeJwt({ tid: 'fabrikam-guid', oid: 'user-b' }), + refreshToken: 'rt-fabrikam', + expiresAt: Date.now() + 3600_000, + }; + + it('saves and loads tokens for a config', () => { + saveTokens(tenantA, client, tokenA); + const loaded = loadTokens(tenantA, client); + expect(loaded).not.toBeNull(); + expect(loaded!.refreshToken).toBe('rt-contoso'); + }); + + it('isolates tokens between different tenants', () => { + saveTokens(tenantA, client, tokenA); + saveTokens(tenantB, client, tokenB); + expect(loadTokens(tenantA, client)!.refreshToken).toBe('rt-contoso'); + expect(loadTokens(tenantB, client)!.refreshToken).toBe('rt-fabrikam'); + }); + + it('isolates tokens between different client IDs', () => { + saveTokens(tenantA, 'client-1', tokenA); + saveTokens(tenantA, 'client-2', tokenB); + expect(loadTokens(tenantA, 'client-1')!.refreshToken).toBe('rt-contoso'); + expect(loadTokens(tenantA, 'client-2')!.refreshToken).toBe('rt-fabrikam'); + }); + + it('returns null for unknown config', () => { + expect(loadTokens('unknown-tenant', client)).toBeNull(); + }); + + it('rejects tokens with mismatched configTenantId metadata', () => { + const path = getTokenPath(tenantA, client); + mockFiles.set(path, JSON.stringify({ ...tokenA, configTenantId: 'wrong-tenant', clientId: client })); + expect(loadTokens(tenantA, client)).toBeNull(); + }); + + it('rejects tokens with mismatched clientId metadata', () => { + const path = getTokenPath(tenantA, client); + mockFiles.set(path, JSON.stringify({ ...tokenA, configTenantId: tenantA, clientId: 'wrong-client' })); + expect(loadTokens(tenantA, client)).toBeNull(); + }); + + it('stores authenticatedTenantId from JWT on save', () => { + saveTokens(tenantA, client, tokenA); + const raw = JSON.parse(mockFiles.get(getTokenPath(tenantA, client))!); + expect(raw.authenticatedTenantId).toBe('contoso-guid'); + expect(raw.authenticatedUserId).toBe('user-a'); + }); + + it('clearTokens removes tokens from disk', () => { + saveTokens(tenantA, client, tokenA); + expect(loadTokens(tenantA, client)).not.toBeNull(); + clearTokens(tenantA, client); + expect(loadTokens(tenantA, client)).toBeNull(); + }); + + it('clearTokens is a no-op for missing files', () => { + expect(() => clearTokens('nonexistent', client)).not.toThrow(); + }); +}); + +// ─── extractJwtClaims ──────────────────────────────────────────────── + +describe('extractJwtClaims', () => { + it('extracts tid and oid from a valid JWT', () => { + const jwt = fakeJwt({ tid: 'my-tenant-guid', oid: 'my-user-guid', aud: 'graph' }); + const claims = extractJwtClaims(jwt); + expect(claims.tid).toBe('my-tenant-guid'); + expect(claims.oid).toBe('my-user-guid'); + }); + + it('returns empty object for non-JWT string', () => { + expect(extractJwtClaims('not-a-jwt')).toEqual({}); + }); + + it('returns empty object for malformed base64', () => { + expect(extractJwtClaims('a.!!!invalid!!!.c')).toEqual({}); + }); + + it('handles JWT without tid/oid claims', () => { + const jwt = fakeJwt({ sub: 'user', aud: 'graph' }); + const claims = extractJwtClaims(jwt); + expect(claims.tid).toBeUndefined(); + expect(claims.oid).toBeUndefined(); + }); + + it('ignores non-string tid/oid values', () => { + const jwt = fakeJwt({ tid: 123, oid: null }); + const claims = extractJwtClaims(jwt); + expect(claims.tid).toBeUndefined(); + expect(claims.oid).toBeUndefined(); + }); +}); + +// ─── Legacy migration ──────────────────────────────────────────────── + +describe('migrateLegacyTokens', () => { + it('migrates legacy token file to identity-scoped path', () => { + const legacyToken = { + accessToken: fakeJwt({ tid: 'legacy-tid', oid: 'legacy-oid' }), + refreshToken: 'legacy-refresh', + expiresAt: Date.now() + 3600_000, + }; + mockFiles.set(LEGACY_TOKEN_PATH, JSON.stringify(legacyToken)); + + migrateLegacyTokens('my-tenant', DEFAULT_CLIENT_ID); + + // Legacy file deleted + expect(mockFiles.has(LEGACY_TOKEN_PATH)).toBe(false); + // Token available at new path + const loaded = loadTokens('my-tenant', DEFAULT_CLIENT_ID); + expect(loaded).not.toBeNull(); + expect(loaded!.refreshToken).toBe('legacy-refresh'); + }); + + it('is a no-op when no legacy file exists', () => { + expect(() => migrateLegacyTokens('my-tenant', DEFAULT_CLIENT_ID)).not.toThrow(); + expect(loadTokens('my-tenant', DEFAULT_CLIENT_ID)).toBeNull(); + }); +}); + +// ─── Logout ────────────────────────────────────────────────────────── + +describe('TeamsCommunicationAdapter.logout()', () => { + it('clears tokens from disk after logout', async () => { + const tenantId = 'logout-test-tenant'; + saveTokens(tenantId, DEFAULT_CLIENT_ID, { + accessToken: fakeJwt({ tid: 'x', oid: 'y' }), + refreshToken: 'rt-logout', + expiresAt: Date.now() + 3600_000, + }); + + const adapter = new TeamsCommunicationAdapter({ tenantId }); + await adapter.logout(); + + expect(loadTokens(tenantId, DEFAULT_CLIENT_ID)).toBeNull(); + }); + + it('is safe to call multiple times', async () => { + const adapter = new TeamsCommunicationAdapter({ tenantId: 'double-logout' }); + await adapter.logout(); + await adapter.logout(); + // Should not throw + }); + + it('logout on one config does not affect another', async () => { + const tenantA = 'iso-tenant-a'; + const tenantB = 'iso-tenant-b'; + const client = DEFAULT_CLIENT_ID; + + saveTokens(tenantA, client, { + accessToken: fakeJwt({ tid: 'a-tid', oid: 'a-oid' }), + refreshToken: 'a-refresh', + expiresAt: Date.now() + 3600_000, + }); + saveTokens(tenantB, client, { + accessToken: fakeJwt({ tid: 'b-tid', oid: 'b-oid' }), + refreshToken: 'b-refresh', + expiresAt: Date.now() + 3600_000, + }); + + const adapterA = new TeamsCommunicationAdapter({ tenantId: tenantA }); + await adapterA.logout(); + + expect(loadTokens(tenantA, client)).toBeNull(); + expect(loadTokens(tenantB, client)).not.toBeNull(); + expect(loadTokens(tenantB, client)!.refreshToken).toBe('b-refresh'); + }); +}); + +// ─── Device-code timeout constants ─────────────────────────────────── + +describe('device-code timeout guards', () => { + it('max timeout is 15 minutes', () => { + expect(DEVICE_CODE_TIMEOUT_MS).toBe(15 * 60 * 1000); + }); + + it('min poll interval is 2 seconds', () => { + expect(DEVICE_CODE_MIN_POLL_MS).toBe(2_000); + }); + + it('max poll interval is 30 seconds', () => { + expect(DEVICE_CODE_MAX_POLL_MS).toBe(30_000); + }); +}); + +// ─── Permanent auth error classification ───────────────────────────── + +describe('PERMANENT_AUTH_ERRORS', () => { + it('includes invalid_grant', () => { + expect(PERMANENT_AUTH_ERRORS).toContain('invalid_grant'); + }); + + it('includes interaction_required', () => { + expect(PERMANENT_AUTH_ERRORS).toContain('interaction_required'); + }); + + it('includes consent_required', () => { + expect(PERMANENT_AUTH_ERRORS).toContain('consent_required'); + }); + + it('includes invalid_client', () => { + expect(PERMANENT_AUTH_ERRORS).toContain('invalid_client'); + }); + + it('does NOT include transient errors', () => { + expect(PERMANENT_AUTH_ERRORS).not.toContain('server_error'); + expect(PERMANENT_AUTH_ERRORS).not.toContain('temporarily_unavailable'); + expect(PERMANENT_AUTH_ERRORS).not.toContain('slow_down'); + }); +}); + +// ─── Multi-config adapter isolation ────────────────────────────────── + +describe('multi-config adapter isolation', () => { + it('different clientIds for same tenant produce different cache files', () => { + const pathA = getTokenPath('organizations', 'app-registration-1'); + const pathB = getTokenPath('organizations', 'app-registration-2'); + expect(pathA).not.toBe(pathB); + }); + + it('different tenantIds for same clientId produce different cache files', () => { + const pathA = getTokenPath('tenant-alpha', DEFAULT_CLIENT_ID); + const pathB = getTokenPath('tenant-beta', DEFAULT_CLIENT_ID); + expect(pathA).not.toBe(pathB); + }); + + it('default "organizations" + default clientId has a stable path', () => { + const p1 = getTokenPath('organizations', DEFAULT_CLIENT_ID); + const p2 = getTokenPath('organizations', DEFAULT_CLIENT_ID); + expect(p1).toBe(p2); + }); +}); From dc3e9f2255d22aa3651055cfd050f3d61f00f418 Mon Sep 17 00:00:00 2001 From: "Dina Berry (She/her)" Date: Thu, 9 Apr 2026 21:05:56 -0700 Subject: [PATCH 2/2] fix(sdk): add refreshToken validation to loadTokens + fix changeset text Add typeof refreshToken === 'string' to shape check in loadTokens to prevent wasted AAD round-trips on incomplete cached tokens. Fix changeset to say logout() instead of revoke(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .changeset/teams-adapter-security.md | 2 +- packages/squad-sdk/src/platform/comms-teams.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/teams-adapter-security.md b/.changeset/teams-adapter-security.md index 99ce0319e..f6e34b3d6 100644 --- a/.changeset/teams-adapter-security.md +++ b/.changeset/teams-adapter-security.md @@ -2,4 +2,4 @@ '@bradygaster/squad-sdk': patch --- -Teams adapter token security: tenant-scoped token cache (keyed by tenant ID hash), explicit revoke() for logout, 15-minute device-code timeout guard, stale token cleanup on permanent auth errors, per-instance user ID cache. Migration guide for async createCommunicationAdapter change. +Teams adapter token security: tenant-scoped token cache (keyed by tenant ID hash), explicit logout() for session teardown, 15-minute device-code timeout guard, stale token cleanup on permanent auth errors, per-instance user ID cache. Migration guide for async createCommunicationAdapter change. diff --git a/packages/squad-sdk/src/platform/comms-teams.ts b/packages/squad-sdk/src/platform/comms-teams.ts index 9cad475de..73db4be9b 100644 --- a/packages/squad-sdk/src/platform/comms-teams.ts +++ b/packages/squad-sdk/src/platform/comms-teams.ts @@ -75,7 +75,7 @@ function loadTokens(tenantId: string, clientId: string): StoredTokens | null { const raw = readFileSync(tokenPath, 'utf-8'); const parsed = JSON.parse(raw) as StoredTokens; // Validate minimum required shape - if (typeof parsed.accessToken !== 'string' || typeof parsed.expiresAt !== 'number') return null; + if (typeof parsed.accessToken !== 'string' || typeof parsed.expiresAt !== 'number' || typeof parsed.refreshToken !== 'string') return null; // Reject tokens from a different config (stale/corrupted cache) if (parsed.configTenantId && parsed.configTenantId !== tenantId) return null; if (parsed.clientId && parsed.clientId !== clientId) return null;