Skip to content

[Identity] Add UsernamePasswordCredential for username/password authentication#4404

Merged
daviwil merged 3 commits intoAzure:masterfrom
daviwil:user-pass-credential
Jul 30, 2019
Merged

[Identity] Add UsernamePasswordCredential for username/password authentication#4404
daviwil merged 3 commits intoAzure:masterfrom
daviwil:user-pass-credential

Conversation

@daviwil
Copy link
Contributor

@daviwil daviwil commented Jul 23, 2019

This change adds username/password authentication as described in the AAD OAuth documentation:

https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth-ropc

Resolves #4310.

@ramya-rao-a ramya-rao-a requested a review from sophiajt July 23, 2019 23:20
Copy link
Contributor

@sophiajt sophiajt left a comment

Choose a reason for hiding this comment

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

Looks good (assuming we always force them to connect via https). I'm guessing Scott should also take a look.

@daviwil
Copy link
Contributor Author

daviwil commented Jul 24, 2019

I don't force it right now, but I probably should. So long as it doesn't negatively impact test scenarios it'll be easy to throw when http is used.

@daviwil daviwil force-pushed the user-pass-credential branch from 39049d0 to b6c7194 Compare July 25, 2019 21:28
@daviwil
Copy link
Contributor Author

daviwil commented Jul 25, 2019

Added the https check in b6c7194


if (!this.baseUri.startsWith("https:")) {
throw new Error("The authorityHost address must use the 'https' protocol.");
}
Copy link

Choose a reason for hiding this comment

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

This check might be too restrictive if this client supports ADFS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I check for it at all, or leave it for now and see if anyone runs into trouble with it?

@daviwil
Copy link
Contributor Author

daviwil commented Jul 30, 2019

I'll go ahead and merge this and then will update the https check in a new PR if @schaabs thinks it should be changed. Thanks all!

@daviwil daviwil merged commit a1cd8e7 into Azure:master Jul 30, 2019
@daviwil daviwil deleted the user-pass-credential branch July 30, 2019 18:23
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.

Identity: Implement Username/password Authentication

3 participants