-
-
Notifications
You must be signed in to change notification settings - Fork 98
Improved GOG & Epic Games auth flow. No more need for code pasting! #480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved GOG & Epic Games auth flow. No more need for code pasting! #480
Conversation
📝 WalkthroughWalkthroughReplaces embedded GOG/Epic manual-code dialogs with in-app OAuth activities and a shared AuthWebViewDialog; adds per-session LoginUrlWithState helpers, URL redaction utility, settings integration for activity result handling, and corresponding string resource cleanup. Changes
Sequence Diagram(s)sequenceDiagram
participant Settings as Settings UI
participant Launcher as Activity Launcher
participant Activity as OAuth Activity
participant WebView as AuthWebViewDialog
participant OAuthServer as OAuth Provider
participant Backend as Backend/API
Settings->>Launcher: launch GOG/Epic OAuth activity
Launcher->>Activity: startActivityForResult()
Activity->>WebView: show(url from LoginUrlWithState)
WebView->>OAuthServer: navigate to OAuth endpoint
OAuthServer->>WebView: redirect to redirect_uri (with state/code)
WebView->>Activity: onUrlChange(url)
Activity->>Activity: validate state/redirect, extract code (query or JS)
Activity->>Launcher: setResult(RESULT_OK, intent with code)
Activity->>Activity: finish()
Launcher->>Settings: onActivityResult(auth code)
Settings->>Backend: handleGogAuthentication/handleEpicAuthentication (exchange code)
Backend-->>Settings: token response → update state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 6 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/src/main/java/app/gamenative/ui/component/dialog/GOGWebViewDialog.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/dialog/GOGWebViewDialog.kt:102">
P2: The WebView is configured to allow file URL access and universal access from file URLs, plus mixed content. Android security guidance flags these settings as risky because they allow file:// pages or injected content to read local files and make network requests, and mixed content permits HTTP content on HTTPS pages. For an auth WebView, this is a security risk unless absolutely required.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/screen/auth/GOGOAuthActivity.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/auth/GOGOAuthActivity.kt:26">
P2: OAuth authorization URL omits a `state` parameter and the callback accepts any returned code, removing standard CSRF protection for the authorization flow.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/ui/component/dialog/GOGWebViewDialog.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/screen/auth/GOGOAuthActivity.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@app/src/main/java/app/gamenative/ui/component/dialog/GOGLoginDialog.kt`:
- Around line 33-82: The login Button currently rendered inside the AlertDialog
body should be moved into the confirmButton slot to match Compose AlertDialog
conventions: remove the Button from the Column in the dialog text and instead
implement it in confirmButton using the same onLaunchInAppLogin, enabled =
!isLoading, Modifier.fillMaxWidth(), contentPadding (respecting isLandscape via
ButtonDefaults.ContentPadding or PaddingValues) and inner Icon+Text layout; keep
the info Text, errorMessage Text and LinearProgressIndicator in the dialog
body/Column, and ensure the dismissButton still uses onDismissRequest and
respects isLoading. This preserves layout, accessibility semantics, and the
original behavior tied to isLoading, isLandscape, errorMessage and
onLaunchInAppLogin.
In `@app/src/main/java/app/gamenative/ui/component/dialog/GOGWebViewDialog.kt`:
- Around line 163-165: onRelease currently only calls
view.saveState(webViewState) and risks leaking the WebView; update the onRelease
handler in GOGWebViewDialog (the lambda referencing
view.saveState(webViewState)) to call view.stopLoading(), set view.webViewClient
= null and view.webChromeClient = null, remove any child views (e.g.,
view.removeAllViews()), then call view.destroy() after saving state so the
WebView is properly torn down and won't leak.
- Around line 93-107: The WebView in GOGWebViewDialog (settings.apply block)
enables file/content access and allows mixed content which exposes the OAuth
surface; change the WebSettings flags to secure defaults by setting
allowFileAccess, allowContentAccess, allowFileAccessFromFileURLs, and
allowUniversalAccessFromFileURLs to false and set mixedContentMode to
WebSettings.MIXED_CONTENT_NEVER_ALLOW (or remove allowing mixed content), and
only enable any of these flags behind a clearly documented, explicit per-use
opt-in (e.g., a parameter or guarded flag) with a comment explaining why it is
required.
- Around line 3-31: The Close icon's IconButton in GOGWebViewDialog.kt currently
sets contentDescription = null; update the Icon composable to provide an
accessibility label by using a localized string resource (e.g.,
stringResource(R.string.close) or an appropriate existing close/close_dialog
string) instead of null so screen readers can announce the action; locate the
Icon inside the CenterAlignedTopAppBar / IconButton in the GOGWebViewDialog
composable and replace the null contentDescription with the localized string.
In `@app/src/main/java/app/gamenative/ui/screen/auth/GOGOAuthActivity.kt`:
- Around line 42-66: Replace the fragile substring check in the onUrlChange
lambda with strict Uri validation: parse the URL into a Uri and confirm scheme
== "https", host == "embed.gog.com" and path == "/on_login_success" before
proceeding; then check for an "error" query parameter and if present call
setResult(Activity.RESULT_CANCELED, Intent().putExtra(EXTRA_ERROR, error)) and
finish(), otherwise call extractAuthCode(uri) (change extractAuthCode to accept
a Uri) and if it returns null treat it as a failure by returning RESULT_CANCELED
with EXTRA_ERROR, else put EXTRA_AUTH_CODE into the success Intent and finish;
also keep the existing Timber.e logging inside extractAuthCode for exceptions.
In
`@app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt`:
- Around line 269-287: The code closes the GOG login dialog immediately in the
gogOAuthLauncher callback (openGOGLoginDialog = false) before calling
handleGogAuthentication, hiding any loading/errors; move that close logic out of
the immediate callback and rely on the handleGogAuthentication
onSuccess/onDialogClose handlers to set openGOGLoginDialog = false (keeping the
dialog open during loading/errors), and add an explicit error assignment to
gogLoginError when the auth code is missing/canceled so the user sees an error
instead of silently returning.
app/src/main/java/app/gamenative/ui/component/dialog/GOGLoginDialog.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/component/dialog/AuthWebViewDialog.kt
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/component/dialog/GOGWebViewDialog.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/component/dialog/AuthWebViewDialog.kt
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/app/gamenative/ui/screen/auth/EpicOAuthActivity.kt`:
- Around line 39-57: Replace the fragile substring checks in the onUrlChange and
onPageFinished handlers with proper URI parsing: use Uri.parse(currentUrl) /
Uri.parse(url) and verify the host equals "epicgames.com" (or the expected host
variant) and that the path exactly matches or startsWith "/id/api/redirect"
before extracting the code; update the logic around extractCodeFromUrl,
finishWithCode and the GOGWebViewDialog callbacks to only proceed when the
parsed URI validation passes (and keep the fallback JSON body extraction via
unquoteJsonString unchanged).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/src/main/java/app/gamenative/ui/screen/auth/EpicOAuthActivity.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/auth/EpicOAuthActivity.kt:40">
P2: Redirect URL validation is too permissive; using a substring match allows non‑Epic domains containing the redirect path to be treated as valid, enabling spoofed auth code extraction.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/ui/screen/auth/EpicOAuthActivity.kt
Outdated
Show resolved
Hide resolved
|
Very nice work. There's a few code clean-ups required, but overall looks good and I've verified it works correctly. As a follow-up we should look to stop the redirection to the LibraryScreen so that we can properly give the "Login Success" correctly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/src/main/res/values/strings.xml">
<violation number="1" location="app/src/main/res/values/strings.xml:995">
P2: Localized strings for GOG login still describe copying/pasting, so non-English locales will show outdated instructions after this change.</violation>
<violation number="2" location="app/src/main/res/values/strings.xml:1019">
P2: Localized strings for Epic login still reference copy/paste steps removed from the English string, causing stale instructions in other locales.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 7 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/src/main/java/app/gamenative/ui/component/dialog/AuthWebViewDialog.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/dialog/AuthWebViewDialog.kt:118">
P2: Auth WebView logs full OAuth URLs, which can include authorization codes or tokens. This risks leaking credentials into logs; redact or avoid logging full URLs in auth flows.</violation>
</file>
<file name="app/src/main/res/values/strings.xml">
<violation number="1" location="app/src/main/res/values/strings.xml:1008">
P2: Format string `epic_logout_failed` now expects a `%s` argument but call sites still call getString without format args, risking MissingFormatArgumentException or showing the placeholder.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/ui/component/dialog/AuthWebViewDialog.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@app/src/main/java/app/gamenative/ui/component/dialog/AuthWebViewDialog.kt`:
- Around line 46-47: Replace the hard-coded default title for topBarTitle with a
string resource: instead of mutableStateOf("Authentication") use
stringResource(R.string.auth_webview_title) via rememberSaveable so the initial
title is localized; add the corresponding <string
name="auth_webview_title">Authentication</string> to your strings.xml. Target
the topBarTitle declaration in AuthWebViewDialog (the rememberSaveable call) and
update it to obtain its initial value from the string resource.
- Around line 118-141: Logs in AuthWebViewDialog are printing full URLs
(including query and fragment) in shouldOverrideUrlLoading, onPageFinished, and
onReceivedError; strip sensitive parts before logging by parsing the URL and
removing query and fragment components and then logging the sanitized URL
instead of the raw one (modify the Timber.d/e calls in shouldOverrideUrlLoading,
onPageFinished, and onReceivedError to use the sanitized URL produced from
Uri.parse(url) with query/fragment cleared). Ensure null-safe handling so if
parsing fails you log a placeholder like "<invalid-url>" or the host only.
In `@app/src/main/java/app/gamenative/ui/screen/auth/EpicOAuthActivity.kt`:
- Around line 69-86: The catch blocks in isValidRedirectUrl and extractAuthCode
silently swallow URI parsing exceptions—modify the catch blocks in those two
functions to log the failure (including the url being parsed and the exception
details/stacktrace) before returning false/null; use the class' logging
mechanism (e.g., Log.e or your app logger) with a clear tag like
"EpicOAuthActivity" and include the exception message and throwable so redirect
parsing failures can be diagnosed.
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/ui/component/dialog/AuthWebViewDialog.kt (1)
116-123: Handle URL overrides viaWebResourceRequesttoo.Only overriding the deprecated
shouldOverrideUrlLoading(WebView, String?)can miss navigations on API 24+. Add theWebResourceRequestoverride and delegate to the existing logic.♻️ Suggested override
+import android.webkit.WebResourceRequest @@ + override fun shouldOverrideUrlLoading( + view: WebView?, + request: WebResourceRequest? + ): Boolean { + return shouldOverrideUrlLoading(view, request?.url?.toString()) + }app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt (1)
261-340: Consider extracting shared OAuth result handling.The GOG and Epic launcher callbacks repeat the same cancel/error/toast and coroutine wiring. A small shared helper would reduce drift and make future changes safer.
app/src/main/java/app/gamenative/ui/component/dialog/AuthWebViewDialog.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/component/dialog/AuthWebViewDialog.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/src/main/java/app/gamenative/ui/screen/auth/EpicOAuthActivity.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/auth/EpicOAuthActivity.kt:33">
P2: OAuth state is regenerated on every onCreate without restoration, so a configuration change (e.g., rotation) recreates the activity with a new state and rejects the in‑flight redirect as a CSRF mismatch.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/ui/screen/auth/EpicOAuthActivity.kt
Outdated
Show resolved
Hide resolved
c8c8039 to
1fda483
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/src/main/java/app/gamenative/ui/component/dialog/AuthWebViewDialog.kt`:
- Around line 118-125: The current WebViewClient in AuthWebViewDialog.kt only
implements the deprecated shouldOverrideUrlLoading(view: WebView?, url: String?)
so onUrlChange misses OAuth redirects on API 24+; fix by extracting the
URL-handling logic (logging via redactUrlForLogging and invoking onUrlChange)
into a shared private helper and override both shouldOverrideUrlLoading(view:
WebView?, url: String?) and shouldOverrideUrlLoading(view: WebView?, request:
WebResourceRequest?) to call that helper (use request.url.toString() safely),
returning the same super result to ensure redirects are captured on all API
levels.
In `@app/src/main/res/values/strings.xml`:
- Around line 1008-1009: Update the two string resources to use "log out" and
"out of" for correct grammar: change the value of epic_logout_success to a
natural phrasing like "Logged out of Epic Games successfully" (or "Successfully
logged out of Epic Games") and change epic_logout_failed to "Failed to log out
of Epic Games: %s" so epic_logout_success and epic_logout_failed reflect the
verb form and "out of" wording.
app/src/main/java/app/gamenative/ui/component/dialog/AuthWebViewDialog.kt
Show resolved
Hide resolved
1fda483 to
a70c80c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/app/gamenative/ui/screen/auth/EpicOAuthActivity.kt`:
- Around line 83-91: When webView.evaluateJavascript(...) returns a null/blank
result from unquoteJsonString(result) the code currently does nothing; add error
handling and a timeout fallback so the user isn't left waiting. Update the block
around webView.evaluateJavascript to: if code is non-empty call
finishWithCode(code) as now, but if code is null/blank then record the failed
extraction and show an error state (e.g., call a new or existing
finishWithError/error UI method or display a Snackbar/AlertDialog) and start a
short timeout/retry (use a Handler, Coroutine with delay, or CountDownTimer) to
retry extraction a couple times before giving the error; reference the
webView.evaluateJavascript call, unquoteJsonString(result) and
finishWithCode(...) when implementing the retries, error dialog, and timeout
logic.
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/service/gog/GOGConstants.kt (1)
48-53: Function naming uses PascalCase instead of Kotlin's camelCase convention.
LoginUrlWithState()should beloginUrlWithState()per Kotlin naming conventions. However, this is a minor style nit.Suggested rename
- fun LoginUrlWithState(): Pair<String, String> { + fun loginUrlWithState(): Pair<String, String> {app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt (1)
96-104: UnusedcoroutineScopeparameter in authentication handlers.The
coroutineScopeparameter in bothhandleGogAuthenticationandhandleEpicAuthenticationis never used within the function body. Consider removing it to avoid confusion.Suggested cleanup
private suspend fun handleGogAuthentication( context: Context, authCode: String, - coroutineScope: CoroutineScope, onLoadingChange: (Boolean) -> Unit, onError: (String?) -> Unit, onSuccess: (Int) -> Unit, onDialogClose: () -> Unit )Apply the same change to
handleEpicAuthentication.
| webView.evaluateJavascript( | ||
| "(function(){ try { var j = JSON.parse(document.body && document.body.innerText || '{}'); return j.authorizationCode || null; } catch(e){ return null; } })();" | ||
| ) { result -> | ||
| val code = unquoteJsonString(result) | ||
| if (!code.isNullOrBlank()) { | ||
| Timber.d("Automatically extracted Epic auth code from page body") | ||
| finishWithCode(code) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No error feedback if JavaScript extraction returns null.
When unquoteJsonString(result) returns null or blank after a valid redirect, the user is left with an unresponsive dialog. Consider adding a timeout or error state.
Suggested improvement
webView.evaluateJavascript(
"(function(){ try { var j = JSON.parse(document.body && document.body.innerText || '{}'); return j.authorizationCode || null; } catch(e){ return null; } })();"
) { result ->
val code = unquoteJsonString(result)
if (!code.isNullOrBlank()) {
Timber.d("Automatically extracted Epic auth code from page body")
finishWithCode(code)
+ } else {
+ Timber.w("Redirect page loaded but no auth code found in body")
+ // Could add a retry mechanism or error after timeout
}
}🤖 Prompt for AI Agents
In `@app/src/main/java/app/gamenative/ui/screen/auth/EpicOAuthActivity.kt` around
lines 83 - 91, When webView.evaluateJavascript(...) returns a null/blank result
from unquoteJsonString(result) the code currently does nothing; add error
handling and a timeout fallback so the user isn't left waiting. Update the block
around webView.evaluateJavascript to: if code is non-empty call
finishWithCode(code) as now, but if code is null/blank then record the failed
extraction and show an error state (e.g., call a new or existing
finishWithError/error UI method or display a Snackbar/AlertDialog) and start a
short timeout/retry (use a Handler, Coroutine with delay, or CountDownTimer) to
retry extraction a couple times before giving the error; reference the
webView.evaluateJavascript call, unquoteJsonString(result) and
finishWithCode(...) when implementing the retries, error dialog, and timeout
logic.
Summary by cubic
Move GOG and Epic sign-in to an in-app WebView with automatic auth code capture and CSRF protection. Users no longer paste codes or leave the app.
Written for commit a70c80c. Summary will update on new commits.
Summary by CodeRabbit
New Features
Refactor