From 08a5d81a4f048b32170a335ccd2c648623870763 Mon Sep 17 00:00:00 2001 From: ayushtr-aws Date: Thu, 9 Jul 2026 11:26:12 -0400 Subject: [PATCH 1/4] feat(cli): add jira invite-user --- cli/src/commands/jira.ts | 319 ++++++++++++++++- cli/test/commands/jira.test.ts | 325 +++++++++++++++++- docs/guides/JIRA_SETUP_GUIDE.md | 30 +- .../content/docs/using/Jira-setup-guide.md | 30 +- 4 files changed, 662 insertions(+), 42 deletions(-) diff --git a/cli/src/commands/jira.ts b/cli/src/commands/jira.ts index 024412c4..10f393c7 100644 --- a/cli/src/commands/jira.ts +++ b/cli/src/commands/jira.ts @@ -28,7 +28,7 @@ import { ResourceExistsException, SecretsManagerClient, } from '@aws-sdk/client-secrets-manager'; -import { DynamoDBDocumentClient, PutCommand } from '@aws-sdk/lib-dynamodb'; +import { DynamoDBDocumentClient, GetCommand, PutCommand } from '@aws-sdk/lib-dynamodb'; import { Command } from 'commander'; import { ApiClient } from '../api-client'; import { loadConfig, loadCredentials } from '../config'; @@ -40,7 +40,9 @@ import { exchangeAuthorizationCode, fetchAccessibleResources, generatePkce, + isAccessTokenExpiring, jiraOauthSecretName, + refreshAccessToken, StoredJiraOauthToken, } from '../jira-oauth'; import { awaitOauthCallback, CALLBACK_URL } from '../oauth-callback-server'; @@ -54,6 +56,9 @@ const DEFAULT_LABEL_FILTER = 'bgagent'; * accepts at creation time. */ const PROJECT_KEY_RE = /^[A-Z][A-Z0-9_]{1,99}$/; +/** Alphabet used by {@link generateInviteCode}. */ +export const INVITE_CODE_ALPHABET = 'abcdefghjkmnpqrstuvwxyz23456789'; + /** Width of the ═ rule used to frame the setup banner. */ const BANNER_WIDTH = 72; @@ -258,6 +263,182 @@ function isWebhookSecretPlaceholder(value: string): boolean { } } +export function generateInviteCode(): string { + const INVITE_CODE_RANDOM_BYTES = 8; + // eslint-disable-next-line @typescript-eslint/no-require-imports + const { randomBytes } = require('crypto') as typeof import('crypto'); + const bytes = randomBytes(INVITE_CODE_RANDOM_BYTES); + let out = 'link-'; + for (const b of bytes) { + out += INVITE_CODE_ALPHABET[b % INVITE_CODE_ALPHABET.length]; + } + return out; +} + +interface JiraUserSearchResult { + readonly accountId?: string; + readonly displayName?: string; + readonly emailAddress?: string; + readonly active?: boolean; + readonly accountType?: string; +} + +function parseStoredJiraOauthToken(secretString: string | undefined, secretId: string): StoredJiraOauthToken { + if (!secretString) { + throw new CliError(`OAuth secret '${secretId}' has no SecretString. Re-run \`bgagent jira setup\`.`); + } + let parsed: unknown; + try { + parsed = JSON.parse(secretString); + } catch { + throw new CliError(`OAuth secret '${secretId}' is not valid JSON. Re-run \`bgagent jira setup\`.`); + } + if (typeof parsed !== 'object' || parsed === null) { + throw new CliError(`OAuth secret '${secretId}' has an unexpected shape. Re-run \`bgagent jira setup\`.`); + } + const obj = parsed as Record; + const required = [ + 'access_token', + 'refresh_token', + 'expires_at', + 'client_id', + 'client_secret', + 'cloud_id', + 'site_url', + ]; + const missing = required.filter((key) => typeof obj[key] !== 'string' || (obj[key] as string).length === 0); + if (missing.length > 0) { + throw new CliError( + `OAuth secret '${secretId}' is missing required field${missing.length === 1 ? '' : 's'} ${missing.join(', ')}. ` + + `Re-run \`bgagent jira setup\` for tenant '${String(obj.cloud_id ?? 'unknown')}'.`, + ); + } + return parsed as StoredJiraOauthToken; +} + +function isJiraUser(value: unknown): value is JiraUserSearchResult { + if (typeof value !== 'object' || value === null) return false; + const obj = value as Record; + return typeof obj.accountId === 'string' && obj.accountId.length > 0; +} + +function isLinkableJiraUser(user: JiraUserSearchResult): boolean { + return user.active !== false && user.accountType !== 'app'; +} + +function formatJiraUserLabel(user: JiraUserSearchResult): string { + const name = user.displayName || user.accountId || '(unknown)'; + return `${name}${user.emailAddress ? ` (${user.emailAddress})` : ''}`; +} + +async function parseJiraJson(response: Response, context: string): Promise { + try { + return await response.json(); + } catch { + throw new CliError(`Atlassian ${context} returned non-JSON: HTTP ${response.status}.`); + } +} + +async function fetchJiraUserByAccountId( + accessToken: string, + cloudId: string, + accountId: string, + fetchImpl: typeof fetch, +): Promise { + const url = new URL(`https://api.atlassian.com/ex/jira/${encodeURIComponent(cloudId)}/rest/api/3/user`); + url.searchParams.set('accountId', accountId); + const response = await fetchImpl(url.toString(), { + method: 'GET', + headers: { + Authorization: `Bearer ${accessToken}`, + Accept: 'application/json', + }, + }); + if (response.status === 404) return null; + if (!response.ok) { + throw new CliError( + `Atlassian Jira user lookup failed: HTTP ${response.status}. ` + + 'The stored OAuth token may be expired/revoked or missing read:jira-user.', + ); + } + const parsed = await parseJiraJson(response, 'Jira user lookup'); + if (!isJiraUser(parsed)) { + throw new CliError(`Atlassian Jira user lookup returned an unexpected shape: ${JSON.stringify(parsed).slice(0, 200)}`); + } + return parsed; +} + +async function searchJiraUsers( + accessToken: string, + cloudId: string, + query: string, + fetchImpl: typeof fetch, +): Promise { + const url = new URL(`https://api.atlassian.com/ex/jira/${encodeURIComponent(cloudId)}/rest/api/3/user/search`); + url.searchParams.set('query', query); + url.searchParams.set('maxResults', '10'); + const response = await fetchImpl(url.toString(), { + method: 'GET', + headers: { + Authorization: `Bearer ${accessToken}`, + Accept: 'application/json', + }, + }); + if (!response.ok) { + throw new CliError( + `Atlassian Jira user search failed: HTTP ${response.status}. ` + + 'The stored OAuth token may be expired/revoked or missing read:jira-user.', + ); + } + const parsed = await parseJiraJson(response, 'Jira user search'); + if (!Array.isArray(parsed)) { + throw new CliError(`Atlassian Jira user search returned an unexpected shape: ${JSON.stringify(parsed).slice(0, 200)}`); + } + return parsed.filter(isJiraUser); +} + +async function resolveJiraUser( + accessToken: string, + cloudId: string, + accountIdOrEmail: string, + fetchImpl: typeof fetch = fetch, +): Promise { + const query = accountIdOrEmail.trim(); + if (!query) { + throw new CliError('Jira account id or email is required.'); + } + + if (!query.includes('@')) { + const direct = await fetchJiraUserByAccountId(accessToken, cloudId, query, fetchImpl); + if (direct) { + if (!isLinkableJiraUser(direct)) { + throw new CliError(`Jira account '${query}' is inactive or is an app account.`); + } + return direct; + } + } + + const users = (await searchJiraUsers(accessToken, cloudId, query, fetchImpl)).filter(isLinkableJiraUser); + const exactAccountId = users.find((user) => user.accountId === query); + if (exactAccountId) return exactAccountId; + + if (query.includes('@')) { + const lowerQuery = query.toLowerCase(); + const exactEmail = users.find((user) => (user.emailAddress ?? '').toLowerCase() === lowerQuery); + if (exactEmail) return exactEmail; + } + + if (users.length === 1) return users[0]; + if (users.length === 0) { + throw new CliError(`No active Jira user found for '${query}' in tenant '${cloudId}'.`); + } + + const candidates = users.map((user) => `- ${formatJiraUserLabel(user)} [${user.accountId}]`).join('\n'); + throw new CliError( + `Jira user lookup for '${query}' returned multiple users. Re-run with the accountId for the intended user:\n${candidates}`, + ); +} + function promptLine(label: string): Promise { return new Promise((resolve) => { const rl = readline.createInterface({ input: process.stdin, output: process.stdout }); @@ -576,12 +757,144 @@ export function makeJiraCommand(): Command { console.log(' 1. Map a Jira project to a GitHub repo:'); console.log(` bgagent jira map ${cloudId} --repo owner/repo`); console.log(' 2. Link your Jira account so triggered tasks attribute to your platform user:'); - console.log(' (an admin runs `bgagent jira invite-user` to issue you a code; this command'); - console.log(' is not yet implemented — populate the user-mapping row manually for now.)'); + console.log(` bgagent jira invite-user ${cloudId} `); + console.log(' bgagent jira link '); console.log(` 3. Add the trigger label '${DEFAULT_LABEL_FILTER}' to a Jira issue in a mapped project.`); }), ); + // ─── invite-user ────────────────────────────────────────────────────────── + jira.addCommand( + new Command('invite-user') + .description('Generate a one-time code for a Jira teammate to redeem via `bgagent jira link `') + .argument('', 'Atlassian tenant cloudId (UUID)') + .argument('', 'Jira accountId or email address for the teammate') + .option('--region ', 'AWS region (defaults to configured region)') + .option('--stack-name ', 'CloudFormation stack name', 'backgroundagent-dev') + .action(async (cloudId: string, accountIdOrEmail: string, opts) => { + const config = loadConfig(); + const region = opts.region || config.region; + const stackName = opts.stackName; + + const [workspaceRegistryTable, userMappingTable] = await Promise.all([ + getStackOutput(region, stackName, 'JiraWorkspaceRegistryTableName'), + getStackOutput(region, stackName, 'JiraUserMappingTableName'), + ]); + const missing: string[] = []; + if (!workspaceRegistryTable) missing.push('JiraWorkspaceRegistryTableName'); + if (!userMappingTable) missing.push('JiraUserMappingTableName'); + if (missing.length > 0) { + throw new CliError( + `Stack '${stackName}' is missing outputs ${missing.join(', ')}. ` + + 'Re-deploy with the JiraIntegration CDK changes (mise //cdk:deploy).', + ); + } + + const creds = loadCredentials(); + if (!creds?.id_token) { + throw new CliError('Not authenticated — run `bgagent login` first.'); + } + const callerCognitoSub = extractCognitoSub(); + + const sm = new SecretsManagerClient({ region }); + const ddb = DynamoDBDocumentClient.from(new DynamoDBClient({ region })); + + const registry = await ddb.send(new GetCommand({ + TableName: workspaceRegistryTable!, + Key: { jira_cloud_id: cloudId }, + })); + const registryRow = registry.Item; + if (!registryRow || registryRow.status !== 'active') { + throw new CliError( + `Jira tenant '${cloudId}' is not in the registry (or status != 'active'). ` + + 'Run `bgagent jira setup` for that tenant first.', + ); + } + const oauthSecretArn = registryRow.oauth_secret_arn as string | undefined; + if (!oauthSecretArn) { + throw new CliError(`Jira tenant '${cloudId}' registry row is missing oauth_secret_arn. Re-run \`bgagent jira setup\`.`); + } + const siteUrl = (registryRow.site_url as string | undefined) ?? ''; + + const oauthSecret = await sm.send(new GetSecretValueCommand({ SecretId: oauthSecretArn })); + let stored = parseStoredJiraOauthToken(oauthSecret.SecretString, oauthSecretArn); + + console.log(`bgagent jira invite-user — tenant '${cloudId}'`); + console.log(` region: ${region}`); + if (siteUrl) { + console.log(` site: ${siteUrl}`); + } + + if (isAccessTokenExpiring(stored.expires_at)) { + process.stdout.write(' → Refreshing Jira OAuth token...'); + const refreshed = await refreshAccessToken({ + refreshToken: stored.refresh_token, + clientId: stored.client_id, + clientSecret: stored.client_secret, + }); + if (!refreshed.refresh_token) { + console.log(' ✗'); + throw new CliError('Atlassian refresh_token grant returned no refresh_token. Re-run `bgagent jira setup`.'); + } + stored = { + ...stored, + access_token: refreshed.access_token, + refresh_token: refreshed.refresh_token, + expires_at: computeExpiresAt(refreshed.expires_in), + scope: refreshed.scope, + updated_at: new Date().toISOString(), + }; + await sm.send(new PutSecretValueCommand({ + SecretId: oauthSecretArn, + SecretString: JSON.stringify(stored), + })); + console.log(' ✓'); + } + + process.stdout.write(' → Resolving Jira user...'); + let picked: JiraUserSearchResult; + try { + picked = await resolveJiraUser(stored.access_token, cloudId, accountIdOrEmail); + console.log(` ✓ (${formatJiraUserLabel(picked)})`); + } catch (err) { + console.log(' ✗'); + throw err; + } + + const code = generateInviteCode(); + const ttl = Math.floor(Date.now() / 1000) + 24 * 60 * 60; + await ddb.send(new PutCommand({ + TableName: userMappingTable!, + Item: { + jira_identity: `pending#${code}`, + status: 'pending', + jira_cloud_id: cloudId, + jira_site_url: siteUrl, + jira_account_id: picked.accountId, + jira_user_name: picked.displayName ?? '', + jira_user_email: picked.emailAddress ?? '', + invited_at: new Date().toISOString(), + invited_by_platform_user_id: callerCognitoSub, + ttl, + }, + })); + + console.log(); + console.log('✅ Invite created.'); + console.log(); + console.log(' Send this to the teammate (Slack/email/etc):'); + console.log(); + console.log(` bgagent jira link ${code}`); + console.log(); + console.log(` Picked Jira user: ${formatJiraUserLabel(picked)}`); + console.log(` Jira tenant: ${siteUrl || cloudId}`); + console.log(` Code expires: ${new Date(ttl * 1000).toISOString()} (24h)`); + console.log(); + console.log(' The teammate sees the Jira identity above and confirms before the'); + console.log(' mapping is written. If you picked the wrong user, the teammate aborts.'); + }), + ); + // ─── link ───────────────────────────────────────────────────────────────── jira.addCommand( new Command('link') diff --git a/cli/test/commands/jira.test.ts b/cli/test/commands/jira.test.ts index 38f245b2..d8fb237d 100644 --- a/cli/test/commands/jira.test.ts +++ b/cli/test/commands/jira.test.ts @@ -22,8 +22,11 @@ import { PutSecretValueCommand, ResourceExistsException, } from '@aws-sdk/client-secrets-manager'; +import { GetCommand, PutCommand } from '@aws-sdk/lib-dynamodb'; import { ApiClient } from '../../src/api-client'; import { + generateInviteCode, + INVITE_CODE_ALPHABET, isWebhookSecretConfigured, JIRA_WEBHOOK_SECRET_PLACEHOLDER_KEY, makeJiraCommand, @@ -38,6 +41,16 @@ import type { StoredJiraOauthToken } from '../../src/jira-oauth'; const execFileMock = jest.fn(); jest.mock('child_process', () => ({ execFile: (...args: unknown[]) => execFileMock(...args) })); +// Secrets Manager — invite-user reads the per-tenant OAuth bundle. +const smSend = jest.fn(); +jest.mock('@aws-sdk/client-secrets-manager', () => { + const actual = jest.requireActual('@aws-sdk/client-secrets-manager'); + return { + ...actual, + SecretsManagerClient: jest.fn(() => ({ send: smSend })), + }; +}); + // OAuth callback server — avoid binding a real localhost socket in `setup`. const awaitOauthCallbackMock = jest.fn(); jest.mock('../../src/oauth-callback-server', () => { @@ -97,12 +110,18 @@ function sampleToken(overrides: Partial = {}): StoredJiraO }; } +function fakeIdToken(sub: string): string { + const header = Buffer.from(JSON.stringify({ alg: 'none' })).toString('base64url'); + const payload = Buffer.from(JSON.stringify({ sub })).toString('base64url'); + return `${header}.${payload}.sig`; +} + describe('makeJiraCommand', () => { test('registers the expected subcommands', () => { const cmd = makeJiraCommand(); expect(cmd.name()).toBe('jira'); const names = cmd.commands.map((c) => c.name()).sort(); - expect(names).toEqual(expect.arrayContaining(['app-template', 'setup', 'link', 'map'])); + expect(names).toEqual(expect.arrayContaining(['app-template', 'setup', 'invite-user', 'link', 'map'])); }); test('`map` exposes a --label option defaulting to the trigger label', () => { @@ -118,6 +137,24 @@ describe('makeJiraCommand', () => { }); }); +describe('generateInviteCode', () => { + test('emits "link-" prefix followed by exactly 8 alphabet characters', () => { + const code = generateInviteCode(); + expect(code).toMatch(/^link-[a-z0-9]{8}$/); + expect(code).toHaveLength(13); + }); + + test('only uses characters from the unambiguous alphabet', () => { + for (let i = 0; i < 200; i++) { + const code = generateInviteCode(); + const chars = code.slice('link-'.length); + for (const c of chars) { + expect(INVITE_CODE_ALPHABET).toContain(c); + } + } + }); +}); + describe('openBrowser', () => { beforeEach(() => execFileMock.mockReset()); @@ -236,6 +273,292 @@ describe('jira link action', () => { }); }); +describe('jira invite-user action', () => { + const originalFetch = global.fetch; + let loadConfigSpy: jest.SpiedFunction; + let loadCredentialsSpy: jest.SpiedFunction; + let logSpy: jest.SpiedFunction; + let writeSpy: jest.SpiedFunction; + + beforeEach(() => { + ddbSend.mockReset(); + smSend.mockReset(); + cfnSend.mockReset().mockResolvedValue({ + Stacks: [{ + Outputs: [ + { OutputKey: 'JiraWorkspaceRegistryTableName', OutputValue: 'JiraRegistryTable' }, + { OutputKey: 'JiraUserMappingTableName', OutputValue: 'JiraUsersTable' }, + ], + }], + }); + loadConfigSpy = jest.spyOn(config, 'loadConfig').mockReturnValue({ region: 'us-west-2' } as ReturnType); + loadCredentialsSpy = jest.spyOn(config, 'loadCredentials').mockReturnValue({ + id_token: fakeIdToken('admin-sub'), + } as ReturnType); + logSpy = jest.spyOn(console, 'log').mockImplementation(); + writeSpy = jest.spyOn(process.stdout, 'write').mockImplementation(() => true); + }); + + afterEach(() => { + global.fetch = originalFetch; + loadConfigSpy.mockRestore(); + loadCredentialsSpy.mockRestore(); + logSpy.mockRestore(); + writeSpy.mockRestore(); + }); + + function mockRegistryAndSecret(overrides: Partial = {}): void { + ddbSend + .mockResolvedValueOnce({ + Item: { + jira_cloud_id: 'cloud-123', + site_url: 'https://acme.atlassian.net', + oauth_secret_arn: 'arn:jira-oauth', + status: 'active', + }, + }) + .mockResolvedValueOnce({}); + smSend.mockResolvedValueOnce({ + SecretString: JSON.stringify(sampleToken({ + cloud_id: 'cloud-123', + site_url: 'https://acme.atlassian.net', + expires_at: new Date(Date.now() + 60 * 60 * 1000).toISOString(), + ...overrides, + })), + }); + } + + async function runInvite(args: string[]): Promise { + const program = makeJiraCommand(); + await program.parseAsync(['node', 'bgagent', 'invite-user', ...args]); + } + + test('writes a pending invite row for a Jira user resolved by email', async () => { + mockRegistryAndSecret(); + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => ([{ + accountId: 'acct-1', + displayName: 'Maya K', + emailAddress: 'maya@example.test', + active: true, + accountType: 'atlassian', + }]), + }); + global.fetch = fetchMock as unknown as typeof fetch; + + await runInvite(['cloud-123', 'maya@example.test']); + + const getCmd = ddbSend.mock.calls[0][0] as GetCommand; + expect(getCmd).toBeInstanceOf(GetCommand); + expect(getCmd.input).toMatchObject({ + TableName: 'JiraRegistryTable', + Key: { jira_cloud_id: 'cloud-123' }, + }); + + const searchUrl = new URL(String(fetchMock.mock.calls[0][0])); + expect(searchUrl.pathname).toBe('/ex/jira/cloud-123/rest/api/3/user/search'); + expect(searchUrl.searchParams.get('query')).toBe('maya@example.test'); + expect(searchUrl.searchParams.get('maxResults')).toBe('10'); + expect(fetchMock.mock.calls[0][1]).toMatchObject({ + headers: { Authorization: 'Bearer access-xyz' }, + }); + + const putCmd = ddbSend.mock.calls[1][0] as PutCommand; + expect(putCmd).toBeInstanceOf(PutCommand); + expect(putCmd.input.TableName).toBe('JiraUsersTable'); + expect(putCmd.input.Item).toMatchObject({ + status: 'pending', + jira_cloud_id: 'cloud-123', + jira_site_url: 'https://acme.atlassian.net', + jira_account_id: 'acct-1', + jira_user_name: 'Maya K', + jira_user_email: 'maya@example.test', + invited_by_platform_user_id: 'admin-sub', + }); + expect(putCmd.input.Item?.jira_identity).toMatch(/^pending#link-[a-z0-9]{8}$/); + expect(putCmd.input.Item?.ttl).toBeGreaterThan(Math.floor(Date.now() / 1000) + 23 * 60 * 60); + expect(logSpy.mock.calls.some((call) => String(call[0]).includes('bgagent jira link link-'))).toBe(true); + }); + + test('resolves a direct Jira accountId without requiring an email search result', async () => { + mockRegistryAndSecret(); + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ + accountId: 'acct-1', + displayName: 'Maya K', + active: true, + accountType: 'atlassian', + }), + }); + global.fetch = fetchMock as unknown as typeof fetch; + + await runInvite(['cloud-123', 'acct-1']); + + expect(fetchMock).toHaveBeenCalledTimes(1); + const lookupUrl = new URL(String(fetchMock.mock.calls[0][0])); + expect(lookupUrl.pathname).toBe('/ex/jira/cloud-123/rest/api/3/user'); + expect(lookupUrl.searchParams.get('accountId')).toBe('acct-1'); + const putCmd = ddbSend.mock.calls[1][0] as PutCommand; + expect(putCmd.input.Item).toMatchObject({ + jira_account_id: 'acct-1', + jira_user_name: 'Maya K', + jira_user_email: '', + }); + }); + + test('refreshes and persists an expired Jira OAuth token before resolving the user', async () => { + mockRegistryAndSecret({ + expires_at: new Date(Date.now() - 60_000).toISOString(), + webhook_signing_secret: 'tenant-secret', + }); + const fetchMock = jest.fn() + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ + access_token: 'new-access', + refresh_token: 'new-refresh', + token_type: 'Bearer', + expires_in: 3600, + scope: 'read:jira-work write:jira-work read:jira-user', + }), + }) + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ([{ + accountId: 'acct-1', + displayName: 'Maya K', + emailAddress: 'maya@example.test', + active: true, + accountType: 'atlassian', + }]), + }); + global.fetch = fetchMock as unknown as typeof fetch; + + await runInvite(['cloud-123', 'maya@example.test']); + + const putSecret = smSend.mock.calls[1][0] as PutSecretValueCommand; + expect(putSecret).toBeInstanceOf(PutSecretValueCommand); + expect(putSecret.input.SecretId).toBe('arn:jira-oauth'); + const persisted = JSON.parse(putSecret.input.SecretString as string) as StoredJiraOauthToken; + expect(persisted).toMatchObject({ + access_token: 'new-access', + refresh_token: 'new-refresh', + webhook_signing_secret: 'tenant-secret', + }); + expect(fetchMock.mock.calls[1][1]).toMatchObject({ + headers: { Authorization: 'Bearer new-access' }, + }); + }); + + test('fails before writing when the admin is not logged in', async () => { + loadCredentialsSpy.mockReturnValue(undefined as ReturnType); + + await expect(runInvite(['cloud-123', 'maya@example.test'])).rejects.toThrow(/Not authenticated/); + + expect(ddbSend).not.toHaveBeenCalled(); + expect(smSend).not.toHaveBeenCalled(); + }); + + test('fails when the Jira tenant is not active in the registry', async () => { + ddbSend.mockResolvedValueOnce({ Item: { jira_cloud_id: 'cloud-123', status: 'revoked' } }); + + await expect(runInvite(['cloud-123', 'maya@example.test'])).rejects.toThrow(/not in the registry/); + + expect(smSend).not.toHaveBeenCalled(); + expect(ddbSend).toHaveBeenCalledTimes(1); + }); + + test('fails when the tenant OAuth secret has no stored value', async () => { + ddbSend.mockResolvedValueOnce({ + Item: { + jira_cloud_id: 'cloud-123', + site_url: 'https://acme.atlassian.net', + oauth_secret_arn: 'arn:jira-oauth', + status: 'active', + }, + }); + smSend.mockResolvedValueOnce({}); + + await expect(runInvite(['cloud-123', 'maya@example.test'])).rejects.toThrow(/has no SecretString/); + + expect(ddbSend).toHaveBeenCalledTimes(1); + }); + + test('falls back to Jira user search when a direct accountId lookup returns 404', async () => { + mockRegistryAndSecret(); + const fetchMock = jest.fn() + .mockResolvedValueOnce({ + ok: false, + status: 404, + json: async () => ({}), + }) + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ([{ + accountId: 'acct-1', + displayName: 'Maya K', + active: true, + accountType: 'atlassian', + }]), + }); + global.fetch = fetchMock as unknown as typeof fetch; + + await runInvite(['cloud-123', 'acct-1']); + + expect(fetchMock).toHaveBeenCalledTimes(2); + const searchUrl = new URL(String(fetchMock.mock.calls[1][0])); + expect(searchUrl.pathname).toBe('/ex/jira/cloud-123/rest/api/3/user/search'); + const putCmd = ddbSend.mock.calls[1][0] as PutCommand; + expect(putCmd.input.Item).toMatchObject({ jira_account_id: 'acct-1' }); + }); + + test('rejects an ambiguous Jira user search before writing an invite', async () => { + mockRegistryAndSecret(); + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => ([ + { accountId: 'acct-1', displayName: 'Maya K', active: true, accountType: 'atlassian' }, + { accountId: 'acct-2', displayName: 'Maya L', active: true, accountType: 'atlassian' }, + ]), + }); + global.fetch = fetchMock as unknown as typeof fetch; + + await expect(runInvite(['cloud-123', 'maya@example.test'])).rejects.toThrow(/multiple users/); + + expect(ddbSend).toHaveBeenCalledTimes(1); + }); + + test('fails if Atlassian refreshes an expired token without returning a rotated refresh token', async () => { + mockRegistryAndSecret({ + expires_at: new Date(Date.now() - 60_000).toISOString(), + }); + const fetchMock = jest.fn().mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ + access_token: 'new-access', + token_type: 'Bearer', + expires_in: 3600, + scope: 'read:jira-work write:jira-work read:jira-user', + }), + }); + global.fetch = fetchMock as unknown as typeof fetch; + + await expect(runInvite(['cloud-123', 'maya@example.test'])).rejects.toThrow(/returned no refresh_token/); + + expect(smSend).toHaveBeenCalledTimes(1); + expect(ddbSend).toHaveBeenCalledTimes(1); + }); +}); + describe('jira setup action', () => { let loadConfigSpy: jest.SpiedFunction; diff --git a/docs/guides/JIRA_SETUP_GUIDE.md b/docs/guides/JIRA_SETUP_GUIDE.md index 3e8cb050..d4507a01 100644 --- a/docs/guides/JIRA_SETUP_GUIDE.md +++ b/docs/guides/JIRA_SETUP_GUIDE.md @@ -115,36 +115,28 @@ This writes an `active` row keyed `#` into the project-mapp ### 5. Link your Jira identity -So tasks triggered from Jira attribute to your platform user (concurrency caps, billing, `bgagent list`), link your Atlassian `accountId` to your ABCA account. An admin issues you a one-time invite code, then you redeem it: +So tasks triggered from Jira attribute to your platform user (concurrency caps, billing, `bgagent list`), link your Atlassian `accountId` to your ABCA account. An admin issues a one-time invite code, then the teammate redeems it. + +#### Admin: generate the invite ```bash -bgagent jira link +bgagent jira invite-user ``` -The CLI shows the Jira identity (name + email) and the tenant, and asks for confirmation **before** writing the mapping row — so a mis-issued code is caught before it binds. +The command resolves the Jira user through the tenant OAuth token, writes a `pending#` row with a 24-hour TTL, and prints the `bgagent jira link ` command to send to the teammate. It requires admin IAM for the stack tables/secrets and a logged-in `bgagent` CLI session for the `invited_by_platform_user_id` audit field. -> The `invite-user` issuing command is not yet implemented (tracked in [#553](https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/issues/553)). Until it lands, an admin can write the user-mapping row directly. Note that until the row exists, a labeled issue from the unlinked user produces no task — the processor comments "Run `bgagent jira link `" on the issue, but no code can be issued yet. +- `` — the tenant UUID from `setup` or `https://.atlassian.net/_edge/tenant_info` +- `` — the teammate's Atlassian `accountId` or email address. If email search is hidden/ambiguous, use `accountId`; Jira profile URLs end in `/people/`. -**Interim manual linking (admin IAM).** Write an `active` row to the user-mapping table (stack output `JiraUserMappingTableName`), keyed exactly as `jira-link.ts` would write it: +#### Teammate: redeem the invite ```bash -aws dynamodb put-item \ - --table-name \ - --item '{ - "jira_identity": {"S": "#"}, - "platform_user_id": {"S": ""}, - "jira_cloud_id": {"S": ""}, - "jira_account_id": {"S": ""}, - "linked_at": {"S": ""}, - "status": {"S": "active"}, - "link_method": {"S": "manual"} - }' +bgagent jira link ``` -Where to find the two identity values: +The CLI shows the Jira identity (name + email) and the tenant, and asks for confirmation **before** writing the mapping row — so a mis-issued code is caught before it binds. -- **Atlassian `accountId`** — open your Jira profile (avatar → Profile); the URL ends `/people/`. Or call `GET https://api.atlassian.com/ex/jira//rest/api/3/myself` with the stored token. -- **Cognito sub (platform user id)** — `aws cognito-idp admin-get-user --user-pool-id --username --query 'UserAttributes[?Name==`sub`].Value' --output text`. +The teammate needs their own ABCA account first (Cognito user + configured CLI). If they do not have one yet, the admin runs `bgagent admin invite-user teammate@example.com`, then the teammate runs `bgagent configure --from-bundle ` and `bgagent login --username teammate@example.com` before redeeming the Jira invite. ### 6. Test diff --git a/docs/src/content/docs/using/Jira-setup-guide.md b/docs/src/content/docs/using/Jira-setup-guide.md index ce67c8a6..97f09f01 100644 --- a/docs/src/content/docs/using/Jira-setup-guide.md +++ b/docs/src/content/docs/using/Jira-setup-guide.md @@ -119,36 +119,28 @@ This writes an `active` row keyed `#` into the project-mapp ### 5. Link your Jira identity -So tasks triggered from Jira attribute to your platform user (concurrency caps, billing, `bgagent list`), link your Atlassian `accountId` to your ABCA account. An admin issues you a one-time invite code, then you redeem it: +So tasks triggered from Jira attribute to your platform user (concurrency caps, billing, `bgagent list`), link your Atlassian `accountId` to your ABCA account. An admin issues a one-time invite code, then the teammate redeems it. + +#### Admin: generate the invite ```bash -bgagent jira link +bgagent jira invite-user ``` -The CLI shows the Jira identity (name + email) and the tenant, and asks for confirmation **before** writing the mapping row — so a mis-issued code is caught before it binds. +The command resolves the Jira user through the tenant OAuth token, writes a `pending#` row with a 24-hour TTL, and prints the `bgagent jira link ` command to send to the teammate. It requires admin IAM for the stack tables/secrets and a logged-in `bgagent` CLI session for the `invited_by_platform_user_id` audit field. -> The `invite-user` issuing command is not yet implemented (tracked in [#553](https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/issues/553)). Until it lands, an admin can write the user-mapping row directly. Note that until the row exists, a labeled issue from the unlinked user produces no task — the processor comments "Run `bgagent jira link `" on the issue, but no code can be issued yet. +- `` — the tenant UUID from `setup` or `https://.atlassian.net/_edge/tenant_info` +- `` — the teammate's Atlassian `accountId` or email address. If email search is hidden/ambiguous, use `accountId`; Jira profile URLs end in `/people/`. -**Interim manual linking (admin IAM).** Write an `active` row to the user-mapping table (stack output `JiraUserMappingTableName`), keyed exactly as `jira-link.ts` would write it: +#### Teammate: redeem the invite ```bash -aws dynamodb put-item \ - --table-name \ - --item '{ - "jira_identity": {"S": "#"}, - "platform_user_id": {"S": ""}, - "jira_cloud_id": {"S": ""}, - "jira_account_id": {"S": ""}, - "linked_at": {"S": ""}, - "status": {"S": "active"}, - "link_method": {"S": "manual"} - }' +bgagent jira link ``` -Where to find the two identity values: +The CLI shows the Jira identity (name + email) and the tenant, and asks for confirmation **before** writing the mapping row — so a mis-issued code is caught before it binds. -- **Atlassian `accountId`** — open your Jira profile (avatar → Profile); the URL ends `/people/`. Or call `GET https://api.atlassian.com/ex/jira//rest/api/3/myself` with the stored token. -- **Cognito sub (platform user id)** — `aws cognito-idp admin-get-user --user-pool-id --username --query 'UserAttributes[?Name==`sub`].Value' --output text`. +The teammate needs their own ABCA account first (Cognito user + configured CLI). If they do not have one yet, the admin runs `bgagent admin invite-user teammate@example.com`, then the teammate runs `bgagent configure --from-bundle ` and `bgagent login --username teammate@example.com` before redeeming the Jira invite. ### 6. Test From 55f5866fdf84c04d0e4411767a0d209a4587edb8 Mon Sep 17 00:00:00 2001 From: ayushtr-aws Date: Fri, 10 Jul 2026 11:52:28 -0400 Subject: [PATCH 2/4] refactor(cli): address PR review for jira invite-user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract generateInviteCode + INVITE_CODE_ALPHABET into shared cli/src/invite-code.ts; import from both jira.ts and linear.ts (drops the require('crypto') + eslint-disable in the Jira copy). - Move parseStoredJiraOauthToken to jira-oauth.ts alongside the other OAuth helpers; jira.ts stays focused on command wiring. - Warn (non-fatal) in `jira invite-user` when the resolved Jira identity is already actively linked, so the admin knows the code will re-link. - Document `bgagent jira setup/map/invite-user/link` in cli/README.md. - Add tests: already-linked warning, inactive/app account rejection, malformed OAuth secret JSON, missing oauth_secret_arn, and missing stack outputs — closing the branches Codecov flagged. --- cli/README.md | 19 ++++++ cli/src/commands/jira.ts | 63 +++++------------- cli/src/commands/linear.ts | 26 +------- cli/src/invite-code.ts | 54 ++++++++++++++++ cli/src/jira-oauth.ts | 43 ++++++++++++ cli/test/commands/jira.test.ts | 108 +++++++++++++++++++++++++++++-- cli/test/commands/linear.test.ts | 3 +- 7 files changed, 236 insertions(+), 80 deletions(-) create mode 100644 cli/src/invite-code.ts diff --git a/cli/README.md b/cli/README.md index 0f3b6b81..b7998ad4 100644 --- a/cli/README.md +++ b/cli/README.md @@ -302,6 +302,25 @@ With no flags, writes to the platform default `GitHubTokenSecretArn` stack outpu Configure the preview-deploy screenshot pipeline webhook. See [Deploy preview screenshots guide](../docs/guides/DEPLOY_PREVIEW_SCREENSHOTS_GUIDE.md). +### `bgagent jira setup` / `map` / `invite-user` / `link` + +Manage the Jira Cloud integration. `setup` authorizes a tenant via OAuth (3LO) and stores the token in Secrets Manager; `map` routes a Jira project to a GitHub repo; the two-step `invite-user` → `link` handshake links a teammate's Jira identity to their platform user. See the [Jira setup guide](../docs/guides/JIRA_SETUP_GUIDE.md) for the full walkthrough. + +``` +bgagent jira setup \ + --stack-name backgroundagent-dev + +bgagent jira map --repo owner/repo + +bgagent jira invite-user \ + --region AWS region (defaults to configured region) \ + --stack-name CloudFormation stack name (default: backgroundagent-dev) + +bgagent jira link +``` + +`invite-user` resolves the teammate's Jira identity through the tenant OAuth token, then writes a `pending#` row (24h TTL) and prints the `bgagent jira link ` the teammate runs from their own machine. The teammate previews the Jira identity before confirming, so a wrong pick can be aborted rather than misattributed. If the identity is already linked, the command warns but still issues the code. + ### `bgagent admin invite-user` / `list-users` / `delete-user` / `reset-password` Manage Cognito users with operator AWS credentials (`cognito-idp:Admin*` on the deployment user pool). Works **before** `bgagent configure` when `--stack-name` is passed (reads `UserPoolId` from CloudFormation). diff --git a/cli/src/commands/jira.ts b/cli/src/commands/jira.ts index 10f393c7..43d0870b 100644 --- a/cli/src/commands/jira.ts +++ b/cli/src/commands/jira.ts @@ -34,6 +34,7 @@ import { ApiClient } from '../api-client'; import { loadConfig, loadCredentials } from '../config'; import { CliError } from '../errors'; import { formatJson } from '../format'; +import { generateInviteCode } from '../invite-code'; import { buildAuthorizationUrl, computeExpiresAt, @@ -42,6 +43,7 @@ import { generatePkce, isAccessTokenExpiring, jiraOauthSecretName, + parseStoredJiraOauthToken, refreshAccessToken, StoredJiraOauthToken, } from '../jira-oauth'; @@ -56,9 +58,6 @@ const DEFAULT_LABEL_FILTER = 'bgagent'; * accepts at creation time. */ const PROJECT_KEY_RE = /^[A-Z][A-Z0-9_]{1,99}$/; -/** Alphabet used by {@link generateInviteCode}. */ -export const INVITE_CODE_ALPHABET = 'abcdefghjkmnpqrstuvwxyz23456789'; - /** Width of the ═ rule used to frame the setup banner. */ const BANNER_WIDTH = 72; @@ -263,18 +262,6 @@ function isWebhookSecretPlaceholder(value: string): boolean { } } -export function generateInviteCode(): string { - const INVITE_CODE_RANDOM_BYTES = 8; - // eslint-disable-next-line @typescript-eslint/no-require-imports - const { randomBytes } = require('crypto') as typeof import('crypto'); - const bytes = randomBytes(INVITE_CODE_RANDOM_BYTES); - let out = 'link-'; - for (const b of bytes) { - out += INVITE_CODE_ALPHABET[b % INVITE_CODE_ALPHABET.length]; - } - return out; -} - interface JiraUserSearchResult { readonly accountId?: string; readonly displayName?: string; @@ -283,39 +270,6 @@ interface JiraUserSearchResult { readonly accountType?: string; } -function parseStoredJiraOauthToken(secretString: string | undefined, secretId: string): StoredJiraOauthToken { - if (!secretString) { - throw new CliError(`OAuth secret '${secretId}' has no SecretString. Re-run \`bgagent jira setup\`.`); - } - let parsed: unknown; - try { - parsed = JSON.parse(secretString); - } catch { - throw new CliError(`OAuth secret '${secretId}' is not valid JSON. Re-run \`bgagent jira setup\`.`); - } - if (typeof parsed !== 'object' || parsed === null) { - throw new CliError(`OAuth secret '${secretId}' has an unexpected shape. Re-run \`bgagent jira setup\`.`); - } - const obj = parsed as Record; - const required = [ - 'access_token', - 'refresh_token', - 'expires_at', - 'client_id', - 'client_secret', - 'cloud_id', - 'site_url', - ]; - const missing = required.filter((key) => typeof obj[key] !== 'string' || (obj[key] as string).length === 0); - if (missing.length > 0) { - throw new CliError( - `OAuth secret '${secretId}' is missing required field${missing.length === 1 ? '' : 's'} ${missing.join(', ')}. ` - + `Re-run \`bgagent jira setup\` for tenant '${String(obj.cloud_id ?? 'unknown')}'.`, - ); - } - return parsed as StoredJiraOauthToken; -} - function isJiraUser(value: unknown): value is JiraUserSearchResult { if (typeof value !== 'object' || value === null) return false; const obj = value as Record; @@ -861,6 +815,19 @@ export function makeJiraCommand(): Command { throw err; } + // Warn (don't block) if this Jira identity is already linked, so the + // admin knows a fresh invite will re-link an existing teammate. The + // active row key mirrors the jira-link handler: `#`. + const existing = await ddb.send(new GetCommand({ + TableName: userMappingTable!, + Key: { jira_identity: `${cloudId}#${picked.accountId}` }, + })); + if (existing.Item && existing.Item.status === 'active') { + console.log(); + console.log(` ⚠ ${formatJiraUserLabel(picked)} is already linked in this tenant.`); + console.log(' Redeeming this code will re-link them to whoever runs `bgagent jira link`.'); + } + const code = generateInviteCode(); const ttl = Math.floor(Date.now() / 1000) + 24 * 60 * 60; await ddb.send(new PutCommand({ diff --git a/cli/src/commands/linear.ts b/cli/src/commands/linear.ts index 4132e2b5..5817fe5b 100644 --- a/cli/src/commands/linear.ts +++ b/cli/src/commands/linear.ts @@ -35,6 +35,7 @@ import { ApiClient } from '../api-client'; import { loadConfig, loadCredentials } from '../config'; import { CliError } from '../errors'; import { formatJson } from '../format'; +import { generateInviteCode } from '../invite-code'; import { buildAuthorizationUrl, computeExpiresAt, @@ -1733,31 +1734,6 @@ async function runSelfLinkPicker(args: { return true; } -/** - * Generate a short, human-typeable invite code for the teammate-invite - * flow. `link-` prefix makes it grep-friendly when an operator pastes it - * into chat alongside the command. 8 random chars from a 31-char alphabet - * = 40 bits of entropy — over a 24h TTL window with ~10 codes outstanding, - * collision probability is negligible. - * - * Alphabet excludes ambiguous glyphs (0/O, 1/l/I) so codes copy-pasted - * across fonts don't get mistyped. - */ -/** Alphabet used by {@link generateInviteCode}. Exposed so tests can - * assert that generated codes only use these characters. */ -export const INVITE_CODE_ALPHABET = 'abcdefghjkmnpqrstuvwxyz23456789'; - -export function generateInviteCode(): string { - const INVITE_CODE_RANDOM_BYTES = 8; - const bytes = new Uint8Array(INVITE_CODE_RANDOM_BYTES); - crypto.getRandomValues(bytes); - let out = 'link-'; - for (const b of bytes) { - out += INVITE_CODE_ALPHABET[b % INVITE_CODE_ALPHABET.length]; - } - return out; -} - /** * Query the Linear `viewer` + `organization` GraphQL fields with whatever * Authorization header the caller hands us. Used both by the legacy diff --git a/cli/src/invite-code.ts b/cli/src/invite-code.ts new file mode 100644 index 00000000..64b90924 --- /dev/null +++ b/cli/src/invite-code.ts @@ -0,0 +1,54 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +/** + * Shared teammate-invite code generation for the Jira and Linear + * `invite-user` → `link ` handshakes. Both integrations mint the + * same code shape and write a `pending#` row consumed by their + * respective `link` handlers, so the generator lives here rather than + * being duplicated per command file. + * + * Alphabet excludes ambiguous glyphs (0/O, 1/l/I) so codes copy-pasted + * across fonts don't get mistyped. Exposed so tests can assert that + * generated codes only use these characters. + */ +export const INVITE_CODE_ALPHABET = 'abcdefghjkmnpqrstuvwxyz23456789'; + +/** Number of random bytes drawn per code (one alphabet char each). */ +const INVITE_CODE_RANDOM_BYTES = 8; + +/** + * Generate a short, human-typeable invite code for the teammate-invite + * flow. `link-` prefix makes it grep-friendly when an operator pastes it + * into chat alongside the command. 8 random chars from a 31-char alphabet + * = ~40 bits of entropy — over a 24h TTL window with ~10 codes + * outstanding, collision probability is negligible. + * + * Uses the Web Crypto `crypto.getRandomValues` global (available in + * Node 18+) so no `require('crypto')` / eslint-disable is needed. + */ +export function generateInviteCode(): string { + const bytes = new Uint8Array(INVITE_CODE_RANDOM_BYTES); + crypto.getRandomValues(bytes); + let out = 'link-'; + for (const b of bytes) { + out += INVITE_CODE_ALPHABET[b % INVITE_CODE_ALPHABET.length]; + } + return out; +} diff --git a/cli/src/jira-oauth.ts b/cli/src/jira-oauth.ts index bbce6d1a..20aaba5c 100644 --- a/cli/src/jira-oauth.ts +++ b/cli/src/jira-oauth.ts @@ -131,6 +131,49 @@ export function jiraOauthSecretName(cloudId: string): string { return `${JIRA_OAUTH_SECRET_PREFIX}${cloudId}`; } +/** + * Deserialize and validate a stored Jira OAuth bundle read from Secrets + * Manager. Enforces the required-field contract so a truncated or + * malformed secret surfaces an actionable "re-run setup" error instead + * of a downstream `undefined` deref. `secretId` is echoed into the error + * so operators know which secret to fix. + */ +export function parseStoredJiraOauthToken( + secretString: string | undefined, + secretId: string, +): StoredJiraOauthToken { + if (!secretString) { + throw new CliError(`OAuth secret '${secretId}' has no SecretString. Re-run \`bgagent jira setup\`.`); + } + let parsed: unknown; + try { + parsed = JSON.parse(secretString); + } catch { + throw new CliError(`OAuth secret '${secretId}' is not valid JSON. Re-run \`bgagent jira setup\`.`); + } + if (typeof parsed !== 'object' || parsed === null) { + throw new CliError(`OAuth secret '${secretId}' has an unexpected shape. Re-run \`bgagent jira setup\`.`); + } + const obj = parsed as Record; + const required = [ + 'access_token', + 'refresh_token', + 'expires_at', + 'client_id', + 'client_secret', + 'cloud_id', + 'site_url', + ]; + const missing = required.filter((key) => typeof obj[key] !== 'string' || (obj[key] as string).length === 0); + if (missing.length > 0) { + throw new CliError( + `OAuth secret '${secretId}' is missing required field${missing.length === 1 ? '' : 's'} ${missing.join(', ')}. ` + + `Re-run \`bgagent jira setup\` for tenant '${String(obj.cloud_id ?? 'unknown')}'.`, + ); + } + return parsed as StoredJiraOauthToken; +} + /** * Compute when an access token should be considered "stale and needs * refresh." We refresh if there's <60s left. diff --git a/cli/test/commands/jira.test.ts b/cli/test/commands/jira.test.ts index d8fb237d..c0ee9ef1 100644 --- a/cli/test/commands/jira.test.ts +++ b/cli/test/commands/jira.test.ts @@ -25,8 +25,6 @@ import { import { GetCommand, PutCommand } from '@aws-sdk/lib-dynamodb'; import { ApiClient } from '../../src/api-client'; import { - generateInviteCode, - INVITE_CODE_ALPHABET, isWebhookSecretConfigured, JIRA_WEBHOOK_SECRET_PLACEHOLDER_KEY, makeJiraCommand, @@ -35,6 +33,7 @@ import { upsertOauthSecret, } from '../../src/commands/jira'; import * as config from '../../src/config'; +import { generateInviteCode, INVITE_CODE_ALPHABET } from '../../src/invite-code'; import type { StoredJiraOauthToken } from '../../src/jira-oauth'; // child_process.execFile — `openBrowser` shells out to the OS opener. @@ -317,6 +316,9 @@ describe('jira invite-user action', () => { status: 'active', }, }) + // Existing-link lookup: no active mapping for the picked Jira identity. + .mockResolvedValueOnce({}) + // PutCommand writing the pending# row. .mockResolvedValueOnce({}); smSend.mockResolvedValueOnce({ SecretString: JSON.stringify(sampleToken({ @@ -365,7 +367,9 @@ describe('jira invite-user action', () => { headers: { Authorization: 'Bearer access-xyz' }, }); - const putCmd = ddbSend.mock.calls[1][0] as PutCommand; + // calls[0] = registry GetCommand, calls[1] = existing-link GetCommand, + // calls[2] = the pending# PutCommand. + const putCmd = ddbSend.mock.calls[2][0] as PutCommand; expect(putCmd).toBeInstanceOf(PutCommand); expect(putCmd.input.TableName).toBe('JiraUsersTable'); expect(putCmd.input.Item).toMatchObject({ @@ -402,7 +406,7 @@ describe('jira invite-user action', () => { const lookupUrl = new URL(String(fetchMock.mock.calls[0][0])); expect(lookupUrl.pathname).toBe('/ex/jira/cloud-123/rest/api/3/user'); expect(lookupUrl.searchParams.get('accountId')).toBe('acct-1'); - const putCmd = ddbSend.mock.calls[1][0] as PutCommand; + const putCmd = ddbSend.mock.calls[2][0] as PutCommand; expect(putCmd.input.Item).toMatchObject({ jira_account_id: 'acct-1', jira_user_name: 'Maya K', @@ -515,7 +519,7 @@ describe('jira invite-user action', () => { expect(fetchMock).toHaveBeenCalledTimes(2); const searchUrl = new URL(String(fetchMock.mock.calls[1][0])); expect(searchUrl.pathname).toBe('/ex/jira/cloud-123/rest/api/3/user/search'); - const putCmd = ddbSend.mock.calls[1][0] as PutCommand; + const putCmd = ddbSend.mock.calls[2][0] as PutCommand; expect(putCmd.input.Item).toMatchObject({ jira_account_id: 'acct-1' }); }); @@ -557,6 +561,100 @@ describe('jira invite-user action', () => { expect(smSend).toHaveBeenCalledTimes(1); expect(ddbSend).toHaveBeenCalledTimes(1); }); + + test('warns (but still writes the invite) when the Jira identity is already linked', async () => { + // Registry GetCommand → active tenant; existing-link GetCommand → an + // active mapping for the resolved account; PutCommand → the pending row. + ddbSend + .mockResolvedValueOnce({ + Item: { + jira_cloud_id: 'cloud-123', + site_url: 'https://acme.atlassian.net', + oauth_secret_arn: 'arn:jira-oauth', + status: 'active', + }, + }) + .mockResolvedValueOnce({ Item: { jira_identity: 'cloud-123#acct-1', status: 'active' } }) + .mockResolvedValueOnce({}); + smSend.mockResolvedValueOnce({ + SecretString: JSON.stringify(sampleToken({ + cloud_id: 'cloud-123', + site_url: 'https://acme.atlassian.net', + expires_at: new Date(Date.now() + 60 * 60 * 1000).toISOString(), + })), + }); + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ accountId: 'acct-1', displayName: 'Maya K', active: true, accountType: 'atlassian' }), + }); + global.fetch = fetchMock as unknown as typeof fetch; + + await runInvite(['cloud-123', 'acct-1']); + + // Existing-link lookup keyed on `#`. + const linkLookup = ddbSend.mock.calls[1][0] as GetCommand; + expect(linkLookup).toBeInstanceOf(GetCommand); + expect(linkLookup.input.Key).toEqual({ jira_identity: 'cloud-123#acct-1' }); + expect(logSpy.mock.calls.some((call) => String(call[0]).includes('already linked'))).toBe(true); + // The invite is still written despite the warning. + expect(ddbSend.mock.calls[2][0]).toBeInstanceOf(PutCommand); + }); + + test('refuses to invite an inactive or app Jira account', async () => { + mockRegistryAndSecret(); + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ accountId: 'acct-1', displayName: 'Bot', active: false, accountType: 'app' }), + }); + global.fetch = fetchMock as unknown as typeof fetch; + + await expect(runInvite(['cloud-123', 'acct-1'])).rejects.toThrow(/inactive or is an app account/); + + // Registry + secret read happened, but no invite row was written. + expect(ddbSend).toHaveBeenCalledTimes(1); + }); + + test('fails with an actionable error when the OAuth secret is malformed JSON', async () => { + ddbSend.mockResolvedValueOnce({ + Item: { + jira_cloud_id: 'cloud-123', + site_url: 'https://acme.atlassian.net', + oauth_secret_arn: 'arn:jira-oauth', + status: 'active', + }, + }); + smSend.mockResolvedValueOnce({ SecretString: '{not-json' }); + + await expect(runInvite(['cloud-123', 'maya@example.test'])).rejects.toThrow(/is not valid JSON/); + + expect(ddbSend).toHaveBeenCalledTimes(1); + }); + + test('fails when the registry row is missing oauth_secret_arn', async () => { + ddbSend.mockResolvedValueOnce({ + Item: { + jira_cloud_id: 'cloud-123', + site_url: 'https://acme.atlassian.net', + status: 'active', + }, + }); + + await expect(runInvite(['cloud-123', 'maya@example.test'])).rejects.toThrow(/missing oauth_secret_arn/); + + expect(smSend).not.toHaveBeenCalled(); + }); + + test('throws a clear error when required stack outputs are missing (not deployed)', async () => { + cfnSend.mockReset().mockResolvedValue({ Stacks: [{ Outputs: [] }] }); + + await expect(runInvite(['cloud-123', 'maya@example.test'])) + .rejects.toThrow(/missing outputs .*JiraWorkspaceRegistryTableName.*JiraUserMappingTableName/s); + + expect(ddbSend).not.toHaveBeenCalled(); + expect(smSend).not.toHaveBeenCalled(); + }); }); describe('jira setup action', () => { diff --git a/cli/test/commands/linear.test.ts b/cli/test/commands/linear.test.ts index 610ac3f2..db0cf3de 100644 --- a/cli/test/commands/linear.test.ts +++ b/cli/test/commands/linear.test.ts @@ -21,13 +21,12 @@ import { PutCommand, ScanCommand } from '@aws-sdk/lib-dynamodb'; import { autoLinkTokenOwner, findReusableOauthAppCredentials, - generateInviteCode, - INVITE_CODE_ALPHABET, isWebhookSecretConfigured, queryLinearTeamKeys, renderLinearAppTemplate, } from '../../src/commands/linear'; import * as config from '../../src/config'; +import { generateInviteCode, INVITE_CODE_ALPHABET } from '../../src/invite-code'; jest.mock('@aws-sdk/lib-dynamodb', () => { const actual = jest.requireActual('@aws-sdk/lib-dynamodb'); From fb01f9ed37802db14f023ff38205ffb953154b85 Mon Sep 17 00:00:00 2001 From: ayushtr-aws Date: Fri, 10 Jul 2026 12:03:01 -0400 Subject: [PATCH 3/4] refactor(cli): address second-pass PR review nits for jira invite-user theagenticguy's review (against pre-refactor 08a5d81) raised two nits not covered by the earlier fixes: - Remove modulo bias in generateInviteCode via rejection sampling: bytes >= the largest alphabet multiple under 256 are discarded so the char distribution is uniform (previously indices 0-7 were favored 9/256). - Guard the pending-row PutCommand with ConditionExpression: attribute_not_exists(jira_identity) so a code collision fails loudly with an actionable error instead of silently overwriting a still-valid invite. Tests: new cli/test/invite-code.test.ts (100% cover incl. rejection + top-up branches and a distribution smoke check); jira.test.ts gains the conditional-put assertion and a ConditionalCheckFailedException case. The review's third nit (inline require('crypto') + eslint-disable) was already resolved by the prior extraction to cli/src/invite-code.ts. --- cli/src/commands/jira.ts | 42 ++++++++++------- cli/src/invite-code.ts | 33 +++++++++++--- cli/test/commands/jira.test.ts | 45 +++++++++++++++++++ cli/test/invite-code.test.ts | 82 ++++++++++++++++++++++++++++++++++ 4 files changed, 180 insertions(+), 22 deletions(-) create mode 100644 cli/test/invite-code.test.ts diff --git a/cli/src/commands/jira.ts b/cli/src/commands/jira.ts index 43d0870b..a85a6743 100644 --- a/cli/src/commands/jira.ts +++ b/cli/src/commands/jira.ts @@ -830,21 +830,33 @@ export function makeJiraCommand(): Command { const code = generateInviteCode(); const ttl = Math.floor(Date.now() / 1000) + 24 * 60 * 60; - await ddb.send(new PutCommand({ - TableName: userMappingTable!, - Item: { - jira_identity: `pending#${code}`, - status: 'pending', - jira_cloud_id: cloudId, - jira_site_url: siteUrl, - jira_account_id: picked.accountId, - jira_user_name: picked.displayName ?? '', - jira_user_email: picked.emailAddress ?? '', - invited_at: new Date().toISOString(), - invited_by_platform_user_id: callerCognitoSub, - ttl, - }, - })); + try { + await ddb.send(new PutCommand({ + TableName: userMappingTable!, + // Guard against clobbering an existing pending invite on the + // (astronomically unlikely) chance the generated code collides. + // Better to fail loudly and let the admin re-run than silently + // overwrite a still-valid code. + ConditionExpression: 'attribute_not_exists(jira_identity)', + Item: { + jira_identity: `pending#${code}`, + status: 'pending', + jira_cloud_id: cloudId, + jira_site_url: siteUrl, + jira_account_id: picked.accountId, + jira_user_name: picked.displayName ?? '', + jira_user_email: picked.emailAddress ?? '', + invited_at: new Date().toISOString(), + invited_by_platform_user_id: callerCognitoSub, + ttl, + }, + })); + } catch (err) { + if ((err as { name?: string }).name === 'ConditionalCheckFailedException') { + throw new CliError(`Invite code '${code}' collided with an existing invite. Re-run the command to mint a fresh code.`); + } + throw err; + } console.log(); console.log('✅ Invite created.'); diff --git a/cli/src/invite-code.ts b/cli/src/invite-code.ts index 64b90924..918e0b87 100644 --- a/cli/src/invite-code.ts +++ b/cli/src/invite-code.ts @@ -30,8 +30,19 @@ */ export const INVITE_CODE_ALPHABET = 'abcdefghjkmnpqrstuvwxyz23456789'; -/** Number of random bytes drawn per code (one alphabet char each). */ -const INVITE_CODE_RANDOM_BYTES = 8; +/** Number of alphabet characters drawn per code. */ +const INVITE_CODE_LENGTH = 8; + +/** Number of distinct values a single random byte can take. */ +const BYTE_RANGE = 256; + +/** + * Largest multiple of the alphabet length that fits in a byte (248 for a + * 31-char alphabet). Bytes at or above this are rejected so the modulo + * maps uniformly — otherwise indices 0–7 would be favored (9/256 vs + * 8/256), skewing generated codes toward the front of the alphabet. + */ +const REJECTION_CEILING = Math.floor(BYTE_RANGE / INVITE_CODE_ALPHABET.length) * INVITE_CODE_ALPHABET.length; /** * Generate a short, human-typeable invite code for the teammate-invite @@ -41,14 +52,22 @@ const INVITE_CODE_RANDOM_BYTES = 8; * outstanding, collision probability is negligible. * * Uses the Web Crypto `crypto.getRandomValues` global (available in - * Node 18+) so no `require('crypto')` / eslint-disable is needed. + * Node 18+) so no `require('crypto')` / eslint-disable is needed, with + * rejection sampling to keep the character distribution unbiased. */ export function generateInviteCode(): string { - const bytes = new Uint8Array(INVITE_CODE_RANDOM_BYTES); - crypto.getRandomValues(bytes); let out = 'link-'; - for (const b of bytes) { - out += INVITE_CODE_ALPHABET[b % INVITE_CODE_ALPHABET.length]; + // Draw random bytes in batches, rejecting any that would bias the modulo. + // A batch of INVITE_CODE_LENGTH bytes usually suffices; the loop tops up + // if enough bytes land in the rejection tail (rare — ~3% per byte). + const buf = new Uint8Array(INVITE_CODE_LENGTH); + while (out.length < 'link-'.length + INVITE_CODE_LENGTH) { + crypto.getRandomValues(buf); + for (const b of buf) { + if (out.length >= 'link-'.length + INVITE_CODE_LENGTH) break; + if (b >= REJECTION_CEILING) continue; + out += INVITE_CODE_ALPHABET[b % INVITE_CODE_ALPHABET.length]; + } } return out; } diff --git a/cli/test/commands/jira.test.ts b/cli/test/commands/jira.test.ts index c0ee9ef1..0ce1a57d 100644 --- a/cli/test/commands/jira.test.ts +++ b/cli/test/commands/jira.test.ts @@ -616,6 +616,51 @@ describe('jira invite-user action', () => { expect(ddbSend).toHaveBeenCalledTimes(1); }); + test('guards the pending row against code collisions with a conditional put', async () => { + mockRegistryAndSecret(); + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ accountId: 'acct-1', displayName: 'Maya K', active: true, accountType: 'atlassian' }), + }); + global.fetch = fetchMock as unknown as typeof fetch; + + await runInvite(['cloud-123', 'acct-1']); + + const putCmd = ddbSend.mock.calls[2][0] as PutCommand; + expect(putCmd.input.ConditionExpression).toBe('attribute_not_exists(jira_identity)'); + }); + + test('surfaces an actionable error when the generated code collides', async () => { + ddbSend + .mockResolvedValueOnce({ + Item: { + jira_cloud_id: 'cloud-123', + site_url: 'https://acme.atlassian.net', + oauth_secret_arn: 'arn:jira-oauth', + status: 'active', + }, + }) + .mockResolvedValueOnce({}) // existing-link lookup: not linked + .mockRejectedValueOnce(Object.assign(new Error('conditional check failed'), { + name: 'ConditionalCheckFailedException', + })); + smSend.mockResolvedValueOnce({ + SecretString: JSON.stringify(sampleToken({ + cloud_id: 'cloud-123', + expires_at: new Date(Date.now() + 60 * 60 * 1000).toISOString(), + })), + }); + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ accountId: 'acct-1', displayName: 'Maya K', active: true, accountType: 'atlassian' }), + }); + global.fetch = fetchMock as unknown as typeof fetch; + + await expect(runInvite(['cloud-123', 'acct-1'])).rejects.toThrow(/collided with an existing invite/); + }); + test('fails with an actionable error when the OAuth secret is malformed JSON', async () => { ddbSend.mockResolvedValueOnce({ Item: { diff --git a/cli/test/invite-code.test.ts b/cli/test/invite-code.test.ts new file mode 100644 index 00000000..597a7e8b --- /dev/null +++ b/cli/test/invite-code.test.ts @@ -0,0 +1,82 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { generateInviteCode, INVITE_CODE_ALPHABET } from '../src/invite-code'; + +describe('generateInviteCode', () => { + test('emits "link-" prefix followed by exactly 8 alphabet characters', () => { + const code = generateInviteCode(); + expect(code).toMatch(/^link-[a-z0-9]{8}$/); + expect(code).toHaveLength(13); + }); + + test('only uses characters from the unambiguous alphabet', () => { + for (let i = 0; i < 200; i++) { + const chars = generateInviteCode().slice('link-'.length); + for (const c of chars) { + expect(INVITE_CODE_ALPHABET).toContain(c); + } + } + }); + + test('rejects biased bytes (>=248) and tops up from the next batch', () => { + // First batch is all-rejected (255 >= 248 rejection ceiling for a + // 31-char alphabet); the second batch is all-zero → every char is + // the alphabet's first character. Exercises both the `continue` + // rejection branch and the top-up loop. + const spy = jest.spyOn(globalThis.crypto, 'getRandomValues') + .mockImplementationOnce((arr) => { + (arr as Uint8Array).fill(255); + return arr; + }) + .mockImplementation((arr) => { + (arr as Uint8Array).fill(0); + return arr; + }); + try { + const code = generateInviteCode(); + expect(code).toBe(`link-${INVITE_CODE_ALPHABET[0].repeat(8)}`); + // At least two draws: the rejected batch + the top-up batch. + expect(spy.mock.calls.length).toBeGreaterThanOrEqual(2); + } finally { + spy.mockRestore(); + } + }); + + test('distribution is not skewed toward the front of the alphabet', () => { + // With modulo bias, indices 0-7 would be over-represented. Draw a + // large sample and assert the front-eighth of the alphabet is not + // meaningfully more frequent than a uniform expectation. This is a + // smoke check, not a rigorous statistical test. + const counts = new Array(INVITE_CODE_ALPHABET.length).fill(0); + const SAMPLES = 5000; + for (let i = 0; i < SAMPLES; i++) { + for (const c of generateInviteCode().slice('link-'.length)) { + counts[INVITE_CODE_ALPHABET.indexOf(c)]++; + } + } + const total = SAMPLES * 8; + const expectedPerChar = total / INVITE_CODE_ALPHABET.length; + // No character should deviate more than 25% from uniform expectation. + for (const count of counts) { + expect(count).toBeGreaterThan(expectedPerChar * 0.75); + expect(count).toBeLessThan(expectedPerChar * 1.25); + } + }); +}); From fb48926ade6b5a90da48d4b21990caaaf751b824 Mon Sep 17 00:00:00 2001 From: ayushtr-aws Date: Fri, 10 Jul 2026 12:17:08 -0400 Subject: [PATCH 4/4] chore: re-trigger CI