From 7c7b404e5719da4168f6191f216e0f4148300356 Mon Sep 17 00:00:00 2001 From: George Fisher Date: Fri, 15 Jul 2022 11:32:32 -0700 Subject: [PATCH 01/10] Azure Oauth Login for Redash --- client/app/assets/images/microsoft_logo.svg | 1 + .../AuthSettings/AzureLoginSettings.jsx | 49 +++++++++++++++++++ .../components/AuthSettings/index.jsx | 4 +- redash/authentication/__init__.py | 3 +- redash/cli/organization.py | 25 ++++++++++ redash/cli/users.py | 25 ++++++++-- redash/handlers/authentication.py | 20 +++++++- redash/handlers/settings.py | 4 ++ redash/models/organizations.py | 5 ++ redash/settings/__init__.py | 5 ++ redash/templates/invite.html | 11 ++++- redash/templates/login.html | 9 +++- tests/handlers/test_settings.py | 20 ++++++++ tests/test_authentication.py | 41 ++++++++++++++++ 14 files changed, 211 insertions(+), 11 deletions(-) create mode 100644 client/app/assets/images/microsoft_logo.svg create mode 100644 client/app/pages/settings/components/AuthSettings/AzureLoginSettings.jsx diff --git a/client/app/assets/images/microsoft_logo.svg b/client/app/assets/images/microsoft_logo.svg new file mode 100644 index 0000000000..1f73976483 --- /dev/null +++ b/client/app/assets/images/microsoft_logo.svg @@ -0,0 +1 @@ +MS-SymbolLockup \ No newline at end of file diff --git a/client/app/pages/settings/components/AuthSettings/AzureLoginSettings.jsx b/client/app/pages/settings/components/AuthSettings/AzureLoginSettings.jsx new file mode 100644 index 0000000000..abeee43e1b --- /dev/null +++ b/client/app/pages/settings/components/AuthSettings/AzureLoginSettings.jsx @@ -0,0 +1,49 @@ +import { isEmpty, join } from "lodash"; +import React from "react"; +import Form from "antd/lib/form"; +import Select from "antd/lib/select"; +import Alert from "antd/lib/alert"; +import DynamicComponent from "@/components/DynamicComponent"; +import { clientConfig } from "@/services/auth"; +import { SettingsEditorPropTypes, SettingsEditorDefaultProps } from "../prop-types"; + +export default function AzureLoginSettings(props) { + const { values, onChange } = props; + + if (!clientConfig.azureLoginEnabled) { + return ( +

+ azure login enabled {clientConfig.azureLoginEnabled} +

+ ); + } + + return ( + +

Microsoft Work or School Account Login

+ + onChange({ auth_azure_roles: value })} + /> + {!isEmpty(values.auth_azure_roles) && ( + + Restrict access to users assigned the {join(values.auth_azure_roles, ", ")} role. +

+ } + className="m-t-16" + /> + )} + ); } diff --git a/redash/authentication/azure_oauth.py b/redash/authentication/azure_oauth.py index 06189d013c..73827b267f 100644 --- a/redash/authentication/azure_oauth.py +++ b/redash/authentication/azure_oauth.py @@ -1,5 +1,7 @@ import logging import requests +import base64 +import json from flask import redirect, url_for, Blueprint, flash, request, session @@ -29,10 +31,31 @@ def verify_profile(org, profile): return False +def get_roles_in_id_token(id_token, logger): + logger.debug("Validating ID token") + id_token_parts = id_token.split(".") + if len(id_token_parts) < 2: + logger.warning("Malformed ID token") + decoded_token_json = json.loads(base64.b64decode(id_token_parts[1] + '==')) + logger.debug("Successfully decoded token") + if "roles" in decoded_token_json: + roles = decoded_token_json["roles"] + logger.debug("Found roles: " + (", ".join(roles))) + return roles + return [] + +def verify_roles(org, roles, logger): + if org.azure_roles: + if not roles: + return False + for azure_role in org.azure_roles: + logger.debug("Verifying role: " + azure_role) + if azure_role in roles: + logger.debug("Role verified: " + azure_role) + return True + return False def create_azure_oauth_blueprint(app): - oauth = OAuth(app) - logger = logging.getLogger("azure_oauth") blueprint = Blueprint("azure_oauth", __name__) @@ -52,8 +75,6 @@ def create_azure_oauth_blueprint(app): def get_user_profile(access_token): headers = {"Authorization": "Bearer {}".format(access_token)} - logger.debug("Graph call =" + access_token + "=") - response = requests.get( "https://graph.microsoft.com/oidc/userinfo", headers=headers ) @@ -95,14 +116,15 @@ def authorized(): session["user"] = user access_token = resp["access_token"] + id_token = resp["id_token"] if access_token is None: logger.warning("Access token missing in call back request.") flash("Validation error. Please retry.") return redirect(url_for("redash.login")) - profile = get_user_profile(access_token) - if profile is None: + if id_token is None: + logger.warning("Id token missing in call back request.") flash("Validation error. Please retry.") return redirect(url_for("redash.login")) @@ -111,6 +133,29 @@ def authorized(): else: org = current_org + profile = get_user_profile(access_token) + + if org.azure_roles: + roles = get_roles_in_id_token(id_token, logger) + else: + roles = [] + + if not verify_roles(org, roles, logger): + logger.warning( + "User tried to login without authorized role assignment: %s. Valid roles are: %s. Provided roles are: %s", + profile["email"], + ", ".join(org.azure_roles), + ", ".join(roles), + ) + flash( + "Your Azure AD account ({}) isn't allowed as you are not assigned a required role: {}. Your assigned roles are: {}".format(profile["email"], ", ".join(org.azure_roles), ", ".join(roles)) + ) + return redirect(url_for("redash.login", org_slug=org.slug)) + + if profile is None: + flash("Validation error. Please retry.") + return redirect(url_for("redash.login")) + if not verify_profile(org, profile): logger.warning( "User tried to login with unauthorized domain name: %s (org: %s)", diff --git a/redash/handlers/settings.py b/redash/handlers/settings.py index 68f7e6528f..ba30e2f07f 100644 --- a/redash/handlers/settings.py +++ b/redash/handlers/settings.py @@ -22,6 +22,7 @@ def get_settings_with_defaults(defaults, org): settings["auth_google_apps_domains"] = org.google_apps_domains settings["auth_azure_apps_domains"] = org.azure_apps_domains + settings["auth_azure_roles"] = org.azure_roles return settings @@ -48,6 +49,9 @@ def post(self): elif k == "auth_azure_apps_domains": previous_values[k] = self.current_org.azure_apps_domains self.current_org.settings[Organization.SETTING_AZURE_APPS_DOMAINS] = v + elif k == "auth_azure_roles": + previous_values[k] = self.current_org.azure_roles + self.current_org.settings[Organization.SETTING_AZURE_ROLES] = v else: previous_values[k] = self.current_org.get_setting( k, raise_on_missing=False diff --git a/redash/models/organizations.py b/redash/models/organizations.py index 0e580050c0..76eb10d13b 100644 --- a/redash/models/organizations.py +++ b/redash/models/organizations.py @@ -13,6 +13,7 @@ class Organization(TimestampMixin, db.Model): SETTING_GOOGLE_APPS_DOMAINS = "google_apps_domains" SETTING_AZURE_APPS_DOMAINS = "azure_apps_domains" + SETTING_AZURE_ROLES = "azure_roles" SETTING_IS_PUBLIC = "is_public" id = primary_key("Organization") @@ -49,6 +50,10 @@ def google_apps_domains(self): def azure_apps_domains(self): return self.settings.get(self.SETTING_AZURE_APPS_DOMAINS, []) + @property + def azure_roles(self): + return self.settings.get(self.SETTING_AZURE_ROLES, []) + @property def is_public(self): return self.settings.get(self.SETTING_IS_PUBLIC, False) diff --git a/tests/handlers/test_settings.py b/tests/handlers/test_settings.py index e94c4257fc..9b6e02962e 100644 --- a/tests/handlers/test_settings.py +++ b/tests/handlers/test_settings.py @@ -52,6 +52,18 @@ def test_updates_azure_apps_domains(self): updated_org = Organization.get_by_slug(self.factory.org.slug) self.assertEqual(updated_org.azure_apps_domains, domains) + def test_updates_azure_roles(self): + admin = self.factory.create_admin() + roles = ["admin"] + rv = self.make_request( + "post", + "/api/settings/organization", + data={"auth_azure_roles": roles}, + user=admin, + ) + updated_org = Organization.get_by_slug(self.factory.org.slug) + self.assertEqual(updated_org.azure_apps_domains, domains) + def test_get_returns_google_appas_domains(self): admin = self.factory.create_admin() domains = ["example.com"] @@ -60,10 +72,18 @@ def test_get_returns_google_appas_domains(self): rv = self.make_request("get", "/api/settings/organization", user=admin) self.assertEqual(rv.json["settings"]["auth_google_apps_domains"], domains) - def test_get_returns_azure_appas_domains(self): + def test_get_returns_azure_app_domains(self): admin = self.factory.create_admin() domains = ["example.com"] admin.org.settings[Organization.SETTING_AZURE_APPS_DOMAINS] = domains rv = self.make_request("get", "/api/settings/organization", user=admin) self.assertEqual(rv.json["settings"]["auth_azure_apps_domains"], domains) + + def test_get_returns_azure_roles(self): + admin = self.factory.create_admin() + roles = ["admin"] + admin.org.settings[Organization.SETTING_AZURE_ROLES] = domains + + rv = self.make_request("get", "/api/settings/organization", user=admin) + self.assertEqual(rv.json["settings"]["auth_azure_roles"], roles) From 8bad0b9db139ec951e75fd9d51447993f3127ca7 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Mon, 10 Oct 2022 21:32:33 +0000 Subject: [PATCH 07/10] Restyled by prettier --- .../settings/components/AuthSettings/AzureLoginSettings.jsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/client/app/pages/settings/components/AuthSettings/AzureLoginSettings.jsx b/client/app/pages/settings/components/AuthSettings/AzureLoginSettings.jsx index d55032facd..5b1fab4020 100644 --- a/client/app/pages/settings/components/AuthSettings/AzureLoginSettings.jsx +++ b/client/app/pages/settings/components/AuthSettings/AzureLoginSettings.jsx @@ -37,11 +37,7 @@ export default function AzureLoginSettings(props) { )} - onChange({ auth_azure_roles: value })} /> {!isEmpty(values.auth_azure_roles) && ( Date: Mon, 10 Oct 2022 15:21:22 -0700 Subject: [PATCH 08/10] Fix tests --- tests/handlers/test_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/handlers/test_settings.py b/tests/handlers/test_settings.py index 9b6e02962e..889f64f85c 100644 --- a/tests/handlers/test_settings.py +++ b/tests/handlers/test_settings.py @@ -62,7 +62,7 @@ def test_updates_azure_roles(self): user=admin, ) updated_org = Organization.get_by_slug(self.factory.org.slug) - self.assertEqual(updated_org.azure_apps_domains, domains) + self.assertEqual(updated_org.azure_apps_roles, roles) def test_get_returns_google_appas_domains(self): admin = self.factory.create_admin() From dc819e79c8dad8b0aec1467b1111f549a8aed967 Mon Sep 17 00:00:00 2001 From: George Fisher Date: Mon, 10 Oct 2022 15:22:53 -0700 Subject: [PATCH 09/10] Fix tests --- tests/handlers/test_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/handlers/test_settings.py b/tests/handlers/test_settings.py index 889f64f85c..92c1838e32 100644 --- a/tests/handlers/test_settings.py +++ b/tests/handlers/test_settings.py @@ -83,7 +83,7 @@ def test_get_returns_azure_app_domains(self): def test_get_returns_azure_roles(self): admin = self.factory.create_admin() roles = ["admin"] - admin.org.settings[Organization.SETTING_AZURE_ROLES] = domains + admin.org.settings[Organization.SETTING_AZURE_ROLES] = roles rv = self.make_request("get", "/api/settings/organization", user=admin) self.assertEqual(rv.json["settings"]["auth_azure_roles"], roles) From d6a47db7f941ed0fa8ca277ad006446d963696b6 Mon Sep 17 00:00:00 2001 From: George Fisher Date: Mon, 10 Oct 2022 16:43:51 -0700 Subject: [PATCH 10/10] Fix Entrypoints object exception see: https://github.com/getredash/redash/pull/5831 --- requirements_bundles.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_bundles.txt b/requirements_bundles.txt index 3f57a20dd2..844b4c6eba 100644 --- a/requirements_bundles.txt +++ b/requirements_bundles.txt @@ -4,5 +4,5 @@ # It's automatically installed when running npm run bundle # These can be removed when upgrading to Python 3.x -importlib-metadata>=1.6 # remove when on 3.8 +importlib-metadata>=1.6,<5.0 # remove when on 3.8 importlib_resources==1.5 # remove when on 3.9