From d70523636cde3cf9450105f83b07bb63f68a4b5c Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> Date: Tue, 14 Dec 2021 14:33:30 -0800 Subject: [PATCH] feat: Guest token (for embedded dashboard auth) (#17517) * generate an embed token * improve existing tests * add some auth setup, and rename token * fix the stuff for compatibility with external request loaders * docs, standard jwt claims, tweaks * black * lint * tests, and safer token decoding * linting * type annotation * prettier * add feature flag * quiet pylint * apparently typing is a problem again * Make guest role name configurable * fake being a non-anonymous user * just one log entry * customizable algo * lint * lint again * 403 works now! * get guest token from header instead of cookie * Revert "403 works now!" This reverts commit df2f49a6d4267b3cccccd66549d54e25bae8e0b6. * fix tests * Revert "Revert "403 works now!"" This reverts commit 883dff38f16537e41f0eb5d699845263c96be5cb. * rename method --- .../src/components/UiConfigContext/index.tsx | 31 +++--- superset/config.py | 8 ++ superset/security/api.py | 69 +++++++++++++- superset/security/guest_token.py | 73 ++++++++++++++ superset/security/manager.py | 95 ++++++++++++++++++- tests/integration_tests/security/api_tests.py | 48 +++++++++- tests/integration_tests/security_tests.py | 60 +++++++++++- 7 files changed, 357 insertions(+), 27 deletions(-) create mode 100644 superset/security/guest_token.py diff --git a/superset-frontend/src/components/UiConfigContext/index.tsx b/superset-frontend/src/components/UiConfigContext/index.tsx index b5c2f53f27dc1..4b77e42015ebc 100644 --- a/superset-frontend/src/components/UiConfigContext/index.tsx +++ b/superset-frontend/src/components/UiConfigContext/index.tsx @@ -39,20 +39,19 @@ export const UiConfigContext = createContext({ export const useUiConfig = () => useContext(UiConfigContext); -export const EmbeddedUiConfigProvider: React.FC = ({ - children, -}) => { - const config = getUrlParam(URL_PARAMS.uiConfig); - const [embeddedConfig] = useState({ - hideTitle: (config & 1) !== 0, - hideTab: (config & 2) !== 0, - hideNav: (config & 4) !== 0, - hideChartControls: (config & 8) !== 0, - }); +export const EmbeddedUiConfigProvider: React.FC = + ({ children }) => { + const config = getUrlParam(URL_PARAMS.uiConfig); + const [embeddedConfig] = useState({ + hideTitle: (config & 1) !== 0, + hideTab: (config & 2) !== 0, + hideNav: (config & 4) !== 0, + hideChartControls: (config & 8) !== 0, + }); - return ( - - {children} - - ); -}; + return ( + + {children} + + ); + }; diff --git a/superset/config.py b/superset/config.py index 337fef882a97a..f9b734340dbc5 100644 --- a/superset/config.py +++ b/superset/config.py @@ -389,6 +389,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # a custom security config could potentially give access to setting filters on # tables that users do not have access to. "ROW_LEVEL_SECURITY": True, + "EMBEDDED_SUPERSET": False, # Enables Alerts and reports new implementation "ALERT_REPORTS": False, # Enable experimental feature to search for other dashboards @@ -1257,6 +1258,13 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument ) GLOBAL_ASYNC_QUERIES_WEBSOCKET_URL = "ws://127.0.0.1:8080/" +# Embedded config options +GUEST_ROLE_NAME = "Public" +GUEST_TOKEN_JWT_SECRET = "test-guest-secret-change-me" +GUEST_TOKEN_JWT_ALGO = "HS256" +GUEST_TOKEN_HEADER_NAME = "X-GuestToken" +GUEST_TOKEN_JWT_EXP_SECONDS = 300 # 5 minutes + # A SQL dataset health check. Note if enabled it is strongly advised that the callable # be memoized to aid with performance, i.e., # diff --git a/superset/security/api.py b/superset/security/api.py index 5aa51f85f0bc4..c0a4a77b24225 100644 --- a/superset/security/api.py +++ b/superset/security/api.py @@ -16,17 +16,38 @@ # under the License. import logging -from flask import Response +from flask import request, Response from flask_appbuilder import expose from flask_appbuilder.api import BaseApi, safe from flask_appbuilder.security.decorators import permission_name, protect from flask_wtf.csrf import generate_csrf +from marshmallow import fields, Schema, ValidationError from superset.extensions import event_logger logger = logging.getLogger(__name__) +class UserSchema(Schema): + username = fields.String() + first_name = fields.String() + last_name = fields.String() + + +class ResourceSchema(Schema): + type = fields.String(required=True) + id = fields.String(required=True) + rls = fields.String() + + +class GuestTokenCreateSchema(Schema): + user = fields.Nested(UserSchema) + resource = fields.Nested(ResourceSchema, required=True) + + +guest_token_create_schema = GuestTokenCreateSchema() + + class SecurityRestApi(BaseApi): resource_name = "security" allow_browser_login = True @@ -60,3 +81,49 @@ def csrf_token(self) -> Response: $ref: '#/components/responses/500' """ return self.response(200, result=generate_csrf()) + + @expose("/guest_token/", methods=["POST"]) + @event_logger.log_this + @protect() + @safe + @permission_name("grant_guest_token") + def guest_token(self) -> Response: + """Response + Returns a guest token that can be used for auth in embedded Superset + --- + post: + description: >- + Fetches a guest token + requestBody: + description: Parameters for the guest token + required: true + content: + application/json: + schema: GuestTokenCreateSchema + responses: + 200: + description: Result contains the guest token + content: + application/json: + schema: + type: object + properties: + token: + type: string + 401: + $ref: '#/components/responses/401' + 500: + $ref: '#/components/responses/500' + """ + try: + body = guest_token_create_schema.load(request.json) + # validate stuff: + # make sure the resource id is valid + # make sure username doesn't reference an existing user + # check rls rules for validity? + token = self.appbuilder.sm.create_guest_access_token( + body["user"], [body["resource"]] + ) + return self.response(200, token=token) + except ValidationError as error: + return self.response_400(message=error.messages) diff --git a/superset/security/guest_token.py b/superset/security/guest_token.py new file mode 100644 index 0000000000000..cbef52f008e34 --- /dev/null +++ b/superset/security/guest_token.py @@ -0,0 +1,73 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from typing import List, Optional, TypedDict, Union + +from flask_appbuilder.security.sqla.models import Role +from flask_login import AnonymousUserMixin + + +class GuestTokenUser(TypedDict, total=False): + username: str + first_name: str + last_name: str + + +class GuestTokenResource(TypedDict): + type: str + id: Union[str, int] + rls: Optional[str] + + +class GuestToken(TypedDict): + iat: float + exp: float + user: GuestTokenUser + resources: List[GuestTokenResource] + + +class GuestUser(AnonymousUserMixin): + """ + Used as the "anonymous" user in case of guest authentication (embedded) + """ + + is_guest_user = True + + @property + def is_authenticated(self) -> bool: + """ + This is set to true because guest users should be considered authenticated, + at least in most places. The treatment of this flag is pretty inconsistent. + """ + return True + + @property + def is_anonymous(self) -> bool: + """ + This is set to false because lots of code assumes that + role = Public if user.is_anonymous == false. + But guest users need to have their own role independent of Public. + """ + return False + + def __init__(self, token: GuestToken, roles: List[Role]): + user = token["user"] + self.guest_token = token + self.username = user.get("username", "guest_user") + self.first_name = user.get("first_name", "Guest") + self.last_name = user.get("last_name", "User") + self.roles = roles + self.resources = token["resources"] diff --git a/superset/security/manager.py b/superset/security/manager.py index 9e45e88afcda6..1a0a4532f5cd2 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -18,6 +18,7 @@ """A set of constants and methods to manage permissions and security""" import logging import re +import time from collections import defaultdict from typing import ( Any, @@ -32,7 +33,8 @@ Union, ) -from flask import current_app, g +import jwt +from flask import current_app, Flask, g, Request from flask_appbuilder import Model from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_appbuilder.security.sqla.manager import SecurityManager @@ -51,7 +53,7 @@ ViewMenuModelView, ) from flask_appbuilder.widgets import ListWidget -from flask_login import AnonymousUserMixin +from flask_login import AnonymousUserMixin, LoginManager from sqlalchemy import and_, or_ from sqlalchemy.engine.base import Connection from sqlalchemy.orm import Session @@ -63,6 +65,12 @@ from superset.constants import RouteMethod from superset.errors import ErrorLevel, SupersetError, SupersetErrorType from superset.exceptions import SupersetSecurityException +from superset.security.guest_token import ( + GuestToken, + GuestTokenResource, + GuestTokenUser, + GuestUser, +) from superset.utils.core import DatasourceName, RowLevelSecurityFilterType if TYPE_CHECKING: @@ -172,6 +180,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods "can_approve", "can_update_role", "all_query_access", + "can_grant_guest_token", } READ_ONLY_PERMISSION = { @@ -221,6 +230,17 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods "all_query_access", ) + guest_user_cls = GuestUser + + def create_login_manager(self, app: Flask) -> LoginManager: + # pylint: disable=import-outside-toplevel + from superset.extensions import feature_flag_manager + + lm = super().create_login_manager(app) + if feature_flag_manager.is_feature_enabled("EMBEDDED_SUPERSET"): + lm.request_loader(self.get_guest_user_from_request) + return lm + def get_schema_perm( # pylint: disable=no-self-use self, database: Union["Database", str], schema: Optional[str] = None ) -> Optional[str]: @@ -1228,3 +1248,74 @@ def can_access_based_on_dashboard(datasource: "BaseDatasource") -> bool: exists = db.session.query(query.exists()).scalar() return exists + + @staticmethod + def _get_current_epoch_time() -> float: + """ This is used so the tests can mock time """ + return time.time() + + def create_guest_access_token( + self, user: GuestTokenUser, resources: List[GuestTokenResource] + ) -> bytes: + secret = current_app.config["GUEST_TOKEN_JWT_SECRET"] + algo = current_app.config["GUEST_TOKEN_JWT_ALGO"] + exp_seconds = current_app.config["GUEST_TOKEN_JWT_EXP_SECONDS"] + + # calculate expiration time + now = self._get_current_epoch_time() + exp = now + (exp_seconds * 1000) + claims = { + "user": user, + "resources": resources, + # standard jwt claims: + "iat": now, # issued at + "exp": exp, # expiration time + } + token = jwt.encode(claims, secret, algorithm=algo) + return token + + def get_guest_user_from_request(self, req: Request) -> Optional[GuestUser]: + """ + If there is a guest token in the request (used for embedded), + parses the token and returns the guest user. + This is meant to be used as a request loader for the LoginManager. + The LoginManager will only call this if an active session cannot be found. + + :return: A guest user object + """ + raw_token = req.headers.get(current_app.config["GUEST_TOKEN_HEADER_NAME"]) + if raw_token is None: + return None + + try: + token = self.parse_jwt_guest_token(raw_token) + except Exception: # pylint: disable=broad-except + # The login manager will handle sending 401s. + # We don't need to send a special error message. + logger.warning("Invalid guest token", exc_info=True) + return None + else: + return self.guest_user_cls( + token=token, + roles=[self.find_role(current_app.config["GUEST_ROLE_NAME"])], + ) + + @staticmethod + def parse_jwt_guest_token(raw_token: str) -> GuestToken: + """ + Parses and validates a guest token. + Raises an error if the jwt is invalid: + if it is not signed with our secret, + or if required claims are not present. + :param raw_token: the token gotten from the request + :return: the same token that was passed in, tested but unchanged + """ + secret = current_app.config["GUEST_TOKEN_JWT_SECRET"] + algo = current_app.config["GUEST_TOKEN_JWT_ALGO"] + + token = jwt.decode(raw_token, secret, algorithms=[algo]) + if token.get("user") is None: + raise ValueError("Guest token does not contain a user claim") + if token.get("resources") is None: + raise ValueError("Guest token does not contain a resources claim") + return cast(GuestToken, token) diff --git a/tests/integration_tests/security/api_tests.py b/tests/integration_tests/security/api_tests.py index 6a81efaa677ca..fcacd7ce668f2 100644 --- a/tests/integration_tests/security/api_tests.py +++ b/tests/integration_tests/security/api_tests.py @@ -15,22 +15,24 @@ # specific language governing permissions and limitations # under the License. # isort:skip_file -"""Unit tests for Superset""" +"""Tests for security api methods""" import json +import jwt + from tests.integration_tests.base_tests import SupersetTestCase from flask_wtf.csrf import generate_csrf -class TestSecurityApi(SupersetTestCase): +class TestSecurityCsrfApi(SupersetTestCase): resource_name = "security" def _assert_get_csrf_token(self): uri = f"api/v1/{self.resource_name}/csrf_token/" response = self.client.get(uri) - assert response.status_code == 200 + self.assert200(response) data = json.loads(response.data.decode("utf-8")) - assert data["result"] == generate_csrf() + self.assertEqual(generate_csrf(), data["result"]) def test_get_csrf_token(self): """ @@ -53,4 +55,40 @@ def test_get_csrf_unauthorized(self): self.logout() uri = f"api/v1/{self.resource_name}/csrf_token/" response = self.client.get(uri) - self.assertEqual(response.status_code, 401) + self.assert401(response) + + +class TestSecurityGuestTokenApi(SupersetTestCase): + uri = f"api/v1/security/guest_token/" + + def test_post_guest_token_unauthenticated(self): + """ + Security API: Cannot create a guest token without authentication + """ + self.logout() + response = self.client.post(self.uri) + self.assert401(response) + + def test_post_guest_token_unauthorized(self): + """ + Security API: Cannot create a guest token without authorization + """ + self.login(username="gamma") + response = self.client.post(self.uri) + self.assert403(response) + + def test_post_embed_token_authorized(self): + self.login(username="admin") + user = {"username": "bob", "first_name": "Bob", "last_name": "Also Bob"} + resource = {"type": "dashboard", "id": "blah", "rls": "1 = 1"} + params = {"user": user, "resource": resource} + + response = self.client.post( + self.uri, data=json.dumps(params), content_type="application/json" + ) + self.assert200(response) + token = json.loads(response.data)["token"] + decoded_token = jwt.decode(token, self.app.config["GUEST_TOKEN_JWT_SECRET"]) + + self.assertEqual(user, decoded_token["user"]) + self.assertEqual(resource, decoded_token["resources"][0]) diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 158b31662c937..2168e8c1f41f3 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -15,14 +15,17 @@ # specific language governing permissions and limitations # under the License. # isort:skip_file +import dataclasses import inspect import re +import time import unittest from collections import namedtuple from unittest import mock from unittest.mock import Mock, patch from typing import Any, Dict +import jwt import prison import pytest @@ -967,9 +970,7 @@ def test_can_access_table(self, mock_raise_for_access): mock_raise_for_access.side_effect = SupersetSecurityException( SupersetError( - "dummy", - SupersetErrorType.TABLE_SECURITY_ACCESS_ERROR, - ErrorLevel.ERROR, + "dummy", SupersetErrorType.TABLE_SECURITY_ACCESS_ERROR, ErrorLevel.ERROR ) ) @@ -1315,3 +1316,56 @@ def test_get_user_datasources_gamma_with_schema( Datasource("database1", "schema1", "table1"), Datasource("database1", "schema1", "table2"), ] + + +class FakeRequest: + headers: Any = {} + + +class TestGuestTokens(SupersetTestCase): + @patch("superset.security.SupersetSecurityManager._get_current_epoch_time") + def test_create_guest_access_token(self, get_time_mock): + now = time.time() + get_time_mock.return_value = now # so we know what it should = + user = {"any": "data"} + resources = [{"some": "resource"}] + + token = security_manager.create_guest_access_token(user, resources) + # unfortunately we cannot mock time in the jwt lib + decoded_token = jwt.decode(token, self.app.config["GUEST_TOKEN_JWT_SECRET"]) + + self.assertEqual(user, decoded_token["user"]) + self.assertEqual(resources, decoded_token["resources"]) + self.assertEqual(now, decoded_token["iat"]) + self.assertEqual( + now + (self.app.config["GUEST_TOKEN_JWT_EXP_SECONDS"] * 1000), + decoded_token["exp"], + ) + + def test_get_guest_user(self): + user = {"username": "test_guest"} + resources = [{"type": "dashboard", "id": 1}] + token = security_manager.create_guest_access_token(user, resources) + fake_request = FakeRequest() + fake_request.headers[current_app.config["GUEST_TOKEN_HEADER_NAME"]] = token + + guest_user = security_manager.get_guest_user_from_request(fake_request) + + self.assertIsNotNone(guest_user) + self.assertEqual("test_guest", guest_user.username) + + @patch("superset.security.SupersetSecurityManager._get_current_epoch_time") + def test_get_guest_user_expired_token(self, get_time_mock): + # make a just-expired token + get_time_mock.return_value = ( + time.time() - (self.app.config["GUEST_TOKEN_JWT_EXP_SECONDS"] * 1000) - 1 + ) + user = {"username": "test_guest"} + resources = [{"type": "dashboard", "id": 1}] + token = security_manager.create_guest_access_token(user, resources) + fake_request = FakeRequest() + fake_request.headers[current_app.config["GUEST_TOKEN_HEADER_NAME"]] = token + + guest_user = security_manager.get_guest_user_from_request(fake_request) + + self.assertIsNone(guest_user)