Skip to content

Passing encrypted key as credential#270

Merged
rayluo merged 2 commits intodevfrom
private-key-with-password
Oct 30, 2020
Merged

Passing encrypted key as credential#270
rayluo merged 2 commits intodevfrom
private-key-with-password

Conversation

@abhidnya13
Copy link
Contributor

@abhidnya13 abhidnya13 commented Oct 23, 2020

Usage: We add one more optional field passphrase in the existing client_credential parameter of ConfidentialClientApplication. The docs will also be updated here

@abhidnya13 abhidnya13 force-pushed the private-key-with-password branch 3 times, most recently from f614596 to 50800e2 Compare October 23, 2020 14:52
@abhidnya13 abhidnya13 requested a review from rayluo October 23, 2020 17:56
@abhidnya13 abhidnya13 linked an issue Oct 27, 2020 that may be closed by this pull request
Copy link

@henrik-me henrik-me left a comment

Choose a reason for hiding this comment

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

:shipit:

@henrik-me
Copy link

unit test?

Copy link
Contributor

@rayluo rayluo left a comment

Choose a reason for hiding this comment

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

Overall it looks good! Leave a couple minor comment below to hear your thoughts.

Copy link
Contributor

@rayluo rayluo left a comment

Choose a reason for hiding this comment

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

Thanks! Per our discussion yesterday, please also add the two new dependencies into our setup.py (otherwise it would break). We can probably use the same version requirements used by Azure SDK.

After that, this would be good to go!

@abhidnya13 abhidnya13 requested a review from rayluo October 29, 2020 02:43
@abhidnya13
Copy link
Contributor Author

I see the unit tests are removed in this commit , can we get them back ?
@rayluo any reason not to have them there ?

@abhidnya13
Copy link
Contributor Author

Thanks for adding the tests @rayluo ! Besides, I have also manually tested this scenario e2e using an encrypted private key and verified it works as expected.

@rayluo rayluo force-pushed the private-key-with-password branch 2 times, most recently from cc3b696 to 2ded277 Compare October 30, 2020 19:06
abhidnya13 and others added 2 commits October 30, 2020 12:07
Code refactoring

Changing reference doc string

Adding tests

PR review 1

Adding dependencies and polishing code

Python 2 compat
Adding missing arguments to api call

Use cryptography lower bound as low as 0.6

Add test cases for _str2bytes()

Choose cryptography upper bound as <4
Copy link
Contributor

@rayluo rayluo left a comment

Choose a reason for hiding this comment

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

Thanks Abhi! We finished this together, in pair-programming style! :-D

@rayluo rayluo merged commit c37f36b into dev Oct 30, 2020
@rayluo rayluo deleted the private-key-with-password branch October 30, 2020 22:08
@rayluo rayluo mentioned this pull request Nov 2, 2020
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.

Accept an encrypted key as client credential

3 participants