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 support for using MS Graph instead of AAD #67
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Azure Active Directory Graph API, now deprecated, does not provide support for atomically creating/removing passwords on an application. As a result, there is a race conditions that can occur when creds are being created for roles configured with an existing service principal that is configured on multiple mounts or across multiple Vault clusters. Unfortunately, [`Azure/azure-sdk-for-go`](https://github.com/Azure/azure-sdk-for-go) does not yet offer a MS Graph API client, therefore, this PR utilizes [`Azure/go-autorest`](https://github.com/Azure/go-autorest) to construct a client the same as [`Azure/azure-sdk-for-go`](https://github.com/Azure/azure-sdk-for-go). This changeset preserves using the AAD Graph API by default but provides a mount configuration option for toggling to the new MS Graph API. This is because the two APIs require different API permissions. This allows users to upgrade to the new plugin version and then switch to the new API. Additionally, although using the MS Graph API is a net benefit, it itself has reliability issues when handling multiple requests in parallel. More details can be found in https://github.com/mdgreenfield/microsoft-graph-api-reliability and I am working with Microsoft to try to get some of these reliability issues resolved. Fixes #58
Closed
jasonodonnell
approved these changes
Sep 23, 2021
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.
LGTM and tested well!
This was referenced Oct 5, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
The Azure Active Directory Graph API (AAD) is being deprecated in favor of the Microsoft Graph API. The AAD API is slated to be removed from Azure mid-2022. This PR updates the secrets engine to support using either API so users can migrate to using MS Graph.
NOTE: Since the AAD API is going to be removed entirely, this engine will stop supporting using the AAD API at some point in the future. All users of this engine should migrate to using the MS Graph API. The exact date for the removal of the AAD API from this engine has not be set.
Big thanks to @mdgreenfield for doing a large part of the heavy lifting for this PR
Design of Change
use_microsoft_graph_api
) that toggles between using AAD and MS Graphgraphrbac
library) and using theautorest
library to use the MS Graph APIapi
)Related Issues/Pull Requests
#63
#59
Contributor Checklist
Test Output
AAD:
MS Graph: