Board Review: Add SharedKey/SAS Credential types to Azure.Core (all languages)

[tg-msft]

We have several services that support Shared Key and SAS credentials with the same API shape and usage:

Service	Shared Key	Shared Access Signature
Storage	x	x
Event Grid		x
Event Hubs	x	x
Service Bus	x	x

I'd like to discuss adding AzureSharedKeyCredential and AzureSharedAccessSignatureCredential in Azure.Core so we don't have multiple copies of each.

(I'll edit in API specifics, a possible plan for refreshing SAS tokens automatically, and a plan for integration with the existing StorageSharedKeyCredential)

[tg-msft]

We should discuss together with #1942

[lilyjma]

scheduled for 10/28

[avg-msft]

@lilyjma Can we get a timeline on this, please? We have an SDK upgrade feature that is blocked on this.

[tg-msft]

Shared Key

After discussion, rather than add a specific AzureNamedKeyCredential we think it would make sense to augment AzureKeyCredential to allow an optional name:

 namespace Azure
 {
      public class AzureKeyCredential  {
         public AzureKeyCredential(string key);
+        public AzureKeyCredential(string name, string key);
+        public string Name { get; }
         [EditorBrowsable(EditorBrowsableState.Never)]
         public string Key { get; }
         public void Update(string key);
+        public void Update(string name, string key);
     }
 }

Additional topics to discuss:

1. I think of signing with a shared key as slightly safer than passing an API key which is why I wanted separate types, but I agree that since it's a huge anti-pattern to use HTTP it won't matter to customers.
2. Both values needs to update atomically. Could consider using a Tuple<string, string> for a single replacement instead of any synchronization.
3. Storage will not update to derive StorageSharedKeyCredential : AzureKeyCredential because the property names don't align and there's no meaningful benefit of forcing it.
4. EventHubs and ServiceBus should use this credential.
5. Tables should replace its current SharedKey credential with this one.

[tg-msft]

SAS

While Storage currently takes SAS tokens as part of the URI and doesn't distinguish between anonymous auth and using SAS for auth, customers have found the inability to roll SAS tokens to be a pain point. See #1942 for more discussion about scenarios. We're proposing an API that looks like:

namespace Azure
{
    public class AzureSharedAccessSignatureCredential {
        public AzureSharedAccessSignatureCredential(string signature);
        [EditorBrowsable(EditorBrowsableState.Never)]
        public string Signature { get; }
        public void Update(string signature);
    }
}

Additional topics to discuss:

1. Should we call it AzureSasCredential instead? Storage uses Sas for everything today.
2. Clients libraries should throw if someone attempts to pass a SAS URI and a SAS credential. It's impossible to get this right in a perfectly forward compatible way, but we'll get a pretty clear auth error in the cases it's wrong.
3. The exception message should point customers to things like BlobUriBuilder which makes it easy to remove a SAS token in code. Similar caveats about forward compat apply.
4. We'd create a "shared source" pipeline policy that tacked on a SAS token to the URI per call. Today I think it would work across implementations.
5. While there's not a scenario that requires it today, we can think about supporting automatic refresh in a future release. The pipeline policy would listen for 403s on the way back, check if the SAS token expired, raise an event (probably sync or async depending on the path), allow the event to compute/fetch a new SAS token, and mark the message as retriable so customers never notice the expiration. This could be added without breaking.
6. Storage, EventHubs, and ServiceBus should use this credential.
7. EventGrid should replace its current credential with this one.

[tg-msft]

Base Type

For anyone dealing with multiple types of credentials today, it's annoying that there's no base type. We're on the fence about creating something like:

namespace Azure.Core
{
    public abstract class AzureCredential { }
    public abstract class TokenCredential : AzureCredential { /* ... */ }
}

namespace Azure
{
    public class AzureKeyCredential : AzureCredential { /* ... */ }
    public class AzureSharedAccessSignatureCredential : AzureCredential { /* ... */ }
}

Additional topics to discuss:

1. There will be no members. It's basically only for tagging.
2. Do we want a less discoverable way of creating a client using the base AzureCredential for customers who are only holding one of these?

[tg-msft]

Recording (MS internal): https://msit.microsoftstream.com/video/0a25a1ff-0400-9fb2-9f3b-f1eb196ee5ee

Highlights:

• AzureSasCredential makes sense to implement immediately
• We'll see if there's time for a quick UX study around the proposed changes to AzureKeyCredential looking at
  • adding Name to AzureKeyCredential,
  • splitting into a separate AzureSharedKeyCredential,
  • or always using dedicated (Client)SharedKeyCredential types
• We'll hold off on a base AzureCredential type for now and library authors can use object internally. We'll revisit when there's a customer scenario.

[lilyjma]

@avg-msft : Please take a look at Ted's last comment on the priorities we have now.

[ramya-rao-a]

I see the PRs in the .Net, Java and Python repo adding both AzureSasCredential and AzureSasCredentialPolicy. While I see the discussion and conclusion for the former in this issue, I don't see any for the latter. Can anyone shed some light on this?

Also, what is the update on AzureSharedKeyCredential?