Azure / Microsoft.Azure.StackExchangeRedis

Azure-specific wrapper for the StackExchange.Redis client library
MIT License
13 stars 12 forks source link

Support authentication using TokenCredential #2

Closed maskati closed 4 months ago

maskati commented 1 year ago

Azure Cache for Redis supports authentication using any AAD principal. The current implementation of this library however only supports ambient managed identities as well as service principals with client secret. This excludes using this for example in local development using AzureCliCredential or alternatives. Azure SDK guidelines on authentication recommend:

DO provide a service client constructor or factory that accepts an instance of the TokenCredential abstraction from Azure Core.

I understand Redis authentication is not pure Bearer token authentication, since it also needs information about the corresponding principal identity (username) in addition to the token (password). However this library should support the typical mechanism of TokenCredential combined with information about the principal, e.g. ConfigureForAzureWithTokenCredential(string principalId, TokenCredential credential). This library could then as necessary refresh the password with a new token and perform possible caching (although this is also against the typical guidelines).

This change would allow the library consumer provide any desired token source for the specified principal.

Ideally this library would follow the typical SDK path of handling authentication using only TokenCredential and not use MSAL, which would drop the need to depend on Azure.Identity and require only Azure.Core.

NickCraver commented 1 year ago

I'm curious about this for sure - do you have examples of other SDKs which do it in this way? I'm all for fewer dependencies, but one of the core goals of the wrapper is to support MSI with as little friction as possible. Are the SDK packages basically implementing the Azure.Identity calls via HTTP directly or some such, rather than a dependency, or some other approach?

IMO, it makes sense to also support the lower level token if that's viable for local development (although I'm not sure what would be refreshing it here). The library overall is a bit of a pairing with a Redis service which recognizes the underlying token - what would we be feeding the token to locally?

If not using tokens or the Azure bits at all, I'm a bit confused on the use case...everything else is supported by the base library (StackExchange.Redis) and the main point of the wrapper is the features above. Of course we want to use the wrapper package in production for identities and package dependencies shouldn't differ for local dev ideally, so if there's a local gap we should fix is so that everything works one way...I'm just confused about the token/refresh semantics of local and what we're feeding. If you have some scenarios that would be a huge help in figuring how to best support it if that makes sense.

maskati commented 1 year ago

do you have examples of other SDKs which do it in this way?

Sure! Here are some examples from other Microsoft SDKs:

Are the SDK packages basically implementing the Azure.Identity calls via HTTP directly or some such, rather than a dependency, or some other approach?

They are not specifically constructing any particular identity client, rather they are taking in the abstract TokenCredential and calling GetTokenAsync as required to acquire an access token for a specified TokenRequestContext (e.g. defining required token scopes).

The library overall is a bit of a pairing with a Redis service which recognizes the underlying token - what would we be feeding the token to locally?

Local development workflows often use Azure resource instances. To avoid having shared key authentication, these can often be configured to use the ambient authentication credential which can be AAD user login locally and managed identity in Azure. From the documentation of DefaultAzureCredential: "The DefaultAzureCredential is appropriate for most scenarios where the application is intended to ultimately be run in Azure. This is because the DefaultAzureCredential combines credentials commonly used to authenticate when deployed, with credentials used to authenticate in a development environment.".

If you have some scenarios that would be a huge help in figuring how to best support it if that makes sense.

It would be great to be able to develop locally against Azure Redis using AAD authentication and then have the same authentication mechanism work when running Azure hosted with managed identity.

eddynaka commented 1 year ago

Hello,

Given that this isn't available yet, is there any way to use the DefaultAzureCredentials and authenticate with Azure Redis? Looking at the examples, I could not find any API that we could use.

Thanks.

philon-msft commented 1 year ago

Yes, you can simply pass the DefaultAzureCredentials.GetToken().Token as the password for a Redis connection. User name is the 'Username' value from the cache's 'Data Access Configuration' blade in the Portal. Note that the connection will be closed when the token expires, unless you periodically re-AUTH the connection with fresh tokens like this extension does.

eddynaka commented 1 year ago

Hi @philon-msft,

Given that we should re-use the ConnectionMultiplexer object, how do we manage to always have a valid token without creating the ConnectionMultiplexer object over and over again?

Reference:

Thanks again for the suggestion!

philon-msft commented 1 year ago

You can update the token for a connection by executing an AUTH command on an existing ConnectionMultiplexer. For example see: https://github.com/Azure/Microsoft.Azure.StackExchangeRedis/blob/2ca6c787c40037094fa0bd05cb96962e9fe46cbe/src/AzureCacheOptionsProviderWithToken.cs#L240

pinkfloydx33 commented 1 year ago

TokenCredential, particularly DefaultAzureCredential would be great as a built-in.

We currently use Azure Workload Identity and managed identities for all of our applications in K8s that need access to Azure resources.

The DefaultAzureCredential makes it possible to use the same simple setup code locally as when deployed--no clunky logic. More importantly, it also has the benefit that your application does not need to be aware of its own identity (nor how to retrieve it). Needing to specify the object/principalId on a per-app basis is practically a non-starter for us since we don't need to do it elsewhere (when using client libs/Azure SDKs for Azure Storage, Azure Postgres, Azure ServiceBus, etc).

DefaultAzureCredential knows exactly how to read the enviroment; so if you are using AZWI like us it will read-in:

We were super excited that Azure offered ACLs/RBAC for managed identities to the Redis Cache. This was the last resource we leverage where we had to maintain a user/pass combination.

The workaround above may suffice (I've not tried it yet), but the simplicity of TokenCredential setup plus the inbox automatic token refresh would be a game changer IMO

maskati commented 1 year ago

Needing to specify the object/principalId on a per-app basis

I am not sure why the Azure Redis AAD implementation needs the principal identifier as the username, since the password (AAD token) component includes the principal identifier as the token subject. It would be more Bearer token like for the Redis username to always be a constant Berarer or something similar.

rudiv commented 1 year ago

Needing to specify the object/principalId on a per-app basis

I am not sure why the Azure Redis AAD implementation needs the principal identifier as the username, since the password (AAD token) component includes the principal identifier as the token subject. It would be more Bearer token like for the Redis username to always be a constant Berarer or something similar.

I agree on this, but it doesn't necessarily even need to do that as I wouldn't see that aspect changing (its service side rather than relevant to this library). The token contains the Principal ID which could be extracted for usage as the username, although it would put an onus on this library to parse the JWT which may result in extra dependencies. That said, the ability is part of .NET.

Either way, a +1 for this. We have a reusable DefaultAzureCredential and so being able to pass in to this library this already existing credential would be marginally beneficial.

maskati commented 1 year ago

At the moment this is possible, because the access tokens for the Redis audience seem to be decodable JWTs. This is not guaranteed however, since access tokens are intended for the target service and should be treated as opaque:

The contents of the token are intended only for the API, which means that access tokens must be treated as opaque strings. For validation and debugging purposes only, developers can decode JWTs using a site like jwt.ms (https://jwt.ms/). Tokens that a Microsoft API receives might not always be a JWT that can be decoded.

rudiv commented 1 year ago

Interesting to know - I haven't seen a token come back that wasn't decodable so far (for a few different services).

Personally, I'd prefer an extension point off of DefaultAzureCredential (or its equivalents) to provide the Principal ID that was resolved, but I feel that's a much bigger ask.

I think a common use case (similar to ours) would be to set up a Managed Identity for a service when in Azure, and having to add in that Principal ID as a configuration value feels a little unnecessary.

maskati commented 1 year ago

Absolutely. The current Redis AAD implementation unfortunately does not allow us an elegant solution. The correct solution would be for the Redis service implementation to be changed to match how all other services do token authentication. Until then we either need to provide the principal username or do unsupported decoding of the token.

sabbadino commented 1 year ago

At the moment this is possible, because the access tokens for the Redis audience seem to be decodable JWTs. This is not guaranteed however, since access tokens are intended for the target service and should be treated as opaque:

The contents of the token are intended only for the API, which means that access tokens must be treated as opaque strings. For validation and debugging purposes only, developers can decode JWTs using a site like jwt.ms (https://jwt.ms/). Tokens that a Microsoft API receives might not always be a JWT that can be decoded.

Opaque tokens are a thing of the past (original oauth2 specs) .. With openidconnect specs everybody expect signed jwt tokens and inspect claims for scopes and other stuff .. should the format change all azure would fall down in a few seconds

maskati commented 1 year ago

@sabbadino that is the case for ID tokens, but access tokens are different:

ID tokens are meant to be read by the OAuth client. Access tokens are meant to be read by the resource server. ID tokens are JWTs. Access tokens can be JWTs but may also be a random string. ID tokens should never be sent to an API. Access tokens should never be read by the client.

Microsoft docs on OIDC ID tokens:

Third-party applications are intended to understand ID tokens. ID tokens shouldn't be used for authorization purposes. Access tokens are used for authorization. The claims provided by ID tokens can be used for UX inside your application, as keys in a database, and providing access to the client application Because ID tokens are always a JWT token, many libraries exist to validate these tokens

Microsoft docs on OAuth2 access tokens:

Per the OAuth specification, access tokens are opaque strings without a set format. Some identity providers (IDPs) use GUIDs and others use encrypted blobs. The format of the access token can depend on the configuration of the API that accepts it. Custom APIs registered by developers on the Microsoft identity platform can choose from two different formats of JSON Web Tokens (JWTs) called v1.0 and v2.0. Microsoft-developed APIs like Microsoft Graph or APIs in Azure have other proprietary token formats. These proprietary formats that can't be validated might be encrypted tokens, JWTs, or special JWT-like.

sabbadino commented 1 year ago

Hi, currently azure aad access tokens are jwt and I doubt Ms is going to change it .. Anyway , I suggest to use the same implementation used by other libraries such as azure key vault: you don't need to provide system managed identity principal Id, one just need to use ManagedIdentityCredential

https://learn.microsoft.com/en-us/azure/azure-app-configuration/use-key-vault-references-dotnet-core?tabs=core6x

maskati commented 1 year ago

currently azure aad access tokens are jwt and I doubt Ms is going to change it

That is not the case. In Azure AD you can encrypt access tokens by setting tokenEncryptionKeyId on the app manifest so that only the holder of the private key (typically the resource server) can view them.

Maybe the Redis service will never configure token encryption, maybe they will, maybe they will do something else to the tokens that make client side decoding problematic. The client shouldn't care, because the client should never be parsing the contents in the first place.

I suggest to use the same implementation used by other libraries such as azure key vault: you don't need to provide system managed identity principal Id, one just need to use ManagedIdentityCredential

That is my suggestion exactly as described in my previous comment:

I am not sure why the Azure Redis AAD implementation needs the principal identifier as the username, since the password (AAD token) component includes the principal identifier as the token subject. It would be more Bearer token like for the Redis username to always be a constant Bearer or something similar.

sabbadino commented 1 year ago

Ah, sorry, only now I see your point (maybe) : services like akv ask for the token only for authentication, while redis wants the principal Id in plain text as well . If I got it well, then it is something that has to be changed on the azure redis cache side , not the c# SDK, am I right?

maskati commented 1 year ago

it is something that has to be changed on the azure redis cache side , not the c# SDK, am I right

Correct. It is the responsibility of the target resource to ensure the token is valid and the token principal is authorized. Having the principal in the username serves no purpose since the same principal must be verified from the token subject.

philon-msft commented 10 months ago

TokenCredential and DefaultAzureCredential are now supported in v2.0.0. Please give it a try and let us know what you think.

For now, the extension still requires UserName/PrincipalId to be passed. If we're able to remove the requirement on the server side in the future, the extension will be updated accordingly.

philon-msft commented 10 months ago

Closing this to clean up, but I'm happy to re-open if needed

sabbadino commented 9 months ago

TokenCredential and DefaultAzureCredential are now supported in v2.0.0. Please give it a try and let us know what you think.

For now, the extension still requires UserName/PrincipalId to be passed. If we're able to remove the requirement on the server side in the future, the extension will be updated accordingly.

i think that as long as we need to provide PrincipalId it is not production ready : in azure web apps .. different slots of same azure web app have different system managed identity PrincipalId .. so .. ;) i think you get the point

pwiens commented 9 months ago

So long as UserName/PrincipalId is required there is no clean way to use Azure Cache for Redis during development. We use DefaultAzureCredential to authenticate with other Azure services and this works really well for local development. We are currently using the following workaround but that's not sustainable:

public static async Task<ConfigurationOptions> ConfigureForAzureWithTokenCredentialAsync(this ConfigurationOptions redisConfigurationOptions, TokenCredential tokenCredential, CancellationToken cancellationToken = default)
{
    var requestContext = new TokenRequestContext([ "acca5fbb-b7e4-4009-81f1-37e38fd66d78/.default" ]);

    var tokenResult = await tokenCredential.GetTokenAsync(requestContext, cancellationToken); ;

    var jwt = new JwtSecurityToken(tokenResult.Token);

    var oid = jwt.Claims.FirstOrDefault(c => c.Type == "oid")?.Value ?? throw new InvalidOperationException("Couln't find principal id");

    return await redisConfigurationOptions.ConfigureForAzureWithTokenCredentialAsync(oid, tokenCredential);
}
stevebus commented 7 months ago

For what it's worth, +1 on having Azure Cache for Redis support DefaultAzureCredential and work the same as other Azure Services... We are developing a new Azure-based solution for a startup, leveraging these azure services:

All of these will use managed identity in azure, an AAD 'user identity' in dev, and DefaultAzureCredential to support moving code from dev to prod with no changes. Having Redis be the oddball here feels unnatural and requires us to consider just dumping AAD auth for redis only and still with access keys.. really hate to do that!

jonsaich commented 7 months ago

I also agree and do not see this as resolved until authentication can by using new DefaultAzureCredential().

I have been going through a project and swapping out connection strings in favour of ManagedIdentity and connecting to Azure Redis cache is the only resource causing grief because it needs the principalId.

philon-msft commented 7 months ago

Thanks all for your feedback on the UserName/PrincipalId requirement. We absolutely understand that this is a pain point that should be improved. We're working on fixing it properly, but don't have a timeline yet. I'll update when we have more clarity.

Meanwhile, I encourage anyone else who's impacted by this (or any other issues with the package) to please report it here.

PascalSenn commented 7 months ago

@philon-msft Is the proposal from @pwiens something that could be incorporated into the library?

PascalSenn commented 7 months ago

I am not sure if @pwiens solution works in all cases but here is a pull request: https://github.com/Azure/Microsoft.Azure.StackExchangeRedis/pull/43

michaeltg17 commented 7 months ago

For what it's worth, +1 on having Azure Cache for Redis support DefaultAzureCredential and work the same as other Azure Services... We are developing a new Azure-based solution for a startup, leveraging these azure services:

  • cosmosdb
  • service bus
  • azure sql
  • key vault
  • blob storage
  • azure redis cache
  • maybe others...

All of these will use managed identity in azure, an AAD 'user identity' in dev, and DefaultAzureCredential to support moving code from dev to prod with no changes. Having Redis be the oddball here feels unnatural and requires us to consider just dumping AAD auth for redis only and still with access keys.. really hate to do that!

Although it would be perfect to have the DefaultAzureCredential work in the redis resource as well, could the workaround be having the redis connection string in the key vault that is accessed using the default credential (managed identity)? So the final result would be more or less the same.

stevebus commented 7 months ago

For what it's worth, +1 on having Azure Cache for Redis support DefaultAzureCredential and work the same as other Azure Services... We are developing a new Azure-based solution for a startup, leveraging these azure services:

  • cosmosdb
  • service bus
  • azure sql
  • key vault
  • blob storage
  • azure redis cache
  • maybe others...

All of these will use managed identity in azure, an AAD 'user identity' in dev, and DefaultAzureCredential to support moving code from dev to prod with no changes. Having Redis be the oddball here feels unnatural and requires us to consider just dumping AAD auth for redis only and still with access keys.. really hate to do that!

Although it would be perfect to have the DefaultAzureCredential work in the redis resource as well, could the workaround be having the redis connection string in the key vault that is accessed using the default credential (managed identity)? So the final result would be more or less the same.

That’s not a bad idea and worth exploring as a workaround until full support is available. I may look into it as a option.

sabbadino commented 7 months ago

For what it's worth, +1 on having Azure Cache for Redis support DefaultAzureCredential and work the same as other Azure Services... We are developing a new Azure-based solution for a startup, leveraging these azure services:

  • cosmosdb
  • service bus
  • azure sql
  • key vault
  • blob storage
  • azure redis cache
  • maybe others...

All of these will use managed identity in azure, an AAD 'user identity' in dev, and DefaultAzureCredential to support moving code from dev to prod with no changes. Having Redis be the oddball here feels unnatural and requires us to consider just dumping AAD auth for redis only and still with access keys.. really hate to do that!

Although it would be perfect to have the DefaultAzureCredential work in the redis resource as well, could the workaround be having the redis connection string in the key vault that is accessed using the default credential (managed identity)? So the final result would be more or less the same.

That's what we did , what else one can do ? But that's just a work around

arealmaas commented 6 months ago

This would be great(!) to have! Using managed identity across the stack here now, and this is the only place where we need to add a workaround with a connection string in key vault etc.

stevebrisson-librestream commented 6 months ago

I am interested in this as well. We are able to use DefaultAzureCredentials for all of the Azure Services we are using aside from Redis Cache. It would make our development and production deployments a lot more secure and easy to maintain.

philon-msft commented 4 months ago

Closing this now that principalId extraction was implemented in #48 and released in 3.0.0. Let us know if you have suggestions or see any issues.