Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.springframework.http.ResponseEntity;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.ModelAttribute;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
Expand All @@ -15,10 +16,13 @@
import lombok.RequiredArgsConstructor;
import project.flipnote.cardset.controller.docs.GroupCardSetControllerDocs;
import project.flipnote.cardset.model.CardSetDetailResponse;
import project.flipnote.cardset.model.CardSetSearchRequest;
import project.flipnote.cardset.model.CardSetSummaryResponse;
import project.flipnote.cardset.model.CardSetUpdateRequest;
import project.flipnote.cardset.model.CreateCardSetRequest;
import project.flipnote.cardset.model.CreateCardSetResponse;
import project.flipnote.cardset.service.CardSetService;
import project.flipnote.common.model.response.PagingResponse;
import project.flipnote.common.security.dto.AuthPrinciple;

@RequiredArgsConstructor
Expand Down Expand Up @@ -62,4 +66,13 @@ public ResponseEntity<CardSetDetailResponse> updateCardSet(
return ResponseEntity.ok(res);
}

@GetMapping
public ResponseEntity<PagingResponse<CardSetSummaryResponse>> getCardSets(
@PathVariable("groupId") Long groupId,
@Valid @ModelAttribute CardSetSearchRequest req
) {
PagingResponse<CardSetSummaryResponse> res = cardSetService.getCardSets(groupId, req);

return ResponseEntity.ok(res);
}
Comment on lines +69 to +77
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

인증 및 그룹 유효성 검증을 추가하세요.

이 엔드포인트에는 다음과 같은 보안 문제가 있습니다:

  1. 인증 누락: 다른 엔드포인트들(createCardSet, getCardSet, updateCardSet)과 달리 @AuthenticationPrincipal 어노테이션이 없어 인증되지 않은 사용자도 접근 가능합니다.
  2. 그룹 존재 검증 누락: 존재하지 않는 groupId에 대한 검증이 없습니다.
  3. 접근 권한 검증 필요 여부 불명확: 공개 카드셋만 반환한다면 인증이 불필요할 수 있지만, 그룹 멤버에게 비공개 카드셋도 보여주어야 한다면 접근 권한 검증이 필요합니다.

요구사항에 따라 다음 중 하나를 선택하여 적용하세요:

옵션 1: 인증된 사용자만 접근 가능하고 그룹 멤버는 비공개 카드셋도 조회 가능

 	@GetMapping
 	public ResponseEntity<PagingResponse<CardSetSummaryResponse>> getCardSets(
 		@PathVariable("groupId") Long groupId,
-		@Valid @ModelAttribute CardSetSearchRequest req
+		@Valid @ModelAttribute CardSetSearchRequest req,
+		@AuthenticationPrincipal AuthPrinciple authPrinciple
 	) {
-		PagingResponse<CardSetSummaryResponse> res = cardSetService.getCardSets(groupId, req);
+		PagingResponse<CardSetSummaryResponse> res = cardSetService.getCardSets(groupId, req, authPrinciple.userId());
 
 		return ResponseEntity.ok(res);
 	}

옵션 2: 인증 없이 공개 카드셋만 조회 가능 (현재 구현과 유사)

 	@GetMapping
 	public ResponseEntity<PagingResponse<CardSetSummaryResponse>> getCardSets(
 		@PathVariable("groupId") Long groupId,
 		@Valid @ModelAttribute CardSetSearchRequest req
 	) {
+		// 그룹 존재 여부 검증은 서비스 레이어에서 수행되어야 함
 		PagingResponse<CardSetSummaryResponse> res = cardSetService.getCardSets(groupId, req);
 
 		return ResponseEntity.ok(res);
 	}

서비스 레이어에서 그룹 존재 검증도 추가해야 합니다.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/main/java/project/flipnote/cardset/controller/GroupCardSetController.java
around lines 69-77, this GET handler lacks authentication and group
existence/permission checks; update it per one of the two allowed behaviors:
Option 1 — require authenticated user by adding @AuthenticationPrincipal to the
method signature and pass the principal to the service, then have the service
validate the group exists and that the caller is a group member so it returns
both public and private cardsets; Option 2 — keep it public but explicitly
validate the group exists in the service and ensure the service only returns
public cardsets; in either case move group existence validation into the service
layer and throw a proper 404 if the group is missing.

}
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package project.flipnote.cardset.model;

import project.flipnote.cardset.entity.CardSet;

public record CardSetSummaryResponse(
Long cardSetId,
Long groupId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;

import project.flipnote.cardset.entity.CardSet;
import project.flipnote.cardset.model.CardSetInfo;
import project.flipnote.group.entity.Category;

Expand All @@ -19,4 +18,11 @@ Page<CardSetInfo> searchByNameContainingAndCategory(
);

List<CardSetInfo> findAllByIdWithImageRefId(Set<Long> cardSets);

Page<CardSetInfo> searchByGroupIdAndNameContainingAndCategory(
Long groupId,
String name,
Category category,
Pageable pageable
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import java.util.List;
import java.util.Set;

import org.checkerframework.checker.units.qual.C;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.Pageable;
Expand All @@ -22,7 +21,6 @@

import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import project.flipnote.cardset.entity.CardSet;
import project.flipnote.cardset.entity.QCardSet;
import project.flipnote.cardset.entity.QCardSetMetadata;
import project.flipnote.cardset.model.CardSetInfo;
Expand All @@ -47,6 +45,36 @@ public Page<CardSetInfo> searchByNameContainingAndCategory(
String name,
Category category,
Pageable pageable
) {
return searchByGroupIdAndNameContainingAndCategory(null, name, category, pageable);
}

public List<CardSetInfo> findAllByIdWithImageRefId(Set<Long> cardSets) {
return queryFactory.select(
Projections.constructor(
CardSetInfo.class,
cardSet,
cardSet.group,
cardSet.name,
cardSet.category,
cardSet.hashtag,
cardSet.imageUrl,
imageRef.id
))
.from(cardSet)
.where(cardSet.id.in(cardSets))
.leftJoin(imageRef)
.on(imageRef.referenceType.eq(ReferenceType.CARD_SET)
.and(imageRef.referenceId.eq(cardSet.id)))
.fetch();
}
Comment on lines +52 to +70
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

@OverRide 어노테이션이 누락되었습니다.

이 메서드가 인터페이스를 구현하는 경우 @Override 어노테이션을 추가하여 명시적으로 표시해야 합니다.

+	@Override
 	public List<CardSetInfo> findAllByIdWithImageRefId(Set<Long> cardSets) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public List<CardSetInfo> findAllByIdWithImageRefId(Set<Long> cardSets) {
return queryFactory.select(
Projections.constructor(
CardSetInfo.class,
cardSet,
cardSet.group,
cardSet.name,
cardSet.category,
cardSet.hashtag,
cardSet.imageUrl,
imageRef.id
))
.from(cardSet)
.where(cardSet.id.in(cardSets))
.leftJoin(imageRef)
.on(imageRef.referenceType.eq(ReferenceType.CARD_SET)
.and(imageRef.referenceId.eq(cardSet.id)))
.fetch();
}
@Override
public List<CardSetInfo> findAllByIdWithImageRefId(Set<Long> cardSets) {
return queryFactory.select(
Projections.constructor(
CardSetInfo.class,
cardSet,
cardSet.group,
cardSet.name,
cardSet.category,
cardSet.hashtag,
cardSet.imageUrl,
imageRef.id
))
.from(cardSet)
.where(cardSet.id.in(cardSets))
.leftJoin(imageRef)
.on(imageRef.referenceType.eq(ReferenceType.CARD_SET)
.and(imageRef.referenceId.eq(cardSet.id)))
.fetch();
}
🤖 Prompt for AI Agents
In
src/main/java/project/flipnote/cardset/repository/CardSetRepositoryCustomImpl.java
around lines 52 to 70, the method findAllByIdWithImageRefId is missing the
@Override annotation; add the @Override annotation immediately above the method
signature to explicitly indicate it implements the interface method (no other
logic changes required).


@Override
public Page<CardSetInfo> searchByGroupIdAndNameContainingAndCategory(
Long groupId,
String name,
Category category,
Pageable pageable
) {
List<OrderSpecifier<?>> orders = new ArrayList<>();

Expand Down Expand Up @@ -91,7 +119,7 @@ public Page<CardSetInfo> searchByNameContainingAndCategory(
imageRef.id
))
.from(cardSet)
.where(buildCardSetSearchFilterConditions(name, category))
.where(buildCardSetSearchFilterConditions(groupId, name, category))
.leftJoin(imageRef)
.on(imageRef.referenceType.eq(ReferenceType.CARD_SET)
.and(imageRef.referenceId.eq(cardSet.id)));
Expand All @@ -109,32 +137,12 @@ public Page<CardSetInfo> searchByNameContainingAndCategory(
Long total = queryFactory
.select(cardSet.count())
.from(cardSet)
.where(buildCardSetSearchFilterConditions(name, category))
.where(buildCardSetSearchFilterConditions(groupId, name, category))
.fetchOne();

return new PageImpl<>(content, pageable, total != null ? total : 0L);
}

public List<CardSetInfo> findAllByIdWithImageRefId(Set<Long> cardSets) {
return queryFactory.select(
Projections.constructor(
CardSetInfo.class,
cardSet,
cardSet.group,
cardSet.name,
cardSet.category,
cardSet.hashtag,
cardSet.imageUrl,
imageRef.id
))
.from(cardSet)
.where(cardSet.id.in(cardSets))
.leftJoin(imageRef)
.on(imageRef.referenceType.eq(ReferenceType.CARD_SET)
.and(imageRef.referenceId.eq(cardSet.id)))
.fetch();
}

private OrderSpecifier<?> toOrderSpecifier(
NumberPath<?> path,
Sort.Order order
Expand All @@ -150,8 +158,13 @@ private BooleanExpression categoryEquals(Category category) {
return category == null ? null : cardSet.category.eq(category);
}

private BooleanExpression[] buildCardSetSearchFilterConditions(String name, Category category) {
return new BooleanExpression[]{
private BooleanExpression groupIdEquals(Long groupId) {
return groupId == null ? null : cardSet.group.id.eq(groupId);
}

private BooleanExpression[] buildCardSetSearchFilterConditions(Long groupId, String name, Category category) {
return new BooleanExpression[] {
groupIdEquals(groupId),
nameContains(name),
categoryEquals(category),
cardSet.publicVisible.isTrue()
Expand Down
25 changes: 23 additions & 2 deletions src/main/java/project/flipnote/cardset/service/CardSetService.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,10 @@
import project.flipnote.group.exception.GroupErrorCode;
import project.flipnote.group.repository.GroupMemberRepository;
import project.flipnote.group.repository.GroupRepository;
import project.flipnote.image.entity.Image;
import project.flipnote.group.service.GroupService;
import project.flipnote.image.entity.ImageMeta;
import project.flipnote.image.entity.ImageRef;
import project.flipnote.image.entity.ReferenceType;
import project.flipnote.image.exception.ImageErrorCode;
import project.flipnote.image.service.ImageRefService;
import project.flipnote.image.service.ImageService;
import project.flipnote.user.entity.UserProfile;
Expand All @@ -61,6 +60,7 @@ public class CardSetService {
private final CardSetMetadataRepository cardSetMetadataRepository;
private final ImageService imageService;
private final ImageRefService imageRefService;
private final GroupService groupService;

@Value("${image.default.cardSet}")
private String defaultCardSetImage;
Expand Down Expand Up @@ -326,4 +326,25 @@ public void decrementBookmarkCount(Long cardSetId) {
public void decrementBookmarkCount(List<Long> cardSetIds) {
cardSetMetadataRepository.decrementBookmarkCount(cardSetIds);
}

/**
* 특정 그룹의 카드셋 목록을 페이지 단위로 조회
*
* @param groupId 조회할 그룹의 ID
* @param req 조회 조건 및 페이징 정보를 포함한 요청 DTO
* @return 페이지 단위로 조회된 카드셋 목록
* @author 윤정환
*/
public PagingResponse<CardSetSummaryResponse> getCardSets(long groupId, CardSetSearchRequest req) {
groupService.validateGroupExists(groupId);

// TODO: Projection 튜닝 필요
Page<CardSetInfo> cardSetPage = cardSetRepository.searchByGroupIdAndNameContainingAndCategory(
groupId, req.getKeyword(), Category.from(req.getCategory()), req.getPageRequest()
);

Page<CardSetSummaryResponse> res = cardSetPage.map(CardSetSummaryResponse::from);

return PagingResponse.from(res);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ public ResponseEntity<ApiResponse<List<ApiResponse.FieldError>>> handleValidatio

@ExceptionHandler(MissingServletRequestParameterException.class)
public ResponseEntity<String> handleMissingServletRequestParameter(
MissingServletRequestParameterException exception) {
MissingServletRequestParameterException exception
) {
String missingParam = exception.getParameterName();
String message = String.format("필수 파라미터 '%s'가 없습니다.", missingParam);
return ResponseEntity.badRequest().body(message);
Expand Down
15 changes: 13 additions & 2 deletions src/main/java/project/flipnote/group/service/GroupService.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,9 @@
import project.flipnote.group.repository.GroupRepository;
import project.flipnote.group.repository.GroupRolePermissionRepository;
import project.flipnote.groupjoin.exception.GroupJoinErrorCode;
import project.flipnote.image.entity.Image;
import project.flipnote.image.entity.ImageMeta;
import project.flipnote.image.entity.ImageRef;
import project.flipnote.image.entity.ReferenceType;
import project.flipnote.image.exception.ImageErrorCode;
import project.flipnote.image.service.ImageRefService;
import project.flipnote.image.service.ImageService;
import project.flipnote.user.entity.UserProfile;
Expand Down Expand Up @@ -448,6 +446,19 @@ public CursorPagingResponse<GroupInfo> findMyGroup(AuthPrinciple authPrinciple,
return createGroupInfoCursorPagingResponse(req, groups);
}

/**
* 지정된 그룹 ID가 존재하는지 검사합니다.
*
* @param groupId 존재 여부를 확인할 그룹의 ID
* @throws BizException 그룹이 존재하지 않을 경우 발생
* @author 윤정환
*/
public void validateGroupExists(Long groupId) {
if (!groupRepository.existsById(groupId)) {
throw new BizException(GroupErrorCode.GROUP_NOT_FOUND);
}
}
Comment on lines +449 to +460
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

소프트 삭제된 그룹에 대한 검증이 누락되었습니다.

이 메서드는 existsById를 사용하여 deletedAt을 확인하지 않습니다. 반면 기존의 validateGroup 메서드(Line 111-115)는 existsByIdAndDeletedAtIsNull을 사용하여 삭제된 그룹을 필터링합니다. 이러한 불일치로 인해 소프트 삭제된 그룹이 유효한 것으로 검증될 수 있습니다.

다음과 같이 수정하세요:

 	public void validateGroupExists(Long groupId) {
-		if (!groupRepository.existsById(groupId)) {
+		if (!groupRepository.existsByIdAndDeletedAtIsNull(groupId)) {
 			throw new BizException(GroupErrorCode.GROUP_NOT_FOUND);
 		}
 	}
🤖 Prompt for AI Agents
In src/main/java/project/flipnote/group/service/GroupService.java around lines
449 to 460, the validateGroupExists method uses groupRepository.existsById which
ignores soft-deleted rows; replace the existence check with the same
soft-delete-aware query used in validateGroup (e.g.,
groupRepository.existsByIdAndDeletedAtIsNull(groupId)) and throw
BizException(GroupErrorCode.GROUP_NOT_FOUND) when that returns false so
soft-deleted groups are treated as not found.


//리스트 조회시 response 생성
private CursorPagingResponse<GroupInfo> createGroupInfoCursorPagingResponse(GroupListRequest req,
List<GroupInfo> groups) {
Expand Down
Loading