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

Auth: Fix US gov azure ad oauth URL parsing #71254

Merged
merged 1 commit into from Jul 10, 2023

Conversation

douglasryanadams
Copy link
Contributor

Updates regex for tenant ID parsing to support .us domains in addition to .com domains for Azure AD.

Fixes #71252

What is this feature?

Allows for Azure AD URLs with domains that end in ".us" in addition to ".com" to support US Gov Azure deployments.

Why do we need this feature?

Fixes a bug that prevents groups using US Gov Azure from integrating Grafana to the [azure.read] Auth scheme.

Who is this feature for?

US Gov Azure organizations.

Which issue(s) does this PR fix?:

Fixes: #71252

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@douglasryanadams douglasryanadams requested a review from a team as a code owner July 7, 2023 22:25
@douglasryanadams douglasryanadams requested review from eleijonmarck and danielkenlee and removed request for a team July 7, 2023 22:25
@CLAassistant
Copy link

CLAassistant commented Jul 7, 2023

CLA assistant check
All committers have signed the CLA.

@grafanabot grafanabot added area/backend pr/external This PR is from external contributor labels Jul 7, 2023
Updates regex for tenant ID parsing to support .us domains in addition
to .com domains for Azure AD.

Fixes grafana#71252
@Jguer Jguer added this to the 10.1.x milestone Jul 10, 2023
Copy link
Contributor

@Jguer Jguer left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @douglasryanadams

@Jguer Jguer merged commit 3a245e4 into grafana:main Jul 10, 2023
16 checks passed
@douglasryanadams
Copy link
Contributor Author

Thank you for reviewing and merging this so switftly, it says a lot to us about how well this project is run.

I was able to validate our integration works with this fix today by building from source for Ubuntu. (Confirming that there were no other bugs impeding Gov Azure AD from working at this time, at least for us.)

@Jguer
Copy link
Contributor

Jguer commented Jul 12, 2023

Thanks for the kind words, we've identified another detail that has caused us to put in some extra work that is relevant #71365

polibb pushed a commit that referenced this pull request Jul 14, 2023
Updates regex for tenant ID parsing to support .us domains in addition
to .com domains for Azure AD.

Fixes #71252
@vtorosyan vtorosyan added backport v10.0.x and removed no-backport Skip backport of PR labels Jul 19, 2023
@grafana-delivery-bot
Copy link
Contributor

The backport to v10.0.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-71254-to-v10.0.x origin/v10.0.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 3a245e49458b4799c105b34807ed8b2a4ebaf192
# When the conflicts are resolved, stage and commit the changes
git add . && git cherry-pick --continue

If you have the GitHub CLI installed:

# Create the PR body template
gh pr view 71254 --json body --template 'Backport 3a245e49458b4799c105b34807ed8b2a4ebaf192 from #71254{{ "\n\n---\n\n" }}{{ index . "body" }}' > .pr-body.txt
# Push the branch to GitHub and a PR
gh pr create --title "[v10.0.x] Auth: Fix US gov azure ad oauth URL parsing" --body-file .pr-body.txt --label "type/bug" --label "area/backend" --label "add to changelog" --label "pr/external" --label "backport" --base v10.0.x --milestone 10.0.x --web

Or, if you don't have the GitHub CLI installed (we recommend you install it!):

# If you don't have the GitHub CLI installed: Push the branch to GitHub and manually create a PR:
git push --set-upstream origin backport-71254-to-v10.0.x
# Remove the local backport branch
git switch main
git branch -D backport-71254-to-v10.0.x

Unless you've used the GitHub CLI above, now create a pull request where the base branch is v10.0.x and the compare/head branch is backport-71254-to-v10.0.x.

@grafana-delivery-bot grafana-delivery-bot bot added the backport-failed Failed to generate backport PR. Please resolve conflicts and create one manually. label Jul 19, 2023
@Jguer Jguer added no-backport Skip backport of PR and removed backport v10.0.x backport-failed Failed to generate backport PR. Please resolve conflicts and create one manually. labels Jul 19, 2023
@ricky-undeadcoders ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to changelog area/backend no-backport Skip backport of PR pr/external This PR is from external contributor type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auth: US Government Azure AD OAuth URL not supported
7 participants