fix(frontend): isolate provider account OAuth callbacks - #270
Conversation
astaxie
left a comment
There was a problem hiding this comment.
Re-reviewed the complete current diff at bbf2e7e. The callback isolation logic is directionally correct: unmarked DingTalk/OIDC callbacks no longer treat generic code or error as Provider-account credentials, while marked and manual callbacks retain the generic names.
[P1] Add regression tests for this authentication-sensitive behavior (frontend/features/admin/core/session.tsx:78-87). The PR changes parsing of OAuth authorization codes and errors but adds no test coverage; the Verification section only lists existing commands. Please add tests proving unmarked query/fragment callbacks with code/error are ignored, marked callbacks still parse them, and manual callback parsing remains compatible.
[P1] Required CI is missing. GitHub currently reports UNSTABLE with an empty status check rollup, so this head cannot be approved or considered merge-ready. Publish the required Backend/Frontend/Deployment/Repository gates and aggregate CI, then request re-review on the exact head.
astaxie
left a comment
There was a problem hiding this comment.
Follow-up re-review of exact head bbf2e7e23a27d91deedce3541cc88282fca42b80: all seven GitHub checks are now SUCCESS and the merge state is MERGEABLE/CLEAN. The implementation still correctly isolates unmarked generic OAuth callbacks.
The prior test-coverage blocker remains: this authentication-sensitive parser change still has no regression tests for unmarked code/error, marked callbacks, and manual callback compatibility. Local frontend lint, typecheck, 64 domain tests, 5 component tests, and production build pass, but the changed behavior is not exercised by a new test. This remains comment-only; please add the focused parser tests and request another review.
Summary
Fix an issue where signing in by scanning the DingTalk QR code unexpectedly opens the Create Provider dialog after the administrator reaches the console.
The DingTalk OAuth callback uses generic parameters such as
code,state, anderror. These parameters could be incorrectly interpreted as a Provider account OAuth callback, causing the Provider creation wizard to open automatically.Related Issue
N/A.
Changes
Type of Change
Verification
npm run lintnpm run typechecknpm testgit diff --checkCompatibility, Security, and Operations
No API, persistence, configuration, or deployment changes. Marked Provider account callbacks and manual callback parsing remain supported.
Checklist
.envfiles, databases, backups, or runtime logs are included.start.sh, and deployment documentation where applicable.data/model-catalog.yamlremains tracked and catalog changes were reviewed where applicable.git diff --checkpasses.