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

Typo in KeyVaultClient causes invalid authorization request #74

Merged
merged 2 commits into from
Nov 16, 2020

Conversation

jefshe
Copy link

@jefshe jefshe commented Nov 8, 2020

I got this error while trying out the KeyVaultClient

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Azure("AuthorizationError(Failed to authenticate to Azure Active Directory
Caused by:
   Generic error: AADSTS70011: The provided request must include a \'scope\' input parameter. The provided value for the input parameter \'scope\' is not valid. 
   The scope https://vault.azure.net.default is not valid

adding a forward slash fixes the crash

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Azure("AuthorizationError(Failed to authenticate to Azure Active Directory
Caused by:
   Generic error: AADSTS70011: The provided request must include a \'scope\' input parameter. The provided value for the input parameter \'scope\' is not valid. 
   The scope https://vault.azure.net.default is not valid
@@ -75,7 +75,7 @@ impl<'a, T: TokenCredential> KeyVaultClient<'a, T> {
// Token is valid, return it.
return Ok(());
}
let resource = format!("https://{}", &self.endpoint_suffix);
let resource = format!("https://{}/", &self.endpoint_suffix);
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should check the endpoint_suffix for an existing beginning / and only add an / if one is not already present.

Copy link
Author

Choose a reason for hiding this comment

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

That's a good point. I guess a nice way might be to make get_token accept Url but I don't know enough about OAUTH2 or this sdk to know if that makes sense.

As a short stop I've done as you've suggested 👍

@jefshe
Copy link
Author

jefshe commented Nov 16, 2020

@MindFlavor , Is there anything else I need to do to get this change merged in?

@MindFlavor
Copy link
Contributor

Sorry @jefshe, it's my fault, I have missed rylev's approval.
Thank you for you help! 👍

@MindFlavor MindFlavor merged commit 93a4b8b into Azure:master Nov 16, 2020
@jefshe
Copy link
Author

jefshe commented Nov 16, 2020

No worries! Thanks for merging it in!

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

Successfully merging this pull request may close these issues.

3 participants