test(connect): test_connect_login_ui can never fail, its only assertion is satisfied before credentials are submitted
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 8
- Forks
- 1
- Avg merge
- 18h 23m
- Merged PRs (30d)
- 63
Description
test_connect_login_ui reports PASS whether or not authentication succeeded, so no lane verifies that a Connect password login actually works.
src/vip_tests/connect/test_auth.py:26 navigates to the login path, the following steps fill and submit the form, and then src/vip_tests/connect/test_auth.py:56-58 is the only assertion that authentication happened:
def user_authenticated(page, connect_url):
# After login, should not be on the login page anymore.
assert "/__login__" not in page.url, "Still on the login page after submitting credentials"
Connect's password provider answers GET /__login__ with a 303 See Other whose Location is the dashboard login route (<dashboard-path>/#/login, so /connect/#/login by default), without attempting any authentication. page.goto(f"{connect_url}/__login__") has therefore already left /__login__ before a single credential is entered, and the assertion is true from that first navigation onward regardless of what gets submitted. A rejected submission does not navigate at all, since the dashboard handles the failure in place, so the URL never returns to /__login__ either.
You can see the redirect without running the suite:
curl -si "$CONNECT_SERVER/__login__" | head -3
The same GET-redirect behaviour applies to the LDAP and PAM providers, so this is not specific to one auth configuration.
Evidence
The published example report shows the contradiction inside a single run: https://posit-dev.github.io/vip/example-report/
src/vip_tests/connect/test_auth.py::test_connect_login_uipassed in 5.99s.src/vip_tests/connect/test_users.py::test_list_usersfailed on the same server withAssertionError: Test user 'testuser' (from 'testuser') not found in user list: ['__bootstrap_admin__'].
The only account on that Connect instance was __bootstrap_admin__. The credentials the login scenario submitted were for testuser, which did not exist. Authentication cannot have succeeded, and the scenario passed anyway.
Why it matters
VIP exists to produce evidence that an installation works, and its report is shown to customers. A PASS on an authentication check that did not authenticate is worse than having no check at all, because it appears in that report as verified.
Options
- Assert on something a successful session actually produces rather than on the URL, by waiting for an authenticated-only element in the dashboard after submission.
src/vip_tests/workbench/test_auth.py:114-117already does this shape of check for the sibling Workbench scenario, assertingHomepage.CURRENT_USERis visible and not empty. - Or assert that Connect issued its session cookie, which is closer to "the server authenticated us" but is heavier and couples the test to cookie naming.
Either way the scenario needs a negative case. Nothing currently proves that a corrected assertion can distinguish success from failure, which is exactly the property the present one lacks: submitting a deliberately wrong password should fail the test.
Related
src/vip/auth.py:858-859 uses the same "we left /__login__" heuristic to detect interactive login completion. That is not wrong for the SAML and OAuth2 providers it ships for, which behave differently on GET /__login__ (OAuth2 renders a provider-choice page in place; SAML redirects to the identity provider's own domain, so the Connect-URL check fails for the duration). It would be wrong if pointed at a password, LDAP or PAM deployment, which is not the documented use case.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with src/vip_tests/connect/test_auth.py, especially test_connect_login_ui and user_authenticated at lines 26 and 56-58; use the curl command in the issue to confirm the initial redirect. Compare the authenticated-element check in src/vip_tests/workbench/test_auth.py:114-117. Done means successful credentials are verified and a deliberately wrong password causes the Connect scenario to fail.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- authentication, testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100