RI-8328 Support Redis Cloud OAuth login with TOTP MFA in desktop app - #6288
RI-8328 Support Redis Cloud OAuth login with TOTP MFA in desktop app#6288VaskoAtanasovRedis wants to merge 20 commits into
Conversation
Redis Cloud OAuth sign-in is completed by a POST /login that can be challenged for MFA. Detect the challenge error codes (user-mfa-required, mfa-invalid-code, mfa-quota-exceeded) in wrapCloudApiError and map them to typed exceptions instead of flattening every 401 into a generic unauthorized error. - callWithAuthRetry no longer retries the MFA exceptions, so the challenge stops burning the server-side attempt quota (the 429 escalation). - ensureAccessToken tolerates a token inside the proactive-renewal buffer when it is still unexpired and there is no refresh token, so the human delay entering the code does not fail the verify. - The verify path re-sends /login with mfa_code + mfa_type, reusing the JSESSIONID from the challenge response, exposed as POST /cloud/me/login/mfa. References: RI-8328
When the post-OAuth user fetch is challenged for MFA, show a dialog to collect the authenticator code and complete sign-in. - Keep the session on the challenge 401 (the global cloudAuthInterceptor used to log the user out on any cloud 401, wiping the tokens before the code could be entered). - Segmented six-digit input: auto-advance, backspace/arrow nav, paste-fill, and auto-submit once the last digit lands. - On a rejected code, show a clear localized message, clear the boxes and refocus the first one; clear the error on the next edit. - On mfa-quota-exceeded, abort with a clear message instead of retrying. - i18n strings (en/bg) for the dialog and the MFA error codes. References: RI-8328
![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: 1ca127938d
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Code Coverage - Backend unit tests
Test suite run success3748 tests passing in 323 suites. Report generated by 🧪jest coverage report action from 442d0d9 |
Code Coverage - Integration Tests
|
- OtpInput: use a fixed-length slot model so clearing a middle box no longer shifts the later digits left. - Reset the SSO flow when the MFA dialog is cancelled or the attempt quota is exceeded, so ConfigOAuth's in-progress guard is released and a later failed sign-in is not silently swallowed. References: RI-8328
Code Coverage - Frontend unit tests
Test suite run success7912 tests passing in 872 suites. Report generated by 🧪jest coverage report action from 442d0d9 |
Covers DTO validation and the happy path, asserting the endpoint re-sends /login with mfa_code + mfa_type. References: RI-8328
1ca1279 to
5e0ec46
Compare
ensureLogin skipped the /login block when apiSessionId was already set, so a stale value would short-circuit verifyMfaCode into a false success without sending the TOTP code. Always re-login when an mfa code is provided. References: RI-8328
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 278489697c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Reject non-6-digit MFA codes in the DTO so a malformed value is not forwarded to Cloud and does not consume a server-side MFA attempt. - Ignore cancel while a verification is in flight (and disable the cancel button), so a pending request cannot resume the flow after the user cancels. - Replace the local getCloudApiErrorCode helper with a shared getApiErrorCustomCode util next to getApiErrorCode (which returns the HTTP status), to avoid two similarly-named getters diverging. References: RI-8328
The /csrf call is skipped when the shared session already has a csrf token, so nock.isDone() was order-dependent. The 200 status already requires the body-matched /login (with mfa_code) to be hit, which is the meaningful check. References: RI-8328
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec71582365
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The challenge forwards the available factors. When totpFactorAvailable is false (e.g. an SMS-only account), opening the TOTP-only dialog would prompt for a code that can never complete and would burn MFA attempts. Detect that and show a clear 'finish in the Cloud console' message instead of the dialog. References: RI-8328
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9550d12. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9550d12e4a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…session - isTokenExpired: a JWT with no exp claim yields NaN; treat it as expired instead of letting NaN <= now report it as still valid. - submitMfaCodeAction: only a rejected code keeps the dialog open; quota exceeded, an expired challenge session (non-MFA 401), or any other error now aborts the flow (close dialog, clear source + ssoFlow) rather than leaving a retry dialog submitting against a dead challenge. References: RI-8328
pawelangelow
left a comment
pawelangelow
left a comment
There was a problem hiding this comment.
One thing I'd still like before merge:
There's no HTTP-level test for the challenge path itself. Everything hinges on the cloud /login user-mfa-required envelope surfacing to the client as errorCode: 11025, and that mapping is still only covered by unit tests with a hand-mocked envelope. A small nock test on GET /cloud/me (cloud replies with the mfa-required envelope, assert the client gets errorCode 11025, no logout, no retry) would lock down the exact contract the whole UI flow depends on. This is the one case that would fail silently if the real error shape differs from the mock.
Have cloud reply to /login with the real user-mfa-required envelope and assert GET /cloud/me surfaces errorCode 11025 with the parsed factors and without retrying /login, covering the exact shape the UI flow depends on.
|
@pawelangelow good call - added exactly that in d2bee92.
Ready for another look whenever you have a minute. |
The strict tsc baseline check counts implicit-any binding elements per file;
the added checkFn tripped a second TS7031. Annotate its param with the
suite's existing `({ body }: any)` idiom so the file stays at its recorded
count instead of ratcheting the baseline up.
GET /cloud/me short-circuits to the cached profile when the shared integration session already holds an apiSessionId + user, so /login was never called and the challenge assertion saw a cached 200. Drive a failed account switch first: its failed re-login makes the auth-retry invalidate the api session and cached user, so the assertion performs a real login and hits the mfa-required challenge deterministically.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11d0c0c6c5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
After the OAuth callback the backend has already stored the access token (and may persist the refresh token), so clearing only renderer state on a canceled or aborted MFA challenge leaves a credentialed session that a later fetch or app restart could silently resume. Route the terminal MFA abort paths (dialog cancel, dead challenge session, unsupported factor) through logoutUserAction so the backend session is deleted, not just local state.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56a99f528b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Container hardcoded gap and justify-content in the styled(Row) CSS, bypassing the layout-prop API. Pass gap="l" / justify="center" as props so the container follows the shared Row layout contract.
vectorSetElementWithAttributesFactory built names from faker.word.words, which can repeat across a buildList. updateElementAttributes matches by name, so a collision updated the wrong element and the attributes assertion failed nondeterministically. Suffix the name with the factory sequence so every built element is unique.
Add vertical padding to the OTP container so a focused box's ring is not clipped by the dialog body, and center the inline error under the boxes instead of left-aligning it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dd532c378
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ntry An already locked-out account returns CloudApiMfaQuotaExceeded (429) on the initial profile fetch, which the auth interceptor does not log out on. The generic failure branch left the backend session and SSO flow alive, so ConfigOAuth stayed in-progress and could swallow a later failed callback. Treat it as a terminal abort with a backend logout, matching the code-submission quota path.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 442d0d9504
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An OS or password-manager autofill delivers the whole code into the focused box via a change event; the handler kept only the last digit, so the login never completed. Route multi-digit change values through the same fill path as paste.
…-totp-mfa-login # Conflicts: # redisinsight/ui/src/i18n/locales/bg.json # redisinsight/ui/src/i18n/locales/en.json
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07424cc620
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // the oauth callback already credentialed the backend session; revoke it so | ||
| // the abandoned sign-in cannot be resumed on a later fetch or app restart. | ||
| // logout also clears the SSO flow, releasing ConfigOAuth's in-progress guard | ||
| dispatch(logoutUserAction()) |
There was a problem hiding this comment.
When a user cancels MFA and immediately starts another sign-in, the dialog and SSO guard are cleared before this asynchronous logout completes. The backend logout waits for the remote refresh-token revocation and only then deletes the local cloud session, so on a slow revocation request the new OAuth callback can store fresh credentials before the older logout deletes them, causing the new sign-in to fail unexpectedly. Keep the flow blocked until logoutUserAction() settles, or otherwise prevent the old logout from deleting a newer session.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9cc4d85cd
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const isMfaChallenge = | ||
| (response?.data as { errorCode?: number })?.errorCode === | ||
| CustomErrorCodes.CloudApiMfaRequired | ||
| if ( | ||
| response?.status === 401 && | ||
| !isMfaChallenge && |
There was a problem hiding this comment.
When the desktop app restarts with a persisted refresh token, the Cloud API JSESSIONID is gone with the in-memory session, so the startup fetchProfile() call can reach /login and receive this MFA challenge. The interceptor now preserves that session, but fetchProfile only records a generic failure and never opens the MFA dialog—the challenge handling exists only in fetchUserInfo—so an MFA-enabled account cannot be restored after restart and receives no verification prompt. Route startup challenges through the same MFA flow or centralize this handling.
Useful? React with 👍 / 👎.
GitHub Actions
Inactive
2 participants
What
Enables Redis Cloud OAuth sign-in (Google/GitHub/SSO) to complete in the
desktop app when the account has TOTP MFA enabled.
The browser OAuth handshake succeeded, but the follow-up Cloud API
POST /loginwas challenged for MFA - which RedisInsight neither surfaced nor completed. It
showed an opaque error and auto-retried, burning the server-side MFA attempt
quota until the account hit
429 mfa-quota-exceeded.Backend: detect the challenge error codes (
user-mfa-required,mfa-invalid-code,mfa-quota-exceeded), map them to typed exceptions, and stopretrying them (no more quota burn). Reuse the challenge
JSESSIONIDand re-send/loginwith the code viaPOST /cloud/me/login/mfa.Frontend: a segmented six-digit verification dialog (auto-advance, paste-fill,
auto-submit) shown on the challenge; the session is kept instead of logging out
on the challenge 401; clear localized messages for an invalid code and for the
attempt-quota lockout. TOTP only (SMS is deprecated in the Cloud web app).
Testing
the verify endpoint (service), the dialog (render / auto-submit / paste /
invalid-code / quota), and the auth interceptor.
enforced: OAuth -> challenge -> code -> sign-in completes; plus the wrong-code
and quota-exceeded paths.
POST /cloud/me/login/mfa(thepath is covered by service-level unit tests today).
Notes
user-mfa-required,mfa-invalid-code,mfa-quota-exceeded); the originating issue documented two.CloudApiMfaRequiredExceptionparses the challenge's available factors(
totpFactorAvailable/smsFactorAvailable/phoneNumber) per the contract,but the UI does not consume them yet (TOTP-only) - open to trimming if preferred.
Manual tests
Success scenario
Screen.Recording.2026-07-27.at.10.56.45.mov
Copy-paste code
Screen.Recording.2026-07-27.at.10.58.57.mov
Wrong or expired code
Screen.Recording.2026-07-27.at.11.00.04.mov
Addresses #6249 (not auto-closed: ships to users only on the
next version release). Jira: RI-8328.
Note
High Risk
Touches cloud authentication, session/JSESSIONID correlation, and token renewal during MFA; mistakes could lock users out or leak partial login state, though coverage is broad.
Overview
Adds end-to-end TOTP MFA handling when Redis Cloud OAuth sign-in is challenged after
POST /login, instead of opaque failures, auth retries that burn MFA quota, and logout on the challenge 401.Backend: Maps Cloud API codes (
user-mfa-required,mfa-invalid-code,mfa-quota-exceeded) to typed errors and customerrorCodes; does not auth-retry or invalidate the session on MFA errors. Persists the challengeJSESSIONIDasmfaApiSessionId, reuses it when submittingmfa_code/mfa_type: Totp, and exposesPOST /cloud/me/login/mfawith 6-digit validation.ensureAccessTokenonly fails without a refresh token when the JWT is actually expired (covers delay while entering the code).Frontend:
OAuthMfaDialogwith segmented OTP input (paste, auto-submit); Redux MFA state andsubmitMfaCodeAction;fetchUserInfoopens the dialog on11025, aborts non-TOTP or quota lockout with backend logout;cloudAuthInterceptorskips logout on MFA-required 401.ConfigOAuthresumes sign-in after verification. i18n for MFA strings.Reviewed by Cursor Bugbot for commit f9cc4d8. Bugbot is set up for automated code reviews on this repo. Configure here.