diff --git a/CHANGELOG.md b/CHANGELOG.md index c9d8b95078..67dea2a18d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [UNRELEASED] +### Bug fixes + +* Fixed `session.user` and `session.groups` raising `AttributeError` in module sessions (`SessionProxy`) and Express apps (`ExpressStubSession`). Both now correctly return the authenticated user's identity from the root session. As part of this fix, `user` and `groups` are now read-only properties on the `Session` ABC — app code can no longer accidentally overwrite credentials that are derived from immutable HTTP headers. (#2276) + ## [1.6.3] - 2026-06-01 ### New features diff --git a/shiny/express/_stub_session.py b/shiny/express/_stub_session.py index d753667598..da008bd50e 100644 --- a/shiny/express/_stub_session.py +++ b/shiny/express/_stub_session.py @@ -48,6 +48,14 @@ def __init__(self, ns: ResolvedId = Root): self.bookmark = BookmarkExpressStub(self) + @property + def user(self) -> str | None: + return None + + @property + def groups(self) -> list[str] | None: + return None + def is_stub_session(self) -> Literal[True]: return True diff --git a/shiny/session/_session.py b/shiny/session/_session.py index f02c3f0b9c..cbd23e6da7 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -196,8 +196,14 @@ class Session(ABC): # iterate over all modules and check the `.bookmark_exclude` list of each proxy # session. bookmark: Bookmark - user: str | None - groups: list[str] | None + + @property + @abstractmethod + def user(self) -> str | None: ... + + @property + @abstractmethod + def groups(self) -> list[str] | None: ... # TODO: not sure these should be directly exposed _outbound_message_queues: OutBoundMessageQueues @@ -740,8 +746,8 @@ def __init__( self.bookmark: Bookmark = BookmarkApp(self) - self.user: str | None = None - self.groups: list[str] | None = None + self._user: str | None = None + self._groups: list[str] | None = None credentials_json: str = "" if "shiny-server-credentials" in self.http_conn.headers: @@ -755,8 +761,8 @@ def __init__( if credentials_json: try: creds = json.loads(credentials_json) - self.user = creds["user"] - self.groups = creds["groups"] + self._user = creds["user"] + self._groups = creds["groups"] except Exception as e: print("Error parsing credentials header: " + str(e), file=sys.stderr) @@ -808,6 +814,14 @@ async def _run_session_ended_tasks(self) -> None: def is_stub_session(self) -> Literal[False]: return False + @property + def user(self) -> str | None: + return self._user + + @property + def groups(self) -> list[str] | None: + return self._groups + async def close(self, code: int = 1001) -> None: await self._conn.close(code, None) await self._run_session_ended_tasks() @@ -1518,6 +1532,14 @@ def __init__(self, root_session: Session, ns: ResolvedId) -> None: self.bookmark = BookmarkProxy(self) + @property + def user(self) -> str | None: + return self._root_session.user + + @property + def groups(self) -> list[str] | None: + return self._root_session.groups + def on_destroy( self, fn: Callable[[], None] | Callable[[], Awaitable[None]] ) -> None: diff --git a/tests/pytest/test_shinysession.py b/tests/pytest/test_shinysession.py index 18017ba440..78c201b3e8 100644 --- a/tests/pytest/test_shinysession.py +++ b/tests/pytest/test_shinysession.py @@ -1,13 +1,58 @@ """Tests for `shiny.Session`.""" +import asyncio +import json + import pytest -from shiny import ui +from shiny import App, Inputs, Outputs, Session, module, ui +from shiny._connection import MockConnection from shiny.reactive import effect, flush, isolate from shiny.session import Inputs from shiny.types import SilentException +def test_stub_session_user_groups(): + from shiny.express._stub_session import ExpressStubSession + + stub = ExpressStubSession() + assert stub.user is None + assert stub.groups is None + + +@pytest.mark.asyncio +async def test_module_session_user_groups(): + """SessionProxy (module session) should delegate user/groups to the root session.""" + captured: dict[str, object] = {} + + @module.server + def mod(input: Inputs, output: Outputs, session: Session): + captured["mod_user"] = session.user + captured["mod_groups"] = session.groups + + def server(input: Inputs, output: Outputs, session: Session): + mod("m") + captured["root_user"] = session.user + captured["root_groups"] = session.groups + + creds = json.dumps({"user": "alice", "groups": ["admin", "dev"]}).encode() + headers = [(b"shiny-server-credentials", creds)] + conn = MockConnection() + conn._http_conn.scope["headers"] = headers + sess = App(ui.TagList(), server)._create_session(conn) + + async def mock_client(): + conn.cause_receive('{"method":"init","data":{}}') + conn.cause_disconnect() + + await asyncio.gather(mock_client(), sess._run()) + + assert captured["root_user"] == "alice" + assert captured["root_groups"] == ["admin", "dev"] + assert captured["mod_user"] == "alice" + assert captured["mod_groups"] == ["admin", "dev"] + + def test_require_active_session_error_messages(): # require_active_session() should report the caller's name when an error occurs. with pytest.raises(RuntimeError, match=r"Progress\(\) must be called"):