Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix /consumers authority caching #3327

Merged
merged 12 commits into from
Mar 31, 2021
Merged
7 changes: 7 additions & 0 deletions change/msal-12d2764f-51e6-49b0-a48b-39ded19ba8a7.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix token cache for /consumers authority #3327",
"packageName": "msal",
"email": "thomas.norling@microsoft.com",
"dependentChangeType": "patch"
}
28 changes: 1 addition & 27 deletions lib/msal-core/src/UserAgentApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1381,7 +1381,7 @@ export class UserAgentApplication {
private getTokenCacheItemByAuthority(authority: string, tokenCacheItems: Array<AccessTokenCacheItem>, requestScopes: Array<string>, tokenType: string): AccessTokenCacheItem {
let filteredAuthorityItems: Array<AccessTokenCacheItem>;

if (UrlUtils.isCommonAuthority(authority) || UrlUtils.isOrganizationsAuthority(authority)) {
if (UrlUtils.isCommonAuthority(authority) || UrlUtils.isOrganizationsAuthority(authority) || UrlUtils.isConsumersAuthority(authority)) {
filteredAuthorityItems = AuthCacheUtils.filterTokenCacheItemsByDomain(tokenCacheItems, UrlUtils.GetUrlComponents(authority).HostNameAndPort);
} else {
filteredAuthorityItems = AuthCacheUtils.filterTokenCacheItemsByAuthority(tokenCacheItems, authority);
Expand Down Expand Up @@ -1463,13 +1463,6 @@ export class UserAgentApplication {

if (!accessTokenCacheItem) {
this.logger.verbose("No matching token found when filtering by scope and authority");
const authorityList = this.getUniqueAuthority(tokenCacheItems, "authority");
if (authorityList.length > 1) {
throw ClientAuthError.createMultipleAuthoritiesInCacheError(scopes.toString());
}

this.logger.verbose("Single authority used, setting authorityInstance");
serverAuthenticationRequest.authorityInstance = AuthorityFactory.CreateInstance(authorityList[0], this.config.auth.validateAuthority);
return null;
} else {
serverAuthenticationRequest.authorityInstance = AuthorityFactory.CreateInstance(accessTokenCacheItem.key.authority, this.config.auth.validateAuthority);
Expand Down Expand Up @@ -1519,25 +1512,6 @@ export class UserAgentApplication {
return TokenUtils.validateExpirationIsWithinOffset(expiration, this.config.system.tokenRenewalOffsetSeconds);
}

/**
* @hidden
* Used to get a unique list of authorities from the cache
* @param {Array<AccessTokenCacheItem>} accessTokenCacheItems - accessTokenCacheItems saved in the cache
* @ignore
*/
private getUniqueAuthority(accessTokenCacheItems: Array<AccessTokenCacheItem>, property: string): Array<string> {
this.logger.verbose("GetUniqueAuthority has been called");
const authorityList: Array<string> = [];
const flags: Array<string> = [];
accessTokenCacheItems.forEach(element => {
if (element.key.hasOwnProperty(property) && (flags.indexOf(element.key[property]) === -1)) {
flags.push(element.key[property]);
authorityList.push(element.key[property]);
}
});
return authorityList;
}

/**
* @hidden
* Check if ADAL id_token exists and return if exists.
Expand Down
9 changes: 0 additions & 9 deletions lib/msal-core/src/error/ClientAuthError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ import { IdToken } from "../IdToken";
import { StringUtils } from "../utils/StringUtils";

export const ClientAuthErrorMessage = {
multipleCacheAuthorities: {
code: "multiple_authorities",
desc: "Multiple authorities found in the cache. Pass authority in the API overload."
},
endpointResolutionError: {
code: "endpoints_resolution_error",
desc: "Error: could not resolve endpoints. Please check network and try again."
Expand Down Expand Up @@ -114,11 +110,6 @@ export class ClientAuthError extends AuthError {
return new ClientAuthError(ClientAuthErrorMessage.endpointResolutionError.code, errorMessage);
}

static createMultipleAuthoritiesInCacheError(scope: string): ClientAuthError {
return new ClientAuthError(ClientAuthErrorMessage.multipleCacheAuthorities.code,
`Cache error for scope ${scope}: ${ClientAuthErrorMessage.multipleCacheAuthorities.desc}.`);
}

static createPopupWindowError(errDetail?: string): ClientAuthError {
let errorMessage = ClientAuthErrorMessage.popUpWindowError.desc;
if (errDetail && !StringUtils.isEmpty(errDetail)) {
Expand Down
1 change: 1 addition & 0 deletions lib/msal-core/src/utils/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export enum SSOTypes {
SID = "sid",
LOGIN_HINT = "login_hint",
ORGANIZATIONS = "organizations",
CONSUMERS = "consumers",
ID_TOKEN ="id_token",
ACCOUNT_ID = "accountIdentifier",
HOMEACCOUNT_ID = "homeAccountIdentifier"
Expand Down
13 changes: 12 additions & 1 deletion lib/msal-core/src/utils/UrlUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export class UrlUtils {
const lowerCaseUrl = url.toLowerCase();
const urlObject = this.GetUrlComponents(lowerCaseUrl);
const pathArray = urlObject.PathSegments;
if (tenantId && (pathArray.length !== 0 && (pathArray[0] === Constants.common || pathArray[0] === SSOTypes.ORGANIZATIONS))) {
if (tenantId && (pathArray.length !== 0 && (pathArray[0] === Constants.common || pathArray[0] === SSOTypes.ORGANIZATIONS || pathArray[0] === SSOTypes.CONSUMERS))) {
pathArray[0] = tenantId;
}
return this.constructAuthorityUriFromObject(urlObject, pathArray);
Expand Down Expand Up @@ -127,6 +127,17 @@ export class UrlUtils {
return (pathArray.length !== 0 && pathArray[0] === SSOTypes.ORGANIZATIONS);
}

/**
* Checks if an authority is for consumers (ex. https://a:b/consumers/)
* @param url The url
* @returns true if authority is for and false otherwise
*/
static isConsumersAuthority(url: string): boolean {
const authority = this.CanonicalizeUri(url);
const pathArray = this.GetUrlComponents(authority).PathSegments;
return (pathArray.length !== 0 && pathArray[0] === SSOTypes.CONSUMERS);
}

/**
* Parses out the components from a url string.
* @returns An object with the various components. Please cache this value insted of calling this multiple times on the same url.
Expand Down
81 changes: 53 additions & 28 deletions lib/msal-core/test/UserAgentApplication.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1140,34 +1140,6 @@ describe("UserAgentApplication.ts Class", function () {
});
});

it("tests getCachedToken without sending authority when no matching accesstoken is found and multiple authorities exist", function (done) {
const tokenRequest : AuthenticationParameters = {
scopes: ["S3"],
account: account
};
const params: kv = { };
params[SSOTypes.SID] = account.sid;
setUtilUnifiedCacheQPStubs(params);

cacheStorage.setItem(JSON.stringify(accessTokenKey), JSON.stringify(accessTokenValue));
accessTokenKey.scopes = "S2";
accessTokenKey.authority = TEST_CONFIG.alternateValidAuthority;
cacheStorage.setItem(JSON.stringify(accessTokenKey), JSON.stringify(accessTokenValue));
cacheStorage.setItem(JSON.stringify(idTokenKey), JSON.stringify(idToken));

msal.acquireTokenSilent(tokenRequest).then(function(response) {
// Won't happen
console.error("Shouldn't have response here. Data: " + JSON.stringify(response));
}).catch(function(err: AuthError) {
expect(err.errorCode).to.include(ClientAuthErrorMessage.multipleCacheAuthorities.code);
expect(err.errorMessage).to.include(ClientAuthErrorMessage.multipleCacheAuthorities.desc);
expect(err.message).to.contain(ClientAuthErrorMessage.multipleCacheAuthorities.desc);
expect(err.name).to.equal("ClientAuthError");
expect(err.stack).to.include("UserAgentApplication.spec.ts");
done();
});
});

it("tests getCachedToken when common authority is passed and single matching accessToken is found", function (done) {
const tokenRequest : AuthenticationParameters = {
authority: TEST_CONFIG.validAuthority,
Expand Down Expand Up @@ -1273,6 +1245,59 @@ describe("UserAgentApplication.ts Class", function () {
});
});

it("tests getCachedToken when consumers authority is passed and single matching accessToken is found", function (done) {

const tokenRequest : AuthenticationParameters = {
authority: TEST_URIS.DEFAULT_INSTANCE + "consumers/",
scopes: ["S1"],
account: account
};
const tokenRequestAlternate : AuthenticationParameters = {
authority: TEST_URIS.ALTERNATE_INSTANCE + "consumers/",
scopes: ["S1"],
account: account
};
const params: kv = { };
params[SSOTypes.SID] = account.sid;
setUtilUnifiedCacheQPStubs(params);

accessTokenKey.authority = TEST_URIS.DEFAULT_INSTANCE + "9188040d-6c67-4c5b-b112-36a304b66dad/";
cacheStorage.setItem(JSON.stringify(accessTokenKey), JSON.stringify(accessTokenValue));

accessTokenKey.authority = TEST_URIS.ALTERNATE_INSTANCE + "9188040d-6c67-4c5b-b112-36a304b66dad/";
accessTokenValue.accessToken = "accessTokenAlternate";
cacheStorage.setItem(JSON.stringify(accessTokenKey), JSON.stringify(accessTokenValue));

idTokenKey.authority = TEST_URIS.DEFAULT_INSTANCE + "9188040d-6c67-4c5b-b112-36a304b66dad/";
cacheStorage.setItem(JSON.stringify(idTokenKey), JSON.stringify(idToken));
idTokenKey.authority = TEST_URIS.ALTERNATE_INSTANCE + "9188040d-6c67-4c5b-b112-36a304b66dad/";
cacheStorage.setItem(JSON.stringify(idTokenKey), JSON.stringify(idToken));

msal.acquireTokenSilent(tokenRequest).then(function(response) {
expect(response.scopes).to.be.deep.eq(["s1"]);
expect(response.account).to.be.eq(account);
expect(response.idToken.rawIdToken).to.eql(TEST_TOKENS.IDTOKEN_V2);
expect(response.idTokenClaims).to.eql(new IdToken(TEST_TOKENS.IDTOKEN_V2).claims);
expect(response.accessToken).to.include(TEST_TOKENS.ACCESSTOKEN);
expect(response.tokenType).to.be.eq(ServerHashParamKeys.ACCESS_TOKEN);
}).catch(function(err: AuthError) {
// Won't happen
console.error("Shouldn't have error here. Data: " + JSON.stringify(err));
});
msal.acquireTokenSilent(tokenRequestAlternate).then(function(response) {
expect(response.scopes).to.be.deep.eq(["s1"]);
expect(response.account).to.be.eq(account);
expect(response.idToken.rawIdToken).to.eql(TEST_TOKENS.IDTOKEN_V2);
expect(response.idTokenClaims).to.eql(new IdToken(TEST_TOKENS.IDTOKEN_V2).claims);
expect(response.accessToken).to.include("accessTokenAlternate");
expect(response.tokenType).to.be.eq(ServerHashParamKeys.ACCESS_TOKEN);
done();
}).catch(function(err: AuthError) {
// Won't happen
console.error("Shouldn't have error here. Data: " + JSON.stringify(err));
});
});

it("tests getCachedToken when tenant authority is passed and single matching accessToken is found", function (done) {

const tokenRequest : AuthenticationParameters = {
Expand Down
21 changes: 0 additions & 21 deletions lib/msal-core/test/error/ClientAuthError.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,27 +48,6 @@ describe("ClientAuthError.ts Class", () => {

});

it("createMultipleAuthoritiesInCacheError creates a ClientAuthError object", () => {

const scope: string = "user.read";
const errorDetail: string = "Cache error for scope";
const multipleAuthoritiesError = ClientAuthError.createMultipleAuthoritiesInCacheError(scope);
let err: ClientAuthError;

try {
throw multipleAuthoritiesError;
} catch (error) {
err = error;
}

expect(err.errorCode).to.equal(ClientAuthErrorMessage.multipleCacheAuthorities.code);
expect(err.errorMessage).to.include(ClientAuthErrorMessage.multipleCacheAuthorities.desc);
expect(err.errorMessage).to.include(`${errorDetail} ${scope}`);
expect(err.message).to.include(ClientAuthErrorMessage.multipleCacheAuthorities.desc);
expect(err.name).to.equal("ClientAuthError");
expect(err.stack).to.include("ClientAuthError.spec.ts");
});

it("createPopupWindowError creates a ClientAuthError object", () => {

const ERROR_DETAIL = "Details:";
Expand Down
14 changes: 14 additions & 0 deletions lib/msal-core/test/utils/UrlUtils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,36 @@ describe("UrlUtils.ts class", () => {
it("/organizations", () => {
expect(UrlUtils.replaceTenantPath("http://a.com/organizations", "1234-5678")).to.eq("http://a.com/1234-5678/");
});

it("/consumers", () => {
expect(UrlUtils.replaceTenantPath("http://login.microsoftonline.com/consumers", "9188040d-6c67-4c5b-b112-36a304b66dad")).to.eq("http://login.microsoftonline.com/9188040d-6c67-4c5b-b112-36a304b66dad/");
});
});

it("isCommonAuthority", () => {
expect(UrlUtils.isCommonAuthority("https://login.microsoftonline.com/common/")).to.eq(true);
expect(UrlUtils.isCommonAuthority("https://login.microsoftonline.com/common")).to.eq(true);
expect(UrlUtils.isCommonAuthority("https://login.microsoftonline.com/organizations/")).to.eq(false);
expect(UrlUtils.isCommonAuthority("https://login.microsoftonline.com/consumers/")).to.eq(false);
expect(UrlUtils.isCommonAuthority("https://login.microsoftonline.com/123456789/")).to.eq(false);
});

it("isOrganizationsAuthority", () => {
expect(UrlUtils.isOrganizationsAuthority("https://login.microsoftonline.com/organizations/")).to.eq(true);
expect(UrlUtils.isOrganizationsAuthority("https://login.microsoftonline.com/organizations")).to.eq(true);
expect(UrlUtils.isOrganizationsAuthority("https://login.microsoftonline.com/common/")).to.eq(false);
expect(UrlUtils.isOrganizationsAuthority("https://login.microsoftonline.com/consumers/")).to.eq(false);
expect(UrlUtils.isOrganizationsAuthority("https://login.microsoftonline.com/123456789/")).to.eq(false);
});

it("isConsumersAuthority", () => {
expect(UrlUtils.isConsumersAuthority("https://login.microsoftonline.com/consumers/")).to.eq(true);
expect(UrlUtils.isConsumersAuthority("https://login.microsoftonline.com/consumers")).to.eq(true);
expect(UrlUtils.isConsumersAuthority("https://login.microsoftonline.com/common/")).to.eq(false);
expect(UrlUtils.isConsumersAuthority("https://login.microsoftonline.com/organizations/")).to.eq(false);
expect(UrlUtils.isConsumersAuthority("https://login.microsoftonline.com/123456789/")).to.eq(false);
});

it("test getHashFromUrl returns hash from url if hash is single character", () => {
const hash = UrlUtils.getHashFromUrl(TEST_URL_HASH_SINGLE_CHAR);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Add here the endpoints for MS Graph API services you would like to use.
const graphConfig = {
graphMeEndpoint: "https://graph.microsoft.com/v1.0/me"
graphMeEndpoint: "https://graph.microsoft-ppe.com/v1.0/me"
};

// Helper function to call MS Graph API endpoint
Expand Down