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

Feature: GitHub OAuth support #4695

Closed
wants to merge 7 commits into from
Closed

Conversation

jupe
Copy link

@jupe jupe commented Feb 29, 2020

What type of PR is this?

  • Feature

Description

Introduce Github OAuth authentication (again, #3629).

Related Tickets & Documents

Covers issue: #2371
Documentation PR: getredash/website#342

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

image

jupe added 5 commits February 29, 2020 11:41
user profile gives only primary email, but sometimes user might have another email address which domain is configured for redash. This change allows to login using secondary address also.
@jupe jupe changed the title Feature: GitHub oauth support Feature: GitHub OAuth support Feb 29, 2020
@jupe jupe marked this pull request as ready for review February 29, 2020 18:25
Copy link
Contributor

@rauchy rauchy left a comment

Choose a reason for hiding this comment

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

Thanks @jupe! Looks good. I just have a few nits 😊

redash/templates/login.html Outdated Show resolved Hide resolved
redash/templates/invite.html Outdated Show resolved Hide resolved
redash/handlers/authentication.py Show resolved Hide resolved
redash/authentication/github_oauth.py Outdated Show resolved Hide resolved
client/app/pages/settings/OrganizationSettings.jsx Outdated Show resolved Hide resolved
client/app/pages/settings/OrganizationSettings.jsx Outdated Show resolved Hide resolved
@jupe
Copy link
Author

jupe commented Mar 1, 2020

@rauchy thanks for review. I’ll update PR tomorrow.

@jupe
Copy link
Author

jupe commented Mar 12, 2020

@rauchy is there still something you would like to change/add ?

@ocadaruma
Copy link

Hi, I plan to use Redash in our company and it'd be very nice if Redash has this feature.

Is there any chance that this PR gets merged?

Or may I re-work the PR based on current impl to fix those conflicts if you don't mind?
I confirmed that this PR works fine even on latest code by just fixing few conflicts.

@susodapop
Copy link
Contributor

Thanks @ocadaruma and @jupe for bumping this. It slipped off our radar. Will see about reviewing this again shortly.

@susodapop susodapop self-requested a review May 11, 2021 20:19
@guidopetri
Copy link
Contributor

@jupe , thanks for the PR! We've updated a lot of things now that we're Community-driven so - if you're still interested in getting this merged - would you mind rebasing off master to re-run the CI, as well as updating merge conflicts?

We're trying to clean up our PR todo list, so if you're not interested, that's fine - we'll close the PR in about a week if we don't hear back. If you're interested in reopening the PR afterwards, we would also very much welcome that.

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

Successfully merging this pull request may close these issues.

6 participants