feat: Allow sign in with Steam Guard TOTP code#1241
Open
peanut250 wants to merge 7 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new public ChangesSteam 2FA flow (single cohesive cohort)
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as TwoFactor UI
participant VM as UserLoginViewModel
participant Auth as SteamAuthenticator
participant Steam as SteamService
User->>UI: tap "Use 2-factor auth code"
UI->>VM: onUseGuardTotp()
VM->>Auth: set useGuardTotp = true
VM->>Steam: startLoginWithCredentials(authenticator=Auth)
Steam->>Auth: request 2FA action (getDeviceCode / getEmailCode / acceptDeviceConfirmation)
Auth->>VM: update loginState (DeviceAuth / EmailAuth / DeviceConfirm)
VM->>UI: render two-factor prompt
User->>UI: submit code
UI->>Auth: send code via submitChannel
Auth->>Steam: complete CompletableFuture with submitted code
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
peanut250
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/service/SteamAuthenticator.kt (2)
15-19: Constructor properties should beval, notvar.
loginState,submitChannel, andviewModelScopeare never reassigned — making themvarunnecessarily widens mutability.useGuardTotprightly staysvarsince it’s toggled.-class SteamAuthenticator(var loginState : MutableStateFlow<UserLoginState>, - var submitChannel : Channel<String>, - var viewModelScope : CoroutineScope) : IAuthenticator { +class SteamAuthenticator( + private val loginState: MutableStateFlow<UserLoginState>, + private val submitChannel: Channel<String>, + private val viewModelScope: CoroutineScope, +) : IAuthenticator {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/service/SteamAuthenticator.kt` around lines 15 - 19, The constructor of SteamAuthenticator declares mutable properties that are never reassigned; change the constructor parameters loginState, submitChannel, and viewModelScope from var to val to make them immutable, leaving useGuardTotp as var since it is toggled; update the class header signature (SteamAuthenticator(...)) to use val for those three identifiers so their intent and thread-safety are clearer.
40-84: Tag inconsistency — log tag readsUserLoginViewModelinsideSteamAuthenticator.Now that this logic lives in its own class, the Timber tag should reflect that to avoid confusing log triage.
- Timber.tag("UserLoginViewModel").d("Two-Factor, device code") + Timber.tag("SteamAuthenticator").d("Two-Factor, device code")(Same for the other two overrides.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/service/SteamAuthenticator.kt` around lines 40 - 84, The Timber logs in SteamAuthenticator use the wrong tag "UserLoginViewModel"; update the Timber.tag calls inside SteamAuthenticator (notably in getDeviceCode and getEmailCode and the other two override methods mentioned) to use a tag reflecting this class (e.g., "SteamAuthenticator" or SteamAuthenticator::class.java.simpleName) so logs correctly identify the source; locate the Timber.tag(...) usages in those override methods and replace the string accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/service/SteamAuthenticator.kt`:
- Around line 21-38: The method acceptDeviceConfirmation in SteamAuthenticator
returns false when useGuardTotp is true but never clears that flag, causing
subsequent logins to always bypass device confirmation; modify
acceptDeviceConfirmation to consume and reset useGuardTotp (set it back to
false) before returning CompletableFuture.completedFuture(false) so the
authenticator can be reused on the next login attempt, keeping the existing
behavior for updating loginState
(loginResult/LoginScreen/isLoggingIn/lastTwoFactorMethod) otherwise unchanged.
In `@app/src/main/java/app/gamenative/ui/model/UserLoginViewModel.kt`:
- Around line 262-275: The authenticator.useGuardTotp flag is being set in
useGuardTotp() and never reset, which permanently disables device-confirmation;
modify the credential-login flow to clear that flag before starting each new
SteamService.startLoginWithCredentials call (e.g., reset
authenticator.useGuardTotp = false at the start of the credential login path)
and also reset it in onLogonEnded whenever a terminal result (Success or Failed)
occurs so each new login starts with a clean authenticator state; locate
useGuardTotp(), the authenticator field, SteamService.startLoginWithCredentials
invocation, and onLogonEnded to apply these resets.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/service/SteamAuthenticator.kt`:
- Around line 15-19: The constructor of SteamAuthenticator declares mutable
properties that are never reassigned; change the constructor parameters
loginState, submitChannel, and viewModelScope from var to val to make them
immutable, leaving useGuardTotp as var since it is toggled; update the class
header signature (SteamAuthenticator(...)) to use val for those three
identifiers so their intent and thread-safety are clearer.
- Around line 40-84: The Timber logs in SteamAuthenticator use the wrong tag
"UserLoginViewModel"; update the Timber.tag calls inside SteamAuthenticator
(notably in getDeviceCode and getEmailCode and the other two override methods
mentioned) to use a tag reflecting this class (e.g., "SteamAuthenticator" or
SteamAuthenticator::class.java.simpleName) so logs correctly identify the
source; locate the Timber.tag(...) usages in those override methods and replace
the string accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3814d63d-eedf-41a0-9708-be8c404f4d2a
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/service/SteamAuthenticator.ktapp/src/main/java/app/gamenative/ui/model/UserLoginViewModel.ktapp/src/main/java/app/gamenative/ui/screen/login/TwoFactorAuthScreen.ktapp/src/main/java/app/gamenative/ui/screen/login/UserLoginScreen.ktapp/src/main/res/values/strings.xml
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/service/SteamAuthenticator.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/SteamAuthenticator.kt:54">
P2: Returned CompletableFuture may never complete on channel closure/cancellation, causing authentication flow hangs.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/model/UserLoginViewModel.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/model/UserLoginViewModel.kt:263">
P2: `useGuardTotp` is set on a shared authenticator instance and never reset, so TOTP mode can leak into subsequent normal logins.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
-35
to
-96
| private val authenticator = object : IAuthenticator { | ||
| override fun acceptDeviceConfirmation(): CompletableFuture<Boolean> { | ||
| Timber.tag("UserLoginViewModel").i("Two-Factor, device confirmation") | ||
|
|
||
| _loginState.update { currentState -> | ||
| currentState.copy( | ||
| loginResult = LoginResult.DeviceConfirm, | ||
| loginScreen = LoginScreen.TWO_FACTOR, | ||
| isLoggingIn = false, | ||
| lastTwoFactorMethod = "steam_guard", | ||
| ) | ||
| } | ||
|
|
||
| return CompletableFuture.completedFuture(true) | ||
| } | ||
|
|
||
| override fun getDeviceCode(previousCodeWasIncorrect: Boolean): CompletableFuture<String> { | ||
| Timber.tag("UserLoginViewModel").d("Two-Factor, device code") | ||
|
|
||
| _loginState.update { currentState -> | ||
| currentState.copy( | ||
| loginResult = LoginResult.DeviceAuth, | ||
| loginScreen = LoginScreen.TWO_FACTOR, | ||
| isLoggingIn = false, | ||
| previousCodeIncorrect = previousCodeWasIncorrect, | ||
| lastTwoFactorMethod = "authenticator_code", | ||
| ) | ||
| } | ||
|
|
||
| return CompletableFuture<String>().apply { | ||
| viewModelScope.launch { | ||
| val code = submitChannel.receive() | ||
| complete(code) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| override fun getEmailCode( | ||
| email: String?, | ||
| previousCodeWasIncorrect: Boolean, | ||
| ): CompletableFuture<String> { | ||
| Timber.tag("UserLoginViewModel").d("Two-Factor, asking for email code") | ||
|
|
||
| _loginState.update { currentState -> | ||
| currentState.copy( | ||
| loginResult = LoginResult.EmailAuth, | ||
| loginScreen = LoginScreen.TWO_FACTOR, | ||
| isLoggingIn = false, | ||
| email = email, | ||
| previousCodeIncorrect = previousCodeWasIncorrect, | ||
| lastTwoFactorMethod = "email_code", | ||
| ) | ||
| } | ||
|
|
||
| return CompletableFuture<String>().apply { | ||
| viewModelScope.launch { | ||
| val code = submitChannel.receive() | ||
| complete(code) | ||
| } | ||
| } | ||
| } | ||
| } |
Owner
There was a problem hiding this comment.
What was the reason to pull this out into a file?
Author
There was a problem hiding this comment.
Just organizational taste/readability since it is slightly expanded to allow for the TOTP code method.
| <string name="steam_2fa_confirmation">Use the Steam Mobile App to confirm your sign in…</string> | ||
| <string name="steam_2fa_device">Please enter your 2-factor auth code from your authenticator app.</string> | ||
| <string name="steam_2fa_email">Please enter the auth code sent to the email at %s</string> | ||
| <string name="steam_2fa_use_guard_totp">Or use 2-factor auth code</string> |
Owner
There was a problem hiding this comment.
Please add translations for other languages also
Author
There was a problem hiding this comment.
For now I can add machine translations as a best effort but have you considered something like Weblate for crowdsourced translations?
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/res/values-da/strings.xml`:
- Line 1109: Update the Danish string resource steam_2fa_use_guard_totp so its
value uses the same 2FA terminology as the rest of the file; replace
"tofaktorautentificering" with the established term family (e.g.,
"tofaktorgodkendelse" or "2-faktor-godkendelseskode") to ensure UI consistency
for that resource.
In `@app/src/main/res/values-de/strings.xml`:
- Line 15: The string resource steam_2fa_use_guard_totp uses the formal
"verwenden Sie" which is inconsistent with the other Steam 2FA strings that use
informal "du" forms; update the value to the informal phrasing (e.g., replace
"Oder verwenden Sie den Code für die Zwei-Faktor-Authentifizierung" with an
informal imperative like "Oder verwende den Code für die
Zwei-Faktor-Authentifizierung") so the formality matches the other strings in
this section.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a14ec348-a66b-4439-975c-59ebf4d3a3c6
📒 Files selected for processing (14)
app/src/main/res/values-da/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ro/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values-zh-rTW/strings.xmlapp/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (4)
- app/src/main/res/values-pt-rBR/strings.xml
- app/src/main/res/values-es/strings.xml
- app/src/main/res/values-uk/strings.xml
- app/src/main/res/values/strings.xml
…ons from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Owner
|
Just tried it out, good change but there are a few changes needed:
|
Owner
|
Oh also, please remove the new SteamAuthenticator.kt file and put the code back where it was - this way it's easier to see if anything was actually diffed in that function or it's just a pure extraction. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Allows signing in via Steam Guard TOTP code for those who have extracted the secrets for use in Bitwarden or other authenticators.
Recording
gamenative_steam_guard_totp_demo.webm
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Add sign-in support for Steam Guard TOTP codes from third-party authenticators. Users can switch from device confirmation to entering a code; spacing is improved on wide layouts.
New Features
steam_2fa_use_guard_totpwith translations across locales.Refactors
UserLoginState; ViewModel exposesuseGuardTotp().onUseGuardTotpfromUserLoginScreentoTwoFactorAuthScreen.Written for commit e9d520f. Summary will update on new commits.
Summary by CodeRabbit
New Features
UI
Localization
Chores