[codex] Add ProtoOS authentication settings E2E tests#416
[codex] Add ProtoOS authentication settings E2E tests#416edgars-avotins wants to merge 3 commits into
Conversation
github-actions
Bot
added
javascript
🔐 Codex Security Review
Review SummaryOverall Risk: MEDIUM Findings[MEDIUM] Password-changing E2E can leave shared admin credentials mutated after partial failure
NotesThe production change in Generated by Codex Security Review | |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9547e4e20f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| await authenticationPage.validateLoggedIn(); | ||
| }); | ||
|
|
||
| updatedPassword = newPassword; |
There was a problem hiding this comment.
When the PUT has already succeeded but any assertion before this assignment fails (for example the payload assertion, toast wait, or logged-in check), updatedPassword is still null, so afterEach skips restoreAdminPassword and leaves the miner using the random password. That breaks subsequent ProtoOS specs or reruns that authenticate with testConfig.admin.password; set the rollback password as soon as the change-password response succeeds, before the remaining UI assertions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds Playwright E2E coverage for ProtoOS “Authentication settings” to validate the admin password change flow, including request payload verification and the “current password required” validation case. This fits into the existing client/e2eTests/protoOS suite by extending the Authentication page object and introducing a small API helper for login/password restoration.
Changes:
- Added a new
authentication.spec.tscovering password update + re-authentication and required-current-password validation. - Expanded the
AuthenticationPagewith settings-form interactions (input + submit helpers). - Introduced an
authenticationHelperfor generating passwords, logging in via API, and restoring the admin password post-test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| client/e2eTests/protoOS/spec/authentication.spec.ts | New E2E spec for admin password update flow + validation coverage, including API request/response assertions and cleanup. |
| client/e2eTests/protoOS/pages/authentication.ts | Extends Authentication page object with settings-form locators and interactions. |
| client/e2eTests/protoOS/helpers/authenticationHelper.ts | New helper for generating a strong password, logging in via API, and restoring admin password after mutation. |
| expect(response.status()).toBe(200); | ||
|
|
||
| await authenticationPage.validateToastMessage("Password updated"); | ||
| await authenticationPage.validateLoggedIn(); | ||
| }); | ||
|
|
||
| updatedPassword = newPassword; | ||
|
|
| const request = await requestPromise; | ||
| const response = await responsePromise; | ||
|
|
||
| expect(request.postDataJSON()).toEqual({ | ||
| current_password: testConfig.admin.password, | ||
| new_password: newPassword, | ||
| }); |
| async validateUsernameFieldDisabledWithValue(expectedValue: string) { | ||
| const usernameField = this.page.locator('input[id="username"]:not([data-testid="username"])'); | ||
| await expect(usernameField).toBeDisabled(); | ||
| await expect(usernameField).toHaveValue(expectedValue); |
🤖 What changed
Why
Validation