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

[Cosmos] AAD authentication sync client #23604

Merged
merged 22 commits into from
Apr 6, 2022
Merged

[Cosmos] AAD authentication sync client #23604

merged 22 commits into from
Apr 6, 2022

Conversation

simorenoh
Copy link
Member

@simorenoh simorenoh commented Mar 22, 2022

This PR has the changes for the sync client to utilize AAD authentication. I'll be making the PR for the async client separate, in order to reduce number of lines per PR.

The way the @azure.identity package uses AAD credentials to authenticate services is by adding those credentials into a policy that runs when requests are sent to the core pipelines. This policy makes sure to refresh the current token if needed and set the authentication header of requests going to the pipeline. The reason why Cosmos had to create their own policy in this instance is due to the prefix we utilize for our tokens, since the bearer token policy given by the identity module sends a different prefix altogether and as such does not work for us.

It was also recommended by the identity team to create our own policies entirely rather than attempting to override a couple methods, since this could break us on their end - specially for the _update_headers() method since it's private.

Sample is a simple run-through of what can and can't be done, if you think adding more examples would be helpful I can do so as well.

@ghost ghost added the Cosmos label Mar 22, 2022
@azure-sdk
Copy link
Collaborator

API changes have been detected in azure-cosmos. You can review API changes here

@simorenoh simorenoh marked this pull request as ready for review March 22, 2022 16:59
@azure-sdk
Copy link
Collaborator

API changes have been detected in azure-cosmos. You can review API changes here

This reverts commit 721bbc7.
@azure-sdk
Copy link
Collaborator

API changes have been detected in azure-cosmos. You can review API changes here

@azure-sdk
Copy link
Collaborator

API changes have been detected in azure-cosmos. You can review API changes here

@azure-sdk
Copy link
Collaborator

API changes have been detected in azure-cosmos. You can review API changes here

sdk/cosmos/azure-cosmos/CHANGELOG.md Show resolved Hide resolved
sdk/cosmos/azure-cosmos/README.md Outdated Show resolved Hide resolved
sdk/cosmos/azure-cosmos/README.md Outdated Show resolved Hide resolved
sdk/cosmos/azure-cosmos/azure/cosmos/_base.py Outdated Show resolved Hide resolved
sdk/cosmos/azure-cosmos/azure/cosmos/auth.py Outdated Show resolved Hide resolved
sdk/cosmos/azure-cosmos/samples/config.py Outdated Show resolved Hide resolved
@azure-sdk
Copy link
Collaborator

API changes have been detected in azure-cosmos. You can review API changes here

@simorenoh simorenoh requested review from annatisch and johanste March 28, 2022 14:18
@simorenoh
Copy link
Member Author

/azp run python - cosmos - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-sdk
Copy link
Collaborator

API changes have been detected in azure-cosmos. You can review API changes here

Copy link
Member

@xinlian12 xinlian12 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@simorenoh
Copy link
Member Author

/azp run python - cosmos - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-sdk
Copy link
Collaborator

API change check for azure-cosmos

API changes have been detected in azure-cosmos. You can review API changes here

@simorenoh
Copy link
Member Author

/azp run python - cosmos - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@simorenoh
Copy link
Member Author

/azp run python - cosmos - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@simorenoh
Copy link
Member Author

/azp run python - cosmos - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@simorenoh simorenoh merged commit 5b928c1 into Azure:main Apr 6, 2022
@simorenoh simorenoh deleted the cosmos-aad branch April 6, 2022 14:49
rakshith91 pushed a commit to rakshith91/azure-sdk-for-python that referenced this pull request Apr 7, 2022
* working authentication to get database account

* working aad authentication for sync client with sample

* readme and changelog

* pylint and better comments on sample

* Update auth.py

* Revert "Update auth.py"

This reverts commit 721bbc7.

* Update auth.py

* Update auth.py

* changes from comments

* quick comment updates

* Update config.py

* Update access_cosmos_with_aad.py

* added sync policy to match async

* small changes

* aad tests for negative path and positive emulator path

* moved logic to be together for each part

* Milis comments

* Update cosmos_client.py

* Update dev_requirements.txt

* Update _auth_policy.py
rakshith91 pushed a commit to rakshith91/azure-sdk-for-python that referenced this pull request Apr 10, 2022
* working authentication to get database account

* working aad authentication for sync client with sample

* readme and changelog

* pylint and better comments on sample

* Update auth.py

* Revert "Update auth.py"

This reverts commit 721bbc7.

* Update auth.py

* Update auth.py

* changes from comments

* quick comment updates

* Update config.py

* Update access_cosmos_with_aad.py

* added sync policy to match async

* small changes

* aad tests for negative path and positive emulator path

* moved logic to be together for each part

* Milis comments

* Update cosmos_client.py

* Update dev_requirements.txt

* Update _auth_policy.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. Cosmos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants