Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into dependabot-npm_and_ya…
Browse files Browse the repository at this point in the history
…rn-pino-8.17.2
  • Loading branch information
mshanemc committed Jan 10, 2024
2 parents c63976b + bd16906 commit 918b43e
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 21 deletions.
29 changes: 29 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,32 @@
## [6.4.6](https://github.com/forcedotcom/sfdx-core/compare/6.4.5...6.4.6) (2024-01-09)


### Bug Fixes

* address review comments ([ea7ad36](https://github.com/forcedotcom/sfdx-core/commit/ea7ad36c65231b9a289c19f5dd185ca5bcf9c735))
* better header check ([ae4757f](https://github.com/forcedotcom/sfdx-core/commit/ae4757f99746866b6bf0e73c4d8f5e9ac8e418c0))
* handle html server response ([90d025d](https://github.com/forcedotcom/sfdx-core/commit/90d025d07e749499206c96f2eabe2055899f38cb))



## [6.4.5](https://github.com/forcedotcom/sfdx-core/compare/6.4.4...6.4.5) (2024-01-09)


### Bug Fixes

* ignore requests for site icons ([aeab4a9](https://github.com/forcedotcom/sfdx-core/commit/aeab4a9eb0c11b987d9d20e217726688fbc3fd9b))



## [6.4.4](https://github.com/forcedotcom/sfdx-core/compare/6.4.3...6.4.4) (2024-01-04)


### Bug Fixes

* remove slash char from error message ([1bb300b](https://github.com/forcedotcom/sfdx-core/commit/1bb300b2bb9f4990c6e19b9b8c33453416c24c95))



## [6.4.3](https://github.com/forcedotcom/sfdx-core/compare/6.4.2...6.4.3) (2023-12-31)


Expand Down
2 changes: 1 addition & 1 deletion LICENSE.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Copyright (c) 2023, Salesforce.com, Inc.
Copyright (c) 2024, Salesforce.com, Inc.
All rights reserved.

Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:
Expand Down
4 changes: 4 additions & 0 deletions messages/auth.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ Invalid request method: %s

Invalid request uri: %s

# error.HttpApi

HTTP response contains html content. Check that the org exists and can be reached.

# pollingTimeout

The device authorization request timed out. After executing force:auth:device:login, you must approve access to the device within 10 minutes. This can happen if the URL wasn’t copied into the browser, login was not attempted, or the 2FA process was not completed within 10 minutes. Request authorization again.
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@salesforce/core",
"version": "6.4.3",
"version": "6.4.6",
"description": "Core libraries to interact with SFDX projects, orgs, and APIs.",
"main": "lib/exported",
"types": "lib/exported.d.ts",
Expand Down
42 changes: 27 additions & 15 deletions src/deviceOauthService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
/* eslint-disable @typescript-eslint/ban-types */

import Transport from 'jsforce/lib/transport';
import { AsyncCreatable, Duration, parseJsonMap } from '@salesforce/kit';
import { AsyncCreatable, Duration, parseJsonMap, sleep } from '@salesforce/kit';
import { HttpRequest, OAuth2Config } from 'jsforce';
import { ensureString, JsonMap, Nullable } from '@salesforce/ts-types';
import { ensureString, isString, JsonMap, Nullable } from '@salesforce/ts-types';
import * as FormData from 'form-data';
import { Logger } from './logger/logger';
import { AuthInfo, DEFAULT_CONNECTED_APP_INFO } from './org/authInfo';
Expand Down Expand Up @@ -39,15 +39,23 @@ export interface DeviceCodePollingResponse extends JsonMap {
issued_at: string;
}

async function wait(ms = 1000): Promise<void> {
return new Promise((resolve) => {
setTimeout(resolve, ms);
});
interface DeviceCodeAuthError extends SfError {
error: string;
error_description: string;
status: number;
}

async function makeRequest<T extends JsonMap>(options: HttpRequest): Promise<T> {
const rawResponse = await new Transport().httpRequest(options);

if (rawResponse?.headers?.['content-type'] === 'text/html') {
const htmlResponseError = messages.createError('error.HttpApi');
htmlResponseError.setData(rawResponse.body);
throw htmlResponseError;
}

const response = parseJsonMap<T>(rawResponse.body);

if (response.error) {
const errorDescription = typeof response.error_description === 'string' ? response.error_description : '';
const error = typeof response.error === 'string' ? response.error : 'Unknown';
Expand Down Expand Up @@ -175,21 +183,25 @@ export class DeviceOauthService extends AsyncCreatable<OAuth2Config> {
);
try {
return await makeRequest<DeviceCodePollingResponse>(httpRequest);
} catch (e) {
/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/restrict-template-expressions */
const err: any = (e as SfError).data;
if (err.error && err.status === 400 && err.error === 'authorization_pending') {
} catch (e: unknown) {
if (e instanceof SfError && e.name === 'HttpApiError') {
throw e;
}

const err = (e instanceof SfError ? e.data : SfError.wrap(isString(e) ? e : 'unknown').data) as
| DeviceCodeAuthError
| undefined;
if (err?.error && err?.status === 400 && err?.error === 'authorization_pending') {
// do nothing because we're still waiting
} else {
if (err.error && err.error_description) {
if (err?.error && err?.error_description) {
this.logger.error(`Polling error: ${err.error}: ${err.error_description}`);
} else {
this.logger.error('Unknown Polling Error:');
this.logger.error(err);
this.logger.error(err ?? e);
}
throw err;
throw err ?? e;
}
/* eslint-enable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/restrict-template-expressions */
}
}

Expand All @@ -212,7 +224,7 @@ export class DeviceOauthService extends AsyncCreatable<OAuth2Config> {
} else {
this.logger.debug(`waiting ${interval} ms...`);
// eslint-disable-next-line no-await-in-loop
await wait(interval);
await sleep(interval);
this.pollingCount += 1;
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/webOAuthServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ import { JwtOAuth2Config } from './org/authInfo';
Messages.importMessagesDirectory(__dirname);
const messages = Messages.loadMessages('@salesforce/core', 'auth');

// Server ignores requests for site icons
const iconPaths = ['/favicon.ico', '/apple-touch-icon-precomposed.png'];

/**
* Handles the creation of a web server for web based login flows.
*
Expand Down Expand Up @@ -186,6 +189,8 @@ export class WebOAuthServer extends AsyncCreatable<WebOAuthServer.Options> {
this.webServer.reportSuccess(response);
} else if (url.pathname === '/OauthError') {
this.webServer.reportError(this.oauthError, response);
} else if (iconPaths.includes(url.pathname ?? '')) {
this.logger.debug(`Ignoring request for icon path: ${url.pathname}`);
} else {
this.webServer.sendError(404, 'Resource not found', response);
const errName = 'invalidRequestUri';
Expand Down Expand Up @@ -213,7 +218,7 @@ export class WebOAuthServer extends AsyncCreatable<WebOAuthServer.Options> {
private parseAuthCodeFromRequest(response: http.ServerResponse, request: WebOAuthServer.Request): Nullable<string> {
if (!this.validateState(request)) {
const error = new SfError('urlStateMismatch');
this.webServer.sendError(400, `${error.message}\n`, response);
this.webServer.sendError(400, error.message, response);
this.closeRequest(request);
this.logger.warn('urlStateMismatchAttempt detected.');
if (!get(this.webServer.server, 'urlStateMismatchAttempt')) {
Expand Down
40 changes: 37 additions & 3 deletions test/unit/deviceOauthServiceTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ const devicePollingResponse = {
issued_at: '1234',
};

const htmlResponse = '<html><body>Server down for maintenance</body></html>';

type UnknownError = {
error: string;
status: number;
Expand Down Expand Up @@ -71,18 +73,40 @@ describe('DeviceOauthService', () => {
describe('requestDeviceLogin', () => {
it('should return the device code response', async () => {
stubMethod($$.SANDBOX, Transport.prototype, 'httpRequest').returns(
Promise.resolve({ body: JSON.stringify(deviceCodeResponse) })
Promise.resolve({
body: JSON.stringify(deviceCodeResponse),
headers: { 'content-type': 'application/json;charset=UTF-8' },
})
);
const service = await DeviceOauthService.create({});
const login = await service.requestDeviceLogin();
expect(login).to.deep.equal(deviceCodeResponse);
});

it('should handle HTML response with proper error', async () => {
stubMethod($$.SANDBOX, Transport.prototype, 'httpRequest').returns(
Promise.resolve({ body: htmlResponse, headers: { 'content-type': 'text/html' } })
);
const service = await DeviceOauthService.create({});
try {
await service.requestDeviceLogin();
expect(true).to.be.false;
} catch (err) {
expect(err).to.have.property('name', 'HttpApiError');
expect(err)
.to.have.property('message')
.and.contain('HTTP response contains html content. Check that the org exists and can be reached.');
}
});
});

describe('awaitDeviceApproval', () => {
it('should return the device polling response', async () => {
stubMethod($$.SANDBOX, Transport.prototype, 'httpRequest').returns(
Promise.resolve({ body: JSON.stringify(devicePollingResponse) })
Promise.resolve({
body: JSON.stringify(devicePollingResponse),
headers: { 'content-type': 'application/json;charset=UTF-8' },
})
);
const service = await DeviceOauthService.create({});
const approval = await service.awaitDeviceApproval(deviceCodeResponse);
Expand All @@ -96,10 +120,16 @@ describe('DeviceOauthService', () => {
Promise.resolve({
statusCode: 400,
body: JSON.stringify({ error: 'authorization_pending' }),
headers: { 'content-type': 'application/json;charset=UTF-8' },
})
)
.onSecondCall()
.returns(Promise.resolve({ body: JSON.stringify(devicePollingResponse) }));
.returns(
Promise.resolve({
body: JSON.stringify(devicePollingResponse),
headers: { 'content-type': 'application/json;charset=UTF-8' },
})
);
const shouldContinuePollingStub = stubMethod($$.SANDBOX, DeviceOauthService.prototype, 'shouldContinuePolling')
.onFirstCall()
.returns(true)
Expand All @@ -119,6 +149,7 @@ describe('DeviceOauthService', () => {
service.pollingCount = DeviceOauthService.POLLING_COUNT_MAX + 1;
try {
await service.awaitDeviceApproval(deviceCodeResponse);
expect(true).to.be.false;
} catch (err) {
expect((err as Error).name).to.equal('PollingTimeoutError');
}
Expand All @@ -132,6 +163,7 @@ describe('DeviceOauthService', () => {
const service = await DeviceOauthService.create({});
try {
await service.awaitDeviceApproval(deviceCodeResponse);
expect(true).to.be.false;
} catch (err) {
// @ts-expect-error: because private member
expect(service.pollingCount).to.equal(0);
Expand All @@ -146,10 +178,12 @@ describe('DeviceOauthService', () => {
error: 'Invalid grant type',
error_description: 'Invalid grant type',
}),
headers: { 'content-type': 'application/json;charset=UTF-8' },
}));
const service = await DeviceOauthService.create({});
try {
await service.awaitDeviceApproval(deviceCodeResponse);
expect(true).to.be.false;
} catch (err) {
// @ts-expect-error: because private member
expect(service.pollingCount).to.equal(0);
Expand Down
67 changes: 67 additions & 0 deletions test/unit/webOauthServerTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,73 @@ describe('WebOauthServer', () => {
expect(reportErrorSpy.callCount).to.equal(1);
expect(reportErrorSpy.args[0][0]).to.equal(authError);
});

it('should ignore requests for favicon and continue', async () => {
const oauthServer = await WebOAuthServer.create({ oauthConfig: {} });
const validateStateStub = stubMethod($$.SANDBOX, oauthServer, 'validateState').returns(true);
await oauthServer.start();

// @ts-expect-error because private member
const webServer = oauthServer.webServer;
const reportSuccessSpy = spyMethod($$.SANDBOX, webServer, 'reportSuccess');

const origOn = webServer.server.on;
let requestListener: http.RequestListener;
stubMethod($$.SANDBOX, webServer.server, 'on').callsFake((event, callback) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-return,@typescript-eslint/no-unsafe-argument
if (event !== 'request') return origOn.call(webServer.server, event, callback);
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
requestListener = callback;
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
callback(
{
method: 'GET',
url: 'http://localhost:1717/favicon.ico',
},
{
setHeader: () => {},
writeHead: () => {},
end: () => {},
}
);
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
callback(
{
method: 'GET',
url: `http://localhost:1717/OauthRedirect?code=${authCode}&state=972475373f51`,
query: { code: authCode },
},
{
setHeader: () => {},
writeHead: () => {},
end: () => {},
}
);
});

// stub the redirect to ensure proper redirect handling and the web server is closed.
redirectStub = stubMethod($$.SANDBOX, webServer, 'doRedirect').callsFake(async (status, url, response) => {
expect(status).to.equal(303);
expect(url).to.equal('/OauthSuccess');
expect(response).to.be.ok;
// eslint-disable-next-line @typescript-eslint/await-thenable
await requestListener(
// @ts-expect-error
{ method: 'GET', url: `http://localhost:1717${url}` },
{
setHeader: () => {},
writeHead: () => {},
end: () => {},
}
);
});

const authInfo = await oauthServer.authorizeAndSave();
expect(authInfo.getFields()).to.deep.equal(authFields);
expect(redirectStub.callCount).to.equal(1);
expect(validateStateStub.callCount).to.equal(1);
expect(reportSuccessSpy.callCount).to.equal(1);
});
});

it('should error if postback has error', async () => {
Expand Down

0 comments on commit 918b43e

Please sign in to comment.