Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "fix: fix device code polling bug",
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "patch"
}
107 changes: 55 additions & 52 deletions lib/msal-common/src/client/DeviceCodeClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,32 @@ export class DeviceCodeClient extends BaseClient {
return parameterBuilder.createQueryString();
}

/**
* Breaks the polling with specific conditions.
* @param request CommonDeviceCodeRequest
* @param deviceCodeResponse DeviceCodeResponse
*/
private continuePolling(
deviceCodeExpirationTime: number,
userSpecifiedTimeout?: number,
userSpecifiedCancelFlag?: boolean,
): boolean {
if (userSpecifiedCancelFlag) {
this.logger.error("Token request cancelled by setting DeviceCodeRequest.cancel = true");
throw ClientAuthError.createDeviceCodeCancelledError();
} else if (userSpecifiedTimeout && userSpecifiedTimeout < deviceCodeExpirationTime && TimeUtils.nowSeconds() > userSpecifiedTimeout) {
this.logger.error(`User defined timeout for device code polling reached. The timeout was set for ${userSpecifiedTimeout}`);
throw ClientAuthError.createUserTimeoutReachedError();
} else if (TimeUtils.nowSeconds() > deviceCodeExpirationTime) {
if (userSpecifiedTimeout) {
this.logger.verbose(`User specified timeout ignored as the device code has expired before the timeout elapsed. The user specified timeout was set for ${userSpecifiedTimeout}`);
}
this.logger.error(`Device code expired. Expiration time of device code was ${deviceCodeExpirationTime}`);
throw ClientAuthError.createDeviceCodeExpiredError();
}
return true;
}

/**
* Creates token request with device code response and polls token endpoint at interval set by the device code
* response
Expand All @@ -151,58 +177,35 @@ export class DeviceCodeClient extends BaseClient {
* Poll token endpoint while (device code is not expired AND operation has not been cancelled by
* setting CancellationToken.cancel = true). POST request is sent at interval set by pollingIntervalMilli
*/
return new Promise<ServerAuthorizationTokenResponse>((resolve, reject) => {

const intervalId: ReturnType<typeof setTimeout> = setInterval(async () => {
try {
if (request.cancel) {

this.logger.error("Token request cancelled by setting DeviceCodeRequest.cancel = true");
clearInterval(intervalId);
reject(ClientAuthError.createDeviceCodeCancelledError());

} else if (userSpecifiedTimeout && userSpecifiedTimeout < deviceCodeExpirationTime && TimeUtils.nowSeconds() > userSpecifiedTimeout) {

this.logger.error(`User defined timeout for device code polling reached. The timeout was set for ${userSpecifiedTimeout}`);
clearInterval(intervalId);
reject(ClientAuthError.createUserTimeoutReachedError());

} else if (TimeUtils.nowSeconds() > deviceCodeExpirationTime) {

if (userSpecifiedTimeout) {
this.logger.verbose(`User specified timeout ignored as the device code has expired before the timeout elapsed. The user specified timeout was set for ${userSpecifiedTimeout}`);
}

this.logger.error(`Device code expired. Expiration time of device code was ${deviceCodeExpirationTime}`);
clearInterval(intervalId);
reject(ClientAuthError.createDeviceCodeExpiredError());

} else {
const thumbprint: RequestThumbprint = {
clientId: this.config.authOptions.clientId,
authority: request.authority,
scopes: request.scopes
};
const response = await this.executePostToTokenEndpoint(
this.authority.tokenEndpoint,
requestBody,
headers,
thumbprint);

if (response.body && response.body.error === Constants.AUTHORIZATION_PENDING) {
// user authorization is pending. Sleep for polling interval and try again
this.logger.info(response.body.error_description || "no_error_description");
} else {
clearInterval(intervalId);
resolve(response.body);
}
}
} catch (error) {
clearInterval(intervalId);
reject(error);
}
}, pollingIntervalMilli);
});
while (this.continuePolling(deviceCodeExpirationTime, userSpecifiedTimeout, request.cancel)) {
const thumbprint: RequestThumbprint = {
clientId: this.config.authOptions.clientId,
authority: request.authority,
scopes: request.scopes
};
const response = await this.executePostToTokenEndpoint(
this.authority.tokenEndpoint,
requestBody,
headers,
thumbprint);

if (response.body && response.body.error === Constants.AUTHORIZATION_PENDING) {
// user authorization is pending. Sleep for polling interval and try again
this.logger.info(response.body.error_description || "Authorization pending. Continue polling.");

await TimeUtils.delay(pollingIntervalMilli);
} else {
this.logger.verbose("Authorization completed successfully. Polling stopped.");
return response.body;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a verbose log message here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

}
}

/*
* The above code should've thrown by this point, but to satisfy TypeScript,
* and in the rare case the conditionals in continuePolling() may not catch everything...
*/
this.logger.error("Polling stopped for unknown reasons.");
throw ClientAuthError.createDeviceCodeUnknownError();
}

/**
Expand Down
11 changes: 11 additions & 0 deletions lib/msal-common/src/error/ClientAuthError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ export const ClientAuthErrorMessage = {
code: "device_code_expired",
desc: "Device code is expired."
},
DeviceCodeUnknownError: {
code: "device_code_unknown_error",
desc: "Device code stopped polling for unknown reasons."
},
NoAccountInSilentRequest: {
code: "no_account_in_silent_request",
desc: "Please pass an account object, silent flow is not supported without account information"
Expand Down Expand Up @@ -388,6 +392,13 @@ export class ClientAuthError extends AuthError {
return new ClientAuthError(ClientAuthErrorMessage.DeviceCodeExpired.code, `${ClientAuthErrorMessage.DeviceCodeExpired.desc}`);
}

/**
* Throws error if device code is expired
*/
static createDeviceCodeUnknownError(): ClientAuthError {
return new ClientAuthError(ClientAuthErrorMessage.DeviceCodeUnknownError.code, `${ClientAuthErrorMessage.DeviceCodeUnknownError.desc}`);
}

/**
* Throws error when silent requests are made without an account object
*/
Expand Down
9 changes: 9 additions & 0 deletions lib/msal-common/src/utils/TimeUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,13 @@ export class TimeUtils {
// If current time + offset is greater than token expiration time, then token is expired.
return (offsetCurrentTimeSec > expirationSec);
}

/**
* Waits for t number of milliseconds
* @param t number
* @param value T
*/
static delay<T>(t: number, value?: T): Promise<T | void> {
return new Promise((resolve) => setTimeout(() => resolve(value), t));
}
}
3 changes: 2 additions & 1 deletion lib/msal-common/test/client/DeviceCodeClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ describe("DeviceCodeClient unit tests", () => {
it("Throw device code expired exception if device code is expired", async () => {
jest.setTimeout(6000);
sinon.stub(DeviceCodeClient.prototype, <any>"executePostRequestToDeviceCodeEndpoint").resolves(DEVICE_CODE_EXPIRED_RESPONSE);
sinon.stub(BaseClient.prototype, <any>"executePostToTokenEndpoint").resolves(AUTHORIZATION_PENDING_RESPONSE);

const request: CommonDeviceCodeRequest = {
authority: TEST_CONFIG.validAuthority,
Expand All @@ -308,7 +309,7 @@ describe("DeviceCodeClient unit tests", () => {
correlationId: "test-correlationId",
scopes: [...TEST_CONFIG.DEFAULT_GRAPH_SCOPE, ...TEST_CONFIG.DEFAULT_SCOPES],
deviceCodeCallback: () => {},
timeout: DEVICE_CODE_RESPONSE.interval, // Setting a timeout equal to the interval polling time to allow for one call to the token endpoint
timeout: DEVICE_CODE_RESPONSE.interval - 1, // Setting a timeout equal to the interval polling time minus one to allow for one call to the token endpoint
};

const client = new DeviceCodeClient(config);
Expand Down