From 89eed5706041a2b2b587a77238b3bc80584911b6 Mon Sep 17 00:00:00 2001 From: zaidusyy Date: Fri, 30 Oct 2026 21:39:02 +0530 Subject: [PATCH 1/2] fix(auth): don't crash on public endpoints when the user row is missing In debug mode a request without a session cookie looks up the sentinel userid 0, which has no row. For a public endpoint the user is then None, but the middleware went on to read user.username (when a superuser is configured) and user.last_active_date, so the first visit to /complete_login ended in "AttributeError: 'NoneType' object has no attribute 'username'" and an error page. That is the step dev.md tells developers to start with, and it fails with debug_userid and debug_username set as well. Continue as an anonymous request instead, the same as the non-debug branch above already does. Endpoints that need a user are still refused with the "unknown cookie userid" error. Refs #608 --- montage/mw/__init__.py | 12 ++++++---- montage/tests/test_web_basic.py | 40 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/montage/mw/__init__.py b/montage/mw/__init__.py index dd8185cf..a82652e4 100644 --- a/montage/mw/__init__.py +++ b/montage/mw/__init__.py @@ -143,10 +143,14 @@ def endpoint(self, next, cookie, rdb_session, _route, config, user = rdb_session.query(User).filter(User.id == userid).first() - if user is None and not ep_is_public: - err = 'unknown cookie userid, try logging in again' - response_dict['errors'].append(err) - return {} + if user is None: + if not ep_is_public: + err = 'unknown cookie userid, try logging in again' + response_dict['errors'].append(err) + return {} + # A public endpoint without a known user, e.g. /complete_login in debug mode + # before the first login, when the sentinel userid 0 has no row. + return next(user=None, user_dao=None) superuser = config.get('superuser') superusers = config.get('superusers', [superuser] if superuser else []) diff --git a/montage/tests/test_web_basic.py b/montage/tests/test_web_basic.py index fec5b342..50782873 100644 --- a/montage/tests/test_web_basic.py +++ b/montage/tests/test_web_basic.py @@ -2049,3 +2049,43 @@ def test_csv_import_warnings_are_dicts(montage_app, api_client, mock_external_ap assert data['warnings'] and all(isinstance(w, dict) for w in data['warnings']) assert any('Gone.jpg' in w.get('import issues', '') for w in data['warnings']) + + +@pytest.fixture +def debug_app(tmpdir): + """App as run for local development: debug mode, and no user row for the sentinel id.""" + config = utils.load_env_config(env_name='devtest') + config['db_url'] = config['db_url'].replace('///', '///' + str(tmpdir) + '/') + # devtest supplies a fixture user to requests without a cookie, which would hide this + config['__env__'] = 'dev' + config['debug'] = True + config['debug_userid'] = 6024474 + config['debug_username'] = 'Slaporte' + config['superuser'] = 'Slaporte' + _create_schema(db_url=config['db_url']) + + index_path = STATIC_PATH + '/index.html' + if not os.path.exists(index_path): + mkdir_p(STATIC_PATH) + with open(index_path, 'w') as f: + f.write('just for tests') + + return create_app('dev', config=config) + + +def test_debug_complete_login_works_before_first_login(debug_app): + """Visiting /complete_login in debug mode without a session logs in, as dev.md says.""" + client = ClasticTestClient(debug_app) + resp = client.get('/complete_login') + assert resp.status_code == 302 + data = _get_cookie_data(client, debug_app.resources['config']['cookie_secret']) + assert data['userid'] == 6024474 + + +def test_debug_without_login_still_refuses_protected_endpoints(debug_app): + """The sentinel user is only accepted by public endpoints.""" + client = ClasticTestClient(debug_app) + resp = client.get('/v1/admin', headers={'Accept': 'application/json'}) + data = json.loads(resp.get_data(as_text=True)) + assert data['status'] == 'failure' + assert data['errors'] From eb65d5845fba93533d6b7e0189a1739f05ec21f6 Mon Sep 17 00:00:00 2001 From: zaidusyy Date: Sat, 31 Oct 2026 21:46:47 +0530 Subject: [PATCH 2/2] test(auth): don't start Sentry from the debug_app fixture The fixture runs the app as the dev environment, and outside devtest create_app starts Sentry with the project's DSN unless MONTAGE_PDB is set. An exception in these tests would then have been reported to the project's Sentry, and sentry_sdk.init stays on for the rest of the test run. Set MONTAGE_PDB, as the devtest environment already implies. --- montage/tests/test_web_basic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/montage/tests/test_web_basic.py b/montage/tests/test_web_basic.py index 50782873..e7f2d5a9 100644 --- a/montage/tests/test_web_basic.py +++ b/montage/tests/test_web_basic.py @@ -2052,8 +2052,9 @@ def test_csv_import_warnings_are_dicts(montage_app, api_client, mock_external_ap @pytest.fixture -def debug_app(tmpdir): +def debug_app(tmpdir, monkeypatch): """App as run for local development: debug mode, and no user row for the sentinel id.""" + monkeypatch.setenv('MONTAGE_PDB', '1') # no Sentry from tests config = utils.load_env_config(env_name='devtest') config['db_url'] = config['db_url'].replace('///', '///' + str(tmpdir) + '/') # devtest supplies a fixture user to requests without a cookie, which would hide this