Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] ITokenAcquisition should have extra methods to expose AuthenticationResult #543

Closed
jmprieur opened this issue Sep 3, 2020 · 2 comments · Fixed by #553
Assignees
Milestone

Comments

@jmprieur
Copy link
Collaborator

jmprieur commented Sep 3, 2020

Is your feature request related to a problem? Please describe.
The Azure SDK has the notion of TokenCredential which is used by various SDKs. It's possible to derive from TokenCredential to provide credentials coming from Microsoft.Identity.Web, however this requires both the token and the expiry. ITokenAcquisition covers the simple scenario where the token is returned, but not the expiry.

In a controller accessing a blob storage customers could write:

 [AuthorizeForScopes(Scopes = new string[] { "https://storage.azure.com/user_impersonation" })]
        public async Task<IActionResult> Blob()
        {
            var scopes = new string[] { "https://storage.azure.com/user_impersonation" }; // I guess the Blob SDK knows already?
            ViewData["Message"] = await CreateBlob(new TokenAcquisitionTokenCredential(_tokenAcquisition),);
            return View();
        }

        private static async Task<string> CreateBlob(TokenAcquisitionTokenCredential tokenCredential)
        {
            // Replace the URL below with the URL to your blob.
            Uri blobUri = new Uri("https://storagesamples.blob.core.windows.net/sample-container/blob1.txt");
            BlobClient blobClient = new BlobClient(blobUri, tokenCredential);

            // Create a blob on behalf of the user.
            string blobContents = "Blob created by Azure AD authenticated user.";
            byte[] byteArray = Encoding.ASCII.GetBytes(blobContents);

            using (MemoryStream stream = new MemoryStream(byteArray))
            {
                await blobClient.UploadAsync(stream);
            }
            return "Blob successfully created";
        }

with

  /// <summary>
    /// Azure SDK token credential based on the ITokenAcquisition service.
    /// </summary>
    public class TokenAcquisitionTokenCredential : TokenCredential
    {
        private ITokenAcquisition _tokenAcquisition;

        /// <summary>
        /// Constructor from an ITokenAcquisition service.
        /// </summary>
        /// <param name="tokenAcquisition">Token acquisition.</param>
        public TokenAcquisitionTokenCredential(ITokenAcquisition tokenAcquisition)
        {
            _tokenAcquisition = tokenAcquisition;
        }

        /// <inheritdoc/>
        public override AccessToken GetToken(TokenRequestContext requestContext, CancellationToken cancellationToken)
        {
            AuthenticationResult result = _tokenAcquisition.GetAuthenticationResultForUserAsync(requestContext.Scopes)
                .GetAwaiter()
                .GetResult();
            return new AccessToken(result.AccessToken, result.ExpiresOn);
        }

        /// <inheritdoc/>
        public override async ValueTask<AccessToken> GetTokenAsync(TokenRequestContext requestContext, CancellationToken cancellationToken)
        {
            AuthenticationResult result = await _tokenAcquisition.GetAuthenticationResultForUserAsync(requestContext.Scopes).ConfigureAwait(false);
            return new AccessToken(result.AccessToken, result.ExpiresOn);
        }
    }

Describe the solution you'd like

  • GetAuthenticationResultForUserAsync
  • Expose TokenAcquisitionTokenCredential (given we already leverage the Azure.Identity NuGet package)

Describe alternatives you've considered
expose the expiry as an optional ref parameter?

  • Expose TokenAcquisitionTokenCredential but not GetAuthenticationResultForUserAsync?

Additional context

  • This is mainly a refactoring of GetTokenForUserAsync by exposing an intermediate product (the AuthenticationResult)
  • Therefor no new tests are needed
  • See the following experimental PR: Add support for the AzureSDK #542
@jmprieur jmprieur added the enhancement New feature or request label Sep 3, 2020
@jmprieur
Copy link
Collaborator Author

jmprieur commented Sep 4, 2020

@jennyf19 @henrik-me @pmaytak
I wonder if we should not just take (for 0-4-0) the GetAuthenticationResultForUserAsync addition (as this is a small refactoring and does not even need more tests). This would unblock this scenario

But I'm not suggesting we take the TokenAcquisitionTokenCredential which requires more specs.

What do you think? PR: #542

cc: @maliksahil

@jennyf19
Copy link
Collaborator

Included in 0.4.0-preview release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants