Skip to content

STU-149 (frontend): reject known provider plus aliases for new magic-link accounts - #77

Closed
sgardoll wants to merge 13 commits into
mainfrom
sgardoll/stu-149-reject-known-provider-plus-aliases-frontend
Closed

sgardoll wants to merge 13 commits into
mainfrom
sgardoll/stu-149-reject-known-provider-plus-aliases-frontend

Conversation

@sgardoll

@sgardoll sgardoll commented Sep 22, 2026 •

Copy link
Copy Markdown
Owner

Frontend part of STU-149.

What this PR does

  • Adds src/authMagicLink.js with shared helpers for provider plus-alias detection, rule explanation, and message mapping.
  • Adds src/authMagicLink.test.js covering allowlisted plus rejection, untagged pass-through, mixed-case domains, custom/unrelated domains, whitespace trimming, and inline messages.
  • Adds src/authMagicLinkFixtures.json keeping the allowlist/codes in sync with the backend.
  • Updates app.js handleMagicLinkRequest() to trim input, show a non-blocking hint for known-provider plus aliases, and render the backend's PLUS_ALIAS_REJECTED response inline while still allowing existing-account recovery to proceed normally.

Verification

  • npm test — 282/282 passing.
  • npm run build — production bundle built successfully.
  • No edits to unrelated auth/session contracts.

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the only remaining concern is a non-blocking overbroad approximation label in deployment results.

Fix All in DevinFindings

  1. P2 Rejection Message Is Not Announced ▶
  2. P2 Approximation Label Overstates Coverage ▶
Fix with agent prompt
### Issue 1
app.js:undefined-4313
The asynchronous rejection updates a plain paragraph while keyboard focus remains on the submit button. Because the message element has no live-region semantics, screen-reader users may not be notified when `PLUS_ALIAS_REJECTED` arrives. The inline validation should use an appropriate status or alert live region. Greptile automatically discovered a related ticket stating that the inline rejection message must be accessible, which informed this comment.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

### Issue 2
src/customCodeVerification.js:217-220
These notices are generated from every unrepresentable project dependency without considering the deployed classes. As a result, even a deployment containing only `dart:`-only classes is shown as “Verified approximately,” although those checks are exact. The notice also does not identify affected classes, so users cannot tell which verification results actually used the reduced package graph.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR adds provider-aware plus-alias handling for magic-link requests and expands custom-code deployment verification, dependency-source parsing, CI, and bundle leak protection.

  • Uses shared frontend fixtures for provider domains, response codes, and rejection messaging.
  • Keeps plus-alias submission non-blocking so existing-account recovery can proceed through the backend.
  • Announces asynchronous sign-in messages through a status live region.
  • Improves pubspec source parsing and reports reduced dependency-graph verification.
  • Adds CI coverage and scans production bundles for supported provider endpoints and credential patterns.

Reviews (4) · Last reviewed commit: "fix: scope the verification skip to dire..."

STU-368. Which packages the Flutter SDK supplies was decided by a
hand-maintained name list kept in three places, and flutter_web_plugins was in
none of them. Every project declaring it - which is every web-enabled project
with a plugin - read it as a dependency that could not be reproduced, and the
deploy was refused for a package most of its classes never imported.

The project's own pubspec already says where each package comes from; the
parser read those lines and threw them away. It now returns them as
sourceKey/sourceValue, so the classification is exact and cannot fall behind
the SDK:

  sdk:                            -> the Flutter SDK, reproduced from the name
  a nested version:               -> a constraint written on its own line
  git: / path: / nested hosted:   -> not expressible in the manifest

SDK_DEPENDENCIES is gone. The runner's allowedSdkPackages is gone too - it
independently refused the same deploy with 'Unsupported SDK package'. That
allowlist was also the only validation of an sdkPackages entry, so the package
name pattern now does that job; without the swap a caller-supplied name would
reach the generated pubspec unchecked. The runner still writes 'sdk: flutter'
itself, so that value never comes from a caller.

Refusal is now scoped per class, to that class's own imports, so one git: entry
no longer leaves an entire deploy uncompiled.

The accept/warn split already existed and is unchanged: a real analyzer error
refuses with 422, and a class that could not be compiled is pushed with its
warning logged before the provisioning request.

Corrects DEPLOYMENT.md, which still described the scaffolding-closure
compilation that reverting a1238f0 removed, and still claimed that nothing
unverified is deployed.

Verified: 274/274 tests, deploy guard 9/9, proxies 30/30, build clean, dart
analyze on the runner reports no issues.
- Add shared authMagicLink helpers with provider allowlist.
- Show a non-blocking hint for plus-tagged known-provider addresses.
- Pass email through to /auth/send-magic-link and render server codes inline.
- Preserve recovery path for existing accounts on plus-tagged addresses.
- Add unit tests and fixture matching the backend contract.
Comment thread src/authMagicLink.test.js Fixed
@sgardoll

Copy link
Copy Markdown
Owner Author

@greptileai

Comment thread src/customCodeVerification.js Resolved
Comment thread src/pubspecSync.js Resolved
Comment thread src/authMagicLink.js Resolved
Comment thread scripts/scan_bundle_for_leaks.py Resolved
Comment thread app.js
if (btn) btn.textContent = 'Sent!'
const data = await sendMagicLink(email)
if (input && data?.code !== 'PLUS_ALIAS_REJECTED') input.value = ''
if (msg) msg.textContent = getMagicLinkResultMessage(data, email)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Rejection Message Is Not Announced

The asynchronous rejection updates a plain paragraph while keyboard focus remains on the submit button. Because the message element has no live-region semantics, screen-reader users may not be notified when PLUS_ALIAS_REJECTED arrives. The inline validation should use an appropriate status or alert live region. Greptile automatically discovered a related ticket stating that the inline rejection message must be accessible, which informed this comment.

Knowledge Base Used: FlutterFlow custom code experience

Source Used: Linear — Reject known provider plus aliases when creating new magic-link accounts

Prompt To Fix With AI
This is a comment left during a code review.
Path: app.js
Line: 4300

Comment:
**Rejection Message Is Not Announced**

The asynchronous rejection updates a plain paragraph while keyboard focus remains on the submit button. Because the message element has no live-region semantics, screen-reader users may not be notified when `PLUS_ALIAS_REJECTED` arrives. The inline validation should use an appropriate status or alert live region. Greptile automatically discovered a related ticket stating that the inline rejection message must be accessible, which informed this comment.

**Knowledge Base Used:** [FlutterFlow custom code experience](https://app.greptile.com/connect-i-o/-/custom-context/knowledge-base/sgardoll/customcodeconnectforflutterflow/-/docs/flutterflow-custom-code-experience.md)

**Source Used:** Linear — [Reject known provider plus aliases when creating new magic-link accounts](https://linear.app/stuart-gardoll/issue/STU-149/reject-known-provider-plus-aliases-when-creating-new-magic-link)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex Fix in Devin

…can reach a class transitively

(cherry picked from commit 9d338ad)
Greptile: the provider domains and response codes were hardcoded in
src/authMagicLink.js while the tests used literals instead of reading
authMagicLinkFixtures.json, so a backend fixture update could leave the
frontend stale while every frontend test still passed.

- src/authMagicLink.js now takes domains, codes and the rejection message
  template from authMagicLinkFixtures.json (JSON import attribute works
  in both the node --test run and the Vite bundle).
- app.js compares the server's validation code against the exported
  PLUS_ALIAS_REJECTED_CODE instead of a local literal, so the button and
  input handling cannot drift from the fixture either.
- src/authMagicLink.test.js feeds every allowlisted domain and response
  code from the same fixture, verifying the wiring against the contract
  instead of restating it.

dist/ rebuilt.
Greptile: the asynchronous PLUS_ALIAS_REJECTED result updated a plain
paragraph while keyboard focus stayed on the submit button, so screen
readers might never hear it. #signin-message now carries role="status",
an implicit polite live region appropriate for a non-critical async
validation result. The element exists at page load and app.js only
mutates its textContent, so the updates are announced without any JS
change; the codebase had no prior live-region pattern to match.

dist/ rebuilt.
CodeQL flags the substring assertions over the email-bearing rejection
message as an incomplete URL-substring-sanitization pattern: 'gmail.com'
can appear anywhere in the message, so the check proves nothing about the
rendered result. Both message assertions now compare the full,
deterministic string rendered from the fixture template.

The pinned strings are a deliberate snapshot of the reviewed backend
contract: a fixture change now breaks these tests loudly and forces a
conscious update, while the fixture-driven wiring tests keep verifying
that the guard honors whatever the fixture says.
greptile-apps[bot]
greptile-apps Bot previously approved these changes Sep 22, 2026
… scratch-graph approximation

(cherry picked from commit e300b76)
Comment on lines +217 to +220
const approximate = unrepresentable.map(
(name) =>
`Your project declares ${name} from a source that cannot be reproduced outside it, so the pre-deploy compile check runs against a scratch package graph that omits ${name}. Classes that do not import ${name} directly are still checked, but against that reduced graph, so their transitive package resolution may differ from your project's and the check is approximate for them.`,
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Approximation Label Overstates Coverage

These notices are generated from every unrepresentable project dependency without considering the deployed classes. As a result, even a deployment containing only dart:-only classes is shown as “Verified approximately,” although those checks are exact. The notice also does not identify affected classes, so users cannot tell which verification results actually used the reduced package graph.

Knowledge Base Used: Custom code verification

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/customCodeVerification.js
Line: 217-220

Comment:
**Approximation Label Overstates Coverage**

These notices are generated from every unrepresentable project dependency without considering the deployed classes. As a result, even a deployment containing only `dart:`-only classes is shown as “Verified approximately,” although those checks are exact. The notice also does not identify affected classes, so users cannot tell which verification results actually used the reduced package graph.

**Knowledge Base Used:** [Custom code verification](https://app.greptile.com/connect-i-o/-/custom-context/knowledge-base/sgardoll/customcodeconnectforflutterflow/-/docs/custom-code-verification.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex Fix in Devin

@sgardoll

Copy link
Copy Markdown
Owner Author

@greptileai — all findings from the previous review have been addressed (see new commits). Please re-review this PR.

@sgardoll

Copy link
Copy Markdown
Owner Author

Closing: this branch is already fully contained in milestone/stu-implement-updated-ui-ux, merged during the earlier integration run as 53d88b2 (STU-149). GitHub refuses to retarget this PR to the milestone branch for that exact reason — there are no new commits between the two. The work therefore ships with the milestone → main PR and this PR no longer corresponds to anything unmerged. Nothing is lost; the commits remain in history.

@sgardoll sgardoll closed this Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants