feat: add dev hack for authentication to access translation platform#50
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the OdooAPI service, primarily by migrating the authentication mechanism from a custom JSON-based API to an XML-RPC client. Key changes include a revised authenticate method, a simplified refreshAccessToken function that appears to be a development stub, and a bypassed logout functionality for development mode. The review comments highlight several issues: the authenticate method's totp parameter removal causes a TypeScript error at the call site, the new XML-RPC client's error object structure will likely break existing error handling, the Promise.resolve() in logout is redundant, and the try-catch block around refreshAccessToken is currently redundant and alters error propagation behavior.
| catch (e: any) { | ||
| } catch (e: any) { | ||
| console.warn("Failed to authenticate: ", e); | ||
| return e; |
There was a problem hiding this comment.
The error object e returned here now originates from the XML-RPC client, which has a different structure than the previous JSON-RPC error response. This will likely break the error handling logic in Login.ts (lines 101-107), which relies on specific properties like res?.name to detect 2FA requirements or login failures.
| } | ||
| console.log("Logout bypassed for development mode"); | ||
| clearStoreCache(); | ||
| return Promise.resolve(); |
There was a problem hiding this comment.
In an async function, returning a value (or nothing) automatically wraps it in a Promise. Explicitly returning Promise.resolve() is redundant.
| return Promise.resolve(); | |
| return; |
| const refreshMarginMs = 10_000; | ||
| if (store.accessToken && options.refreshIfExpired !== false && | ||
| new Date(store.accessTokenExpiresAt!).getTime() < Date.now() + refreshMarginMs) { | ||
| try { await refreshAccessToken(); } catch { clearStoreCache(); return; } |
There was a problem hiding this comment.
The try-catch block is redundant because the current refreshAccessToken implementation is a stub that never throws. Additionally, the error handling behavior has changed: it now swallows the error and returns undefined, whereas the previous implementation re-threw the error. If this logic is intended to be restored later, consider maintaining the error propagation.
1 participant
Introduce a temporary authentication workaround to allow connection to the translation platform without relying on the auth_external module.
This implementation uses native XML-RPC authentication and treats the user password as an access token, while keeping the overall API structure compatible with the future token-based system.
This is a temporary solution intended for development purposes only and will be removed once the migration to auth_external is completed.