Proposal to enable MISE with SAL compatibility with Microsoft.Identity.Web #1483
Conversation
| @@ -28,12 +29,12 @@ internal static void StoreTokenUsedToCallWebAPI(this HttpContext httpContext, Jw | |||
| /// </summary> | |||
| /// <param name="httpContext">HTTP context associated with the current request.</param> | |||
| /// <returns><see cref="JwtSecurityToken"/> used to call the web API.</returns> | |||
| internal static JwtSecurityToken? GetTokenUsedToCallWebAPI(this HttpContext httpContext) | |||
| internal static SecurityToken? GetTokenUsedToCallWebAPI(this HttpContext httpContext) | |||
There was a problem hiding this comment.
JwtBearer generates a JwtSecurityToken, whereas SAL generates a JsonWebToken (new generation that does not depend on NewtonSoft).
In order for Microsoft.Identity.Web to support both, we now return the base class (SecurityToken)
There was a problem hiding this comment.
This will also allow other token types.
| @@ -13,7 +13,7 @@ namespace Microsoft.Identity.Web | |||
| /// Options for configuring authentication using Azure Active Directory. It has both AAD and B2C configuration attributes. | |||
| /// Merges the MicrosoftIdentityWebOptions and the ConfidentialClientApplicationOptions. | |||
| /// </summary> | |||
| internal class MergedOptions : MicrosoftIdentityOptions | |||
| public class MergedOptions : MicrosoftIdentityOptions | |||
There was a problem hiding this comment.
MergedOptions needs to be exposed in the MISE extension for Id.Web.
Alternatively have a friend assembly, but I think we said we prefer not too (for breaking changes)
| @@ -252,7 +252,7 @@ internal static void UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftId | |||
| } | |||
| } | |||
|
|
|||
| internal static void UpdateMergedOptionsFromConfidentialClientApplicationOptions(ConfidentialClientApplicationOptions confidentialClientApplicationOptions, MergedOptions mergedOptions) | |||
| public static void UpdateMergedOptionsFromConfidentialClientApplicationOptions(ConfidentialClientApplicationOptions confidentialClientApplicationOptions, MergedOptions mergedOptions) | |||
There was a problem hiding this comment.
Needs to be called in the Mise extension for Id.Web
| @@ -251,6 +253,12 @@ internal MergedOptions GetOptions(string authenticationScheme) | |||
| authenticationScheme = GetEffectiveAuthenticationScheme(authenticationScheme); | |||
| MergedOptions mergedOptions = GetOptions(authenticationScheme); | |||
|
|
|||
| if (string.IsNullOrEmpty(mergedOptions.Instance)) | |||
There was a problem hiding this comment.
Like what was done in GetAuthenticationResultForApp in the case of anonymous controllers
| @@ -767,15 +775,12 @@ private IConfidentialClientApplication BuildConfidentialClientApplication(Merged | |||
| try | |||
| { | |||
| // In web API, validatedToken will not be null | |||
| JwtSecurityToken? validatedToken = CurrentHttpContext?.GetTokenUsedToCallWebAPI(); | |||
| SecurityToken? validatedToken = CurrentHttpContext?.GetTokenUsedToCallWebAPI(); | |||
There was a problem hiding this comment.
Jwt returns a JwtToken, whereas SAL returns a JsonWebToken. Now providing the base class and a new method that tries both
| @@ -817,6 +822,26 @@ private IConfidentialClientApplication BuildConfidentialClientApplication(Merged | |||
| } | |||
| } | |||
|
|
|||
| private static string? GetActualToken(SecurityToken? validatedToken) | |||
There was a problem hiding this comment.
New method that returns the token to use for OBO:
- whether it's an encrypted token or not
- whether its a JwtSecurityToken or a JsonWebToken.
There was a problem hiding this comment.
How do we handle RPS (MSA tickets)?
There was a problem hiding this comment.
Today, this is not handled. What do you suggest, @brentschmaltz ?
There was a problem hiding this comment.
@jmprieur i suggest that long term, id.web uses SAL for creating all inbound / outbound requests.
There was a problem hiding this comment.
@brentschmaltz Today, SAL is 1P. We wouldn't be able to use in a 3P library.
jmprieur
changed the title
|
SonarCloud Quality Gate failed. |
|
work is complete. closing this POC> |
@jennyf19
This is an initial proposal, in order to trying things out and prototype APIs on the MISE side
this is just to open the discussion, but nothing definitive and plenty of work to do