Skip to content

Commit

Permalink
fix: Reduce number of warnings on XSUAA selection (#1593)
Browse files Browse the repository at this point in the history
  • Loading branch information
FrankEssenberger authored Sep 16, 2021
1 parent 27b799d commit 5ac6849
Show file tree
Hide file tree
Showing 9 changed files with 153 additions and 100 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

## Improvements

-
- [core] Reduce the number of warnings when selecting an XSUAA instance.

## Fixed Issues

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ describe('destination loading precedence', () => {
{ cacheVerificationKeys: false }
)
).rejects.toThrowErrorMatchingInlineSnapshot(
'"Unable to get access token for \\"destination\\" service. No service instance of type \\"destination\\" found."'
'"No binding to an XSUAA service instance found. Please make sure to bind an instance of the XSUAA service to your application."'
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ describe('jwtType x selection strategy combinations. Possible values are {subscr
});

it('it warns if you use iss property and user jwt', async () => {
mockServiceBindings();
const logger = createLogger({
package: 'core',
messageContext: 'destination-accessor-service'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@ import { jwtBearerToken, serviceToken } from '../token-accessor';
import { addProxyConfigurationOnPrem } from '../connectivity-service';
import {
getDestinationService,
getDestinationServiceCredentialsList
getDestinationServiceCredentialsList,
getXsuaaServiceCredentials
} from '../environment-accessor';
import { isIdenticalTenant } from '../tenant';
import { DestinationServiceCredentials } from '../environment-accessor-types';
import {
DestinationServiceCredentials,
XsuaaServiceCredentials
} from '../environment-accessor-types';
import { Destination } from './destination-service-types';
import {
alwaysProvider,
Expand Down Expand Up @@ -80,13 +84,16 @@ class DestinationFromServiceRetriever {
): Promise<Destination | null> {
const subscriberToken =
await DestinationFromServiceRetriever.getSubscriberToken(options);

const xsuaaCredentials = getXsuaaServiceCredentials(options.userJwt);
const providerToken =
await DestinationFromServiceRetriever.getProviderClientCredentialsToken(
xsuaaCredentials,
options
);
const da = new DestinationFromServiceRetriever(
name,
options,
{ ...options, xsuaaCredentials },
subscriberToken,
providerToken
);
Expand Down Expand Up @@ -163,11 +170,15 @@ class DestinationFromServiceRetriever {
}

private static async getProviderClientCredentialsToken(
xsuaaCredentials: XsuaaServiceCredentials,
options: DestinationOptions
): Promise<JwtPair> {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { userJwt, ...optionsWithoutJwt } = options;
const encoded = await serviceToken('destination', optionsWithoutJwt);
const encoded = await serviceToken('destination', {
...optionsWithoutJwt,
xsuaaCredentials
});
return { encoded, decoded: decodeJwt(encoded) };
}

Expand All @@ -189,7 +200,7 @@ class DestinationFromServiceRetriever {

private constructor(
readonly name: string,
options: DestinationOptions,
options: DestinationOptions & { xsuaaCredentials: XsuaaServiceCredentials },
readonly subscriberToken: JwtPair | undefined,
readonly providerClientCredentialsToken: JwtPair
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ export type ServiceCredentials = {
/**
* Credentials for the Destination service.
*/
export interface DestinationServiceCredentials {
clientid: string;
clientsecret: string;
export type DestinationServiceCredentials = ServiceCredentials & {
identityzone: string;
instanceid: string;
tenantid: string;
Expand All @@ -37,14 +35,12 @@ export interface DestinationServiceCredentials {
url: string;
verificationkey: string;
xsappname: string;
}
};

/**
* Credentials for the XSUAA service.
*/
export interface XsuaaServiceCredentials {
clientid: string;
clientsecret: string;
export type XsuaaServiceCredentials = ServiceCredentials & {
identityzone: string;
identityzoneid: string;
sburl: string;
Expand All @@ -54,4 +50,4 @@ export interface XsuaaServiceCredentials {
url: string;
verificationkey: string;
xsappname: string;
}
};
170 changes: 110 additions & 60 deletions packages/core/src/connectivity/scp-cf/environment-accessor.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { createLogger, ErrorWithCause, first } from '@sap-cloud-sdk/util';
import {
createLogger,
ErrorWithCause,
first,
unique
} from '@sap-cloud-sdk/util';
import * as xsenv from '@sap/xsenv';
import { JwtPayload } from 'jsonwebtoken';
import { audiences, decodeJwt } from './jwt';
Expand Down Expand Up @@ -201,10 +206,9 @@ export function getDestinationServiceUri(): string | null {
export function getXsuaaServiceCredentials(
token?: JwtPayload | string
): XsuaaServiceCredentials {
if (typeof token === 'string') {
return getXsuaaServiceCredentials(decodeJwt(token)); // Decode without verifying
}
return selectXsuaaInstance(token);
return typeof token === 'string'
? selectXsuaaInstance(decodeJwt(token))
: selectXsuaaInstance(token);
}

/**
Expand Down Expand Up @@ -238,91 +242,137 @@ export function resolveService(service: string | Service): Service {
export function extractClientCredentials(
serviceCreds: ServiceCredentials
): ClientCredentials {
if (!serviceCreds.clientsecret) {
throw new Error(
`Cloud not extract client secret for clientId: ${serviceCreds.clientid}.`
);
}
return {
username: serviceCreds.clientid,
password: serviceCreds.clientsecret
};
}

function selectXsuaaInstance(token?: JwtPayload): XsuaaServiceCredentials {
const xsuaaInstances = getServiceList('xsuaa');
const xsuaaCredentials: XsuaaServiceCredentials[] = getServiceList(
'xsuaa'
).map(service => service.credentials as XsuaaServiceCredentials);

if (!xsuaaInstances.length) {
if (!arrayHasAtLeastOneElement(xsuaaCredentials)) {
throw Error(
'No binding to an XSUAA service instance found. Please make sure to bind an instance of the XSUAA service to your application.'
);
}

const strategies = [matchingClientId, matchingAudience, takeFirstAndWarn];
const selected = applyStrategiesInOrder(strategies, xsuaaInstances, token);
return token
? selectXsuaaInstanceWithJwt(xsuaaCredentials, token)
: selectXsuaaInstanceWithoutJwt(xsuaaCredentials);
}

if (selected.length === 0) {
throw Error('No XSUAA instances are found from the given JWT.');
}
function handleOneXsuuaInstance(
xsuaaCredentials: [XsuaaServiceCredentials]
): XsuaaServiceCredentials {
logger.debug(
`Only one XSUAA instance bound. This one is used: ${xsuaaCredentials[0].xsappname}`
);
return xsuaaCredentials[0];
}

function arrayHasAtLeastOneElement<T>(
array: T[] | [T, ...T[]]
): array is [T, ...T[]] {
return !!array.length && array.length > 0;
}

function arrayHasExactlyOneElement<T>(array: T[] | [T]): array is [T] {
return array.length === 1;
}

if (selected.length > 1) {
function selectXsuaaInstanceWithoutJwt(
xsuaaCredentials: [XsuaaServiceCredentials, ...XsuaaServiceCredentials[]]
): XsuaaServiceCredentials {
if (!arrayHasExactlyOneElement(xsuaaCredentials)) {
logger.warn(
`Multiple XSUAA instances could be matched to the given JWT! Choosing the first one (xsappname: ${
first(selected)!.credentials.xsappname
}).`
`The following XSUAA instances are bound: ${xsuaaCredentials.map(
x => '\n\t- {x.xsappname}'
)}
No JWT given to select XSUAA instance. ${choseFirstOneText(
xsuaaCredentials
)}`
);
return xsuaaCredentials[0];
}

return first(selected)!.credentials;
return handleOneXsuuaInstance(xsuaaCredentials);
}

function applyStrategiesInOrder(
selectionStrategies: SelectionStrategyFn[],
xsuaaInstances: Record<string, any>[],
token?: JwtPayload
): Record<string, any>[] {
return selectionStrategies.reduce(
(result, strategy) =>
result.length ? result : strategy(xsuaaInstances, token),
[]
function selectXsuaaInstanceWithJwt(
xsuaaCredentials: [XsuaaServiceCredentials, ...XsuaaServiceCredentials[]],
jwt: JwtPayload
): XsuaaServiceCredentials {
const selectedAll = [
...matchingClientId(xsuaaCredentials, jwt),
...matchingAudience(xsuaaCredentials, jwt)
];

const selectedUnique = unique(selectedAll.map(x => x.clientid)).map(
id => selectedAll.find(y => y.clientid === id)!
);
}

type SelectionStrategyFn = (
xsuaaInstances: Record<string, any>[],
token?: JwtPayload
) => Record<string, any>[];
if (selectedUnique.length === 1) {
logger.debug(
`One XSUAA instance found using JWT in service binding. Used service name is: ${xsuaaCredentials[0].xsappname}`
);
return xsuaaCredentials[0];
}

function matchingClientId(
xsuaaInstances: Record<string, any>[],
token?: JwtPayload
): Record<string, any>[] {
if (!token) {
return [];
if (selectedUnique.length > 1) {
logger.warn(
`Multiple XSUAA instances could be matched to the given JWT: ${xsuaaCredentials.map(
x => `\n\t- ${x.xsappname}`
)}
${choseFirstOneText(xsuaaCredentials)}`
);
return selectedUnique[0];
}
return xsuaaInstances.filter(
xsuaa => xsuaa.credentials.clientid === token.client_id
);
}

function matchingAudience(
xsuaaInstances: Record<string, any>[],
token?: JwtPayload
): Record<string, any>[] {
if (!token) {
return [];
if (!arrayHasExactlyOneElement(xsuaaCredentials)) {
logger.warn(
`Multiple XSUAA instances found: ${xsuaaCredentials.map(
x => `\n\t- ${x.xsappname}`
)}
None of those match either client id or audience from the given JWT. ${choseFirstOneText(
xsuaaCredentials
)}`
);
return xsuaaCredentials[0];
}
return xsuaaInstances.filter(xsuaa =>
audiences(token).has(xsuaa.credentials.xsappname)
return handleOneXsuuaInstance(xsuaaCredentials);
}

function choseFirstOneText(
xsuaaCredentials: XsuaaServiceCredentials[]
): string {
return `Choosing the first one (xsappname: "${
first(xsuaaCredentials)!.xsappname
}").`;
}

function matchingClientId(
xsuaaCredentials: XsuaaServiceCredentials[],
token: JwtPayload
): XsuaaServiceCredentials[] {
return xsuaaCredentials.filter(
credentials => credentials.clientid === token.client_id
);
}

function takeFirstAndWarn(
xsuaaInstances: Record<string, any>[]
): Record<string, any>[] {
logger.warn(
`Unable to match a specific XSUAA service instance to the given JWT. The following XSUAA instances are bound: ${xsuaaInstances.map(
x => x.credentials.xsappname
)}. The following one will be selected: ${
xsuaaInstances[0].credentials.xsappname
}. This might produce errors in other parts of the system!`
function matchingAudience(
xsuaaCredentials: XsuaaServiceCredentials[],
token: JwtPayload
): XsuaaServiceCredentials[] {
return xsuaaCredentials.filter(credentials =>
audiences(token).has(credentials.xsappname)
);
return xsuaaInstances.slice(0, 1);
}

interface BasicCredentials {
Expand Down
15 changes: 0 additions & 15 deletions packages/core/src/connectivity/scp-cf/jwt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,21 +350,6 @@ export function readPropertyWithWarn(
return jwtPayload[property];
}

/**
* Fetches the URL from the JWT header which exposes the verification key for that JWT.
* @param token - Encoded JWT as a string.
* @returns The value of the `jku` property of the JWT header.
*/
function getVerificationKeyUrl(token: string): string {
const decodedJwt = decodeJwtComplete(token);
if (!decodedJwt.header.jku || !decodedJwt.header.kid) {
throw new Error(
'JWT does not contain verification key URL (`jku`) and/or key ID (`kid`).'
);
}
return decodedJwt.header.jku;
}

/**
* @deprecated Since v1.46.0. This interface will not be replaced. Use the higher level JWT types directly.
* Interface to represent the registered claims of a JWT.
Expand Down
Loading

0 comments on commit 5ac6849

Please sign in to comment.