-
-
Notifications
You must be signed in to change notification settings - Fork 163
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
Add Microsoft device code example with tenant id #222
Add Microsoft device code example with tenant id #222
Conversation
"https://login.microsoftonline.com/{tenant}/oauth2/v2.0/authorize".to_string(), | ||
)?, | ||
Some(TokenUrl::new( | ||
"https://login.microsoftonline.com/{tenant}/v2.0/oauth2/token".to_string(), |
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.
shouldn't this be {tenant}/oauth2/v2.0/token
?
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 will change it. Thanks for finding it :)
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 is the same issue @randomairborne fixed in #220 for the common case
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.
@ramosbugs ohh, I already changed that in this PR.
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.
@ramosbugs ok you can merge it, I will just do a rebase
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.
@ramosbugs rebase done
#[tokio::main] | ||
async fn main() -> Result<(), Box<dyn Error>> { | ||
let device_auth_url = DeviceAuthorizationUrl::new( | ||
"https://login.microsoftonline.com/{tenant}/oauth2/v2.0/devicecode".to_string(), |
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.
to make it clear to users that this needs to be substituted for their tenant, I'd suggest defining a TENANT_ID
global at the top, and then using the format!
macro here to automatically fill it in so that they only have to update it in one place at the top. and then also a comment next to that global instructing them to do so, maybe with a link for where they can find that tenant ID
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.
Done
2a41e25
to
72cd0cd
Compare
To make it more understandable for Microsoft Azure endpoint users, an endpoint with tenant is added and edited the existing example as common user.
72cd0cd
to
977442a
Compare
Thanks! |
To make it more understandable for Microsoft Azure endpoint users, an endpoint with tenant is added and edited the existing example as common user.