B2C Authority fixes #1276

tnorling · 2020-02-11T23:29:03Z

Updated B2C Authority to extend the generic Authority class, rather than AADAuthority.
Removed url slicing which was breaking instantiation of B2CAuthority for certain authorities
Added knownAuthorities config parameter which sets the trusted authorities for B2C use cases
Authority factory will create B2C Authority instance if knownAuthoritites is set, AAD Authority instance otherwise

jasonnutter · 2020-02-11T23:31:20Z

@tnorling What was the reasoning behind changing the base class?

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

coveralls · 2020-02-11T23:33:08Z

Coverage increased (+1.0%) to 77.973% when pulling 10779ca on known-authorities into 153d810 on dev.

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

tnorling · 2020-02-11T23:43:10Z

@tnorling What was the reasoning behind changing the base class?

AAD and B2C are distinct instances, it didnt make sense for AADAuthority to extend Authority and then B2C to extend AADAuthority when we are overwriting the AAD functions anyway within B2C

…oft-authentication-library-for-js into known-authorities

lib/msal-core/src/Configuration.ts

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

sameerag

Added some comments but overall looks good!

jmckennon

Looks good, nice job on the descriptive tests! :D

pkanher617 · 2020-03-26T18:54:54Z

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

@@ -107,6 +107,8 @@ export const AADTrustedHostList =  {
    "login.microsoftonline.us": "login.microsoftonline.us"
 };

+export const B2CTrustedHostList = {};


I would consider moving this to AuthorityFactory or B2CAuthority since it isn't actually a constant.

I'm also not a huge fan of having a global variable like this, I think it should be a property or field of an object or class

Moved this to AuthorityFactory but not sure how to avoid making it global since it needs to be read from the static detectAuthorityFromUrl function.

pkanher617

Looks good for the most part, one concern about B2CTrustedHostList but otherwise it is fine. I would like to see some samples or e2e tests for this before we merge.

…oft-authentication-library-for-js into known-authorities

sameerag

LGTM. Make sure error messages match across the board.
Good work!

…oft-authentication-library-for-js into known-authorities

jasonnutter · 2020-04-08T18:58:06Z

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

-import { ClientConfigurationErrorMessage } from "../error/ClientConfigurationError";
-import { UrlUtils } from "../utils/UrlUtils";
+import { ClientConfigurationError } from "../error/ClientConfigurationError";
+import { AuthorityFactory, B2CTrustedHostList } from "./AuthorityFactory";


This creates a circular dependency :( sorry didn't catch this before.

jasonnutter · 2020-04-08T18:58:52Z

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

@@ -9,25 +9,42 @@
 import { AadAuthority } from "./AadAuthority";
 import { B2cAuthority } from "./B2cAuthority";


Other end of the circle

Per @tnorling this circular dep was resolved in #1479

B2C Authority fixes

775d59a

tnorling requested review from sameerag, jasonnutter, jmckennon and pkanher617 February 11, 2020 23:29

sameerag reviewed Feb 11, 2020

View reviewed changes

lib/msal-core/src/authority/B2cAuthority.ts Show resolved Hide resolved

jasonnutter reviewed Feb 11, 2020

View reviewed changes

lib/msal-core/src/authority/B2cAuthority.ts Show resolved Hide resolved

tnorling added 9 commits February 20, 2020 11:59

Implement known authorities for B2C

e6e44ca

Update setKnownAuthorities to use params

8bef92f

Include optional authorityData configuration type

4fd2c47

Update Errors

cf918c3

Authority Unit Tests

02f9528

Update TestConstants

1798a83

Update AAD and B2C tests

b9c7680

Merge branch 'known-authorities' of https://github.com/AzureAD/micros…

3b59547

…oft-authentication-library-for-js into known-authorities

Removed authority type parameter

0827b81

sameerag reviewed Mar 2, 2020

View reviewed changes

lib/msal-core/src/Configuration.ts Show resolved Hide resolved

sameerag reviewed Mar 2, 2020

View reviewed changes

lib/msal-core/src/authority/AuthorityFactory.ts Outdated Show resolved Hide resolved

sameerag reviewed Mar 2, 2020

View reviewed changes

Update return to ternary

a78b15b

sameerag mentioned this pull request Mar 13, 2020

Add KnownAuthorities() API #1187

Closed

tnorling added 4 commits March 20, 2020 10:41

Add authorityType parameter

07e6dd2

Update string replace

92f2067

More unit tests

0a96df5

Fix unit test

da3cf68

sameerag mentioned this pull request Mar 24, 2020

[msal-common] Add knownAuthorities parameter to support B2C validation #1416

Merged

Remove authorityType

bc90b09

Merge branch 'dev' into known-authorities

352e3f1

jmckennon approved these changes Mar 25, 2020

View reviewed changes

sameerag mentioned this pull request Mar 26, 2020

[msal-common][msal-node] Authority changes #1424

Merged

pkanher617 reviewed Mar 26, 2020

View reviewed changes

pkanher617 suggested changes Mar 26, 2020

View reviewed changes

tnorling added 4 commits March 26, 2020 13:47

Move B2CTrustedHostList and setKnownAuthorities

95c5fcb

Merge branch 'dev' into known-authorities

f8de5a2

Update function description

15d65d9

Merge branch 'known-authorities' of https://github.com/AzureAD/micros…

8f5d27b

…oft-authentication-library-for-js into known-authorities

pkanher617 approved these changes Apr 6, 2020

View reviewed changes

sameerag approved these changes Apr 7, 2020

View reviewed changes

tnorling added 4 commits April 7, 2020 16:03

Update to untrustedAuthority error

ee97b68

Merge branch 'dev' into known-authorities

21324e5

Add unit test

3c3e5c7

Merge branch 'known-authorities' of https://github.com/AzureAD/micros…

10779ca

…oft-authentication-library-for-js into known-authorities

tnorling merged commit 39b9592 into dev Apr 7, 2020

jasonnutter reviewed Apr 8, 2020

View reviewed changes

tnorling deleted the known-authorities branch April 13, 2020 20:08

jasonnutter added this to the [email protected] - Release milestone Apr 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

B2C Authority fixes #1276

B2C Authority fixes #1276

tnorling commented Feb 11, 2020 •

edited

Loading

jasonnutter commented Feb 11, 2020

coveralls commented Feb 11, 2020 •

edited

Loading

tnorling commented Feb 11, 2020

sameerag left a comment

jmckennon left a comment

pkanher617 Mar 26, 2020

pkanher617 Mar 26, 2020

tnorling Mar 26, 2020

pkanher617 left a comment

sameerag left a comment

jasonnutter Apr 8, 2020

jasonnutter Apr 8, 2020

mmacy Apr 9, 2020

		@@ -9,25 +9,42 @@
		import { AadAuthority } from "./AadAuthority";
		import { B2cAuthority } from "./B2cAuthority";

B2C Authority fixes #1276

B2C Authority fixes #1276

Conversation

tnorling commented Feb 11, 2020 • edited Loading

jasonnutter commented Feb 11, 2020

coveralls commented Feb 11, 2020 • edited Loading

tnorling commented Feb 11, 2020

sameerag left a comment

Choose a reason for hiding this comment

jmckennon left a comment

Choose a reason for hiding this comment

pkanher617 Mar 26, 2020

Choose a reason for hiding this comment

pkanher617 Mar 26, 2020

Choose a reason for hiding this comment

tnorling Mar 26, 2020

Choose a reason for hiding this comment

pkanher617 left a comment

Choose a reason for hiding this comment

sameerag left a comment

Choose a reason for hiding this comment

jasonnutter Apr 8, 2020

Choose a reason for hiding this comment

jasonnutter Apr 8, 2020

Choose a reason for hiding this comment

mmacy Apr 9, 2020

Choose a reason for hiding this comment

tnorling commented Feb 11, 2020 •

edited

Loading

coveralls commented Feb 11, 2020 •

edited

Loading