Skip to content

Commit 6696005

Browse files
author
Prithvi Kanherkar
authored
Merge pull request #2994 from AzureAD/remove-expireson-null-type
Fix typing for expiresOn field
2 parents afb615e + 2846601 commit 6696005

File tree

6 files changed

+28
-15
lines changed

6 files changed

+28
-15
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Fix typing for expiresOn field (#2994)",
4+
"packageName": "@azure/msal-common",
5+
"email": "prkanher@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

lib/msal-common/src/error/ClientAuthError.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,11 @@ export const ClientAuthErrorMessage = {
169169
},
170170
noAuthorizationCodeFromServer: {
171171
code: "authorization_code_missing_from_server_response",
172-
desc: "Srver response does not contain an authorization code to proceed"
172+
desc: "Server response does not contain an authorization code to proceed"
173+
},
174+
accessTokenEntityNullError: {
175+
code: "access_token_entity_null",
176+
desc: "Access token entity is null, please check logs and cache to ensure a valid access token is present."
173177
}
174178
};
175179

@@ -486,4 +490,11 @@ export class ClientAuthError extends AuthError {
486490
static createNoAuthCodeInServerResponseError(): ClientAuthError {
487491
return new ClientAuthError(ClientAuthErrorMessage.noAuthorizationCodeFromServer.code, ClientAuthErrorMessage.noAuthorizationCodeFromServer.desc);
488492
}
493+
494+
/**
495+
* Throws error when access token entity is null when handling a response.
496+
*/
497+
static createAccessTokenEntityNullError(): ClientAuthError {
498+
return new ClientAuthError(ClientAuthErrorMessage.accessTokenEntityNullError.code, ClientAuthErrorMessage.accessTokenEntityNullError.desc);
499+
}
489500
}

lib/msal-common/src/response/AuthenticationResult.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export type AuthenticationResult = {
3030
idTokenClaims: object;
3131
accessToken: string;
3232
fromCache: boolean;
33-
expiresOn: Date | null;
33+
expiresOn: Date;
3434
tokenType: string;
3535
extExpiresOn?: Date;
3636
state?: string;

lib/msal-common/src/response/ResponseHandler.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ export class ResponseHandler {
305305
requestState?: RequestStateObject): Promise<AuthenticationResult> {
306306
let accessToken: string = "";
307307
let responseScopes: Array<string> = [];
308-
let expiresOn: Date | null = null;
308+
let expiresOn: Date;
309309
let extExpiresOn: Date | undefined;
310310
let familyId: string = Constants.EMPTY_STRING;
311311
if (cacheRecord.accessToken) {
@@ -318,6 +318,8 @@ export class ResponseHandler {
318318
responseScopes = ScopeSet.fromString(cacheRecord.accessToken.target).asArray();
319319
expiresOn = new Date(Number(cacheRecord.accessToken.expiresOn) * 1000);
320320
extExpiresOn = new Date(Number(cacheRecord.accessToken.extendedExpiresOn) * 1000);
321+
} else {
322+
throw ClientAuthError.createAccessTokenEntityNullError();
321323
}
322324

323325
if (cacheRecord.appMetadata) {

lib/msal-common/test/client/SilentFlowClient.spec.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,6 @@ describe("SilentFlowClient unit tests", () => {
137137
expect(response.idTokenClaims).to.deep.eq(ID_TOKEN_CLAIMS);
138138
expect(response.accessToken).to.deep.eq(testAccessTokenEntity.secret);
139139
expect(response.state).to.be.empty;
140-
console.log();
141140
});
142141

143142
it("acquireCachedToken() looks up Bearer token when AuthenticationScheme is not set in request", async () => {

lib/msal-common/test/response/ResponseHandler.spec.ts

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -200,23 +200,17 @@ describe("ResponseHandler.ts", () => {
200200
});
201201

202202
describe("generateAuthenticationResult", async () => {
203-
it("sets default values if access_token not in cacheRecord", async () => {
203+
it("throws error if access_token not in cacheRecord", async () => {
204+
const testResponse: ServerAuthorizationTokenResponse = {...AUTHENTICATION_RESULT.body};
205+
testResponse.access_token = undefined;
204206
const testRequest: BaseAuthRequest = {
205207
authority: testAuthority.canonicalAuthority,
206208
correlationId: "CORRELATION_ID",
207209
scopes: ["openid", "profile", "User.Read", "email"]
208210
};
209-
const testResponse: ServerAuthorizationTokenResponse = {...AUTHENTICATION_RESULT.body};
210-
testResponse.access_token = null;
211-
212211
const responseHandler = new ResponseHandler("this-is-a-client-id", testCacheManager, cryptoInterface, new Logger(loggerOptions), null, null);
213212
const timestamp = TimeUtils.nowSeconds();
214-
const result = await responseHandler.handleServerTokenResponse(testResponse, testAuthority, timestamp, testRequest);
215-
216-
expect(result.accessToken).to.be.eq("");
217-
expect(result.scopes).to.be.length(0);
218-
expect(result.expiresOn).to.be.null;
219-
expect(result.extExpiresOn).to.be.undefined;
213+
await expect(responseHandler.handleServerTokenResponse(testResponse, testAuthority, timestamp, testRequest)).rejectedWith(ClientAuthErrorMessage.accessTokenEntityNullError.desc);
220214
});
221215

222216
it("sets default values if refresh_token not in cacheRecord", async () => {
@@ -226,7 +220,7 @@ describe("ResponseHandler.ts", () => {
226220
scopes: ["openid", "profile", "User.Read", "email"]
227221
};
228222
const testResponse: ServerAuthorizationTokenResponse = {...AUTHENTICATION_RESULT.body};
229-
testResponse.refresh_token = null;
223+
testResponse.refresh_token = undefined;
230224

231225
const responseHandler = new ResponseHandler("this-is-a-client-id", testCacheManager, cryptoInterface, new Logger(loggerOptions), null, null);
232226
const timestamp = TimeUtils.nowSeconds();

0 commit comments

Comments
 (0)