madhushreeag commented on code in PR #44365: URL: https://github.com/apache/superset/pull/44365#discussion_r4199555580
########## tests/integration_tests/security/login_token_api_tests.py: ########## @@ -0,0 +1,443 @@ +# 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. +"""Route-level tests for the one-time login token endpoints. + +These complement ``tests/unit_tests/security/login_token_test.py``, which +exercises mint and consume directly. Driving the HTTP routes additionally covers +the decorator stack -- ``@transaction()`` in particular, which does not nest -- +and the session cookie that ``login_user`` writes, neither of which the unit +tests can see. +""" + +from contextlib import contextmanager +from typing import Any +from unittest.mock import patch +from uuid import UUID + +from flask_appbuilder.security.sqla.manager import user_updating + +from superset import db +from superset.daos.key_value import KeyValueDAO +from superset.extensions import csrf +from superset.key_value.models import KeyValueEntry +from superset.key_value.types import KeyValueResource +from superset.utils import json +from tests.conftest import with_config +from tests.integration_tests.base_tests import SupersetTestCase +from tests.integration_tests.conftest import with_feature_flags +from tests.integration_tests.constants import GAMMA_USERNAME + +ENDPOINT = "/api/v1/security/login-token/" +MINT_SECRET = "integration-test-secret" # noqa: S105 + + +def _resolver(request: Any, **kwargs: Any) -> dict[str, Any] | None: + """Stand-in for a deployment resolver: a shared header plus a username. + + A real one would validate an id token or call an internal service; all this + needs to do is distinguish an authorized caller from an unauthorized one. + """ + if request.headers.get("X-Test-Mint-Secret") != MINT_SECRET: + return None + username = (request.get_json(silent=True) or {}).get("username") + return {"username": username} if username else None + + +def _verbatim_resolver(request: Any, **kwargs: Any) -> dict[str, Any] | None: + """Return the request body as ``userinfo``, unmodified. + + Lets a test drive an arbitrary resolver result through the real route, which + is the only way to cover shapes a sensible resolver would not produce but a + misbehaving one will. + """ + if request.headers.get("X-Test-Mint-Secret") != MINT_SECRET: + return None + return request.get_json(silent=True) or None + + +@contextmanager +def csrf_enabled(app: Any) -> Any: + """Turn real CSRF protection on for the duration of a test. + + The integration config sets ``WTF_CSRF_ENABLED = False``, so + ``SupersetAppInitializer.configure_wtf`` never ran ``csrf.init_app`` and + never applied ``WTF_CSRF_EXEMPT_LIST``. Flipping the config alone does + nothing: there is no ``before_request`` hook to flip, and the defaults + ``validate_csrf`` reads (``WTF_CSRF_METHODS``, ``WTF_CSRF_FIELD_NAME`` and + the rest) are only set by ``init_app``. + + So the real ``init_app`` runs, rather than those defaults being restated + here -- restating them would mean reimplementing part of what this is meant + to verify. It registers a ``before_request`` hook and a context processor, + which Flask refuses once a request has been served, so the setup guard is + lifted for that call and everything it appended is removed afterwards. This + keeps the test independent of its position in the suite. + """ + got_first_request = app._got_first_request # noqa: SLF001 + before = list(app.before_request_funcs.get(None, [])) + processors = list(app.template_context_processors.get(None, [])) + extension = app.extensions.get("csrf") + exempt_views = set(csrf._exempt_views) # noqa: SLF001 + config = { + key: app.config[key] for key in list(app.config) if key.startswith("WTF_CSRF_") + } + + try: + app._got_first_request = False # noqa: SLF001 + csrf.init_app(app) + for view in app.config["WTF_CSRF_EXEMPT_LIST"]: + csrf.exempt(view) + app._got_first_request = got_first_request # noqa: SLF001 + app.config["WTF_CSRF_ENABLED"] = True + yield + finally: + app._got_first_request = got_first_request # noqa: SLF001 + app.before_request_funcs[None] = before + app.template_context_processors[None] = processors + csrf._exempt_views = exempt_views # noqa: SLF001 + for key in [k for k in list(app.config) if k.startswith("WTF_CSRF_")]: + if key in config: + app.config[key] = config[key] + else: + del app.config[key] + if extension is None: + app.extensions.pop("csrf", None) + else: + app.extensions["csrf"] = extension + + +class TestLoginTokenApi(SupersetTestCase): + def _mint(self, username: str = GAMMA_USERNAME) -> str: + """Mint a token through the route and return it.""" + response = self.client.post( + ENDPOINT, + data=json.dumps({"username": username}), + content_type="application/json", + headers={"X-Test-Mint-Secret": MINT_SECRET}, + ) + assert response.status_code == 200, response.data + token = json.loads(response.data)["access_token"] + assert token + return token + + @staticmethod + def _stored_tokens() -> int: + return ( + db.session.query(KeyValueEntry) + .filter(KeyValueEntry.resource == KeyValueResource.LOGIN_TOKEN.value) + .count() + ) + + @staticmethod + def _is_stored(token: str) -> bool: + """Whether this specific token still has a row. + + Scoped to one key rather than counting the table: the metadata database + is shared across the suite, so an absolute count couples these + assertions to whatever other tests happen to have left behind. + """ + return ( + KeyValueDAO.get_entry(KeyValueResource.LOGIN_TOKEN, UUID(token)) is not None + ) + + # ---------------------------------------------------------------- closed off + + def test_endpoints_are_404_without_the_feature_flag(self): + """Closed by default: neither verb exists until the flag is on.""" + assert self.client.post(ENDPOINT).status_code == 404 + assert self.client.get(f"{ENDPOINT}?token=x").status_code == 404 + + @with_feature_flags(LOGIN_TOKEN=True) + @with_config({"LOGIN_TOKEN_IDENTITY_RESOLVER": None}) + def test_endpoints_are_404_without_a_resolver(self): + """The flag alone is not enough -- a resolver must also be configured.""" + assert self.client.post(ENDPOINT).status_code == 404 + assert self.client.get(f"{ENDPOINT}?token=x").status_code == 404 + + # --------------------------------------------------------------------- mint + + @with_feature_flags(LOGIN_TOKEN=True) + @with_config({"LOGIN_TOKEN_IDENTITY_RESOLVER": _resolver}) + def test_mint_rejects_a_caller_the_resolver_declines(self): + """No shared header, no token -- and nothing written to the store.""" + before = self._stored_tokens() + response = self.client.post( + ENDPOINT, + data=json.dumps({"username": GAMMA_USERNAME}), + content_type="application/json", + ) + assert response.status_code == 401 + assert self._stored_tokens() == before + + # ------------------------------------------------------------------ consume + + @with_feature_flags(LOGIN_TOKEN=True) + @with_config({"LOGIN_TOKEN_IDENTITY_RESOLVER": _resolver}) + def test_consume_establishes_a_session_and_redirects(self): Review Comment: Added `test_consume_provisions_a_new_user_with_mapped_roles_only`. It mints and redeems for a brand new user with `AUTH_USER_REGISTRATION` on, `AUTH_ROLES_MAPPING = {"superset_alpha": ["Alpha"]}` and `role_keys: ["superset_alpha", "Admin"]`, then asserts the saved roles are exactly `["Alpha", "Gamma"]`, that email and first/last name were saved, and that the session belongs to the new user. I ran it against two broken versions of the code. Passing only the username to `auth_user_oauth` fails with `['Gamma'] == ['Alpha', 'Gamma']`, and treating `role_keys` as role names fails with `['Admin', 'Alpha', 'Gamma'] == ['Alpha', 'Gamma']`, so it catches both lost roles and escalation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
