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_ui passed in 5.99s.
src/vip_tests/connect/test_users.py::test_list_users failed on the same server with AssertionError: 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-117 already does this shape of check for the sibling Workbench scenario, asserting Homepage.CURRENT_USER is 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.
test_connect_login_uireports PASS whether or not authentication succeeded, so no lane verifies that a Connect password login actually works.src/vip_tests/connect/test_auth.py:26navigates to the login path, the following steps fill and submit the form, and thensrc/vip_tests/connect/test_auth.py:56-58is the only assertion that authentication happened:Connect's password provider answers
GET /__login__with a303 See OtherwhoseLocationis the dashboard login route (<dashboard-path>/#/login, so/connect/#/loginby 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:
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 fortestuser, 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
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.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-859uses 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 onGET /__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.