-
Couldn't load subscription status.
- Fork 32
Fix Azure OIDC endpoint selection to support both U2M and M2M authentication flows #454
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
Fix Azure OIDC endpoint selection to support both U2M and M2M authentication flows #454
Conversation
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
| } | ||
| if (isAzure() && getAzureClientId() != null) { | ||
|
|
||
| if (isAzure() && shouldUseAzureOidcEndpoints()) { |
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.
Something to check/test, is whether this breaks Databricks OIDC. If I am reading this correct, Databricks OIDC will also try to connect to Azure endpoints.
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.
Thanks for the review, Hector!
The flows I have tested for this change is :
- U2M for azure databricks workspace
- M2M with databricks based credentials for Azure workspace
- M2M with Azure service principals for Azure workspace
Is there any other testing that you suggest?
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.
Databricks OIDC for Azure workspace:
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.
(Adding context from our offline discussion from the past week) : our product does not use this flow and we will need to build the testing infra to be able to test out the suggested flow.
|
Close as per discussion |
What changes are proposed in this pull request?
How is this tested?
Note to the reviewers
We can't work around this by adding a dummy Azure client ID during U2M external browser auth (e.g.,
.setAzureClientId("test")), because none of these fields are populated for U2M before the OIDC endpoints are fetched.