test(ui): cover the open auth, members and API key e2e cases - #7272
Conversation
|
Claude finished @luizhf42's task in 5m 40s —— View job Code Review CompleteReviewed 14 files. Posted 1 inline issue. Standards: 0 violations, 1 judgement call. Spec: not run. Correctness: 0. StandardsNo documented rule is broken. The one judgement call is a Mysterious Name: SpecThe Spec axis did not run. The PR references shellhub-io/team#243, but that repo isn't readable from this workflow, so there was no spec text to check against. Additional notes
If you've addressed the feedback and want a new review, tag |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
2492843 to
19630cf
Compare
|
/review |
|
Claude finished @gustavosbarreto's task in 5m 39s —— View job Code Review CompleteReviewed 14 files. Posted 1 inline issue. Standards: 1 violation, 0 judgement calls. Spec: not run. Correctness: 0. Standards
SpecThe Spec axis did not run. The PR references shellhub-io/team#243, and that repo can't be read from this workflow, so there was no spec text to check against. Additional notes
If you've addressed the feedback and want a new review, tag |
19630cf to
2a13edd
Compare
aef7f7a to
d7c9704
Compare
|
/review |
d7c9704 to
dac4879
Compare
|
/review |
|
Claude finished @otavio's task in 4m 38s —— View job Code Review CompleteReviewed 14 files. Posted 1 inline issue. Standards: 1 violation, 0 judgement calls. Spec: not run. Correctness: 0. Standards
SpecThe Spec axis did not run. The PR references shellhub-io/team#243, and that repo can't be read from this workflow, so there was no spec text to check against. Additional notes
If you've addressed the feedback and want a new review, tag |
d73f023 to
a005d61
Compare
Enterprise and cloud accept devices and open the admin users and instance API key pages only under a license. Without one, accepting a device answers 402 "device license limit reached" and the admin routes redirect to /admin/license, so the e2e cases for paired devices and instance keys can't run. Up now requires SHELLHUB_LICENSE_FILE for both editions and fails before building anything when it is unset or the file is missing, the same way the cloud stack fails without the Stripe keys. The value comes from the shell first, then from .env.override, through the lookup the Stripe keys already used. A relative path resolves against the repo root, where .env.override lives, because the stack runs from tests/ and the e2e script from ui/, so the working directory means nothing to whoever set it. The path is made absolute because Compose would resolve a relative bind mount against the compose file's directory. CI writes the TEST_LICENSE secret to a file with printf, which adds no trailing newline, and fails when the secret is empty: an empty file would pass the existence check and only fail later, inside the server. Fork pull requests never reach this step, because the path filter only runs the licensed editions for pull requests from this repository.
A detail panel rendered its label as a dt and its value as a dd with nothing linking the two, so assistive tech read the value without its label and a test could only reach it through the DOM structure. The dd now points at its dt with aria-labelledby. Playwright's getByRole never names a definition, even with aria-labelledby set, so tests reach the value with getByLabel, which follows the same attribute.
Adds the open Members & Invitations cases from team#243: signing up through an invitation consumes
it, a non-admin's invitee waits for an instance admin's approval on enterprise, and a member's
paired devices follow the member through pairing, demotion, removal, leaving and account deletion.
The invitation status is read from user_invitations because no endpoint returns it once the
invitee has an account. The approval case runs only on enterprise, the one edition that holds a
non-admin's invitee for approval. It also carries the sign-up checks for enterprise, which is why
the plain sign-up case skips that edition with the same reason. The instance admin comes from the
server CLI's --admin flag; the first user an instance creates is an admin too, which is why the
existing sign-up test, invited by the seeded admin, never hit approval.
Devices pair over the API, so no agent runs. Each pairing request sends a fresh RSA key, because
the server hands back the code it already issued for a key it has seen, and a random MAC whose
first byte is 0x02, a locally administered unicast address that can't match a real device. The
"Keep <name> as a team device" checkboxes are visually hidden inputs under a styled span that
intercepts clicks, so the tests tick them with the Space key, as the role radios already were.
"Paired by" is read with getByLabel, not getByRole("definition", { name }). Playwright follows
ARIA 1.3, where the definition role a <dd> carries prohibits a name, so the role query never
matches in a browser even though jsdom's Testing Library computes one.
createTeam, createTeamWithMember, signInAndOpen and signUpFromInvite move to helpers.ts because
the auth and API key specs need them too. The 15s waits on the sign-up headings are the ones the
existing sign-up test already used.
Adds the open Authentication cases from team#243. MFA is turned on through the database, since no API or CLI enables it without a live TOTP code from the setup flow. The secret is a fixed, public base32 test value: the test stops at the code prompt and never computes a code. It checks the 401 and the MFA token header on the login response, which is what sends the console to /mfa-login. The session case removes the member through the API, then waits for the 401 from /api/devices before checking the redirect to /login, so the logout is tied to the navigation. The member signs in without signInAndOpen: its extra page load fires requests that race the removal and log the member out before the Devices click.
Adds the open API Keys cases from team#243 for namespace and instance keys. Each test reads the key from the dialog that generates it, proves the key authenticates, then checks that neither the page nor the list endpoint returns it. The instance key runs on enterprise and cloud, where the admin panel lives.
e8a9145 to
7f5efa7
Compare
… refused The pairing test now reads the "Accepting as" section before accepting, so it fails if the page names someone other than the signed-in member. The issue asks for the owner to be shown on the accept page, and the test only checked the success message and device details. The demote, remove, leave and account-deletion tests now authenticate as each paired device's agent after the member is gone, with the same hostname, identity, public key and tenant the pairing used. A device kept for the team still authenticates (200). A removed one gets 401: the server refuses a removed device that was enrolled by pairing until it pairs again (authDevice in server/api/services/auth.go), rather than reviving it as pending. This is the agent's side of these cases, which team#243 marks as UI plus agent and which the tests only covered from the API's device list.
7f5efa7 to
a359114
Compare
|
/review |
|
Claude finished @otavio's task in 3m 14s —— View job Code Review CompleteReviewed 14 files on two axes: Standards (repo rules, Go and React conventions, tests, code smells) and Correctness (security and runtime defects). No new issues found, so the code can stay as it is. The Spec axis didn't run because the PR references shellhub-io/team#243, and this workflow can't read that repo. Earlier rounds raised three issues: the
If you push additional changes and want a new review, tag |
3 participants
Summary
Adds the 11 open Playwright cases from shellhub-io/team#243 for Authentication, Members & Invitations and API Keys. Enterprise and cloud accept devices and open the admin panel only under a license, so the e2e stack now mounts one on those editions.
The license covers seven tests: the five paired-device cases (the server refuses to accept a device without one) and the approval and instance key cases (every
/admin/*page redirects to/admin/licensewithout one). CI writes theTEST_LICENSEsecret to a file for the enterprise and cloud jobs.InfoItemnow links each value to its label witharia-labelledby, so screen readers announce the label and the "Paired by" check can find it withgetByLabel.Evidence
/admin/usersand/admin/instance-api-keysredirected to/admin/license.After: fresh stacks on each edition:
stack upwith a missing license path fails before building anything.Merge Danger
Door: two-way
Blast Radius: CI
Enterprise and cloud e2e jobs fail at the license step until the
TEST_LICENSEsecret is set on the repo. Fork pull requests skip those jobs, so they're unaffected. The one production change is thearia-labelledbyonInfoItem, which only adds an accessible name.