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 @@ -48,7 +48,7 @@ SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
.requestMatchers("/auth/signup").permitAll()
.requestMatchers("/auth/login").permitAll()
.requestMatchers(HttpMethod.PUT, "/auth/password-reset").permitAll()
.requestMatchers("/composers").permitAll()
.requestMatchers(HttpMethod.GET, "/composers").permitAll()
.requestMatchers(HttpMethod.GET, "/posts/**").permitAll()
.requestMatchers("/users/check-nickname").permitAll()
.requestMatchers("/composers/{composerId}/posts").permitAll()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package com.daramg.server.composer.application;

import com.daramg.server.common.application.EntityUtils;
import com.daramg.server.common.exception.BusinessException;
import com.daramg.server.common.exception.CommonErrorStatus;
import com.daramg.server.composer.domain.Composer;
import com.daramg.server.composer.domain.ComposerLike;
import com.daramg.server.composer.dto.ComposerCreateDto;
import com.daramg.server.composer.dto.ComposerLikeResponseDto;
import com.daramg.server.composer.repository.ComposerLikeRepository;
import com.daramg.server.composer.repository.ComposerRepository;
import com.daramg.server.user.domain.User;
import lombok.RequiredArgsConstructor;
import org.springframework.dao.DataIntegrityViolationException;
Expand All @@ -16,8 +20,29 @@
public class ComposerService {

private final ComposerLikeRepository composerLikeRepository;
private final ComposerRepository composerRepository;
private final EntityUtils entityUtils;

@Transactional
public void createComposer(ComposerCreateDto dto, User user) {
if (user.getId() != 1L) {
throw new BusinessException(CommonErrorStatus.FORBIDDEN);
}
Comment on lines +28 to +30

Choose a reason for hiding this comment

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

security-high high

The createComposer method uses a hardcoded user ID (user.getId() != 1L) for administrative checks, which is a critical security vulnerability. This brittle approach allows for easy privilege escalation, as the first registered user (ID 1) would gain admin privileges, potentially enabling an attacker to bypass authorization. It is highly recommended to implement a robust Role-Based Access Control (RBAC) system, such as adding an isAdmin() method to the User entity or utilizing Spring Security's @PreAuthorize annotation, instead of relying on hardcoded user IDs.

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

Composer composer = Composer.builder()
.koreanName(dto.getKoreanName())
.englishName(dto.getEnglishName())
.nativeName(dto.getNativeName())
.gender(dto.getGender())
.nationality(dto.getNationality())
.birthYear(dto.getBirthYear())
.deathYear(dto.getDeathYear())
.bio(dto.getBio())
.era(dto.getEra())
.continent(dto.getContinent())
.build();
composerRepository.save(composer);
}

/**
* 작곡가 좋아요 상태를 토글(Toggle)합니다.
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,15 @@ public class Composer extends BaseEntity<Composer> {
@Builder
private Composer(@NonNull String koreanName, @NonNull String englishName,
String nativeName, @NonNull Gender gender, String nationality,
Short birthYear, Short deathYear, Era era, Continent continent) {
Short birthYear, Short deathYear, String bio, Era era, Continent continent) {
this.koreanName = koreanName;
this.englishName = englishName;
this.nativeName = nativeName;
this.gender = gender;
this.nationality = nationality;
this.birthYear = birthYear;
this.deathYear = deathYear;
this.bio = bio;
this.era = era;
this.continent = continent;
}
Expand Down
20 changes: 20 additions & 0 deletions src/main/java/com/daramg/server/composer/dto/ComposerCreateDto.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.daramg.server.composer.dto

import com.daramg.server.composer.domain.Continent
import com.daramg.server.composer.domain.Era
import com.daramg.server.composer.domain.Gender
import jakarta.validation.constraints.NotBlank
import jakarta.validation.constraints.NotNull

data class ComposerCreateDto(
@get:NotBlank val koreanName: String,
@get:NotBlank val englishName: String,
val nativeName: String? = null,
@get:NotNull val gender: Gender,
val nationality: String? = null,
val birthYear: Short? = null,
val deathYear: Short? = null,
val bio: String? = null,
val era: Era? = null,
val continent: Continent? = null,
)
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package com.daramg.server.composer.presentation;

import com.daramg.server.composer.application.ComposerService;
import com.daramg.server.composer.dto.ComposerCreateDto;
import com.daramg.server.composer.dto.ComposerLikeResponseDto;
import com.daramg.server.user.domain.User;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.bind.annotation.*;

@RestController
@RequiredArgsConstructor
Expand All @@ -17,6 +17,12 @@ public class ComposerController {

private final ComposerService composerService;

@PostMapping
@ResponseStatus(HttpStatus.CREATED)
public void createComposer(@Valid @RequestBody ComposerCreateDto dto, User user) {

Choose a reason for hiding this comment

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

security-high high

The createComposer method accepts a User object as a parameter without the @AuthenticationPrincipal annotation. In Spring MVC, if a POJO parameter is not annotated, the framework may attempt to bind request parameters (query strings, form data) to the object's properties. An attacker could potentially exploit this by providing an id parameter in the request (e.g., POST /composers?id=1) to impersonate the administrator or other users, bypassing the intended security checks in the service layer.

Recommendation: Use the @AuthenticationPrincipal annotation to ensure that the User object is resolved exclusively from the authenticated security context.

Suggested change
public void createComposer(@Valid @RequestBody ComposerCreateDto dto, User user) {
public void createComposer(@Valid @RequestBody ComposerCreateDto dto, @AuthenticationPrincipal User user) {

composerService.createComposer(dto, user);
}

@PostMapping("/{composerId}/like")
public ResponseEntity<ComposerLikeResponseDto> toggleComposerLike(@PathVariable Long composerId, User user) {
ComposerLikeResponseDto response = composerService.toggleLike(composerId, user);
Expand Down
Loading