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

Add generic OIDC authentication flow #6783

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions client/app/assets/images/openid.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 5 additions & 2 deletions redash/authentication/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ def init_app(app):
from redash.authentication.google_oauth import (
create_google_oauth_blueprint,
)
from redash.authentication.oidc import create_oidc_blueprint

login_manager.init_app(app)
login_manager.anonymous_user = models.AnonymousUser
Expand All @@ -257,12 +258,14 @@ def extend_session():
# Authlib's flask oauth client requires a Flask app to initialize
for blueprint in [
create_google_oauth_blueprint(app),
create_oidc_blueprint(app),
saml_auth.blueprint,
remote_user_auth.blueprint,
ldap_auth.blueprint,
]:
csrf.exempt(blueprint)
app.register_blueprint(blueprint)
if blueprint:
csrf.exempt(blueprint)
app.register_blueprint(blueprint)

user_logged_in.connect(log_user_logged_in)
login_manager.request_loader(request_loader)
Expand Down
106 changes: 106 additions & 0 deletions redash/authentication/oidc.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
import logging

import requests
from authlib.integrations.flask_client import OAuth
from flask import Blueprint, flash, redirect, request, session, url_for

from redash import models, settings
from redash.authentication import (
create_and_login_user,
get_next_path,
logout_and_redirect_to_index,
)
from redash.authentication.org_resolving import current_org


def create_oidc_blueprint(app):
if not settings.OIDC_ENABLED:
return None

oauth = OAuth(app)

Check warning on line 20 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L20

Added line #L20 was not covered by tests

logger = logging.getLogger("oidc")
blueprint = Blueprint("oidc", __name__)

Check warning on line 23 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L22-L23

Added lines #L22 - L23 were not covered by tests

def get_oidc_config(url):
resp = requests.get(url=url)

Check warning on line 26 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L25-L26

Added lines #L25 - L26 were not covered by tests
if resp.status_code != 200:
logger.warning(

Check warning on line 28 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L28

Added line #L28 was not covered by tests
f"Unable to get configuration details (response code {resp.status_code}). Configuration URL: {url}"
)
Comment on lines +27 to +30
Copy link
Member

Choose a reason for hiding this comment

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

  1. Let's consistently use response (I prefer) or resp.
        if not response.ok:
            logger.warning(
                f"Unable to get configuration details (response code {resp.status_code}). Configuration URL: {url} - {response.text}"
            )

return None
return resp.json()

Check warning on line 32 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L31-L32

Added lines #L31 - L32 were not covered by tests

oidc_config = get_oidc_config(settings.OIDC_COFIGURATION_URL)
oauth = OAuth(app)
oauth.register(

Check warning on line 36 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L34-L36

Added lines #L34 - L36 were not covered by tests
name="oidc",
server_metadata_url=settings.OIDC_COFIGURATION_URL,
client_kwargs={
"scope": "openid email profile",
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we make the needed scopes a var too?
([Future PR?] propagate/sync groups or roles for "redash_admin"?)

},
)

def get_user_profile(access_token):
Copy link
Member

Choose a reason for hiding this comment

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

maybe the canonical get_userinfo instead of get_user_profile as profile is just one of the possible scopes. let's to avoid confusion.

headers = {"Authorization": "Bearer {}".format(access_token)}
response = requests.get(oidc_config["userinfo_endpoint"], headers=headers)

Check warning on line 46 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L44-L46

Added lines #L44 - L46 were not covered by tests

if response.status_code == 401:
logger.warning("Failed getting user profile (response code 401).")
return None

Check warning on line 50 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L49-L50

Added lines #L49 - L50 were not covered by tests
Comment on lines +48 to +50
Copy link
Member

Choose a reason for hiding this comment

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

what about something like:

        if not response.ok:
            logger.warning(f"Failed getting user profile (response code {response.status_code}) - {response.text}.")
            return None


return response.json()

Check warning on line 52 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L52

Added line #L52 was not covered by tests

@blueprint.route("/<org_slug>/oidc", endpoint="authorize_org")
def org_login(org_slug):
session["org_slug"] = current_org.slug
return redirect(url_for(".authorize", next=request.args.get("next", None)))

Check warning on line 57 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L54-L57

Added lines #L54 - L57 were not covered by tests

@blueprint.route("/oidc", endpoint="authorize")
def login():
redirect_uri = url_for(".callback", _external=True)

Check warning on line 61 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L59-L61

Added lines #L59 - L61 were not covered by tests

next_path = request.args.get("next", url_for("redash.index", org_slug=session.get("org_slug")))
logger.debug("Callback url: %s", redirect_uri)
logger.debug("Next is: %s", next_path)

Check warning on line 65 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L63-L65

Added lines #L63 - L65 were not covered by tests

session["next_url"] = next_path

Check warning on line 67 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L67

Added line #L67 was not covered by tests

return oauth.oidc.authorize_redirect(redirect_uri)

Check warning on line 69 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L69

Added line #L69 was not covered by tests

@blueprint.route("/oidc/callback", endpoint="callback")
def authorized():
logger.debug("Authorized user inbound")

Check warning on line 73 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L71-L73

Added lines #L71 - L73 were not covered by tests

resp = oauth.oidc.authorize_access_token()
user = resp.get("userinfo")

Check warning on line 76 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L75-L76

Added lines #L75 - L76 were not covered by tests
if user:
session["user"] = user

Check warning on line 78 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L78

Added line #L78 was not covered by tests

access_token = resp["access_token"]

Check warning on line 80 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L80

Added line #L80 was not covered by tests

if access_token is None:
logger.warning("Access token missing in call back request.")
Copy link
Member

Choose a reason for hiding this comment

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

typo call back -> callback

flash("Validation error. Please retry.")
return redirect(url_for("redash.login"))

Check warning on line 85 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L83-L85

Added lines #L83 - L85 were not covered by tests

profile = get_user_profile(access_token)

Check warning on line 87 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L87

Added line #L87 was not covered by tests
if profile is None:
flash("Validation error. Please retry.")
return redirect(url_for("redash.login"))

Check warning on line 90 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L89-L90

Added lines #L89 - L90 were not covered by tests

if "org_slug" in session:
org = models.Organization.get_by_slug(session.pop("org_slug"))

Check warning on line 93 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L93

Added line #L93 was not covered by tests
else:
org = current_org

Check warning on line 95 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L95

Added line #L95 was not covered by tests

user = create_and_login_user(org, profile["name"], profile["email"])

Check warning on line 97 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L97

Added line #L97 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

  1. maybe allow choosing what field to use as the name?
  2. if email not verified, should we allow user creation? (avoid possible future CVE)

if user is None:
return logout_and_redirect_to_index()

Check warning on line 99 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L99

Added line #L99 was not covered by tests

unsafe_next_path = session.get("next_url") or url_for("redash.index", org_slug=org.slug)
next_path = get_next_path(unsafe_next_path)

Check warning on line 102 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L101-L102

Added lines #L101 - L102 were not covered by tests

return redirect(next_path)

Check warning on line 104 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L104

Added line #L104 was not covered by tests

return blueprint

Check warning on line 106 in redash/authentication/oidc.py

View check run for this annotation

Codecov / codecov/patch

redash/authentication/oidc.py#L106

Added line #L106 was not covered by tests
16 changes: 16 additions & 0 deletions redash/handlers/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@
return google_auth_url


def get_oidc_auth_url(next_path):
if not settings.OIDC_ENABLED:
return None
if settings.MULTI_ORG:
oidc_auth_url = url_for("oidc.authorize_org", next=next_path, org_slug=current_org.slug)

Check warning on line 35 in redash/handlers/authentication.py

View check run for this annotation

Codecov / codecov/patch

redash/handlers/authentication.py#L35

Added line #L35 was not covered by tests
else:
oidc_auth_url = url_for("oidc.authorize", next=next_path)
return oidc_auth_url

Check warning on line 38 in redash/handlers/authentication.py

View check run for this annotation

Codecov / codecov/patch

redash/handlers/authentication.py#L37-L38

Added lines #L37 - L38 were not covered by tests


def render_token_login_page(template, org_slug, token, invite):
try:
user_id = validate_token(token)
Expand Down Expand Up @@ -89,12 +99,15 @@
return redirect(url_for("redash.index", org_slug=org_slug))

google_auth_url = get_google_auth_url(url_for("redash.index", org_slug=org_slug))
oidc_auth_url = get_oidc_auth_url(url_for("redash.index", org_slug=org_slug))

return (
render_template(
template,
show_google_openid=settings.GOOGLE_OAUTH_ENABLED,
show_oidc_login=settings.OIDC_ENABLED,
google_auth_url=google_auth_url,
oidc_auth_url=oidc_auth_url,
show_saml_login=current_org.get_setting("auth_saml_enabled"),
show_remote_user_login=settings.REMOTE_USER_LOGIN_ENABLED,
show_ldap_login=settings.LDAP_LOGIN_ENABLED,
Expand Down Expand Up @@ -204,14 +217,17 @@
flash("Password login is not enabled for your organization.")

google_auth_url = get_google_auth_url(next_path)
oidc_auth_url = get_oidc_auth_url(next_path)

return render_template(
"login.html",
org_slug=org_slug,
next=next_path,
email=request.form.get("email", ""),
show_google_openid=settings.GOOGLE_OAUTH_ENABLED,
show_oidc_login=settings.OIDC_ENABLED,
google_auth_url=google_auth_url,
oidc_auth_url=oidc_auth_url,
show_password_login=current_org.get_setting("auth_password_login_enabled"),
show_saml_login=current_org.get_setting("auth_saml_enabled"),
show_remote_user_login=settings.REMOTE_USER_LOGIN_ENABLED,
Expand Down
5 changes: 5 additions & 0 deletions redash/settings/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@
GOOGLE_CLIENT_SECRET = os.environ.get("REDASH_GOOGLE_CLIENT_SECRET", "")
GOOGLE_OAUTH_ENABLED = bool(GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET)

OIDC_CLIENT_ID = os.environ.get("REDASH_OIDC_CLIENT_ID", "")
OIDC_CLIENT_SECRET = os.environ.get("REDASH_OIDC_CLIENT_SECRET", "")
OIDC_COFIGURATION_URL = os.environ.get("REDASH_OIDC_COFIGURATION_URL", "")
palash247 marked this conversation as resolved.
Show resolved Hide resolved
OIDC_ENABLED = bool(OIDC_CLIENT_ID and OIDC_CLIENT_SECRET and OIDC_COFIGURATION_URL)

# If Redash is behind a proxy it might sometimes receive a X-Forwarded-Proto of HTTP
# even if your actual Redash URL scheme is HTTPS. This will cause Flask to build
# the SAML redirect URL incorrect thus failing auth. This is especially common if
Expand Down
9 changes: 8 additions & 1 deletion redash/templates/invite.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<div class="fixed-width-page">
<div class="bg-white tiled">
<div class="m-b-25">
{% if show_google_openid or show_saml_login or show_remote_user_login or show_ldap_login %}
{% if show_google_openid or show_oidc_login or show_saml_login or show_remote_user_login or show_ldap_login %}
To create your account, please choose a password or login with your SSO provider.
{% else %}
To create your account, please choose a password.
Expand All @@ -28,6 +28,13 @@
</a>
{% endif %}

{% if show_oidc_login %}
<a href="{{ oidc_auth_url }}" class="login-button btn btn-default btn-block">
<img src="/static/images/openid.svg">
Login with OIDC
</a>
{% endif %}

{% if show_saml_login %}
<a href="{{ url_for('saml_auth.sp_initiated', org_slug=org_slug) }}" class="login-button btn btn-default btn-block">SAML Login</a>
{% endif %}
Expand Down
7 changes: 7 additions & 0 deletions redash/templates/login.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@
</a>
{% endif %}

{% if show_oidc_login %}
<a href="{{ oidc_auth_url }}" class="login-button btn btn-default btn-block">
<img src="/static/images/openid.svg">
Login with OIDC
</a>
{% endif %}

{% if show_saml_login %}
<a href="{{ url_for('saml_auth.sp_initiated', org_slug=org_slug, next=next) }}" class="login-button btn btn-default btn-block">SAML Login</a>
{% endif %}
Expand Down