Skip to content

Commit

Permalink
Fix token refresh with relative redirectUri (#6761)
Browse files Browse the repository at this point in the history
In 3.2.0 we added redirectUri to the /token request in order to support
broker scenarios. This caused a regression when refreshing tokens with
relative URLs. This PR addresses this issue.

Fixes #6733
  • Loading branch information
tnorling authored Dec 13, 2023
1 parent 793b5e6 commit 462e876
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix token refreshes with relative redirectUri #6761",
"packageName": "@azure/msal-browser",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix token refreshes with relative redirectUri #6761",
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ export class SilentRefreshClient extends StandardInteractionClient {
...request,
...baseRequest,
};

if (request.redirectUri) {
// Make sure any passed redirectUri is converted to an absolute URL - redirectUri is not a required parameter for refresh token redemption so only include if explicitly provided
silentRequest.redirectUri = this.getRedirectUri(
request.redirectUri
);
}

const serverTelemetryManager = this.initializeServerTelemetryManager(
ApiId.acquireTokenSilent_silentFlow
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,62 @@ describe("SilentRefreshClient", () => {
expect(tokenResp).toEqual(testTokenResponse);
});

it("Relative redirectUri is converted to absolute", async () => {
const testServerTokenResponse = {
token_type: TEST_CONFIG.TOKEN_TYPE_BEARER,
scope: TEST_CONFIG.DEFAULT_SCOPES.join(" "),
expires_in: TEST_TOKEN_LIFETIMES.DEFAULT_EXPIRES_IN,
ext_expires_in: TEST_TOKEN_LIFETIMES.DEFAULT_EXPIRES_IN,
access_token: TEST_TOKENS.ACCESS_TOKEN,
refresh_token: TEST_TOKENS.REFRESH_TOKEN,
id_token: TEST_TOKENS.IDTOKEN_V2,
};
const testTokenResponse: AuthenticationResult = {
authority: TEST_CONFIG.validAuthority,
uniqueId: testIdTokenClaims.oid || "",
tenantId: testIdTokenClaims.tid || "",
scopes: ["scope1"],
idToken: testServerTokenResponse.id_token,
idTokenClaims: testIdTokenClaims,
accessToken: testServerTokenResponse.access_token,
fromCache: false,
correlationId: RANDOM_TEST_GUID,
expiresOn: new Date(
Date.now() + testServerTokenResponse.expires_in * 1000
),
account: testAccount,
tokenType: AuthenticationScheme.BEARER,
};
const silentATStub = jest
.spyOn(
RefreshTokenClient.prototype,
<any>"acquireTokenByRefreshToken"
)
.mockResolvedValue(testTokenResponse);
const tokenRequest: CommonSilentFlowRequest = {
scopes: ["scope1"],
account: testAccount,
authority: TEST_CONFIG.validAuthority,
authenticationScheme: AuthenticationScheme.BEARER,
correlationId: TEST_CONFIG.CORRELATION_ID,
forceRefresh: false,
redirectUri: "/", // relative redirectUri
};
const expectedTokenRequest: CommonSilentFlowRequest = {
...tokenRequest,
scopes: ["scope1"],
authority: `${Constants.DEFAULT_AUTHORITY}`,
correlationId: RANDOM_TEST_GUID,
forceRefresh: false,
redirectUri: "https://localhost:8081/", // absolute redirectUri
};
const tokenResp = await silentRefreshClient.acquireToken(
tokenRequest
);
expect(silentATStub).toHaveBeenCalledWith(expectedTokenRequest);
expect(tokenResp).toEqual(testTokenResponse);
});

describe("storeInCache tests", () => {
beforeEach(() => {
const rtEntity = {
Expand Down
2 changes: 2 additions & 0 deletions lib/msal-common/src/request/CommonSilentFlowRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export type CommonSilentFlowRequest = BaseAuthRequest & {
account: AccountInfo;
/** Skip cache lookup and forces network call(s) to get fresh tokens */
forceRefresh: boolean;
/** RedirectUri registered on the app registration - only required in brokering scenarios */
redirectUri?: string;
/** Key value pairs to include on the POST body to the /token endpoint */
tokenBodyParameters?: StringDict;
/** If refresh token will expire within the configured value, consider it already expired. Used to pre-emptively invoke interaction when cached refresh token is close to expiry. */
Expand Down

0 comments on commit 462e876

Please sign in to comment.