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.

Sign up for GitHub

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

Jump to bottom

Auth: Make GitHub auth's allowed_organizations be case insensitive #66879

Merged
merged 2 commits into from
Apr 20, 2023
Merged

Auth: Make GitHub auth's allowed_organizations be case insensitive #66879

merged 2 commits into from
Apr 20, 2023

Conversation

consideRatio
Copy link
Contributor

@consideRatio consideRatio commented Apr 19, 2023 •
edited
Loading

What is this feature?

Its a bugfix for the github authentication for the allowed_organizations configuration that incorrectly was case sensitive. Some github organizations have a capitcal case, for example https://github.com/smithsonian, which can be seen using the GitHub CLI to work against GitHub's API:

gh api /orgs/smithsonian | jq .login
"Smithsonian"

Which issue(s) does this PR fix?:

Fixes #66876

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.

@consideRatio consideRatio requested a review from a team as a code owner April 19, 2023 16:14
@consideRatio consideRatio requested review from mgyongyosi and linoman and removed request for a team April 19, 2023 16:14
@CLAassistant
Copy link

CLAassistant commented Apr 19, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@grafanabot grafanabot added area/backend pr/external This PR is from external contributor labels Apr 19, 2023
@consideRatio consideRatio mentioned this pull request Apr 19, 2023
Closed
8 tasks
mgyongyosi
mgyongyosi approved these changes Apr 20, 2023
View reviewed changes
Copy link
Contributor

@mgyongyosi mgyongyosi left a comment

Choose a reason for hiding this comment

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

Hey @consideRatio, thank you for this PR!

@mgyongyosi mgyongyosi added the no-backport Skip backport of PR label Apr 20, 2023
@mgyongyosi mgyongyosi added this to the 10.0.x milestone Apr 20, 2023
mgyongyosi
mgyongyosi requested changes Apr 20, 2023
View reviewed changes
pkg/login/social/github_oauth.go Outdated Show resolved Hide resolved
@mgyongyosi mgyongyosi merged commit 4089514 into grafana:main Apr 20, 2023
@mgyongyosi
Copy link
Contributor

@consideRatio Thanks for the PR! It has been merged! 🚀

mdvictor pushed a commit that referenced this pull request Apr 21, 2023
@consideRatio @mdvictor
Auth: Make GitHub auth's allowed_organizations be case insensitive (#…
ab9cfd4 
…66879)

* Auth: Make GitHub auth's allowed_organizations be case insensitive

* Apply code suggestion to use strings.EqualFold
@zerok zerok modified the milestones: 10.0.x, 10.0.0 May 2, 2023
@consideRatio consideRatio mentioned this pull request May 8, 2023
Merged
@zerok zerok modified the milestones: 10.0.0, 10.0.0-preview May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgyongyosi mgyongyosi mgyongyosi approved these changes

@linoman linoman Awaiting requested review from linoman linoman was automatically assigned from grafana/grafana-authnz-team

Assignees
No one assigned
Labels
add to changelog area/backend no-backport Skip backport of PR pr/external This PR is from external contributor
Projects
None yet
Milestone
10.0.0-preview
Development

Successfully merging this pull request may close these issues.

Auth: GitHub auth's allowed_organizations is case sensitive, but shouldn't be
5 participants
@consideRatio @CLAassistant @mgyongyosi @zerok @grafanabot