ADFS 2019 Support

change/msal-2020-08-12-11-09-30-msal-core-adfs.json

@@ -0,0 +1,8 @@
+{
+  "type": "minor",
+  "comment": "ADFS 2019 Support (#1668)",
+  "packageName": "msal",
+  "email": "[email protected]",
+  "dependentChangeType": "patch",
+  "date": "2020-08-12T18:09:30.073Z"
+}

lib/msal-core/src/Account.ts

@@ -66,8 +66,8 @@ export class Account {
         const utid: string = clientInfo ? clientInfo.utid : "";
 
         let homeAccountIdentifier: string;
-        if (!StringUtils.isEmpty(uid) && !StringUtils.isEmpty(utid)) {
-            homeAccountIdentifier = CryptoUtils.base64Encode(uid) + "." + CryptoUtils.base64Encode(utid);
+        if (!StringUtils.isEmpty(uid)) {
+            homeAccountIdentifier = StringUtils.isEmpty(utid)? CryptoUtils.base64Encode(uid): CryptoUtils.base64Encode(uid) + "." + CryptoUtils.base64Encode(utid);
         }
         return new Account(accountIdentifier, homeAccountIdentifier, idToken.preferredName, idToken.name, idToken.claims, idToken.sid, idToken.issuer);
     }

lib/msal-core/src/ClientInfo.ts

@@ -6,6 +6,7 @@
 import { CryptoUtils } from "./utils/CryptoUtils";
 import { ClientAuthError } from "./error/ClientAuthError";
 import { StringUtils } from "./utils/StringUtils";
+import { IdToken } from "./IdToken";
 
 /**
  * @hidden
@@ -30,6 +31,15 @@ export class ClientInfo {
         this._utid = utid;
     }
 
+    static createClientInfoFromIdToken(idToken:IdToken): string {
+        const clientInfo = {
+            uid: idToken.subject, 
+            utid: ""
+        };
+
+        return CryptoUtils.base64Encode(JSON.stringify(clientInfo));
+    }
+
     constructor(rawClientInfo: string) {
         if (!rawClientInfo || StringUtils.isEmpty(rawClientInfo)) {
             this.uid = "";

lib/msal-core/src/IdToken.ts

@@ -58,6 +58,8 @@ export class IdToken {
 
                 if (this.claims.hasOwnProperty("preferred_username")) {
                     this.preferredName = this.claims["preferred_username"];
+                } else if (this.claims.hasOwnProperty("upn")) {
+                    this.preferredName = this.claims["upn"];
                 }
 
                 if (this.claims.hasOwnProperty("name")) {

lib/msal-core/src/UserAgentApplication.ts

@@ -7,7 +7,7 @@ import { AccessTokenCacheItem } from "./cache/AccessTokenCacheItem";
 import { AccessTokenKey } from "./cache/AccessTokenKey";
 import { AccessTokenValue } from "./cache/AccessTokenValue";
 import { ServerRequestParameters } from "./ServerRequestParameters";
-import { Authority } from "./authority/Authority";
+import { Authority, AuthorityType } from "./authority/Authority";
 import { ClientInfo } from "./ClientInfo";
 import { IdToken } from "./IdToken";
 import { Logger } from "./Logger";
@@ -1798,9 +1798,10 @@ export class UserAgentApplication {
                     if (hashParams.hasOwnProperty(ServerHashParamKeys.CLIENT_INFO)) {
                         this.logger.verbose("Fragment has clientInfo");
                         clientInfo = hashParams[ServerHashParamKeys.CLIENT_INFO];
+                    } else if (this.authorityInstance.AuthorityType === AuthorityType.Adfs) {
+                        clientInfo = ClientInfo.createClientInfoFromIdToken(idTokenObj);
                     } else {
                         this.logger.warning("ClientInfo not received in the response from AAD");
-                        throw ClientAuthError.createClientInfoNotPopulatedError("ClientInfo not received in the response from the server");
                     }
 
                     response.account = Account.createAccount(idTokenObj, new ClientInfo(clientInfo));
@@ -1853,6 +1854,8 @@ export class UserAgentApplication {
                     if (hashParams.hasOwnProperty(ServerHashParamKeys.CLIENT_INFO)) {
                         this.logger.verbose("Fragment has clientInfo");
                         clientInfo = hashParams[ServerHashParamKeys.CLIENT_INFO];
+                    } else if (this.authorityInstance.AuthorityType === AuthorityType.Adfs) {
+                        clientInfo = ClientInfo.createClientInfoFromIdToken(idTokenObj);
                     } else {
                         this.logger.warning("ClientInfo not received in the response from AAD");
                     }

lib/msal-core/src/authority/Authority.ts

@@ -11,7 +11,7 @@ import { UrlUtils } from "../utils/UrlUtils";
 import TelemetryManager from "../telemetry/TelemetryManager";
 import HttpEvent from "../telemetry/HttpEvent";
 import { TrustedAuthority } from "./TrustedAuthority";
-import { NetworkRequestType } from "../utils/Constants";
+import { NetworkRequestType, Constants, WELL_KNOWN_SUFFIX } from "../utils/Constants";
 
 /**
  * @hidden
@@ -33,6 +33,17 @@ export class Authority {
         this.tenantDiscoveryResponse = authorityMetadata;
     }
 
+    public static isAdfs(authorityUrl: string): boolean {
+        const components = UrlUtils.GetUrlComponents(authorityUrl);
+        const pathSegments = components.PathSegments;
+
+        return (pathSegments.length && pathSegments[0].toLowerCase() === Constants.ADFS);
+    }
+
+    public get AuthorityType(): AuthorityType {
+        return Authority.isAdfs(this.canonicalAuthority)? AuthorityType.Adfs : AuthorityType.Default;
+    };
+
     public IsValidationEnabled: boolean;
 
     public get Tenant(): string {
@@ -87,7 +98,7 @@ export class Authority {
 
     // http://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata
     protected get DefaultOpenIdConfigurationEndpoint(): string {
-        return `${this.CanonicalAuthority}v2.0/.well-known/openid-configuration`;
+        return (this.AuthorityType === AuthorityType.Adfs)? `${this.CanonicalAuthority}${WELL_KNOWN_SUFFIX}` : `${this.CanonicalAuthority}v2.0/${WELL_KNOWN_SUFFIX}`;
     }
 
     /**

lib/msal-core/src/authority/AuthorityFactory.ts

@@ -11,8 +11,6 @@ import { StringUtils } from "../utils/StringUtils";
 import { ClientConfigurationError } from "../error/ClientConfigurationError";
 import { ITenantDiscoveryResponse, OpenIdConfiguration } from "./ITenantDiscoveryResponse";
 import TelemetryManager from "../telemetry/TelemetryManager";
-import { Constants } from "../utils/Constants";
-import { UrlUtils } from "../utils/UrlUtils";
 
 export class AuthorityFactory {
     private static metadataMap = new Map<string, ITenantDiscoveryResponse>();
@@ -63,15 +61,4 @@ export class AuthorityFactory {
 
         return new Authority(authorityUrl, validateAuthority, this.metadataMap.get(authorityUrl));
     }
-
-    public static isAdfs(authorityUrl: string): boolean {
-        const components = UrlUtils.GetUrlComponents(authorityUrl);
-        const pathSegments = components.PathSegments;
-
-        if (pathSegments.length && pathSegments[0].toLowerCase() === Constants.ADFS) {
-            return true;
-        }
-
-        return false;
-    }
 }

lib/msal-core/src/telemetry/TelemetryUtils.ts

@@ -1,14 +1,14 @@
 import { TENANT_PLACEHOLDER, EVENT_NAME_PREFIX } from "./TelemetryConstants";
 import { CryptoUtils } from "../utils/CryptoUtils";
 import { UrlUtils } from "../utils/UrlUtils";
-import { AuthorityFactory } from "../authority/AuthorityFactory";
+import { Authority } from "../authority/Authority";
 
 export const scrubTenantFromUri = (uri: string): String => {
 
     const url = UrlUtils.GetUrlComponents(uri);
 
     // validate trusted host
-    if (AuthorityFactory.isAdfs(uri)) {
+    if (Authority.isAdfs(uri)) {
         /**
          * returning what was passed because the library needs to work with uris that are non
          * AAD trusted but passed by users such as B2C or others.

lib/msal-core/src/utils/Constants.ts

@@ -104,6 +104,7 @@ export enum ErrorCacheKeys {
 
 export const DEFAULT_AUTHORITY: string = "https://login.microsoftonline.com/common/";
 export const AAD_INSTANCE_DISCOVERY_ENDPOINT: string = `${DEFAULT_AUTHORITY}/discovery/instance?api-version=1.1&authorization_endpoint=`;
+export const WELL_KNOWN_SUFFIX: string = ".well-known/openid-configuration";
 
 /**
  * @hidden

lib/msal-core/test/Account.spec.ts

@@ -6,46 +6,107 @@ import { Account } from "../src/Account";
 import { TEST_TOKENS, TEST_DATA_CLIENT_INFO } from "./TestConstants";
 import { CryptoUtils } from "../src/utils/CryptoUtils";
 
-
 describe("Account.ts Class", function() {
 
-    const idToken: IdToken = new IdToken(TEST_TOKENS.IDTOKEN_V2);
-    const clientInfo: ClientInfo = new ClientInfo(TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO);
+    let idToken: IdToken = new IdToken(TEST_TOKENS.IDTOKEN_V2);
+    let clientInfo: ClientInfo = new ClientInfo(TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO);
+
+    describe("createAccount", () => {
+        it("verifies account object is created", () => {
+            const account = Account.createAccount(idToken, clientInfo);
+            expect(account instanceof Account).to.be.true;
+        });
+
+        it("verifies homeAccountIdentifier matches", () => {
+            const account = Account.createAccount(idToken, clientInfo);
+            const homeAccountIdentifier = CryptoUtils.base64Encode(TEST_DATA_CLIENT_INFO.TEST_UID) + "." + CryptoUtils.base64Encode(TEST_DATA_CLIENT_INFO.TEST_UTID);
+
+            expect(account.homeAccountIdentifier).to.equal(homeAccountIdentifier);
+        });
+
+        it("verifies Account object created matches the idToken parameters", () => {
+            const account = Account.createAccount(idToken, clientInfo);
+
+            expect(account.accountIdentifier).to.equal(idToken.objectId);
+            expect(account.userName).to.equal(idToken.preferredName);
+            expect(account.name).to.equal(idToken.name);
+            // This will be deprecated soon
+            expect(account.idToken).to.equal(idToken.claims);
+            expect(account.idTokenClaims).to.equal(idToken.claims);
+            expect(account.sid).to.equal(idToken.sid);
+            expect(account.environment).to.equal(idToken.issuer);
+        });
+
+        it("verifies accountIdentifier equal subject claim if objectId not present", () => {
+            const tempIdToken = idToken;
+            tempIdToken.objectId = "";
+
+            const account = Account.createAccount(tempIdToken, clientInfo);
+            expect(account.accountIdentifier).to.equal(tempIdToken.subject);
+        });
+
+        it("verifies homeAccountIdentifier is undefined if ClientInfo is empty", () => {
+            const tempIdToken = idToken;
+            tempIdToken.subject = "";
+
+            const emptyClientInfo = new ClientInfo(ClientInfo.createClientInfoFromIdToken(tempIdToken));
+            const account = Account.createAccount(idToken, emptyClientInfo);
+
+            expect(account.homeAccountIdentifier).to.be.undefined;
+        });
+    });
 
+    describe("compareAccounts", () => {
+        it("returns false if a1 is null", () => {
+            const account2 = Account.createAccount(idToken, clientInfo);
+            expect(Account.compareAccounts(null, account2)).to.be.false;
+        });
 
-    it("verifies account object is created", function () {
+        it("returns false if a2 is null", () => {
+            const account1 = Account.createAccount(idToken, clientInfo);
+            expect(Account.compareAccounts(account1, null)).to.be.false;
+        });
 
-        const account = Account.createAccount(idToken, clientInfo);
-        expect(account instanceof Account).to.be.true;
-    });
+        it("returns false if a1.homeAccountIdentifier evaluates to false", () => {
+            const tempIdToken = idToken;
+            tempIdToken.subject = "";
 
-    it("verifies homeAccountIdentifier matches", function () {
+            const clientInfo2 = new ClientInfo(ClientInfo.createClientInfoFromIdToken(tempIdToken));
+            const account1 = Account.createAccount(idToken, clientInfo2);
+            const account2 = Account.createAccount(idToken, clientInfo);
 
-        const account = Account.createAccount(idToken, clientInfo);
-        const homeAccountIdentifier = CryptoUtils.base64Encode(TEST_DATA_CLIENT_INFO.TEST_UID) + "." + CryptoUtils.base64Encode(TEST_DATA_CLIENT_INFO.TEST_UTID);
+            expect(account1.homeAccountIdentifier).to.undefined;
+            expect(Account.compareAccounts(account1, account2)).to.be.false;
+        });
 
-        expect(account.homeAccountIdentifier).to.equal(homeAccountIdentifier);
-    });
+        it("returns false if a2.homeAccountIdentifier evaluates to false", () => {
+            const tempIdToken = idToken;
+            tempIdToken.subject = "";
 
-    it("verifies Account object created matches the idToken parameters", function () {
+            const clientInfo2 = new ClientInfo(ClientInfo.createClientInfoFromIdToken(tempIdToken));
+            const account1 = Account.createAccount(idToken, clientInfo);
+            const account2 = Account.createAccount(idToken, clientInfo2);
 
-        const account = Account.createAccount(idToken, clientInfo);
+            expect(account2.homeAccountIdentifier).to.undefined;
+            expect(Account.compareAccounts(account1, account2)).to.be.false;
+        });
 
-        if(idToken.objectId != null) {
-            expect(account.accountIdentifier).to.equal(idToken.objectId);
-        }
-        else {
-            expect(account.accountIdentifier).to.equal(idToken.subject);
-        }
-
-        expect(account.userName).to.equal(idToken.preferredName);
-        expect(account.name).to.equal(idToken.name);
-        // This will be deprecated soon
-        expect(account.idToken).to.equal(idToken.claims);
-        expect(account.idTokenClaims).to.equal(idToken.claims);
-        expect(account.sid).to.equal(idToken.sid);
-        expect(account.environment).to.equal(idToken.issuer);
-    });
+        it("returns true if a1.homeAccountIdentifier === a2.homeAccountIdentifier", () => {
+            const account1 = Account.createAccount(idToken, clientInfo);
+            const account2 = Account.createAccount(idToken, clientInfo);
+
+            expect(Account.compareAccounts(account1, account2)).to.be.true;
+        });
 
+        it("returns false if a1.homeAccountIdentifier !== a2.homeAccountIdentifier", () => {
+            const tempIdToken = idToken;
+            tempIdToken.subject = "test-oid";
 
+            const clientInfo2 = new ClientInfo(ClientInfo.createClientInfoFromIdToken(tempIdToken));
+            const account1 = Account.createAccount(idToken, clientInfo);
+            const account2 = Account.createAccount(idToken, clientInfo2);
+
+            expect(Account.compareAccounts(account1, account2)).to.be.false;
+        });
+    });
 });

lib/msal-core/test/ClientInfo.spec.ts

@@ -3,8 +3,9 @@ import sinon from "sinon";
 import { ClientInfo } from "../src/ClientInfo";
 import { ClientAuthError, AuthError } from "../src";
 import { ClientAuthErrorMessage } from "../src/error/ClientAuthError";
-import { TEST_DATA_CLIENT_INFO } from "./TestConstants";
+import { TEST_DATA_CLIENT_INFO, TEST_TOKENS } from "./TestConstants";
 import { CryptoUtils } from "../src/utils/CryptoUtils";
+import { IdToken } from "../src/IdToken";
 
 describe("Client Info", function () {
 
@@ -33,6 +34,20 @@ describe("Client Info", function () {
 
     });
 
+    describe("createClientInfoFromIdToken", () => {
+        it("Returns encoded ClientInfo Object", () => {
+            const tempIdToken: IdToken = new IdToken(TEST_TOKENS.IDTOKEN_V2);;
+            tempIdToken.subject = "test-oid";
+
+            const clientInfo = ClientInfo.createClientInfoFromIdToken(tempIdToken);
+
+            const clientInfoObj = new ClientInfo(clientInfo);
+
+            expect(clientInfoObj.uid).to.equal("test-oid");
+            expect(clientInfoObj.utid).to.equal("");
+        });
+    });
+
     describe("Parsing raw client info string", function () {
 
         let clientInfoObj : ClientInfo;
@@ -96,6 +111,34 @@ describe("Client Info", function () {
             expect(clientInfoObj.utid).to.be.eq(TEST_DATA_CLIENT_INFO.TEST_UTID);
         });
 
+        it("Does not set anything if uid and utid are not part of clientInfo", () => {
+            sinon.stub(CryptoUtils, "base64Decode").returns(`{"test-uid":"123-test-uid","test-utid":"456-test-utid"}`);
+            // What we pass in here doesn't matter since we are stubbing
+            clientInfoObj = new ClientInfo(TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO);
+            expect(clientInfoObj).to.not.be.null;
+            expect(clientInfoObj.uid).to.be.eq("");
+            expect(clientInfoObj.utid).to.be.eq("");
+        });
+
+        it("Does not set utid member if utid not part of ClientInfo", () => {
+            sinon.stub(CryptoUtils, "base64Decode").returns(`{"uid":"123-test-uid","test-utid":"456-test-utid"}`);
+            // What we pass in here doesn't matter since we are stubbing
+            clientInfoObj = new ClientInfo(TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO);
+            expect(clientInfoObj).to.not.be.null;
+            expect(clientInfoObj.uid).to.be.eq(TEST_DATA_CLIENT_INFO.TEST_UID);
+            expect(clientInfoObj.utid).to.be.eq("");
+
+        });
+
+        it("Does not set uid member if uid not part of ClientInfo", () => {
+            sinon.stub(CryptoUtils, "base64Decode").returns(`{"test-uid":"123-test-uid","utid":"456-test-utid"}`);
+            // What we pass in here doesn't matter since we are stubbing
+            clientInfoObj = new ClientInfo(TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO);
+            expect(clientInfoObj).to.not.be.null;
+            expect(clientInfoObj.uid).to.be.eq("");
+            expect(clientInfoObj.utid).to.be.eq(TEST_DATA_CLIENT_INFO.TEST_UTID);
+        });
+
     });
 
 });

lib/msal-core/test/TestConstants.ts

@@ -73,6 +73,10 @@ export const B2C_TEST_CONFIG = {
     MSAL_CLIENT_ID: "e760cab2-b9a1-4c0d-86fb-ff7084abd902"
 };
 
+export const ADFS_TEST_CONFIG = {
+    validAuthority: "https://fs.msidlab8.com/adfs"
+};
+
 export const TEST_RESPONSE_TYPE = {
     id_token: "id_token",
     token: "token",