diff --git a/README.md b/README.md index cc1fe1a..4473991 100644 --- a/README.md +++ b/README.md @@ -610,7 +610,8 @@ FLAGS -y, --yml Output yml --help Show help --license-config=... Product profile(s) for a service, format: - '=[,...]'. Repeat for multiple services. + '=[,...]'. Repeat for + multiple services. --orgId= Organization id --projectName= (required) Name of the project containing the workspace --service-code= (required) Comma-separated list of API service codes to add (e.g. @@ -941,7 +942,8 @@ FLAGS -y, --yml Output yml --help Show help --license-config=... Product profile(s) for a service, format: - '=[,...]'. Repeat for multiple services. + '=[,...]'. Repeat for + multiple services. --orgId= Organization id --projectName= (required) Name of the project containing the workspace --service-code= (required) Comma-separated list of API service codes to add (e.g. diff --git a/src/commands/console/workspace/api/add.js b/src/commands/console/workspace/api/add.js index d578b23..c469c56 100644 --- a/src/commands/console/workspace/api/add.js +++ b/src/commands/console/workspace/api/add.js @@ -21,7 +21,7 @@ const LibConsoleCLI = require('@adobe/aio-cli-lib-console') * Format: "=[,...]" * * @param {string[]} values raw flag values - * @returns {Object} map of sdkCode to list of profile identifiers + * @returns {{[sdkCode: string]: string[]}} map of sdkCode to list of profile identifiers */ function parseLicenseConfigFlags (values) { const result = {} @@ -46,10 +46,15 @@ function parseLicenseConfigFlags (values) { /** * Match requested profile identifiers against a service's available - * licenseConfigs by either id or name. + * licenseConfigs by id, name, or productId. * - * @param {Array<{id: string, name: string, productId: string}>} available - * @param {string[]} requested profile names or ids + * Matching by productId lets users pass the value they see in + * `properties.licenseConfigs[].productId` from `aio console api list`, + * which is convenient for services like Frame.io that expose a single + * profile per product. + * + * @param {Array<{id: string, name: string, productId: string}>} available licenseConfigs reported for the service + * @param {string[]} requested profile names, ids, or productIds * @param {string} sdkCode service code for error messages * @returns {Array} selected licenseConfig objects */ @@ -57,7 +62,7 @@ function resolveLicenseConfigs (available, requested, sdkCode) { const selected = [] const notFound = [] for (const id of requested) { - const match = available.find(lc => lc.id === id || lc.name === id) + const match = available.find(lc => lc.id === id || lc.name === id || lc.productId === id) if (match) { selected.push(match) } else { @@ -74,6 +79,112 @@ function resolveLicenseConfigs (available, requested, sdkCode) { return selected } +/** + * Pick the best service record to subscribe when multiple records share + * the same sdkCode. + * + * Some services (notably Frame.io) appear twice in `getEnabledServicesForOrg`: + * once as `type: 'adobeid'` with no licenseConfigs (browser/SPA flow) and + * once as `type: 'entp'` with the product profile metadata required for + * OAuth Server-to-Server. `Array.find` returns whichever the API lists + * first, which silently drops `--license-config` when the adobeid record + * wins and causes JIL to reject the subscription. Since this command + * always uses OAuth Server-to-Server credentials, prefer the `entp` record + * (and, within that, the one that actually carries licenseConfigs). + * + * @param {Array} services full enabled-services list + * @param {string} code sdkCode to look up + * @returns {object|undefined} the chosen service record, or undefined + */ +function pickServiceForCode (services, code) { + const matches = services.filter(s => s.code === code) + if (matches.length === 0) { + return undefined + } + const hasLicenseConfigs = s => + s.properties && + Array.isArray(s.properties.licenseConfigs) && + s.properties.licenseConfigs.length > 0 + const entpWithProfiles = matches.find(s => s.type === 'entp' && hasLicenseConfigs(s)) + if (entpWithProfiles) { + return entpWithProfiles + } + const entp = matches.find(s => s.type === 'entp') + if (entp) { + return entp + } + return matches[0] +} + +/** + * Reduce the enabled-services list to one record per sdkCode using + * pickServiceForCode. Preserves the original ordering of the chosen records. + * + * @param {Array} services enabled services + * @returns {Array} deduplicated services + */ +function dedupeServicesByCode (services) { + const seen = new Set() + const result = [] + for (const s of services) { + if (seen.has(s.code)) continue + seen.add(s.code) + // pickServiceForCode is guaranteed to return a record because s itself + // is in services and matches by code. + result.push(pickServiceForCode(services, s.code)) + } + return result +} + +/** + * Merge new service-subscription requests with existing services on the + * credential. JIL's PUT-services endpoint replaces the credential's + * service list rather than appending to it, so without this merge a + * subsequent `aio console workspace api add` call would silently wipe + * the services subscribed by an earlier call. + * + * For codes present in both, the new entry wins (the user is overriding + * the existing subscription, including any licenseConfig changes). + * + * @param {Array} existing serviceProperties currently on the credential + * @param {Array} requested serviceProperties the user is adding + * @returns {Array} merged serviceProperties + */ +function mergeServiceProperties (existing, requested) { + const requestedCodes = new Set(requested.map(sp => sp.sdkCode)) + const kept = existing.filter(sp => !requestedCodes.has(sp.sdkCode)) + return [...kept, ...requested] +} + +/** + * Detect JIL subscription errors embedded in a 200 response and throw + * a CLI-friendly error if any are found. + * + * JIL returns `{ error: [...], errorDetails: [{ sdkCode, domain, code, message }...] }` + * for partial/total failures inside an otherwise successful HTTP response, + * so without this check `--json` output silently looks like success. + * + * @param {object} response the subscribe response body + */ +function assertSubscribeSuccess (response) { + if (!response || typeof response !== 'object') { + return + } + const errorDetails = Array.isArray(response.errorDetails) ? response.errorDetails : [] + const errorCodes = Array.isArray(response.error) ? response.error : [] + if (errorDetails.length === 0 && errorCodes.length === 0) { + return + } + const formatted = errorDetails.length > 0 + ? errorDetails.map(d => { + const where = d && d.sdkCode ? `${d.sdkCode}: ` : '' + const message = d == null ? '(unknown error)' : (d.message || JSON.stringify(d)) + return ` ${where}${message}` + }).join('\n') + : ` ${errorCodes.join(', ')}` + throw new Error(`Failed to add API service(s):\n${formatted}`) +} + class AddCommand extends ConsoleCommand { async run () { const { flags } = await this.parse(AddCommand) @@ -107,14 +218,27 @@ class AddCommand extends ConsoleCommand { const licenseConfigMap = parseLicenseConfigFlags(flags['license-config'] || []) + // Fail fast if --license-config references a service code that isn't in + // --service-code. Otherwise the entry is silently ignored, which is the + // exact silent-drop class of bug this command was patched against. + const requestedSet = new Set(requestedCodes) + const orphanLicenseConfigs = Object.keys(licenseConfigMap).filter(c => !requestedSet.has(c)) + if (orphanLicenseConfigs.length > 0) { + this.error( + `--license-config given for service code(s) not in --service-code: ${orphanLicenseConfigs.join(', ')}. ` + + `Requested service codes: ${requestedCodes.join(', ')}.` + ) + } + const enabledServices = await this.consoleCLI.getEnabledServicesForOrg(orgId) - aioConsoleLogger.debug(`Enabled services: ${JSON.stringify(enabledServices.map(s => s.code))}`) + const supportedServices = dedupeServicesByCode(enabledServices) + aioConsoleLogger.debug(`Enabled services (deduped): ${JSON.stringify(supportedServices.map(s => s.code))}`) const serviceProperties = [] const notFound = [] const missingProfiles = [] for (const code of requestedCodes) { - const service = enabledServices.find(s => s.code === code) + const service = supportedServices.find(s => s.code === code) if (!service) { notFound.push(code) continue @@ -154,14 +278,33 @@ class AddCommand extends ConsoleCommand { ) } + // JIL's PUT-services endpoint replaces the credential's service list, + // so fetch what's already subscribed and submit the union — otherwise + // a later `api add` call silently wipes services attached by an earlier + // one. The lib returns [] (not throws) for a workspace without a + // credential yet, so any thrown error here is real (auth, network, + // server) and we let it propagate: proceeding with an empty list + // would cause the very overwrite this merge is supposed to prevent. + const existingProperties = await this.consoleCLI.getServicePropertiesFromWorkspaceWithCredentialType({ + orgId, + projectId: project.id, + workspace, + supportedServices, + credentialType: LibConsoleCLI.OAUTH_SERVER_TO_SERVER_CREDENTIAL + }) + const mergedProperties = mergeServiceProperties(existingProperties, serviceProperties) + aioConsoleLogger.debug(`Submitting service list: ${JSON.stringify(mergedProperties.map(sp => sp.sdkCode))}`) + const result = await this.consoleCLI.subscribeToServicesWithCredentialType({ orgId, project, workspace, - serviceProperties, + serviceProperties: mergedProperties, credentialType: LibConsoleCLI.OAUTH_SERVER_TO_SERVER_CREDENTIAL }) + assertSubscribeSuccess(result) + if (flags.json) { this.printJson(result) } else if (flags.yml) { @@ -200,7 +343,7 @@ AddCommand.flags = { required: true }), 'license-config': Flags.string({ - description: 'Product profile(s) for a service, format: \'=[,...]\'. Repeat for multiple services.', + description: 'Product profile(s) for a service, format: \'=[,...]\'. Repeat for multiple services.', multiple: true }), json: Flags.boolean({ @@ -222,3 +365,7 @@ AddCommand.aliases = [ module.exports = AddCommand module.exports.parseLicenseConfigFlags = parseLicenseConfigFlags module.exports.resolveLicenseConfigs = resolveLicenseConfigs +module.exports.assertSubscribeSuccess = assertSubscribeSuccess +module.exports.pickServiceForCode = pickServiceForCode +module.exports.dedupeServicesByCode = dedupeServicesByCode +module.exports.mergeServiceProperties = mergeServiceProperties diff --git a/test/commands/console/workspace/api/add.test.js b/test/commands/console/workspace/api/add.test.js index 6802071..2c9f7f3 100644 --- a/test/commands/console/workspace/api/add.test.js +++ b/test/commands/console/workspace/api/add.test.js @@ -45,6 +45,7 @@ const mockConsoleCLIInstance = { getProjects: jest.fn().mockResolvedValue([mockProject]), getWorkspaces: jest.fn().mockResolvedValue([mockWorkspace]), getEnabledServicesForOrg: jest.fn().mockResolvedValue(mockEnabledServices), + getServicePropertiesFromWorkspaceWithCredentialType: jest.fn().mockResolvedValue([]), subscribeToServicesWithCredentialType: jest.fn().mockResolvedValue(mockSubscribeResponse) } @@ -56,7 +57,7 @@ jest.mock('@adobe/aio-cli-lib-console', () => ({ })) const TheCommand = require('../../../../../src/commands/console/workspace/api/add') -const { parseLicenseConfigFlags, resolveLicenseConfigs } = TheCommand +const { parseLicenseConfigFlags, resolveLicenseConfigs, assertSubscribeSuccess, pickServiceForCode, dedupeServicesByCode, mergeServiceProperties } = TheCommand describe('parseLicenseConfigFlags', () => { it('should parse a single sdkCode with one profile', () => { @@ -121,6 +122,13 @@ describe('resolveLicenseConfigs', () => { expect(resolveLicenseConfigs(available, ['ProfileB'], 'X')).toEqual([available[1]]) }) + it('should match a profile by productId', () => { + const single = [ + { id: '875473476', name: 'Default Frame.io Enterprise', productId: 'AA458DFE4F7020A0441A' } + ] + expect(resolveLicenseConfigs(single, ['AA458DFE4F7020A0441A'], 'FrameioAPISDK')).toEqual([single[0]]) + }) + it('should match multiple profiles mixing id and name', () => { expect(resolveLicenseConfigs(available, ['lc1', 'ProfileB'], 'X')).toEqual(available) }) @@ -131,6 +139,134 @@ describe('resolveLicenseConfigs', () => { }) }) +describe('pickServiceForCode', () => { + it('should return undefined when no service matches the code', () => { + expect(pickServiceForCode([{ code: 'A', type: 'entp' }], 'B')).toBeUndefined() + }) + + it('should return the single match when there is no duplicate', () => { + const s = { code: 'A', type: 'entp', properties: { licenseConfigs: [{ id: 'lc1' }] } } + expect(pickServiceForCode([s], 'A')).toBe(s) + }) + + it('should prefer the entp record with populated licenseConfigs when a code appears twice', () => { + // Repro of the Frame.io case in getEnabledServicesForOrg + const adobeid = { code: 'FrameioAPISDK', type: 'adobeid', properties: null } + const entp = { + code: 'FrameioAPISDK', + type: 'entp', + properties: { licenseConfigs: [{ id: '875473476', productId: 'AA458DFE4F7020A0441A' }] } + } + expect(pickServiceForCode([adobeid, entp], 'FrameioAPISDK')).toBe(entp) + }) + + it('should fall back to an entp record without profiles before an adobeid record', () => { + const adobeid = { code: 'X', type: 'adobeid', properties: null } + const entp = { code: 'X', type: 'entp', properties: null } + expect(pickServiceForCode([adobeid, entp], 'X')).toBe(entp) + }) + + it('should fall back to the first match when no entp record exists', () => { + const a = { code: 'Y', type: 'adobeid' } + const b = { code: 'Y', type: 'adobeid' } + expect(pickServiceForCode([a, b], 'Y')).toBe(a) + }) +}) + +describe('dedupeServicesByCode', () => { + it('should return a single record per code, preferring entp+licenseConfigs', () => { + const services = [ + { code: 'FrameioAPISDK', type: 'adobeid', properties: null }, + { code: 'FrameioAPISDK', type: 'entp', properties: { licenseConfigs: [{ id: 'lc1' }] } }, + { code: 'OtherSDK', type: 'entp' } + ] + const deduped = dedupeServicesByCode(services) + expect(deduped).toHaveLength(2) + expect(deduped.find(s => s.code === 'FrameioAPISDK')).toBe(services[1]) + expect(deduped.find(s => s.code === 'OtherSDK')).toBe(services[2]) + }) + + it('should be a no-op when no duplicates exist', () => { + const services = [{ code: 'A', type: 'entp' }, { code: 'B', type: 'entp' }] + expect(dedupeServicesByCode(services)).toEqual(services) + }) +}) + +describe('mergeServiceProperties', () => { + it('should append new services when there is no overlap', () => { + const existing = [{ sdkCode: 'A' }] + const requested = [{ sdkCode: 'B' }] + expect(mergeServiceProperties(existing, requested)).toEqual([{ sdkCode: 'A' }, { sdkCode: 'B' }]) + }) + + it('should let the requested entry win for overlapping sdkCodes', () => { + const existing = [{ sdkCode: 'A', licenseConfigs: [{ id: 'old' }] }] + const requested = [{ sdkCode: 'A', licenseConfigs: [{ id: 'new' }] }] + expect(mergeServiceProperties(existing, requested)).toEqual([{ sdkCode: 'A', licenseConfigs: [{ id: 'new' }] }]) + }) + + it('should preserve existing services not in the requested list', () => { + const existing = [{ sdkCode: 'KeepMe' }, { sdkCode: 'Override' }] + const requested = [{ sdkCode: 'Override', updated: true }] + expect(mergeServiceProperties(existing, requested)).toEqual([ + { sdkCode: 'KeepMe' }, + { sdkCode: 'Override', updated: true } + ]) + }) +}) + +describe('assertSubscribeSuccess', () => { + it('should not throw on a normal success response', () => { + expect(() => assertSubscribeSuccess({ sdkList: ['AdobeAnalyticsSDK'] })).not.toThrow() + }) + + it('should not throw on null/undefined', () => { + expect(() => assertSubscribeSuccess(null)).not.toThrow() + expect(() => assertSubscribeSuccess(undefined)).not.toThrow() + }) + + it('should not throw on empty error arrays', () => { + expect(() => assertSubscribeSuccess({ sdkList: [], error: [], errorDetails: [] })).not.toThrow() + }) + + it('should surface the JIL "requires selection of a product" error', () => { + const jilErrorResponse = { + error: ['FrameioAPISDK'], + errorDetails: [{ + sdkCode: 'FrameioAPISDK', + domain: 'JIL', + code: 400, + message: 'Service FrameioAPISDK requires selection of a product' + }] + } + expect(() => assertSubscribeSuccess(jilErrorResponse)) + .toThrow(/Failed to add API service\(s\)[\s\S]*FrameioAPISDK: Service FrameioAPISDK requires selection of a product/) + }) + + it('should fall back to error[] when errorDetails is missing', () => { + expect(() => assertSubscribeSuccess({ error: ['SomeSDK'] })) + .toThrow(/Failed to add API service\(s\)[\s\S]*SomeSDK/) + }) + + it('should format error details that lack a sdkCode', () => { + expect(() => assertSubscribeSuccess({ + errorDetails: [{ domain: 'JIL', code: 500, message: 'kaboom' }] + })).toThrow(/Failed to add API service\(s\)[\s\S]*kaboom/) + }) + + it('should fall back to JSON.stringify when an error detail lacks a message', () => { + expect(() => assertSubscribeSuccess({ + errorDetails: [{ sdkCode: 'WeirdSDK', code: 418 }] + })).toThrow(/Failed to add API service\(s\)[\s\S]*WeirdSDK:[\s\S]*"code":\s*418/) + }) + + it('should render a null entry inside errorDetails as "(unknown error)"', () => { + expect(() => assertSubscribeSuccess({ + errorDetails: [null] + })).toThrow(/Failed to add API service\(s\)[\s\S]*\(unknown error\)/) + }) +}) + describe('console:workspace:api:add', () => { let command @@ -140,6 +276,7 @@ describe('console:workspace:api:add', () => { mockConsoleCLIInstance.getProjects.mockResolvedValue([mockProject]) mockConsoleCLIInstance.getWorkspaces.mockResolvedValue([mockWorkspace]) mockConsoleCLIInstance.getEnabledServicesForOrg.mockResolvedValue(mockEnabledServices) + mockConsoleCLIInstance.getServicePropertiesFromWorkspaceWithCredentialType.mockResolvedValue([]) mockConsoleCLIInstance.subscribeToServicesWithCredentialType.mockResolvedValue(mockSubscribeResponse) }) @@ -246,6 +383,21 @@ describe('console:workspace:api:add', () => { await expect(command.run()).rejects.toThrow('Product profile(s) not found for service AdobeAnalyticsSDK: UnknownProfile') }) + it('should error when --license-config references a code not in --service-code', async () => { + // Catches typos / casing mismatches that would otherwise silently + // drop the license-config entry while the unrelated --service-code + // succeeds. + command.argv = [ + '--service-code', 'AppBuilderDataServicesSDK', + '--projectName', 'myproject', + '--workspaceName', 'Stage', + '--orgId', '12345', + '--license-config', 'FrameIOAPISDK=875473476' + ] + await expect(command.run()).rejects.toThrow(/--license-config given for service code\(s\) not in --service-code: FrameIOAPISDK[\s\S]*Requested service codes: AppBuilderDataServicesSDK/) + expect(mockConsoleCLIInstance.subscribeToServicesWithCredentialType).not.toHaveBeenCalled() + }) + it('should error on malformed --license-config value', async () => { command.argv = [ '--service-code', 'AdobeAnalyticsSDK', @@ -351,6 +503,94 @@ describe('console:workspace:api:add', () => { await expect(command.run()).rejects.toThrow('SDK subscribe failed') }) + it('should prefer the entp duplicate service record when adobeid is listed first', async () => { + // Repro of the Frame.io shape in getEnabledServicesForOrg: same sdkCode + // appears once as adobeid (no licenseConfigs) and once as entp (with + // licenseConfigs). The pre-dedup code subscribed against the adobeid + // record and dropped --license-config silently. + mockConsoleCLIInstance.getEnabledServicesForOrg.mockResolvedValue([ + { name: 'Frame.io API', code: 'FrameioAPISDK', type: 'adobeid', properties: null }, + { + name: 'Frame.io API', + code: 'FrameioAPISDK', + type: 'entp', + properties: { + roles: [{ id: 1102, code: 'frame.s2s.all', name: null }], + licenseConfigs: [{ id: '875473476', name: 'Default Frame.io Enterprise', productId: 'AA458DFE4F7020A0441A' }] + } + } + ]) + command.argv = [ + '--service-code', 'FrameioAPISDK', + '--projectName', 'myproject', + '--workspaceName', 'Stage', + '--orgId', '12345', + '--license-config', 'FrameioAPISDK=875473476' + ] + await command.run() + + const call = mockConsoleCLIInstance.subscribeToServicesWithCredentialType.mock.calls[0][0] + expect(call.serviceProperties).toHaveLength(1) + expect(call.serviceProperties[0].sdkCode).toBe('FrameioAPISDK') + expect(call.serviceProperties[0].licenseConfigs).toEqual([ + { id: '875473476', name: 'Default Frame.io Enterprise', productId: 'AA458DFE4F7020A0441A' } + ]) + }) + + it('should merge new services with services already on the credential', async () => { + // JIL's PUT-services replaces the credential's service list, so the + // command must submit the union of existing + new. + mockConsoleCLIInstance.getServicePropertiesFromWorkspaceWithCredentialType.mockResolvedValue([ + { name: 'Existing SDK', sdkCode: 'ExistingSDK', roles: null, licenseConfigs: null } + ]) + command.argv = [ + '--service-code', 'AppBuilderDataServicesSDK', + '--projectName', 'myproject', + '--workspaceName', 'Stage', + '--orgId', '12345' + ] + await command.run() + + const call = mockConsoleCLIInstance.subscribeToServicesWithCredentialType.mock.calls[0][0] + expect(call.serviceProperties.map(sp => sp.sdkCode)).toEqual(['ExistingSDK', 'AppBuilderDataServicesSDK']) + }) + + it('should propagate failures to fetch existing services instead of overwriting silently', async () => { + // The lib returns [] (not throws) for a workspace without a credential + // yet, so a thrown error here is a real auth/network/server failure. + // Swallowing it and proceeding with [] would cause the credential's + // current services to be replaced by just the requested adds — the + // exact overwrite this merge is supposed to prevent. + mockConsoleCLIInstance.getServicePropertiesFromWorkspaceWithCredentialType.mockRejectedValue(new Error('network blew up')) + command.argv = [ + '--service-code', 'AppBuilderDataServicesSDK', + '--projectName', 'myproject', + '--workspaceName', 'Stage', + '--orgId', '12345' + ] + await expect(command.run()).rejects.toThrow('network blew up') + expect(mockConsoleCLIInstance.subscribeToServicesWithCredentialType).not.toHaveBeenCalled() + }) + + it('should surface JIL embedded errors as a CLI error', async () => { + mockConsoleCLIInstance.subscribeToServicesWithCredentialType.mockResolvedValue({ + error: ['AppBuilderDataServicesSDK'], + errorDetails: [{ + sdkCode: 'AppBuilderDataServicesSDK', + domain: 'JIL', + code: 400, + message: 'Service AppBuilderDataServicesSDK requires selection of a product' + }] + }) + command.argv = [ + '--service-code', 'AppBuilderDataServicesSDK', + '--projectName', 'myproject', + '--workspaceName', 'Stage', + '--orgId', '12345' + ] + await expect(command.run()).rejects.toThrow('requires selection of a product') + }) + it('should output JSON when --json is used', async () => { const stdoutSpy = jest.spyOn(process.stdout, 'write').mockImplementation(() => true) const stderrSpy = jest.spyOn(process.stderr, 'write').mockImplementation(() => true)