minghuaw / azeventhubs

Unofficial Azure Event Hubs SDK over AMQP 1.0 for rust
5 stars 2 forks source link

`EventHubTokenCredential` is not publicly exported from lib #10

Closed jackgerrits closed 1 year ago

jackgerrits commented 1 year ago

EventHubTokenCredential is not accessible due to the module containing it not being publicly exported from the library.

It is public here: https://github.com/minghuaw/azure-sdk-for-rust/blob/de21c92e74c99d7b4cb4afbde6d1703a3f4aeb46/sdk/messaging_eventhubs/src/authorization/event_hub_token_credential.rs#L7C6-L7C6

But it is not reexported in lib.rs

The variants of the enum also should be exported so they can be used by a consumer.

minghuaw commented 1 year ago

I will probably change the version number to follow that of azure_core starting with this fix

minghuaw commented 1 year ago

@jackgerrits I have something (not published on crates.io yet) that already works with AzureNamedKeyCredential and technically should work with managed identity from azure_identity crate. However, I've run a quick test on an azure VM which simply tries to get the managed identity and try to get token (see below), and somehow it just fails to get any credential.

// rust
use azure_identity::DefaultAzureCredential;
use azure_core::auth::TokenCredential;

const DEFAULT_SCOPE: &str = "https://eventhubs.azure.net/.default";

#[tokio::main]
async fn main() {
    println!("Hello, world!");

    let credential = DefaultAzureCredential::default();
    let token = credential.get_token(DEFAULT_SCOPE).await.unwrap();

}

Then I tried the same thing but with dotnet sdk (the code snippet can be found below), and everything just works fine.

// dotnet
using Azure.Core;
using Azure.Identity;

var credential = new DefaultAzureCredential();
var context = new TokenRequestContext(new[] {"https://eventhubs.azure.net/.default"});
var accessToken = credential.GetToken(context);
String accessTokenString = accessToken.Token.ToString();

Console.WriteLine(accessTokenString);

This could very likely be my fault as I am not super familiar with managed identity on azure, but I wonder if there's bug with azure_identity.

minghuaw commented 1 year ago

I will make a preview release on crates.io so that you can give it a try

minghuaw commented 1 year ago

Hi @jackgerrits I have made a preview release ("0.14.0-alpha") on crates.io. I want to make sure that EventHubTokenCredential works properly with azure managed identity before making it "0.14.0"

minghuaw commented 1 year ago

There seems to be a long queue on docs.rs. I will just list some example showcasing how to work with credentials in this preview release.

  1. Working with AzureNamedKeyCredential or AzureSasCredential
// You can turn `AzureNamedKeyCredential` or `AzureSasCredential` into
// `EventHubTokenCredential` and then use 
// `EventHubConnection::from_namespace_and_credential`

let named_key_credential = AzureNamedKeyCredential::new(key_name, key);
// `AzureSasCredential` doesn't need the signature authorization resource
let resource = build_connection_signature_authorization_resource(transport_type, &fully_qualified_namespace, &event_hub_name).unwrap();
let shared_access_credential = SharedAccessCredential::try_from_named_key_credential(credential, resource).unwrap();
let connection = EventHubConnection::from_namespace_and_credential(
    fully_qualified_namespace,
    event_hub_name,
    shared_access_credential,
    EventHubConnectionOptions::default(),
).await.unwrap();

// Or you can use the convenience function
let named_key_credential = AzureNamedKeyCredential::new(key_name, key);
let connection = EventHubConnection::from_namespace_and_named_key_credential(
    fully_qualified_namespace,
    event_hub_name,
    named_key_credential,
    options,
).await.unwrap();
  1. Working with other credential types that implements azure_core::TokenCredential (ie. credential types provided in azure_identity)
// `azure_identity::DefaultAzureCredential` implements `azure_core::TokenCredential`
let credential = azure_identity::DefaultAzureCredential::default();
let connection = EventHubConnection::from_namespace_and_credential(
    fully_qualified_namespace,
    event_hub_name,
    credential,
    EventHubConnectionOptions::default(),
).await.unwrap();
jackgerrits commented 1 year ago

Thanks! I will try it out.

jackgerrits commented 1 year ago

@minghuaw I was able to get a token using the DefaultAzureCredentail (CLI) by changing the scope to (removing .default):

const DEFAULT_SCOPE: &str = "https://eventhubs.azure.net/";

This seems to be documented here: https://learn.microsoft.com/en-us/azure/event-hubs/authenticate-application

However, I am a bit confused because the scope you originally used also seems to be used often if you search for it on GitHub.

minghuaw commented 1 year ago

Thanks for the information. That indeed worked. But the original DEFAULT_SCOPE was taken from the dotnet SDK, and it's kinda hard coded into the dotnet sdk for eventhubs. I will do some more research

minghuaw commented 1 year ago

It seems like "https://eventhubs.azure.net/" is used in the legacy Event Hubs SDK (https://github.com/Azure/azure-sdk-for-net/blob/fbd50bd3d03e6483a85b3f75ed194ded4c29a1e5/sdk/eventhub/Microsoft.Azure.EventHubs/src/Primitives/ManagedIdentityTokenProvider.cs#L39)

jackgerrits commented 1 year ago

It looks like the resource is https://eventhubs.azure.net/ and the scope would be https://eventhubs.azure.net/.default. (see here, however resource is audience here?)

Looking at the insides of the identity module for CLI credentials it is passing the parameter that is passed through to --resource parameter of az cli. See here

This would indicate that the resource and not scope should be used.

$ az account get-access-token --help 

Command
    az account get-access-token : Get a token for utilities to access Azure.
        The token will be valid for at least 5 minutes with the maximum at 60 minutes. If the
        subscription argument isn't specified, the current account is used.

Arguments
    --name --subscription -n -s : Name or ID of subscription.
    --resource                  : Azure resource endpoints in AAD v1.0.
    --resource-type             : Type of well-known resource.  Allowed values: aad-graph, arm,
                                  batch, data-lake, media, ms-graph, oss-rdbms.
    --scope                     : Space-separated AAD scopes in AAD v2.0. Default to Azure Resource
                                  Manager.
    --tenant -t                 : Tenant ID for which the token is acquired. Only available for user
                                  and service principal account, not for MSI or Cloud Shell account.

It seems like it is using AAD 1.0, and hence the reason why the .default needs to be omitted. This matches up with this: https://learn.microsoft.com/en-us/azure/active-directory/develop/msal-v1-app-scopes

image
minghuaw commented 1 year ago

I think I figured out why it didn't work. https://github.com/Azure/azure-sdk-for-rust/blob/c0d938c7ae0f5fef878956617ff73245e94026a9/sdk/identity/src/token_credentials/client_secret_credentials.rs#L144 azure_identity is appending ".default" internally.

jackgerrits commented 1 year ago

Ah that makes sense, great!

minghuaw commented 1 year ago

@jackgerrits The problem with azure_identity credentials has been fixed (tested on an azure VM and added a new example). I have released version "0.14.0" on crates.io.

jackgerrits commented 1 year ago

Thank you!

minghuaw commented 1 year ago

I will leave this issue open for now. Feel free to close it if the new version works as expected for you :)

minghuaw commented 1 year ago

Hi @jackgerrits , I opened another issue minghuaw/azeventhubs#13 which discusses whether methods like EventHubConnection::from_connection_string() should be changed to EventHubConnection::try_from_connection_string() (or something else), and I was wondering if you could provide some input on this?

jackgerrits commented 1 year ago

It looks like the default credential is working well now, thank you! (I replied to that in the other issue)