Skip to content

Commit

Permalink
chore: prevent storing of refresh token or usage of client id/secret
Browse files Browse the repository at this point in the history
Signed-off-by: Dustin Popp <dustinpopp@ibm.com>
  • Loading branch information
dpopp07 committed Oct 8, 2024
1 parent 9932e85 commit ff8f4a0
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 4 deletions.
31 changes: 31 additions & 0 deletions auth/token-managers/iam-assume-token-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import logger from '../../lib/logger';
import { onlyOne, validateInput } from '../utils/helpers';
import { buildUserAgent } from '../../lib/build-user-agent';
import { IamRequestBasedTokenManager, IamRequestOptions } from './iam-request-based-token-manager';
Expand Down Expand Up @@ -104,6 +105,12 @@ export class IamAssumeTokenManager extends IamRequestBasedTokenManager {
// be ignored, so we can pass the options structure to that constructor as-is.
this.iamDelegate = new IamTokenManager(options);

// These options are used by the delegate token manager
// but they are not supported by this token manager.
this.clientId = undefined;
this.clientSecret = undefined;
this.scope = undefined;

// Set the grant type and user agent for this flavor of authentication.
this.formData.grant_type = 'urn:ibm:params:oauth:grant-type:assume';
this.userAgent = buildUserAgent('iam-assume-authenticator');
Expand All @@ -127,4 +134,28 @@ export class IamAssumeTokenManager extends IamRequestBasedTokenManager {

return super.requestToken();
}

/**
* Extend this method from the parent class to erase the refresh token from
* the class - we do not want to expose it for IAM Assume authentication.
*
* @param tokenResponse - the response object from JWT service request
*/
protected saveTokenInfo(tokenResponse): void {
super.saveTokenInfo(tokenResponse);
this.refreshToken = undefined;
}

/**
* This token manager doesn't save the refresh token but this method is still
* exposed by the underlying class, so we override it here to log a warning.
*
* @returns the refresh token
*/
public getRefreshToken(): string {
logger.warn(
'The IamAssumeTokenManager does not store the refresh token - it will be undefined.'
);
return super.getRefreshToken();
}
}
6 changes: 3 additions & 3 deletions auth/token-managers/iam-request-based-token-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ export interface IamRequestOptions extends JwtTokenManagerOptions {
* class be extended with specific implementations.
*/
export class IamRequestBasedTokenManager extends JwtTokenManager {
private clientId: string;
protected clientId: string;

private clientSecret: string;
protected clientSecret: string;

private scope: string;
protected scope: string;

protected refreshToken: string;

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
"eslint:fix": "eslint . --fix",
"eslint:check": "eslint . --cache",
"lint": "npm run eslint:check",
"lint:fix": "npm run eslint:fix",
"fix": "npm run eslint:fix",
"jest": "jest",
"test": "jest test/unit/",
"test-travis": "jest --runInBand test/unit/",
Expand Down
63 changes: 63 additions & 0 deletions test/unit/iam-assume-token-manager.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ describe('IAM Assume Token Manager', () => {
RequestWrapper.mockImplementation(() => ({
sendRequest: sendRequestMock,
}));
afterEach(() => {
sendRequestMock.mockClear();
});
afterAll(() => {
sendRequestMock.mockRestore();
});
Expand Down Expand Up @@ -210,6 +213,39 @@ describe('IAM Assume Token Manager', () => {

expect(instance.userAgent).toMatch('iam-assume-authenticator');
});

it('should store client id, secret, and scope in delegate but not in the class', () => {
const instance = new IamAssumeTokenManager({
apikey: IAM_APIKEY,
iamProfileCrn: IAM_PROFILE_CRN,
clientId: 'some-id',
clientSecret: 'some-secret',
scope: 'some-scope',
});

expect(instance.clientId).toBeUndefined();
expect(instance.clientSecret).toBeUndefined();
expect(instance.scope).toBeUndefined();
expect(instance.iamDelegate.clientId).toBe('some-id');
expect(instance.iamDelegate.clientSecret).toBe('some-secret');
expect(instance.iamDelegate.scope).toBe('some-scope');
});
});

describe('getters', () => {
it('should warn if user calls getRefreshToken', () => {
const warnSpy = jest.spyOn(logger, 'warn');
const instance = new IamAssumeTokenManager({
apikey: IAM_APIKEY,
iamProfileCrn: IAM_PROFILE_CRN,
});

instance.getRefreshToken();
expect(warnSpy).toHaveBeenCalledWith(
'The IamAssumeTokenManager does not store the refresh token - it will be undefined.'
);
warnSpy.mockRestore();
});
});

describe('requestToken', () => {
Expand Down Expand Up @@ -306,6 +342,33 @@ describe('IAM Assume Token Manager', () => {
const accessToken = await instance.getToken();

expect(accessToken).toBe(OTHER_ACCESS_TOKEN);

// Ensure the refresh token is NOT saved after `getToken` is called.
expect(instance.refreshToken).toBeUndefined();
});

it('should use client id, secret, and scope in delegate request but not the manager request', async () => {
const instance = new IamAssumeTokenManager({
apikey: IAM_APIKEY,
iamProfileCrn: IAM_PROFILE_CRN,
clientId: 'some-id',
clientSecret: 'some-secret',
scope: 'some-scope',
});

await instance.requestToken();

// Check the IAM delegate's request first.
const iamRequestOptions = getRequestOptions(sendRequestMock, 0);
expect(iamRequestOptions.headers).toBeDefined();
expect(iamRequestOptions.headers.Authorization).toBe('Basic c29tZS1pZDpzb21lLXNlY3JldA==');
expect(iamRequestOptions.form.scope).toBe('some-scope');

// Then, look at the second request from the IAM Assume token manager.
const assumeRequestOptions = getRequestOptions(sendRequestMock, 1);
expect(assumeRequestOptions.headers).toBeDefined();
expect(assumeRequestOptions.headers.Authorization).toBeUndefined();
expect(assumeRequestOptions.form.scope).toBeUndefined();
});
});
});

0 comments on commit ff8f4a0

Please sign in to comment.