The previous commit (2e14bd95d) made the MFA test package discoverable
and resilient but left one scenario --
'Validation view of a MFA method should return a HTML tags' -- as a
documented unittest.SkipTest. The skip reason was that
validate.html -> security/render_page.html -> base.html references
'current_app' and 'csrf_token()' in Jinja, and the dummy Flask app
built by test_create_dummy_app() does not expose either: pgAdmin's
real create_app() injects current_app via an @app.context_processor
(web/pgadmin/__init__.py:922) and Flask-WTF registers csrf_token() in
the Jinja env. The bare Flask(name, ...) used by the dummy app has
neither, so the GET-path render fails with
UndefinedError: 'current_app' is undefined.
Add the same two globals to the dummy app:
* A @app.context_processor returning {'current_app': current_app} so
the Flask proxy is bound at render time, mirroring what
create_app() does in production.
* A jinja_env.globals['csrf_token'] = lambda: 'dummy-csrf-token' that
matches what Flask-WTF would otherwise install. Tests asserting on
the rendered HTML can rely on the marker being present and stable.
With both globals available the template renders, /mfa/validate
returns a 200 HTML response, and the scenario goes from SkipTest to
a real assertion. The fix is intentionally scoped to the dummy harness
in tests/utils.py -- no production code change.
Result on this worktree: pgadmin.authenticate.mfa.tests now runs as
13 passed / 0 failed / 0 skipped (was 12/0/1). Full suite goes from
1818/0/444 to 1819/0/443.
The MFA test directory was missing __init__.py, so
find_modules('pgadmin', False, True) in regression/runtests.py could
not walk into it. The test classes therefore never reached the
TestsGeneratorRegistry registry and the entire pgadmin.authenticate.mfa
test suite was silently absent from the default test run. The
regression for #10028 added in this branch was caught by that gap.
Adding the missing __init__.py exposes a second pre-existing problem:
TestMFATests.setUp called BaseTestGenerator.setUp, which posts to
/browser/server/connect/... and asserts a 200 response. The MFA
scenarios all run against a dummy Flask app (or pure mocks), so that
endpoint is not registered and the connect_server assertion fires
before any check_*() function runs. Skip the BaseTestGenerator setUp
since these scenarios deliberately do not need a real PostgreSQL
server.
Three further fixes that surface once the suite actually runs:
* mfa_enabled() and init_app() both short-circuit when SERVER_MODE
is False. test_config.json defaults to DESKTOP mode, so every
scenario in the suite was taking the disabled path. Force
SERVER_MODE=True for the duration of the TestMFATests class and
restore the previous value in tearDownClass.
* check_validation_view_content patched flask.current_app to capture
logger.exception() calls. Those only fire on the POST path of
/mfa/validate; the test exercises only the GET path, so the patch
was dead code -- and additionally turned flask.current_app into a
MagicMock, which broke Jinja's lookup of current_app in
validate.html. Drop the patch and the now-unused ValidationException
import.
* check_validation_view_content still cannot render validate.html
against the bare dummy Flask app because the template references
current_app.config and extends Flask-Security's
security/render_page.html, neither of which the dummy app provides.
Skip the scenario with a clear unittest.SkipTest reason rather than
paper over with a brittle patch; rebuilding the dummy harness to
expose those globals is its own follow-up.
Result on this worktree: pgadmin.authenticate.mfa.tests now runs as
12 passed / 0 failed / 1 skipped (the dummy-app/template gap),
up from a silent 0/0/0. Full suite goes from 1806/0/443 to 1818/0/444.
The MFA flow honoured the user-supplied "next" query/form parameter
without checking that it pointed back inside pgAdmin, so an attacker
who got a logged-in user to click /mfa/validate?next=<external> could
land them on an attacker-controlled host straight out of the auth
flow -- a trusted-domain redirect ideal for credential-phishing
follow-on.
Add a single _is_safe_redirect_url helper that allows only same-origin
http(s) targets (relative paths included) and rejects:
* external hosts in absolute and protocol-relative ("//host") form;
* non-http schemes a browser will still follow (javascript:, data:,
mailto:);
* userinfo tricks ("http://localhost@attacker.example/");
* backslash variants ("/\\host", "\\\\host") that some browsers
normalize to forward slashes, enabling protocol-relative bypasses;
* empty / missing targets.
Gate every redirect that consumes the user-supplied next value through
the helper -- the GET and POST branches of /mfa/validate and the POST
branch of /mfa/register -- falling back to the internal browser index
when the target is anything other than safe. The registration POST
keeps its existing 'internal' sentinel ("rendered from the in-app
dialog") since that string is matched exactly and is not a URL.
Ship the validator with a dedicated table-driven unit test covering
each accept/reject category and an integration test that POCs the
attacker payload from the report and asserts the response redirects
to the internal index instead of attacker.example.
Reported by: Mai Phạm Hiền <mai.phamhien171@gmail.com>
Reviewed by: Kundan Sable <kundan.sable@enterprisedb.com>
- Remove the SCSS dependency completely and use MUI for theming.
- Update - date-fns, @date-io, notistack. Remove - popper.js, sass-loader.
- Cleanup webpack config.
- Port PSQL tool to use MUI themes instead of SCSS theme.
- Theme change will reflect realtime without refreshing pgAdmin.
1. Delete unreachable code or refactor the code to make it reachable.
2. Unexpected var, use let or const instead.
3. Remove useless assignment to variable.
4. Define a constant instead of duplicating the literal
5. Remove commented out code
Ashesh Vashi
Dave Page
Akshay Joshi
Dave Page
Pravesh Sharma
Ahmad
Khushboo Vashi
Yogesh Mahajan
Anil Sahoo
Aditya Toshniwal
Aditya Toshniwal
Mark Mayo