fix some comments and namings #244
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jmprieur
approved these changes
Jan 7, 2020
| @@ -24,8 +24,8 @@ public static ClaimsPrincipal ToClaimsPrincipal(this IAccount account) | |||
| return new ClaimsPrincipal( | |||
| new ClaimsIdentity(new Claim[] | |||
| { | |||
| new Claim(ClaimConstants.Oid, account.HomeAccountId.ObjectId), | |||
| new Claim(ClaimConstants.Tid, account.HomeAccountId.TenantId), | |||
| new Claim(ClaimConstants.Oid, account.HomeAccountId?.ObjectId), | |||
There was a problem hiding this comment.
Should we even add the claims if the HomeAccountId is null?
jmprieur
added a commit
that referenced
this pull request
Feb 19, 2020
* Renamings: - AddMicrosoftIdentityPlatformAuthentication => AddSignIn - AddMsal => AddWebAppCallProtectedWebApi * More renaming for consistency * Updating the README.md with a breaking changes section * Attempt to move to pure OIDC and JwtBearer implementation (dropping the comfort AzureAD layers) This code does not work yet .... * Raw OIDC configuration suggestion * missing commit * Using AzureAD appsettings section * Experimenting UI as external RazorLibrary project * fix some comments and namings (#244) * Removing AzureADOptions class * Added missing OpenIdConnectOptions binding Added logic to check if authority has v2.0 before modifying it * removed Microsoft.AspNetCore.Authentication.AzureAD.UI reference from web project * reversing testing method * Commented non-working method * Partial changes for B2C login * Fixes for B2C sign-in * Fixed B2C NameClaimType to "name" * Renaming2 (#269) * Renaming GetAccessTokenOnBehalfOfUserAsync into GetAccessTokenForUserAsync as the OnBehalfOfUser was telling too much about OBO, whereas in the case of a Web App this was not the OBO flow. * Addressing PR feedback * New class MicrosoftIdentityOptions, so we can merge AAD and B2C options in the same class Add AuthorityHelpers Helper method to check if authority is V2 Helper method to build AAD or B2C AuthorityHelpers Refactored AddSignIn so parameters shows in a logical order * Attempt to merge B2C and AAD acquireToken logic * Allowing multiple OIDC schemes to be configured * Added B2C ResetPassword and EditProfile in UI project Added B2C exclusive OIDC event handlers Fixed 1-5 samples to use Microsoft.Identity.UI project * Added comments and preparation for PR * Preparation for PR * Havin the authority in the appsettings is not mandatory * Fixed sample 5-1 to use refactored code * missing commit * Samples 5-2 to use refactored code * Moved samesite cookie logic to CookiePolicyOptionsExtensions class * Gitignore for test project * Added IsV2Authority tests * Added license info * Added BuildAuthority unit tests * Addressing PR reviews * Added B2C tests on AadIssuerValidatorTests * Renaming policy to userFlow * Fix spelling mistakes * Added missing error page in UI project * Addressing PR comments about overwriting custom OIDC and JWT events from the developer * Fixed MT scenarios Fixed broken unit test Added b2c NameClaimType config inside AddSignIn * Fixed all samples in folder 1 to use refactored code * Updated all samples to use new code * Renaming policy to user flow * Added B2C Password Reset Policy Added B2C Password Reset Policy ID to app settings. * Moving Web app calling Web API to its own sub folder * Jmprieur/webappwebapib2c (#298) * Experiment for a B2C Web app calling a B2C Web API. Needs more work. * Test and fix B2C web app calls web api scenario Fix issue in AadIssuerValidator when certains claims are not present (ex. policy) in the token Fix bug w/scopes and scopeKeySection Send "client_info=1" to request to get client info back from AAD B2C Add ClientInfo class + methods to pull out the TenantId from the ClientInfo Add full value to UserFlow as it was not picking up on short version In TokenAcquisition, check first for b2c and then do null tenant and then tenant specific, was using the wrong authority for b2c previously Fix bug in determining audience validation * Fixing missing references * Uncommenting a needed line * Fixing a build break * Fixing the configuration and a problem brought bad a by merge by jmprieur * add uitid to the claims for b2c * moving Web app calling Web API with B2C in own folder * Reading the 4-1-MyOrg folder * Update README.md Co-authored-by: jennyf19 <jeferrie@microsoft.com> * - Cleaned appsettings - Removed DisallowsSameSiteNone from WebAppServiceCollectionExtensions because it got moved to another class - Added edit profile button on login bar - Fixed merge on AuthorizeForScopesAttribute - Added options.TokenValidationParameters.NameClaimType = "name" on web api Startup to fix todo list * Added older role claim const * Added policy on APIs to check for scope 'read' * Refactored scopes policy * Renamings and PR comments * Fixing the failing unit test (because the tenant ID is now retrieved from the issuer) (#302) Co-authored-by: Jean-Marc Prieur <jmprieur@microsoft.com> Co-authored-by: jennyf19 <jeferrie@microsoft.com> Co-authored-by: pmaytak <34331512+pmaytak@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.