fix(#396): 캠페인 제안 상세 조회에 제품 이름 추가#397
Conversation
Summary of ChangesHello @ParkJiYeoung8297, 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은 캠페인 제안 상세 조회 시 제품 이름을 응답에 포함시켜 사용자 경험을 향상시키는 것을 목표로 합니다. 기존에는 제품 ID만 제공되었으나, 이제 관련 브랜드의 스폰서십에서 제품 이름을 가져와 상세 응답 DTO에 통합함으로써, 캠페인 제안에 대한 보다 완전하고 이해하기 쉬운 정보를 제공합니다. Highlights
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
|
![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 PR adds the product name when querying campaign proposal details. However, it introduces a significant security vulnerability: an Insecure Direct Object Reference (IDOR) in the getProposalDetail method, as it retrieves proposal details without verifying user authorization. An authorization check must be implemented to ensure only authorized users can view details. Additionally, the format of the newly added repository method is inconsistent, and the logic for fetching product names could be refactored for better readability.
| CampaignProposal proposal = campaignProposalRepository.findByIdWithTags(proposalId) | ||
| .orElseThrow(() -> new CustomException(BusinessErrorCode.CAMPAIGN_PROPOSAL_NOT_FOUND)); |
There was a problem hiding this comment.
The getProposalDetail method takes a userId and a proposalId but fails to verify if the user is authorized to access the proposal. An attacker could potentially view any campaign proposal by providing its ID. The code even contains a TODO comment acknowledging the missing authorization check. Implement an authorization check to ensure the userId matches either the senderUserId or receiverUserId of the proposal (or has appropriate administrative privileges).
| CampaignProposal proposal = campaignProposalRepository.findByIdWithTags(proposalId) | |
| .orElseThrow(() -> new CustomException(BusinessErrorCode.CAMPAIGN_PROPOSAL_NOT_FOUND)); | |
| CampaignProposal proposal = campaignProposalRepository.findByIdWithTags(proposalId) | |
| .orElseThrow(() -> new CustomException(BusinessErrorCode.CAMPAIGN_PROPOSAL_NOT_FOUND)); | |
| if (!proposal.getSenderUserId().equals(userId) && !proposal.getReceiverUserId().equals(userId)) { | |
| throw new CustomException(BusinessErrorCode.CAMPAIGN_PROPOSAL_FORBIDDEN); | |
| } |
There was a problem hiding this comment.
추후 권한 확인 로직 추가 예정 (데모데이 이후)
...main/java/com/example/RealMatch/brand/domain/repository/BrandAvailableSponsorRepository.java
Outdated
Show resolved
Hide resolved
| String productName = null; | ||
|
|
||
| // TODO: 데모데이 이후에 해당 제품이 없으면 에러 던지는 방향으로 수정 필요!! + 조회 권한 로직 추가 필요(본인만 조회 가능) | ||
| if (proposal.getProductId() != null) { | ||
| productName = brandAvailableSponsorRepository | ||
| .findByBrandIdAndId( | ||
| proposal.getBrand().getId(), | ||
| proposal.getProductId() | ||
| ) | ||
| .map(BrandAvailableSponsor::getName) | ||
| .orElse(null); | ||
| } |
There was a problem hiding this comment.
productName을 가져오는 로직은 Optional 체이닝을 사용하여 더 간결하고 표현력 있게 만들 수 있습니다. 이렇게 하면 명시적인 if 문을 제거하고 코드를 더 간결하게 만들 수 있습니다.
다음과 같이 리팩토링할 수 있습니다:
String productName = Optional.ofNullable(proposal.getProductId())
.flatMap(productId -> brandAvailableSponsorRepository
.findByBrandIdAndId(proposal.getBrand().getId(), productId))
.map(BrandAvailableSponsor::getName)
.orElse(null);이 코드는 현재 31, 32, 34-42 라인의 구현을 대체할 수 있습니다.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds the product name to the campaign proposal detail view. However, a significant security flaw has been identified: the getProposalDetail method lacks an authorization check, leading to an Insecure Direct Object Reference (IDOR) vulnerability. Security controls must be implemented to prevent unauthorized access to sensitive campaign proposal data before merging to production. Additionally, it is suggested to add comments to clarify the use of Optional.ofNullable for the productId field in the CampaignProposal entity, especially since productId is marked as nullable=false.
| // TODO: 데모데이 이후에 해당 제품이 없으면 에러 던지는 방향으로 수정 필요!! + 조회 권한 로직 추가 필요(본인만 조회 가능) | ||
| String productName = Optional.ofNullable(proposal.getProductId()) | ||
| .flatMap(productId -> brandAvailableSponsorRepository | ||
| .findByBrandIdAndId(proposal.getBrand().getId(), productId)) | ||
| .map(BrandAvailableSponsor::getName) | ||
| .orElse(null); | ||
|
|
||
| return CampaignProposalDetailResponse.from(proposal); | ||
| return CampaignProposalDetailResponse.from(proposal, productName); |
There was a problem hiding this comment.
The getProposalDetail method currently lacks an authorization check, leading to an Insecure Direct Object Reference (IDOR) vulnerability. The userId parameter is passed but unused, allowing unauthorized access to campaign proposal details. It is critical to verify that the userId matches either the senderUserId or receiverUserId of the proposal before returning the response. Additionally, please add a comment to clarify why Optional.ofNullable is used for the productId field in the CampaignProposal entity, given that productId is marked as nullable=false.
if (!proposal.getSenderUserId().equals(userId) && !proposal.getReceiverUserId().equals(userId)) {
throw new CustomException(BusinessErrorCode.CAMPAIGN_PROPOSAL_USER_MISMATCH);
}
String productName = Optional.ofNullable(proposal.getProductId())
.flatMap(productId -> brandAvailableSponsorRepository
.findByBrandIdAndId(proposal.getBrand().getId(), productId))
.map(BrandAvailableSponsor::getName)
.orElse(null);
return CampaignProposalDetailResponse.from(proposal, productName);1 participant
fix(#396): 캠페인 제안 상세 조회에 제품 이름 추가
Summary
캠페인 제안 상세조회에서 제품 이름 추가
Changes
Type of Change
Related Issues
#118
참고 사항
브랜드와 제품이 제대로 매칭되어야만 조회가 가능합니다. 만약 브랜드와 제품이 제대로 매칭되지않은 더미 데이터의 경우 productName이 null이 반환됩니다. (데모데이 이후 에러 반환하는 것으로 수정 필요)
또한 데모데이 이후 본인이 보낸 캠페인 제안만 조회 가능하도록 권한 확인 로직도 추가예정입니다.