Skip to content

fix: allow orgs with default github provider #16755

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

Merged
merged 1 commit into from
Mar 3, 2025

Conversation

hugodutka
Copy link
Contributor

This PR fixes 2 bugs:

Problem 1

The server would fail to start when the default github provider was configured and the flag --oauth2-github-allowed-orgs was set. The error was

error: configure github oauth2: allow everyone and allowed orgs cannot be used together

This PR fixes it by enabling "allow everone" with the default provider only if "allowed orgs" isn't set.

Problem 2

The default github provider uses the device flow to authorize users, and that's handled differently by our web UI than the standard oauth flow. In particular, the web UI only handles JSON responses rather than HTTP redirects. There were 2 code paths that returned redirects, and the PR changes them to return JSON messages instead if the device flow is configured.

@hugodutka hugodutka changed the title fix: allowed orgs with default github provider fix: allow orgs with default github provider Feb 28, 2025
@hugodutka hugodutka marked this pull request as ready for review February 28, 2025 14:49
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

The change looks reasonable to me, but cc'ing @Emyrk for a second pair of eyes.

@johnstcn johnstcn requested review from Emyrk and removed request for dannykopping March 3, 2025 12:51
Copy link
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

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

Before this change, if neither allowEveryone or allowOrgs was set, did we implicitly allow anyone?

@hugodutka
Copy link
Contributor Author

@Emyrk we set allowEveryone to true by default, so we implicitly allowed everyone.

@hugodutka hugodutka merged commit 95347b2 into main Mar 3, 2025
36 checks passed
@hugodutka hugodutka deleted the hugodutka/github-oauth2-default-orgs-fix branch March 3, 2025 15:05
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2025
@stirby
Copy link
Collaborator

stirby commented Mar 3, 2025

/cherry-pick release/2.20

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants