Skip to content

Add Interactive Browser Authentication Support #2418

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

M-Patrone
Copy link

@M-Patrone M-Patrone commented Apr 1, 2025

This PR introduces interactive browser-based authentication (#2417) using a new credential type: InteractiveBrowserCredential. This implementation allows developers to authenticate via the default system browser and obtain access tokens through the OAuth2 authorization code flow.

This is particularly useful for experimentation, development, or local testing, where a developer can authenticate with their Azure identity and access resources without needing to set up service principals or managed identities.

New module: interactive_credential::interactive_browser_credential

  • Implements InteractiveBrowserCredential, which handles launching the browser, receiving the auth code via a local web server and retrieving an access token.
  • Internal server logic: Added internal_server.rs to handle local TCP server communication and cross-platform browser launching (Windows, macOS, Linux).

New example: examples/interactive_credentials.rs

  • Demonstrates how to authenticate using this credential and retrieve a list of Azure Storage accounts using the obtained token.

Tests:

  • Unit tests for interactive_browser_credential and internal_server including validation of browser command availability and auth code extraction.

PS: I also added the async-process crate to launch the browser asynchronously.

@Copilot Copilot AI review requested due to automatic review settings April 1, 2025 07:53
@github-actions github-actions bot added Azure.Identity The azure_identity crate Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Apr 1, 2025
Copy link

github-actions bot commented Apr 1, 2025

Thank you for your contribution @M-Patrone! We will review the pull request and get back to you soon.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds interactive browser-based authentication support using an OAuth2 authorization code flow. The key changes include:

  • Adding a new interactive_credential module with support for browser-based authentication.
  • Implementing a local TCP server (internal_server) to capture the authorization code.
  • Introducing an example and tests to demonstrate and verify the new authentication flow.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
sdk/identity/azure_identity/src/lib.rs Exposes the new interactive_credential module.
sdk/identity/azure_identity/src/interactive_credential/mod.rs Sets up the module structure for interactive credentials.
sdk/identity/azure_identity/src/interactive_credential/internal_server.rs Implements local TCP server logic to handle browser redirection and auth code extraction.
sdk/identity/azure_identity/src/interactive_credential/interactive_browser_credential.rs Implements the interactive browser authentication credential using OAuth2.
sdk/identity/azure_identity/examples/interactive_credentials.rs Provides a working example of interactive browser authentication.
sdk/identity/azure_identity/Cargo.toml Adds dependency for async process handling on non-WASM targets.

@heaths heaths requested a review from chlowell April 1, 2025 19:35
Copy link
Member

@heaths heaths left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! I'll let @chlowell review further and decide what to do since he's working on azure_identity currently, but I've made a few comments since he's ramping up.

Comment on lines +34 to +36
client_id: Option<ClientId>,
tenant_id: Option<String>,
redirect_url: Option<Url>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be passed into an InteractiveBrowserCredentialOptions per guidelines and some of the newer credential types/changes.

@github-project-automation github-project-automation bot moved this from Untriaged to In Progress in Azure Identity SDK Improvements Apr 1, 2025
@M-Patrone
Copy link
Author

@heaths Thank you for your helpful feedback! I really appreciate you taking the time to review this. I'll make the suggested improvements and update the code accordingly. 🙏

Copy link
Member

@chlowell chlowell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this! I'm afraid it's a bigger feature than it might appear at first glance because we don't yet have the infrastructure to support user authentication. It may be best to revisit this in the future once we have the necessary caching features--they aren't trivial to implement

#[allow(dead_code)]
pub async fn get_token(
&self,
scopes: Option<&[&str]>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should require at least one scope and add ["offline_access", "openid", "profile"] to the caller's scopes. There are a few reasons scopes mustn't be optional:

  1. credentials must implement TokenCredential to function with SDK clients
  2. get_token(None) means something nonsensical like "give me an access token, I don't care what it's for"
  3. default scopes can cause mysterious behavior

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chlowell Thank you very much for your feedback, and apologies for the delayed response.

I'm currently working on implementing TokenCredential. However, I’ve run into an issue that I find a bit puzzling. When compiling, I get the following error:

 implementation of `std::marker::Send` is not general enough
   --> sdk/identity/azure_identity/src/interactive_credential/interactive_browser_credential.rs:150:83
    |
150 |       async fn get_token(&self, scopes: &[&str]) -> azure_core::Result<AccessToken> {
    |  ___________________________________________________________________________________^
151 | |         let scopes_owned: Vec<Cow<'_, str>> = scopes.iter().map(|s| Cow::Borrowed(*s)).collect();
152 | |         self.get_access_token(scopes_owned).await
153 | |     }
    | |_____^ implementation of `std::marker::Send` is not general enough
    |
    = note: `std::marker::Send` would have to be implemented for the type `&str`
    = note: ...but `std::marker::Send` is actually implemented for the type `&'0 str`, for some specific lifetime `'0`

I haven’t had the chance to fully dive into this yet, but maybe someone has encountered a similar issue before. The error seems to be triggered by this part of the code:

authorization_code_flow
                    .exchange(
                        options.http_client.clone(),
                        AuthorizationCode::new(code).clone(),
                    )
                    .await
                    .map(|r| {
                        return AccessToken::new(
                            r.access_token().secret().clone(),
                            OffsetDateTime::now_utc() + r.expires_in().unwrap().clone(),
                        );
                    });

This eventually calls into the function here: exchange

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran into the same problem last week. It appears related to rust-lang/rust#64552. I gave up on finding a workaround because replacing oauth2 with my own code was much faster. That's what I would do in this case as well. The protocol is documented here, though that doc doesn't cover the public client case we're interested in--in the exchange request, a public client would omit client_secret and client_assertion.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chlowell Thanks a lot for your helpful response! I really appreciate you taking the time.

Over the last couple of days, I dug into the issue to better understand the root cause—mainly because the original error message was so vague that it didn’t point me in any useful direction. That was the most frustrating part.

So I started isolating the problem step by step. It became clear that the error happens during the exchange call in the authorization code flow. To narrow it down further, I stripped my code down to this minimal example:

let b = AuthorizationCode::new("test_auth_code".to_string()).clone();
 let c = self.options.http_client.clone();
 let a = authorization_code_flow.exchange(c, b).await; 

That’s where the error triggered. I then created a dedicated test function to extract and observe the behavior more closely:

    fn get_access_token_test(
        &self,
        scopes: &[&str],
    ) -> impl Send + Future<Output = azure_core::Result<AccessToken>> {
        assert_send(async move {
            let authorization_code_flow = authorization_code_flow::authorize(
                ClientId::new("jkadjfa".to_string()),
                None,
                &"jkadjfa".to_string(),
                Url::from_str("str").unwrap(),
                &scopes,
            );

            let b = AuthorizationCode::new("djfak".to_string()).clone();
            let c = new_http_client();

            let a = authorization_code_flow.exchange(c, b).await?.clone();

            Ok(AccessToken::new("test", OffsetDateTime::now_utc()))
        })
    }
}

And here's the helper function for asserting Send:

fn assert_send<T>(fut: impl Send + Future<Output = T>) -> impl Send + Future<Output = T> {
    fut
}
#[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))]
#[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)]
impl TokenCredential for InteractiveBrowserCredential {
    async fn get_token(&self, scopes: &[&str]) -> azure_core::Result<AccessToken> {
        self.get_access_token_test(scopes).await
    }
}

I used this in a custom TokenCredential implementation so I could observe the issue directly when calling get_token.

This gave me a much more helpful error message:

error: implementation of std::marker::Send is not general enough
   --> sdk/identity/azure_identity/src/interactive_credential/interactive_browser_credential.rs:154:9
    |
154 | /         assert_send(async move {
155 | |             let authorization_code_flow = authorization_code_flow::authorize(
156 | |                 ClientId::new("jkadjfa".to_string()),
157 | |                 None,
...   |
168 | |             Ok(AccessToken::new("test", OffsetDateTime::now_utc()))
169 | |         })
    | |__________^ implementation of std::marker::Send is not general enough
    |
    = note: std::marker::Send would have to be implemented for the type &Oauth2HttpClient
    = note: ...but std::marker::Send is actually implemented for the type &'0 Oauth2HttpClient, for some specific lifetime '0

So the issue was clearly a Send problem related to the lifetime of the closure passed to request_async.

To fix it, I restructured my exchange method to use Arc and clone it inside the closure, like this:

 pub async fn exchange(
        self,
        http_client: Arc<dyn HttpClient>,
        code: oauth2::AuthorizationCode,
    ) -> azure_core::Result<
        oauth2::StandardTokenResponse<oauth2::EmptyExtraTokenFields, oauth2::basic::BasicTokenType>,
    > {
        //        let oauth_http_client = Oauth2HttpClient::new(http_client.clone());
        //        let client = |request: HttpRequest| oauth_http_client.request(request);

        //improve problem with implementing the `send`
        let oauth_http_client = Arc::new(Oauth2HttpClient::new(http_client.clone()));
        let client = {
            let oauth_http_client = oauth_http_client.clone();
            move |request: HttpRequest| {
                let oauth_http_client = oauth_http_client.clone();
                async move { oauth_http_client.request(request).await }
            }
        };

        self.client
            .exchange_code(code)
            // Send the PKCE code verifier in the token request
            .set_pkce_verifier(self.pkce_code_verifier)
            .request_async(&client)
            .await
            .context(
                ErrorKind::Credential,
                "exchanging an authorization code for a token failed",
            )
    }

This resolved the Send issue for now—though I still want to run a few more tests to be completely sure.

Thanks again for your time and suggestion to replace the oauth2 crate. That’s still on the table, but I wanted to understand the root problem first—and I think I’ve got a clearer picture now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry to make this feature bigger, however it will also need an auth code flow implementation because we want to remove the oauth2 dependency. The flow is documented here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chlowell, no worries at all. I really appreciate your guidance throughout this process.

Digging into this implementation has been quite insightful for me. It also gave me the opportunity to learn how to handle custom OAuth2 properties returned in the token response body, such as client_info 😁. To achieve this, I extended the OAuth2 client using a custom oauth2_http_client::Oauth2HttpClient and defined a new client type to work with the additional fields.

Based on your comment, I understand that you want to remove the oauth2 dependency entirely and implement the authorization code flow ourselves.

That brings me to a quick question: Would you prefer that I

  • implement a general authorization code flow in a separate PR and then extend it for the interactive flow (to handle client_info),
  • or focus only on implementing the interactive variant directly with client_info,
  • or possibly handle everything directly in this current PR?

I'm totally flexible and happy to go with whichever option fits the project's needs and your preferences best — just let me know how you'd like to proceed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add it in this PR. In my imagination it isn't a lot of code because you can assume the server is Entra ID (e.g. simply request client_info in all cases) and that only this credential will use the auth code flow.

}

#[tokio::test]
async fn interactive_auth_flow_should_return_token() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mustn't run by default. CI runs can't complete the interactive login and we don't want to open a browser every time someone working on the crate runs cargo test

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a good place for something under examples/ with a comment here in interactive_browser_credential.rs asking devs changing the code to run it manually. You should put assertions in there - at least that it works.

///
/// If no scopes are provided, default scopes will be used.
#[allow(dead_code)]
pub async fn get_token(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be wrapped in the internal cache to prevent every call redundantly authenticating. See for example ClientAssertionCredential.

Here's a gotcha though: the cache implicitly assumes it holds tokens for only one identity. That won't work for this particular credential because users may choose a different account every time they authenticate. We have to prevent this credential getting into a state where, say, it returns Cosmos tokens for user A and Key Vault tokens for user B. I imagine the simplest solution would be having the credential remember the last authenticated user (this would require parsing id tokens) and clear the cache when the application authenticates a new user. That may be complex to implement

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @chlowell , I've now implemented the cache similar to how it's done in ` ClientAssertionCredential. However, I'm still trying to fully understand the concern regarding caching tokens from potentially different identities.

From my current perspective, it seems that when someone wants to authenticate with a different account (e.g., switching from user A to user B), they would typically create a new instance of InteractiveBrowserCredential, configured with the appropriate options. Based on this assumption, I thought the risk of mixing tokens between different users would be naturally avoided. Here's a simplified example of how I imagined this working:

fn foo(){
// User A
let options_a = InteractiveBrowserCredentialOptions {
    client_id: None,
    tenant_id: Some(tenant_id_a),
    redirect_url: None,
};
let credential_a = InteractiveBrowserCredential::new(options_a)?;
let token_a = credential_a.get_token(&["https://management.azure.com/.default"]).await?;

// Do something with token_a...

// User B
let options_b = InteractiveBrowserCredentialOptions {
    client_id: None,
    tenant_id: Some(tenant_id_b),
    redirect_url: None,
};
let credential_b = InteractiveBrowserCredential::new(options_b)?;
let token_b = credential_b.get_token(&["https://management.azure.com/.default"]).await?;

// Do something with token_b...
}

So my (possibly naive) assumption was: if the developer needs to authenticate a different user, they would instantiate a separate credential, similar to how other credentials behave.

That said, I might very well be missing an important part of the picture. I'm still relatively new to this, and I want to make sure I understand the full context. If you have time, could you perhaps elaborate a bit more on the potential issue – maybe even with some pseudocode or a concrete example where this problem could arise?

Thanks again for your guidance and patience. I really appreciate it and I’m learning a lot through this process.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So my (possibly naive) assumption was: if the developer needs to authenticate a different user, they would instantiate a separate credential, similar to how other credentials behave.

That assumption doesn't hold for an interactive credential like this because the developer can't control the authenticated account or know it before get_token returns. This credential sends the user to a web page where they can log in any account they want. In your example, credential_a and credential_b could authenticate the same account or different accounts--that's up to the user when they complete the interactive login.

You're correct that a developer who wanted to get tokens for different users simultaneously would need multiple instances of this credential, however that isn't a common scenario and would be difficult to accomplish because again, the user (of the program) completes the interactive login and therefore decides which account to authenticate. The common case is a single-user program like the Azure CLI, a tool users run locally to do something with Azure resources.

I should reiterate that implementing this credential is particularly difficult because it has more moving parts than other types and entails additional work for caching and handling user accounts, parts of which may not be documented. Honestly, it's a bad first issue.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @chlowell, thank you so much for the detailed explanation. I really appreciate it!

I realize now that I had a bit of a misunderstanding around the caching behavior. I assumed that caching would inherently avoid the situation where different users get mixed up, because once a token is cached, no new browser flow would be triggered – so no opportunity for a different user to log in.
What I didn’t quite see is that once the token expires, a new login can happen, and that’s where a different user could authenticate. And because the cache isn’t user-aware, that can lead to the wrong token being used or stored. That totally makes sense now.

So the correct behavior would be: the credential needs to detect when a different user logs in (e.g., by parsing the ID token) and clear the cache when that happens, so tokens from different identities don’t get mixed up. Got it!

Even though this might not be the ideal "first issue" 😅, I’d still really like to give it a proper shot and learn from it. If you agree, I’ll try to model the solution after the existing caching mechanisms like in ClientAssertionCredential and expand from there.

Thanks again for your help and patience! 🙌

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the correct behavior would be: the credential needs to detect when a different user logs in (e.g., by parsing the ID token) and clear the cache when that happens, so tokens from different identities don’t get mixed up.

I believe that's the simplest way to get the correct behavior. An ID token won't always provide the data necessary to identify a user account uniquely and consistently, so you'd instead use the "home account ID", which so far as I know isn't documented. To get one, add client_info=1 to the token request body. The response will then have a field "client_info": "some base64". The base64 decodes to a string of JSON like {"uid":"A","utid":"B"}. A and B are consistent and unique for the authenticated account, and by convention the home account ID is "A.B".

I don't want to discourage you working on this PR--your contribution's welcome--only to make sure you're aware that this feature is particularly complex and getting this PR to the finish line will probably require significant effort all around. Continuing to work on it and abandoning it are both reasonable choices ☺️

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chlowell Thanks a lot for the helpful reply!

That’s really good to know about the ID token. I had just implemented the Hybrid Flow and was planning to cache the oid,tid and also the sub claim. Based on this StackOverflow thread, I know now, that I wasn’t too far off. But if the ID token doesn’t guarantee a stable identifier, then I’ll definitely switch gears and give client_info a try instead, that sounds like the more reliable way to go.

Whether or not this PR ends up being merged is totally fine with me. I’ve already learned so much and that’s a huge win on its own for me 😄. If at some point I start to clutter your inbox too much, feel free to wave the white flag, no hard feelings 😄. For now, I’d still love to keep exploring and pushing it a bit further if that’s okay!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chlowell, apologies for the delay. I'm just finalizing the changes.
Before wrapping up, I wanted to ask if it would make sense to provide an option for users to choose between using the hybrid auth flow and the approach that includes client_info=1?
I'm asking because I still have the implementation for the hybrid flow and could include it if it's useful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that's an implementation detail. The get_token contract is "scopes in, access token out". User account data isn't relevant at that level because it doesn't affect the access token. Come to think of it, we should avoid acquiring an ID token if possible, because that's a waste of cycles and bytes given we have no use for the token at this time. That would mean not adding the openid and profile scopes. I don't know whether client_info=1 works without those scopes so, please give it a try.

Copy link
Author

@M-Patrone M-Patrone May 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chlowell I tested the behavior, and it seems that when both openid and profile scopes are removed, I no longer receive the user info (client_info). I also tried including only one of them at a time, but that didn’t work either. It appears that both openid and profile need to be present in order to receive the user info:
image
Given that, acquiring an ID token seems to be unavoidable if we need access to client_info, unless there's another approach I'm missing. Let me know if you have any suggestions on how we might avoid the extra scopes while still retrieving the necessary user data.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing the science. We need client_info in any case, so it's okay to acquire an ID token if that's necessary to get client_info.

@MoChilia
Copy link
Member

MoChilia commented Apr 9, 2025

Hi team, this is a lovely feature and exactly what I'm looking for. Are there any plans or milestones to bring it to production?

@M-Patrone
Copy link
Author

Hi team, this is a lovely feature and exactly what I'm looking for. Are there any plans or milestones to bring it to production?

Hello @MoChilia, I'm currently incorporating some improvements based on the feedback I received. The work is still in progress on my side, but I can’t speak for the core maintainers regarding if or when it might be merged into production.

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 Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants