Dec 9, 2022 · Dec 7, 2022 · Dec 9, 2022 · Dec 7, 2022 · Dec 8, 2022 · Dec 8, 2022
diff --git a/3-Authorization-II/2-call-api-b2c/API/TodoListAPI/Controllers/TodoListController.cs b/3-Authorization-II/2-call-api-b2c/API/TodoListAPI/Controllers/TodoListController.cs

            if (!IsAppOnlyToken() && _currentPrincipal != null)
            {
                _currentPrincipalId = _currentPrincipal.GetHomeObjectId(); // use "sub" claim as a unique identifier in B2C
                // The default behavior of the JwtSecurityTokenHandler is to map inbound claim names to new values in the generated ClaimsPrincipal.
                // The result is that "sub" claim that identifies the subject of the incoming JWT token is mapped to a claim
                // named "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier". An alternative approach is to
                // disable this mapping by setting JwtSecurityTokenHandler.DefaultMapInboundClaims to false in Startup.cs and
                // then calling _currentPrincipal.FindFirstValue(ClaimConstants.Sub) to obtain the value of the unmapped "sub" claim.
                _currentPrincipalId = _currentPrincipal.GetNameIdentifierId(); // use "sub" claim as a unique identifier in B2C
                PopulateDefaultToDos(_currentPrincipalId);
            }
        }
            return false;
        }
    }
 }
 }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -37,7 +37,12 @@ public TodoListController(TodoContext context, IHttpContextAccessor contextAcces

		if (!IsAppOnlyToken() && _currentPrincipal != null)
		{
		_currentPrincipalId = _currentPrincipal.GetHomeObjectId(); // use "sub" claim as a unique identifier in B2C
		// The default behavior of the JwtSecurityTokenHandler is to map inbound claim names to new values in the generated ClaimsPrincipal.
		// The result is that "sub" claim that identifies the subject of the incoming JWT token is mapped to a claim
		// named "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier". An alternative approach is to
		// disable this mapping by setting JwtSecurityTokenHandler.DefaultMapInboundClaims to false in Startup.cs and
		// then calling _currentPrincipal.FindFirstValue(ClaimConstants.Sub) to obtain the value of the unmapped "sub" claim.
		_currentPrincipalId = _currentPrincipal.GetNameIdentifierId(); // use "sub" claim as a unique identifier in B2C
Copy link Contributor derisenDec 7, 2022• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. @John-Garland thank you so much for catching this -I can reproduce it. I feel like instead of using`.GetNameIdentifierId()` for retrievingsub, we can be more explicit with something like`.FindFirstValue(ClaimConstants.Sub);`. In that case we would also need to set theJwtSecurityTokenHandler.DefaultMapInboundClaims tofalse in`Startup.cs` though -happy to discuss! Copy link ContributorAuthor John-GarlandDec 8, 2022 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Good question. My inclination would be to keep the sample as small/self-contained as possible. I worry that turning off claims mapping would imply it as a necessary step for using MSAL/MSIW with B2C. Perhaps, however we can achieve the goal of being explicit about the origin of the claim (I agree that sub-->nameidentifier is hardly obvious) with some strategic comments calling out the expected conversion and also suggesting the approach above as an alternative? (If that idea resonates, I can update the PR accordingly.) Copy link Contributor derisenDec 8, 2022 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Comments sound good to me! As long as we explain what we're after and what alternatives out there we're all good (I just came across a good discussion on this issuehere) Copy link ContributorAuthor John-GarlandDec 9, 2022 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Good thread. I guess this takes me back to my original question - should these samples advocate for a "cleaner" approach to handling JWT token claims and recommend that claims mapping be disabled across the board? I could see using this as an opportunity to retrofit the other exercises in this sequence as well (though done as a separate PR...) Thinking about it, I'm going to use the comment approach I mentioned, but I would love to have a broader conversation about disabling the mapping and if it should eb something we advocate across the samples (and elsewhere!) Copy link Contributor derisenDec 9, 2022• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Claims mapping usually functions well in AAD use cases, at least I don't remember an instance where it led us astray. But for B2C use cases it might be a good idea, since it has various idiosyncrasies like this. Let me bring this up with my team, at least for B2C samples cc@kalyankrishna1
		PopulateDefaultToDos(_currentPrincipalId);
		}
		}
Expand DownExpand Up		@@ -198,4 +203,4 @@ private bool IsAppOnlyToken()
		return false;
		}
		}
		}
		}