-
Notifications
You must be signed in to change notification settings - Fork 10
Fix: Critical login state and authentication issues #60
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
base: main
Are you sure you want to change the base?
Conversation
dc63785 to
6b37d75
Compare
|
Notice: I utilized Gemini 3 Pro to accelerate the development of this PR. Therefore, I consider a single-person review (2-eye principle) insufficient. I strictly prefer a 4-eye principle review by another contributor to ensure correctness. |
cf23ce7 to
3cfa0d8
Compare
|
Looks fine in general 👍 I will give it a test |
| SingleSessionManager.setUserAgent(userAgent) | ||
|
|
||
| // initialise thumbnails cache on background thread | ||
| ThumbnailsCacheManager.InitDiskCacheTask().execute() |
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.
@zerox80 Why is this line here?
If this is a glitch from something else, can you please re-base this PR so we see if it's still there?
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.
its not glitch from something else, i always rebase all PRs after u merge something. What do u mean ? im confused
|
New app installation, fails immediatly after initial launch (I can still accept that the app sends me notifications) Might also be fishy that I see |
|
yes i solved that super stuff in other PR xD |
|
will focus this PR soon, i am cooking stuff with conflict management while syncing. |
|
OK, yes, rebase because I think all the stuff in ic_action_open_shortcut.xml is also not relevant herew :-) |
…various service instances.
… prevent login failure after process death
…omTabsIntent for Firefox compatibility - Add setIntent() in onNewIntent to properly handle OAuth redirects - Add logging for OAuth redirect debugging
bfbce96 to
5586f7e
Compare
Fix: Critical Login State Loss & Authentication Issues
📋 Overview
This PR resolves critical issues in the login process, specifically regarding OAuth authentication (OIDC). The primary issue was that the
LoginActivitystate (and thus crucial authentication parameters) was lost when the user switched to the browser view (Custom Tabs) and returned. This resulted in login failures or app crashes/restarts, particularly on devices with aggressive memory management.🐛 The Problem
When the app opens the browser for OAuth login,
LoginActivityis pushed to the background. Android may terminate the app process at this point to reclaim memory.When the user returns after logging in:
LoginActivityis recreated (onCreate).codeVerifier,codeChallenge,serverBaseUrl) are gone.🛠 The Solution
The solution consists of three main components:
codeVerifier,codeChallenge,oidcState) are now explicitly saved toSharedPreferencesbefore the browser is opened. This is more robust thanonSaveInstanceStateas it survives complete process restarts.onCreateandonSaveInstanceState, the UI state (URL input, Auth method) is now correctly saved and restored.LoginActivityfrom being placed on the stack when the browser returns. Instead, the existing instance is reused.🔍 Detailed Code Analysis
1.
AuthenticationViewModel.kt- Mutability for RestoreWe need to make the OAuth parameters mutable so we can overwrite them when restoring the old state from storage.
2.
LoginActivity.kt- Intent & Task ManagementWhen the browser returns with the Auth Code, we don't want Android to layer a new
LoginActivityover the old one. We want to reactivate the old one.3.
LoginActivity.kt- Saving & Loading Auth StateHere lies the magic of persistence. We don't just rely on RAM.
Saving before Browser Launch:
Restoring on Start (
onCreate):Cleanup on Success:
4.
LoginActivity.kt- UI State RestorationWe ensure that the UI (input fields, button visibility) is correctly restored so the user isn't faced with an empty screen.
✅ Verification / Testing