Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
base: main
Are you sure you want to change the base?
Add Interactive Browser Authentication Support #2418
Changes from all commits
caa690d
3e5f48f
d42509e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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
There was a problem hiding this comment.
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: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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
andcredential_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.
There was a problem hiding this comment.
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! 🙌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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☺️
There was a problem hiding this comment.
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 thesub
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 giveclient_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!
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 theopenid
andprofile
scopes. I don't know whetherclient_info=1
works without those scopes so, please give it a try.There was a problem hiding this comment.
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
andprofile
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 bothopenid
andprofile
need to be present in order to receive the user info: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.There was a problem hiding this comment.
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 getclient_info
.There was a problem hiding this comment.
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:get_token(None)
means something nonsensical like "give me an access token, I don't care what it's for"There was a problem hiding this comment.
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: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:
This eventually calls into the function here: exchange
There was a problem hiding this comment.
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
andclient_assertion
.There was a problem hiding this comment.
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:
That’s where the error triggered. I then created a dedicated test function to extract and observe the behavior more closely:
And here's the helper function for asserting Send:
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:
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:
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.
There was a problem hiding this comment.
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 hereThere was a problem hiding this comment.
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 customoauth2_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
authorization code flow
in a separate PR and then extend it for the interactive flow (to handleclient_info
),client_info
,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.
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 ininteractive_browser_credential.rs
asking devs changing the code to run it manually. You should put assertions in there - at least that it works.