Ajit/fix fetch auth config#2068
Conversation
…ion imports - Replaced ajax calls with fetch in sponsor, sponsors, and themeSetting pages. - Updated constants import to config in various files for better configuration management. - Improved error handling for API responses. - Adjusted logic for premium theme checks based on new config settings. - Enhanced cookie management in the Authenticator plugin for better security. - Fixed minor typos and improved code readability across multiple files.
Greptile SummaryThis PR refactors the auth system from a custom-header XHR flow to a cookie-based scheme: a new
Confidence Score: 3/5Not safe to merge — multiple P1 issues mean the new cookie-based auth flow is broken end-to-end in the Cordova WebView. Three independent P1 findings across the core login check, logout, and sponsor purchase all using fetch without credentials:include, making the new cookie auth scheme non-functional for critical paths. src/lib/auth.js (core login/logout fetch calls), src/pages/sponsor/sponsor.js (purchase POST), and previously-flagged plugin purchase/refund fetch calls.
|
| Filename | Overview |
|---|---|
| src/lib/auth.js | Core auth refactored to cookie-based flow via fetch; both getLoggedInUser() and logout() fetch calls are missing credentials: 'include', breaking login detection and server-side logout. |
| src/lib/ajax.js | New local XHR-based ajax utility replacing @deadlyjack/ajax; withCredentials is wired up via ajax.configure in main.js. |
| src/lib/config.js | New config module consolidating constants.js; includes HAS_PRO getter/setter replacing IS_FREE_VERSION global. |
| src/lib/adRewards.js | getRewardIdentity references undefined user variable (previously flagged); canShowAds/isRewardedSupported correctly migrated to config.HAS_PRO. |
| src/plugins/auth/src/android/Authenticator.java | Refactored to inject auth cookie via CookieManager with HttpOnly; Secure; SameSite=None — addresses previous HttpOnly finding. |
| src/pages/plugin/plugin.js | Migrated from ajax to fetch for order/refund calls; credentials: 'include' still missing (previously flagged). |
| src/main.js | Correctly wires ajax.configure with withCredentials=true for API XHR calls; migrates IS_FREE_VERSION to config.HAS_PRO. |
| src/pages/sponsor/sponsor.js | POST to /api/sponsor is missing credentials: 'include', so the server receives an unauthenticated purchase confirmation. |
| src/lib/checkPluginsUpdate.js | Correctly migrated from ajax to fetch with proper res.ok check and JSON parsing; previous options-object bug is fixed. |
Sequence Diagram
sequenceDiagram
participant App as JS WebView
participant CM as CookieManager
participant Server as API Server
Note over App,CM: Login
App->>CM: saveToken via cordova.exec
CM->>CM: setCookie with HttpOnly Secure SameSite=None
CM-->>App: success
Note over App,Server: Auth check - getLoggedInUser
App->>Server: fetch GET /login - no credentials include
Server-->>App: 401 Unauthorized - cookie not forwarded
Note over App,Server: XHR path via ajax utility
App->>Server: XHR with withCredentials true
Server-->>App: 200 OK - cookie forwarded correctly
Note over App,Server: Logout
App->>Server: fetch DELETE /login - no credentials include
Server-->>App: request without session - session NOT cleared
App->>CM: clearTokenCookie via cordova.exec
CM-->>App: local cookie cleared only
Reviews (2): Last reviewed commit: "fix: update cookie settings to include H..." | Re-trigger Greptile
| ).readFile("json"); | ||
|
|
||
| const res = await ajax({ | ||
| const res = await fetch({ |
There was a problem hiding this comment.
| device?.uuid || | ||
| "guest"; | ||
| return String(userId); | ||
| const userId = user?.id || "Guest"; |
There was a problem hiding this comment.
user is undefined and userId is never returned
The refactor removed the auth.getUserInfo() call but kept the reference to user, which is never declared in this scope. This throws a ReferenceError at runtime, so getRewardIdentity always falls into the catch branch and returns device.uuid || "guest". Additionally, even if user were defined, there is no return userId statement, so the function would return undefined.
|
|
||
| private void logout(CallbackContext callbackContext) { | ||
| Log.d(TAG, "Logging out, removing token."); | ||
| prefManager.remove(KEY_TOKEN); | ||
| if (callbackContext != null) callbackContext.success(); | ||
| } | ||
|
|
||
| private void isLoggedIn(CallbackContext callbackContext) { | ||
| String token = prefManager.getString(KEY_TOKEN, null); | ||
| if (token == null) { | ||
| Log.d(TAG, "isLoggedIn check: No token found in preferences."); | ||
| callbackContext.error(0); | ||
| return; | ||
| } | ||
|
|
||
| Log.d(TAG, "isLoggedIn check: Token found, validating with server..."); | ||
| final String tokenToValidate = token; // Make effectively final for lambda | ||
|
|
||
| cordova.getThreadPool().execute(() -> { | ||
| try { | ||
| String result = validateToken(tokenToValidate); | ||
| if (result != null) { | ||
| Log.i(TAG, "Token validation successful."); | ||
| callbackContext.success(); | ||
| } else { | ||
| Log.w(TAG, "Token validation failed (result was null)."); | ||
| callbackContext.error(401); | ||
| } | ||
| } catch (Exception e) { | ||
| Log.e(TAG, "CRITICAL error in isLoggedIn thread: " + e.getMessage(), e); | ||
| callbackContext.error("Internal Plugin Error: " + e.getMessage()); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| private void getUserInfo(CallbackContext callbackContext) { | ||
| Log.d(TAG, "getUserInfo: Fetching token..."); | ||
| String token = prefManager.getString(KEY_TOKEN, null); | ||
|
|
||
| if (token == null) { | ||
| Log.e(TAG, "getUserInfo: No token found."); | ||
| callbackContext.error(0); | ||
| return; | ||
| private void setTokenCookie(String token) { | ||
| CookieManager cm = CookieManager.getInstance(); | ||
| for (String origin : API_ORIGINS) { | ||
| cm.setCookie(origin, "token=" + token + "; Path=/; Secure; SameSite=None"); | ||
| } | ||
|
|
||
| final String tokenToValidate = token; | ||
| cordova.getThreadPool().execute(() -> { | ||
| try { | ||
| String response = validateToken(tokenToValidate); | ||
| if (response != null) { | ||
| Log.d(TAG, "getUserInfo: Successfully fetched user info."); | ||
| callbackContext.success(response); | ||
| } else { | ||
| Log.e(TAG, "getUserInfo: Validation failed or unauthorized."); | ||
| callbackContext.error("Unauthorized"); | ||
| } | ||
| } catch (Exception e) { | ||
| Log.e(TAG, "Error in getUserInfo: " + e.getMessage(), e); | ||
| callbackContext.error("Error: " + e.getMessage()); | ||
| } | ||
| }); | ||
| cm.flush(); |
There was a problem hiding this comment.
Auth token cookie missing
HttpOnly flag
The token cookie is set without the HttpOnly attribute. Any JavaScript running in the WebView can read it via document.cookie, which means a malicious or compromised plugin could exfiltrate the session token. Adding HttpOnly prevents JS access without breaking the withCredentials XHR/fetch flow, since the browser still sends HttpOnly cookies automatically.
|
@greptile_apps review again with confidence score |
| } | ||
|
|
||
| async getUserInfo() { | ||
| try { | ||
| const data = await this._exec("getUserInfo"); | ||
| return typeof data === "string" ? JSON.parse(data) : data; | ||
| } catch (error) { | ||
| console.error("Failed to fetch user data.", error); | ||
| return null; | ||
| } | ||
| } | ||
| /** | ||
| * | ||
| * @returns {User} | ||
| */ | ||
| async getLoggedInUser() { | ||
| if (loggedInUser) return loggedInUser; | ||
|
|
||
| async getAvatar() { | ||
| try { | ||
| const userData = await this.getUserInfo(); | ||
| if (!userData) return null; | ||
|
|
||
| if (userData.github) { | ||
| return `https://avatars.githubusercontent.com/${userData.github}`; | ||
| const res = await fetch(`${config.API_BASE}/login`); | ||
|
|
||
| if (res.ok) { | ||
| loggedInUser = await res.json(); | ||
| localStorage.setItem(CACHE_USER_KEY, JSON.stringify(loggedInUser)); | ||
| clearTimeout(cacheTimeout); | ||
| cacheTimeout = setTimeout(() => (loggedInUser = null), 600_000); |
There was a problem hiding this comment.
Core auth fetch missing
credentials: 'include'
getLoggedInUser() calls fetch against acode.app/api without credentials: 'include'. The Cordova WebView loads from a local origin (file:// or localhost), so these are cross-origin requests. Without credentials: 'include' the auth cookie set by CookieManager is not forwarded — the server will always see an unauthenticated request, return 401, and getLoggedInUser() will return null, meaning loginEvents.emit() in main.js is never called and the user always appears logged out. By contrast, every ajax/XHR call explicitly sets withCredentials = true via ajax.configure in main.js, showing this omission is unintentional.
| } | ||
|
|
||
| async openLoginUrl() { | ||
| const url = "https://acode.app/login?redirect=app"; | ||
|
|
||
| async logout() { | ||
| try { | ||
| await new Promise((resolve, reject) => { | ||
| CustomTabs.open(url, { showTitle: true }, resolve, reject); | ||
| const res = await fetch(`${config.API_BASE}/login`, { | ||
| method: "DELETE", | ||
| }); | ||
| if (!res.ok) { |
There was a problem hiding this comment.
logout() server-side session not invalidated without credentials
The DELETE /login call also lacks credentials: 'include'. The cookie is not sent to the server so the session is never invalidated server-side — the server-side session remains alive even after the user "logs out" locally. Add credentials: "include" here as well.
| ); | ||
|
|
||
| try { | ||
| const res = await ajax.post(`${constants.API_BASE}/sponsor`, { | ||
| data: { | ||
| const res = await fetch(`${config.API_BASE}/sponsor`, { | ||
| method: "POST", | ||
| body: JSON.stringify({ | ||
| ...sponsorDetails, | ||
| tier: productId, | ||
| packageName: BuildInfo.packageName, | ||
| purchaseToken: order.purchaseToken, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
Sponsor purchase
fetch missing credentials: 'include'
The POST to /api/sponsor doesn't include credentials: "include", so the auth cookie won't be forwarded in this cross-origin Cordova WebView context. The server will receive an unauthenticated purchase confirmation request. Every ajax/XHR call sets withCredentials = true via ajax.configure in main.js, so the omission here appears unintentional.
* fix: hooks not working on free version * Update hooks/post-process.js Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Update post-process.js ---------
No description provided.