Conversation
Summary of ChangesHello @dohy-eon, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 자산 동기화 과정에서 발생하는 비효율성을 개선하고, 사용자에게 보다 신속하고 정확한 최신 잔액 정보를 제공하는 것을 목표로 합니다. 동기화 직후 발생할 수 있는 데이터 불일치(레이스 컨디션)를 방지하기 위해, API로부터 수집된 실시간 잔액 데이터를 우선적으로 활용하도록 로직을 변경하여 DB 조회 부하를 줄이고 응답성을 향상시켰습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This Pull Request enhances the asset synchronization logic by preventing redundant DB queries and immediately returning real-time parsed balance data, improving user experience and reducing race conditions. It introduces a latestBalances field in AssetResDTO.AssetSyncResult and streamlines data flow from AssetFetchService to AssetBalanceService. However, a security audit revealed two significant concerns: a potential Insecure Direct Object Reference (IDOR) vulnerability due to inadequate ownership verification for the memberId parameter, and the logging of sensitive financial information (account balances) in application logs. Addressing these security issues is essential to safeguard user data and maintain proper access controls.
| * 동기화 실패 시 기존 잔액을 반환합니다. | ||
| */ | ||
| @Transactional(propagation = Propagation.NOT_SUPPORTED) | ||
| public Long syncAndGetLatestBalance(Long memberId, Long accountId) { |
There was a problem hiding this comment.
The method syncAndGetLatestBalance accepts memberId and accountId as parameters and uses them to fetch and return sensitive financial data (account balance). While it verifies that the accountId belongs to the memberId (line 50), it does not verify that the memberId belongs to the currently authenticated user. An attacker could potentially guess or discover another user's memberId and accountId to retrieve their real-time account balance. It is recommended to verify that the memberId matches the ID of the authenticated user, or retrieve the authenticated user's ID directly from the security context.
🔗 Related Issue
📝 Summary
동기화 작업 시 발생하는 DB 중복 조회를 방지하고 실시간으로 파싱된 잔액 데이터를 즉시 반환하도록 개선
🔄 Changes
💬 Questions & Review Points
레이스 컨디션 방지: 동기화 직후 DB 반영 지연으로 인해 이전 잔액이 조회되는 문제를 방지하기 위해 메모리 내 데이터를 우선 사용하도록 했습니다. 이 접근 방식이 서비스의 다른 도메인과 일관성이 있는지, 그냥 사용해도 괜찮은지 체크 부탁드립니다.
예외 처리: AssetBalanceService에서 동기화 실패 시 fallback으로 기존 db값 사용하게 처리했는데 코드가 적절한지 체크 부탁드리겠습니다.
API 테스트는 따로 진행하지 않았습니다.
✅ Checklist