Feat/fe testing infrastructure#144
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
7acd03d to
deb8d3c
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
deb8d3c to
94943cf
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
94943cf to
3b111f5
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
567eaf8 to
b434b4e
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
b434b4e to
b02637e
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
b02637e to
ece0a61
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
ece0a61 to
88d296e
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
88d296e to
f7b8536
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
f7b8536 to
127ce7a
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
127ce7a to
6248346
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
2 similar comments
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
e995647 to
1b6f66d
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
e0b8519 to
1f2e9b6
Compare
Signed-off-by: romanetar <roman_ag@hotmail.com>
Signed-off-by: romanetar <roman_ag@hotmail.com>
Signed-off-by: romanetar <roman_ag@hotmail.com>
Comment out login-mfa-flow.spec.ts and register.spec.ts so CI runs login.spec.ts alone to verify it now passes without account lockout interference. Also fix the MFA beforeEach mock: fulfill() must run before unroute(), otherwise Playwright auto-resolves the in-flight route on unroute and the later fulfill() throws "Route is already handled" - which was letting the real POST through with a wrong password and locking out test@test.com.
login.spec.ts verified green in isolation; re-enable the MFA flow suite (route-ordering fix already applied) and the registration suite now that the account-lockout cascade is gone. Signed-off-by: romanetar <roman_ag@hotmail.com>
PR #142 reverted the password login step from AJAX back to a native form POST + server redirect/session flow (commit 0eca371), removing handleAuthenticatePasswordFlow/Ok/Error, authenticateWithPassword, and the MFA_CHALLENGE_REQUIRED constant. The tests added by this branch were written against the old AJAX contract and needed to be realigned. - tests/js/login/login.mfa.test.js: remove the handleAuthenticatePasswordOk describe block - it tested a client-side AJAX handler that no longer exists in login.js. - tests/e2e/tests/auth/login-mfa-flow.spec.ts: - beforeEach no longer mocks the password POST as JSON; it performs a real native login against a real MFA-enforced account, matching how postLogin() actually issues a challenge (redirect + session state). - Each TS-* test now uses its own seeded MFA user (mfa-ts-NNN@test.com) instead of sharing one fixed account - a real challenge issuance counts against two_factor.rate_limit.max_otp_requests, so 8 tests sharing one account exhausted the limit before the suite finished. - Fixed VERIFY_URL/RESEND_URL/RECOVERY_URL/CANCEL_URL glob patterns to end with '**': postRawRequest() appends every param as a query string in addition to the body, so the exact-suffix glob never matched and silently left every route mock inert (requests were hitting the real backend instead). - TS-004/TS-007: resetToPasswordFlow() keeps the verified identity and returns to the password step (authFlow: FLOW.PASSWORD) - it does not clear user_name/user_verified. Both tests asserted the email step was shown instead, contradicting their own titles and the function's name. - TS-002: widened the post-verify assertion timeout - onVerify2FA() always assigns window.location.href on success, so even a same-URL mock response occasionally triggers a real navigation that raced the original 1s timeout. - .github/workflows/{pull_request,push}_frontend_tests.yml: seed the 8 mfa-ts-NNN@test.com accounts alongside the existing test@test.com / e2e@test.com fixtures. - .gitignore: add /test-results/ (Playwright's screenshot/video/trace output directory) - only /tests/e2e/report/ was previously ignored. Verified: 40/40 PHP (TwoFactorLoginFlowTest), 23/23 Jest, 13/13 Playwright e2e, stable across repeated runs via `docker compose --profile e2e run --rm playwright npx playwright test`.
Adds tests/e2e/tests/oauth2/auth-code-flow.spec.ts, exercising the full
authorization code grant end to end - including the memento (pending
OAuth2 request) surviving a real MFA detour, consent-bypass for a
returning user, and MFA-skip for a trusted device:
- unauthenticated /oauth2/auth redirects to login (memento serialized).
- full flow: real login -> real MFA challenge -> real OTP -> consent
screen for the correct client -> Accept -> authorization code ->
code exchanged at the token endpoint for a real access_token.
- returning user with prior consent: a second /oauth2/auth for the same
client+scope skips the consent screen entirely and redirects straight
to redirect_uri (InteractiveGrantType::handle()'s has_former_consent +
auto_approval branch).
- trusted device: checking "Trust this device" during MFA sets the
Secure device_trust_token cookie; logging out and logging back in
then skips the MFA challenge entirely.
Infrastructure needed to drive this for real (no mocks):
- app/Console/Commands/GetLatestOtp.php (idp:get-latest-otp {email}):
prints the newest not-yet-redeemed OTP for a user, since the mailer
queues via Redis and there is no catchable local mailbox to read the
code from. Registered in app/Console/Kernel.php.
- tests/e2e/utils/otp.ts: reads that OTP from the test runner - directly
via `php artisan` when reachable in-process (CI, host dev), or via
`docker exec idp-app php artisan ...` when running against the
dockerized stack (APP_URL points at nginx).
- docker-compose/playwright/Dockerfile + docker-compose.yml: the
playwright service now builds this image (adds the Docker CLI on top
of the stock Playwright image) and mounts /var/run/docker.sock so the
above `docker exec` path works from inside that container. Scoped to
the e2e profile only.
- The suite works around two config('app.url')-vs-actual-origin
mismatches (e.g. app.url=http://localhost but this suite runs against
http://nginx in the docker-compose e2e profile - cookies are
domain-scoped, so following the server's literal absolute redirect/
form-action URLs client-side would drop the session): verify2FA's
redirect_url, the consent form's action, and the password step's
postLogin() redirect are all replayed via page.request (shares the
page's cookies) instead of trusting the browser/client-side JS to
follow them unassisted.
- .github/workflows/{pull_request,push}_frontend_tests.yml: seed
mfa-oauth2-consent@test.com and mfa-oauth2-trust@test.com alongside
the existing mfa-oauth2@test.com fixture.
Known environment limitation (not a bug): the trusted-device assertion
requires a "potentially trustworthy origin" for the Secure cookie to
persist - true for http://localhost (host dev, and CI, which already
uses APP_URL=http://localhost:8001) but not for the docker-compose e2e
profile's http://nginx, where browsers silently drop the cookie.
Verified: 16/17 e2e via `docker compose --profile e2e run --rm
playwright npx playwright test` (the trusted-device test is the one
expected miss, per the above), 4/4 in tests/e2e/tests/oauth2/ via host
(`npx playwright test`), 40/40 PHP (TwoFactorLoginFlowTest), 23/23 Jest.
1b6f66d to
052220a
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-144/ This page is automatically updated on each push to this PR. |
2 participants
ref https://app.clickup.com/t/9014802374/86bak2ur6