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 @@ -55,6 +55,15 @@ public void createComposer(ComposerCreateDto dto, User user) {
* @param user 요청한 유저 (인증 객체)
* @return 좋아요 생성 시 {@code true}, 취소 시 {@code false}를 담은 DTO
*/
@Transactional
public void deleteComposer(Long composerId, User user) {
if (user.getId() != 5L) {
throw new BusinessException(CommonErrorStatus.FORBIDDEN);
}
Comment on lines +60 to +62

Choose a reason for hiding this comment

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

security-high high

The authorization check relies on a hardcoded user ID (5L) to identify the administrator. This is a critical security vulnerability as user IDs are often predictable, potentially leading to privilege escalation if a regular user is assigned this ID. It also introduces maintainability issues, as this 'magic number' is difficult to understand and manage if admin policies change. It is strongly recommended to implement a proper Role-Based Access Control (RBAC) system. This involves adding a role field to the User entity and checking for an ADMIN role instead of a hardcoded ID. This will enhance both security and maintainability.

Suggested change
if (user.getId() != 5L) {
throw new BusinessException(CommonErrorStatus.FORBIDDEN);
}
if (!user.isAdmin()) {
throw new BusinessException(CommonErrorStatus.FORBIDDEN);
}

Composer composer = entityUtils.getEntity(composerId, Composer.class);
composerRepository.delete(composer);
}

@Transactional
public ComposerLikeResponseDto toggleLike(Long composerId, User user) {
Composer composer = entityUtils.getEntity(composerId, Composer.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ public void createComposer(@Valid @RequestBody ComposerCreateDto dto, User user)
composerService.createComposer(dto, user);
}

@DeleteMapping("/{composerId}")
@ResponseStatus(HttpStatus.NO_CONTENT)
public void deleteComposer(@PathVariable Long composerId, User user) {
composerService.deleteComposer(composerId, user);
}

@PostMapping("/{composerId}/like")
public ResponseEntity<ComposerLikeResponseDto> toggleComposerLike(@PathVariable Long composerId, User user) {
ComposerLikeResponseDto response = composerService.toggleLike(composerId, user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static org.springframework.restdocs.cookies.CookieDocumentation.requestCookies;
import static org.springframework.restdocs.payload.PayloadDocumentation.fieldWithPath;
import static org.springframework.restdocs.request.RequestDocumentation.parameterWithName;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

Expand Down Expand Up @@ -76,6 +77,35 @@ public class ComposerControllerTest extends ControllerTestSupport {
));
}

@Test
void 작곡가를_삭제한다() throws Exception {
Comment on lines +80 to +81

Choose a reason for hiding this comment

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

medium

현재 테스트는 작곡가 삭제 성공 케이스(204 No Content)만 검증하고 있습니다. 하지만 서비스 로직에는 관리자(ID=5)만 삭제를 수행할 수 있는 권한 검사 로직이 포함되어 있습니다.

권한이 없는 사용자가 삭제를 시도했을 때 예상대로 403 Forbidden 응답을 반환하는지에 대한 테스트 케이스를 추가하여 코드의 안정성을 높이는 것이 좋습니다.

// given
Long composerId = 1L;
Cookie cookie = new Cookie(COOKIE_NAME, "access_token");

// when
ResultActions result = mockMvc.perform(delete("/composers/{composerId}", composerId)
.cookie(cookie)
);

// then
result.andExpect(status().isNoContent())
.andDo(restDocsHandler.document(
resource(ResourceSnippetParameters.builder()
.tag("Composer API")
.summary("작곡가 삭제")
.description("관리자가 작곡가를 삭제합니다.")
.pathParameters(
parameterWithName("composerId").description("삭제할 작곡가 ID")
)
.build()
),
requestCookies(
cookieWithName(COOKIE_NAME).description("유저의 토큰")
)
));
}

@Test
void 작곡가_좋아요를_토글한다() throws Exception {
// given
Expand Down
Loading