From 17320c0de1bb6342ce817b873301f897164f3317 Mon Sep 17 00:00:00 2001 From: Frank Essenberger <56544999+FrankEssenberger@users.noreply.github.com> Date: Thu, 26 Aug 2021 19:41:56 +0200 Subject: [PATCH] fix: basic authentication and onPrem Connectivity (#1547) --- CHANGELOG.md | 1 + .../scp-cf/connectivity-service.spec.ts | 4 +- .../scp-cf/connectivity-service.ts | 52 ++++-- ...ation-accessor-authentication-type.spec.ts | 163 +++++++++++++++--- ...ation-accessor-proxy-configuration.spec.ts | 19 +- .../destination/destination-cache.spec.ts | 6 +- .../destination/destination-from-service.ts | 8 +- .../example-destination-service-responses.ts | 27 ++- .../test/auth-flows/auth-flow-util.ts | 3 + .../test/auth-flows/auth-flow.spec.ts | 17 +- 10 files changed, 236 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15517718a0..48180f9173 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ ## Fixed Issues +- [core] Fix failing destination retrieval for `OnPremise` proxy type and basic authentication. - [openapi-generator] Fix generation of options per service configuration files to always use POSIX-style file path separators independent of operating system. diff --git a/packages/core/src/connectivity/scp-cf/connectivity-service.spec.ts b/packages/core/src/connectivity/scp-cf/connectivity-service.spec.ts index 868d9ceae8..4224c824a2 100644 --- a/packages/core/src/connectivity/scp-cf/connectivity-service.spec.ts +++ b/packages/core/src/connectivity/scp-cf/connectivity-service.spec.ts @@ -52,12 +52,14 @@ describe('connectivity-service', () => { const input: Destination = { url: 'https://example.com', - proxyType: 'OnPremise' + proxyType: 'OnPremise', + authentication: 'PrincipalPropagation' }; const expected: Destination = { url: 'https://example.com', proxyType: 'OnPremise', + authentication: 'PrincipalPropagation', proxyConfiguration: { ...mockedConnectivityServiceProxyConfig, headers: { diff --git a/packages/core/src/connectivity/scp-cf/connectivity-service.ts b/packages/core/src/connectivity/scp-cf/connectivity-service.ts index 1b764d0d65..3089b01878 100644 --- a/packages/core/src/connectivity/scp-cf/connectivity-service.ts +++ b/packages/core/src/connectivity/scp-cf/connectivity-service.ts @@ -1,10 +1,14 @@ import { createLogger, ErrorWithCause } from '@sap-cloud-sdk/util'; import { Protocol } from './protocol'; import { ProxyConfiguration } from './connectivity-service-types'; -import { Destination } from './destination/destination-service-types'; +import { + AuthenticationType, + Destination +} from './destination/destination-service-types'; import { EnvironmentAccessor } from './environment-accessor'; import { Service } from './environment-accessor-types'; import { serviceToken } from './token-accessor'; +import { decodeJwt, isUserToken, JwtPair } from './jwt'; const logger = createLogger({ package: 'core', @@ -28,18 +32,30 @@ export function addProxyConfiguration( ): Promise { return Promise.resolve() .then(() => proxyHostAndPort()) - .then(hostAndPort => addHeaders(hostAndPort, jwt)) + .then(hostAndPort => + addHeaders(hostAndPort, destination.authentication, jwt) + ) .then(proxyConfiguration => ({ ...destination, proxyConfiguration })); } +// TODO: remove string argument in v2.0 export function addProxyConfigurationOnPrem( destination: Destination, - jwt?: string + jwt: string | JwtPair | undefined ): Promise { - return addProxyConfiguration(destination, jwt); + const jwtPair = + typeof jwt === 'string' ? { encoded: jwt, decoded: decodeJwt(jwt) } : jwt; + if ( + destination.authentication === 'PrincipalPropagation' && + !isUserToken(jwtPair) + ) { + throw new Error('For principal propagation a user JWT is needed.'); + } + + return addProxyConfiguration(destination, jwtPair?.encoded); } interface HostAndPort { @@ -73,6 +89,7 @@ function readConnectivityServiceBinding(): Service { function addHeaders( hostAndPort: HostAndPort, + authenticationType: AuthenticationType | undefined, jwt?: string ): Promise { const connServiceBinding = readConnectivityServiceBinding(); @@ -81,7 +98,7 @@ function addHeaders( .then(() => proxyAuthorizationHeader(connServiceBinding, jwt)) .then(proxyAuthHeader => ({ ...proxyAuthHeader, - ...sapConnectivityAuthenticationHeader(jwt) + ...sapConnectivityAuthenticationHeader(authenticationType, jwt) })) .then( headers => @@ -109,16 +126,25 @@ function proxyAuthorizationHeader( } function sapConnectivityAuthenticationHeader( + authenticationType: AuthenticationType | undefined, jwt?: string ): Record { - if (jwt) { - return { - 'SAP-Connectivity-Authentication': `Bearer ${jwt}` - }; + if (authenticationType === 'PrincipalPropagation') { + if (jwt) { + return { + 'SAP-Connectivity-Authentication': `Bearer ${jwt}` + }; + } + throw new Error( + `Unable to create "SAP-Connectivity-Authentication" header: no JWT found on the current request. + Connecting to on-premise systems via principle propagation is not possible.` + ); + } + if (authenticationType === 'BasicAuthentication') { + logger.warn( + 'You are connecting to an On-Premise system using basic authentication. For productive usage Principal propagation is recommended.' + ); } - logger.warn( - `Unable to create "SAP-Connectivity-Authentication" header: no JWT found on the current request. - Continuing without header. Connecting to on-premise systems may not be possible.` - ); + return {}; } diff --git a/packages/core/src/connectivity/scp-cf/destination/destination-accessor-authentication-type.spec.ts b/packages/core/src/connectivity/scp-cf/destination/destination-accessor-authentication-type.spec.ts index aaa281bb47..d347a745bd 100644 --- a/packages/core/src/connectivity/scp-cf/destination/destination-accessor-authentication-type.spec.ts +++ b/packages/core/src/connectivity/scp-cf/destination/destination-accessor-authentication-type.spec.ts @@ -1,6 +1,10 @@ import nock from 'nock'; -import { mockServiceBindings } from '../../../../test/test-util/environment-mocks'; import { + mockServiceBindings, + onlyIssuerXsuaaUrl +} from '../../../../test/test-util/environment-mocks'; +import { + expectAllMocksUsed, mockJwtBearerToken, mockServiceToken } from '../../../../test/test-util/token-accessor-mocks'; @@ -11,6 +15,7 @@ import { mockVerifyJwt } from '../../../../test/test-util/destination-service-mocks'; import { + onlyIssuerServiceToken, providerJwtBearerToken, providerServiceToken, providerUserJwt, @@ -30,7 +35,10 @@ import { oauthPasswordSingleResponse, oauthSingleResponse, oauthUserTokenExchangeMultipleResponse, - oauthUserTokenExchangeSingleResponse + oauthUserTokenExchangeSingleResponse, + onPremiseBasicMultipleResponse, + onPremiseBasicSingleResponse, + onPremisePrincipalPropagationMultipleResponse } from '../../../../test/test-util/example-destination-service-responses'; import { clientCredentialsTokenCache } from '../client-credentials-token-cache'; import { wrapJwtInHeader } from '../jwt'; @@ -77,7 +85,7 @@ describe('authentication types', () => { userJwt: subscriberUserJwt }); expect(actual).toMatchObject(expected); - httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); + expectAllMocksUsed(httpMocks); }); it('returns a destination with authTokens if its authenticationType is OAuth2SAMLBearerFlow, provider tenant', async () => { @@ -110,7 +118,7 @@ describe('authentication types', () => { }); expect(actual).toMatchObject(expected); - httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); + expectAllMocksUsed(httpMocks); }); it('should use provider client credentials token for SystemUser exists in provider destination', async () => { @@ -145,7 +153,7 @@ describe('authentication types', () => { cacheVerificationKeys: false }); expect(actual).toMatchObject(expected); - httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); + expectAllMocksUsed(httpMocks); }); it('should use subscriber client credentials token for SystemUser exists in subscriber destination', async () => { @@ -181,7 +189,7 @@ describe('authentication types', () => { userJwt: subscriberUserJwt }); expect(actual).toMatchObject(expected); - httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); + expectAllMocksUsed(httpMocks); }); }); @@ -214,7 +222,7 @@ describe('authentication types', () => { userJwt: subscriberUserJwt }); expect(actual).toMatchObject(expected); - httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); + expectAllMocksUsed(httpMocks); }); it('returns a destination with authTokens if its authenticationType is OAuth2ClientCredentials, provider tenant', async () => { @@ -243,7 +251,7 @@ describe('authentication types', () => { const expected = parseDestination(oauthClientCredentialsSingleResponse); const actual = await getDestination(destinationName); expect(actual).toMatchObject(expected); - httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); + expectAllMocksUsed(httpMocks); }); }); @@ -276,7 +284,7 @@ describe('authentication types', () => { userJwt: providerUserJwt }); expect(actual).toMatchObject(expected); - httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); + expectAllMocksUsed(httpMocks); }); it('should use the provider access token as auth header and subscriber jwt as exchange header for provider destination and provider jwt', async () => { @@ -311,7 +319,7 @@ describe('authentication types', () => { userJwt: subscriberUserJwt }); expect(actual).toMatchObject(expected); - httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); + expectAllMocksUsed(httpMocks); }); it('should use the subscriber access token as auth header and subscriber jwt as exchange header for provider destination and provider jwt', async () => { @@ -346,7 +354,7 @@ describe('authentication types', () => { userJwt: subscriberUserJwt }); expect(actual).toMatchObject(expected); - httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); + expectAllMocksUsed(httpMocks); }); }); @@ -381,7 +389,7 @@ describe('authentication types', () => { expect(actual!.certificates!.length).toBe(1); expect(actual!.keyStoreName).toBe('key.p12'); expect(actual!.keyStorePassword).toBe('password'); - httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); + expectAllMocksUsed(httpMocks); }); it('returns a destination with certificates if the authentication type is ClientCertificateAuthentication, provider tenant', async () => { @@ -413,7 +421,7 @@ describe('authentication types', () => { expect(actual!.certificates!.length).toBe(1); expect(actual!.keyStoreName).toBe('key.p12'); expect(actual!.keyStorePassword).toBe('password'); - httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); + expectAllMocksUsed(httpMocks); }); }); @@ -422,18 +430,54 @@ describe('authentication types', () => { clientCredentialsTokenCache.clear(); }); + it('returns a destination with OnPrem connectivity and basic auth', async () => { + mockServiceBindings(); + mockVerifyJwt(); + mockServiceToken(); + const httpMocks = [ + mockInstanceDestinationsCall(nock, [], 200, subscriberServiceToken), + mockSubaccountDestinationsCall( + nock, + onPremiseBasicMultipleResponse, + 200, + subscriberServiceToken + ), + mockSingleDestinationCall( + nock, + onPremiseBasicSingleResponse, + 200, + destinationName, + wrapJwtInHeader(subscriberServiceToken).headers + ) + ]; + + const actual = await getDestination('OnPremise', { + userJwt: subscriberServiceToken, + cacheVerificationKeys: false, + selectionStrategy: alwaysSubscriber + }); + expect(actual?.proxyConfiguration).toMatchObject({ + host: 'proxy.example.com', + port: 12345, + protocol: 'http', + headers: { 'Proxy-Authorization': expect.stringMatching(/Bearer.*/) } + }); + }); + it('returns a destination without authTokens if its authenticationType is Basic', async () => { mockServiceBindings(); mockVerifyJwt(); const serviceTokenSpy = mockServiceToken(); - mockInstanceDestinationsCall(nock, [], 200, subscriberServiceToken); - mockSubaccountDestinationsCall( - nock, - basicMultipleResponse, - 200, - subscriberServiceToken - ); + const httpMocks = [ + mockInstanceDestinationsCall(nock, [], 200, subscriberServiceToken), + mockSubaccountDestinationsCall( + nock, + basicMultipleResponse, + 200, + subscriberServiceToken + ) + ]; const expected = parseDestination(basicMultipleResponse[0]); const actual = await getDestination(destinationName, { @@ -442,6 +486,83 @@ describe('authentication types', () => { }); expect(actual).toMatchObject(expected); expect(serviceTokenSpy).toHaveBeenCalled(); + expectAllMocksUsed(httpMocks); + }); + }); + + describe('authentication type Principal Propagation', () => { + it('returns a destination with onPrem connectivity and principal propagation', async () => { + mockServiceBindings(); + mockVerifyJwt(); + mockServiceToken(); + + const httpMocks = [ + mockInstanceDestinationsCall(nock, [], 200, subscriberServiceToken), + mockSubaccountDestinationsCall( + nock, + onPremisePrincipalPropagationMultipleResponse, + 200, + subscriberServiceToken + ) + ]; + + const actual = await getDestination('OnPremise', { + userJwt: subscriberServiceToken, + cacheVerificationKeys: false, + selectionStrategy: alwaysSubscriber + }); + expect(actual?.proxyConfiguration).toMatchObject({ + host: 'proxy.example.com', + port: 12345, + protocol: 'http', + headers: { + 'Proxy-Authorization': expect.stringMatching(/Bearer.*/), + 'SAP-Connectivity-Authentication': expect.stringMatching(/Bearer.*/) + } + }); + expectAllMocksUsed(httpMocks); + }); + + it('fails for Principal Propagation and no user JWT', async () => { + mockServiceBindings(); + mockVerifyJwt(); + mockServiceToken(); + + const httpMocks = [ + mockInstanceDestinationsCall(nock, [], 200, providerServiceToken), + mockSubaccountDestinationsCall( + nock, + onPremisePrincipalPropagationMultipleResponse, + 200, + providerServiceToken + ) + ]; + + await expect(getDestination('OnPremise')).rejects.toThrowError( + 'For principal propagation a user JWT is needed.' + ); + expectAllMocksUsed(httpMocks); + }); + + it('fails for Principal Propagation and issuer JWT', async () => { + mockServiceBindings(); + mockVerifyJwt(); + mockServiceToken(); + + const httpMocks = [ + mockInstanceDestinationsCall(nock, [], 200, onlyIssuerServiceToken), + mockSubaccountDestinationsCall( + nock, + onPremisePrincipalPropagationMultipleResponse, + 200, + onlyIssuerServiceToken + ) + ]; + + await expect( + getDestination('OnPremise', { iss: onlyIssuerXsuaaUrl }) + ).rejects.toThrowError('For principal propagation a user JWT is needed.'); + expectAllMocksUsed(httpMocks); }); }); @@ -476,7 +597,7 @@ describe('authentication types', () => { const expected = parseDestination(oauthPasswordSingleResponse); const actual = await getDestination(destinationName, {}); expect(actual).toMatchObject(expected); - httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); + expectAllMocksUsed(httpMocks); }); }); }); diff --git a/packages/core/src/connectivity/scp-cf/destination/destination-accessor-proxy-configuration.spec.ts b/packages/core/src/connectivity/scp-cf/destination/destination-accessor-proxy-configuration.spec.ts index 4d28d1663b..e75a9c2f44 100644 --- a/packages/core/src/connectivity/scp-cf/destination/destination-accessor-proxy-configuration.spec.ts +++ b/packages/core/src/connectivity/scp-cf/destination/destination-accessor-proxy-configuration.spec.ts @@ -14,13 +14,13 @@ import { } from '../../../../test/test-util/destination-service-mocks'; import { providerServiceToken, - subscriberServiceToken, - subscriberUserJwt + subscriberServiceToken } from '../../../../test/test-util/mocked-access-tokens'; import { basicMultipleResponse, destinationName, - onPremiseMultipleResponse + onPremiseMultipleResponse, + onPremisePrincipalPropagationMultipleResponse } from '../../../../test/test-util/example-destination-service-responses'; import { Protocol } from '../protocol'; import { getDestination } from './destination-accessor'; @@ -69,7 +69,7 @@ describe('proxy configuration', () => { const httpMocks = [ mockInstanceDestinationsCall( nock, - onPremiseMultipleResponse, + onPremisePrincipalPropagationMultipleResponse, 200, subscriberServiceToken ), @@ -99,15 +99,13 @@ describe('proxy configuration', () => { mockServiceToken(); const httpMocks = [ - mockSubaccountDestinationsCall(nock, [], 200, subscriberServiceToken), mockInstanceDestinationsCall(nock, [], 200, providerServiceToken), mockSubaccountDestinationsCall( nock, onPremiseMultipleResponse, 200, providerServiceToken - ), - mockInstanceDestinationsCall(nock, [], 200, subscriberServiceToken) + ) ]; const expected = { @@ -120,14 +118,11 @@ describe('proxy configuration', () => { proxyConfiguration: { ...mockedConnectivityServiceProxyConfig, headers: { - 'Proxy-Authorization': `Bearer ${subscriberServiceToken}`, - 'SAP-Connectivity-Authentication': `Bearer ${subscriberUserJwt}` + 'Proxy-Authorization': `Bearer ${providerServiceToken}` } } }; - const actual = await getDestination('OnPremise', { - userJwt: subscriberUserJwt - }); + const actual = await getDestination('OnPremise'); expect(actual).toEqual(expected); httpMocks.forEach(mock => expect(mock.isDone()).toBe(true)); }); diff --git a/packages/core/src/connectivity/scp-cf/destination/destination-cache.spec.ts b/packages/core/src/connectivity/scp-cf/destination/destination-cache.spec.ts index 0af2504c7b..a19d180605 100644 --- a/packages/core/src/connectivity/scp-cf/destination/destination-cache.spec.ts +++ b/packages/core/src/connectivity/scp-cf/destination/destination-cache.spec.ts @@ -28,7 +28,7 @@ import { destinationName, oauthMultipleResponse, oauthSingleResponse, - onPremiseMultipleResponse + onPremisePrincipalPropagationMultipleResponse } from '../../../../test/test-util/example-destination-service-responses'; import { destinationServiceCache } from './destination-service-cache'; import { getDestination } from './destination-accessor'; @@ -372,7 +372,7 @@ describe('caching destination integration tests', () => { mockInstanceDestinationsCall(nock, [], 200, providerServiceToken), mockSubaccountDestinationsCall( nock, - onPremiseMultipleResponse, + onPremisePrincipalPropagationMultipleResponse, 200, providerServiceToken ) @@ -398,7 +398,7 @@ describe('caching destination integration tests', () => { Name: 'OnPremise', URL: 'my.on.premise.system:54321', ProxyType: 'OnPremise', - Authentication: 'NoAuthentication' + Authentication: 'PrincipalPropagation' }), proxyConfiguration: { ...mockedConnectivityServiceProxyConfig, diff --git a/packages/core/src/connectivity/scp-cf/destination/destination-from-service.ts b/packages/core/src/connectivity/scp-cf/destination/destination-from-service.ts index 081eb2c76a..2dca5a83bd 100644 --- a/packages/core/src/connectivity/scp-cf/destination/destination-from-service.ts +++ b/packages/core/src/connectivity/scp-cf/destination/destination-from-service.ts @@ -420,13 +420,7 @@ class DestinationFromServiceRetriever { ): Promise { switch (proxyStrategy(destination)) { case ProxyStrategy.ON_PREMISE_PROXY: - if (!isUserToken(this.subscriberToken)) { - throw new Error('For principal propagation a user JWT is needed.'); - } - return addProxyConfigurationOnPrem( - destination, - this.subscriberToken.encoded - ); + return addProxyConfigurationOnPrem(destination, this.subscriberToken); case ProxyStrategy.INTERNET_PROXY: return addProxyConfigurationInternet(destination); case ProxyStrategy.NO_PROXY: diff --git a/packages/core/test/test-util/example-destination-service-responses.ts b/packages/core/test/test-util/example-destination-service-responses.ts index 1160a6c2ca..be29fbec73 100644 --- a/packages/core/test/test-util/example-destination-service-responses.ts +++ b/packages/core/test/test-util/example-destination-service-responses.ts @@ -95,12 +95,7 @@ export const oauthClientCredentialsSingleResponse: DestinationJson = destinationSingleResponse(oauthClientCredentialsMultipleResponse); export const onPremiseMultipleResponse: DestinationConfiguration[] = [ - { - Name: 'OnPremise', - URL: 'my.on.premise.system:54321', - ProxyType: 'OnPremise', - Authentication: 'NoAuthentication' as AuthenticationType - } + getOnPremDestination('NoAuthentication') ]; export const basicMultipleResponse: DestinationConfiguration[] = [ @@ -115,3 +110,23 @@ export const basicMultipleResponse: DestinationConfiguration[] = [ Password: 'password' } ]; + +export const onPremiseBasicMultipleResponse: DestinationConfiguration[] = [ + getOnPremDestination('BasicAuthentication') +]; +export const onPremiseBasicSingleResponse: DestinationJson = + destinationSingleResponse(oauthMultipleResponse); + +export const onPremisePrincipalPropagationMultipleResponse: DestinationConfiguration[] = + [getOnPremDestination('PrincipalPropagation')]; +export const onPremisePrincipalPropagationSingleResponse: DestinationJson = + destinationSingleResponse(onPremisePrincipalPropagationMultipleResponse); + +function getOnPremDestination(authType: AuthenticationType) { + return { + Name: 'OnPremise', + URL: 'my.on.premise.system:54321', + ProxyType: 'OnPremise', + Authentication: authType + }; +} diff --git a/test-packages/integration-tests/test/auth-flows/auth-flow-util.ts b/test-packages/integration-tests/test/auth-flows/auth-flow-util.ts index 06203730c4..1db4d320f9 100644 --- a/test-packages/integration-tests/test/auth-flows/auth-flow-util.ts +++ b/test-packages/integration-tests/test/auth-flows/auth-flow-util.ts @@ -36,6 +36,9 @@ export interface UserAccessTokens { } export interface Systems { + s4onPrem: { + providerBasic: string; + }; s4: { providerBasic: string; providerClientCert: string; diff --git a/test-packages/integration-tests/test/auth-flows/auth-flow.spec.ts b/test-packages/integration-tests/test/auth-flows/auth-flow.spec.ts index 15aef604c1..8ef3783fea 100644 --- a/test-packages/integration-tests/test/auth-flows/auth-flow.spec.ts +++ b/test-packages/integration-tests/test/auth-flows/auth-flow.spec.ts @@ -6,7 +6,8 @@ import { serviceToken, userApprovedServiceToken, wrapJwtInHeader, - jwtBearerToken + jwtBearerToken, + getDestinationFromDestinationService } from '@sap-cloud-sdk/core'; import { BusinessPartner } from '@sap/cloud-sdk-vdm-business-partner-service'; import { @@ -85,6 +86,20 @@ describe('OAuth flows', () => { expect(result.length).toBe(1); }, 60000); + xit('BasicAuth onPrem Basic Authentication', async () => { + const destination = await getDestinationFromDestinationService( + systems.s4onPrem.providerBasic, + {} + ); + + expect(destination!.proxyConfiguration).toMatchObject({ + headers: { 'Proxy-Authorization': expect.stringMatching(/Bearer.*/) }, + host: expect.stringMatching(/.*sap\.hana\.ondemand\.com/), + port: expect.stringMatching(/\d+/), + protocol: 'http' + }); + }, 60000); + xit('BasicAuth: Provider Destination & Provider Token + PUT request (csrf token)', async () => { const clientGrant = await serviceToken('destination', { userJwt: accessToken.provider