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

review resource, scopes & permissions for TokenCredentials #61

Closed
ctaggart opened this issue Oct 22, 2020 · 6 comments
Closed

review resource, scopes & permissions for TokenCredentials #61

ctaggart opened this issue Oct 22, 2020 · 6 comments
Labels
Azure.Identity The azure_identity crate

Comments

@ctaggart
Copy link
Contributor

ctaggart commented Oct 22, 2020

My first thought was that resource should be a list of scopes instead to match Microsoft Authentication Library MSAL v2, but I'm not sure.

/// Represents a credential capable of providing an OAuth token.
#[async_trait::async_trait]
pub trait TokenCredential {
    /// Gets a `TokenResponse` for the specified resource
    async fn get_token(&self, resource: &str) -> Result<TokenResponse, AzureError>;
}

My guess is that resource here is modeled after he az command line and is the resource identifier. I think it becomes the scope query param. In MSAL v2, it is a space delimited list.

Links:

@heaths
Copy link
Member

heaths commented Jun 1, 2021

The Go SDK is currently track 1-based and shouldn't be used as an example. Instead, we focus on track 2. See https://github.com/Azure/azure-sdk-for-net/tree/master/sdk/identity/Azure.Identity/ for the .NET implementations, which is owned by the person driving identity across languages. For example, we don't currently expose scopes but there is an effort happening soon to do something along those lines.

@cataggar
Copy link
Member

Pretty sure scopes: &[&str] or similar like #484.

@heaths
Copy link
Member

heaths commented Nov 16, 2021

Fairly recently, TokenRequestContext was added across languages that takes an optional scope and can be used for multi-tenant authentication when supported by the client SDK. We should be idiomatically consistent. See .NET's implementation at https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/core/Azure.Core/src/TokenRequestContext.cs.

@cataggar
Copy link
Member

cataggar commented Nov 16, 2021

I asked about BearerTokenAuthenticationPolicy in Azure/azure-sdk-for-net#21281, since I'm not sure the best way to allow scopes in new .NET clients, but this worked for the clients I made.

https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/core/Azure.Core/src/Pipeline/BearerTokenAuthenticationPolicy.cs

        public MyClient(Uri endpoint, TokenCredential credential, IEnumerable<string> scopes, MyClientOptions options)
        : this(new ClientDiagnostics(options), HttpPipelineBuilder.Build(options, new BearerTokenAuthenticationPolicy(credential, scopes)), endpoint)
        {
        }

        public MyClient(Uri endpoint, TokenCredential credential, IEnumerable<string> scopes)
        : this(endpoint, credential, scopes, new MyClientOptions())
        {
        }

For #520, I'm going to pass in endpoint, credential, and scopes like this for now.

@heaths
Copy link
Member

heaths commented Nov 16, 2021

Sounds reasonable for v1, but we'll need to be consistent eventually. I've opened #521 to track.

@cataggar cataggar removed this from the azure_identity 0.1.0 milestone Jan 20, 2022
@cataggar
Copy link
Member

#1493 moved to scopes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Identity The azure_identity crate
Projects
None yet
Development

No branches or pull requests

3 participants