[BE] feat: GroupAccessCode를 암호화 #456
Conversation
Test Results72 tests 72 ✅ 3s ⏱️ Results for commit 3ff04d2. ♻️ This comment has been updated with latest results. |
donghoony
requested review from
skylar1220,
Kimprodp and
nayonsoso
and removed request for
skylar1220 and
Kimprodp
...d/src/main/java/reviewme/reviewgroup/domain/exception/InvalidProjectNameLengthException.java
Show resolved
Hide resolved
.../src/main/java/reviewme/reviewgroup/domain/exception/InvalidRevieweeNameLengthException.java
Outdated
Show resolved
Hide resolved
...rc/main/java/reviewme/reviewgroup/domain/exception/ReviewGroupNotFoundByReviewException.java
Outdated
Show resolved
Hide resolved
...a/reviewme/reviewgroup/domain/exception/ReviewGroupNotFoundByReviewRequestCodeException.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/reviewme/reviewgroup/repository/ReviewGroupRepository.java
Show resolved
Hide resolved
...nd/src/main/java/reviewme/reviewgroup/domain/exception/ReviewGroupUnAuthorizedException.java
Outdated
Show resolved
Hide resolved
| @Service | ||
| @Transactional(readOnly = true) | ||
| @RequiredArgsConstructor | ||
| public class ReviewGroupFinder { |
There was a problem hiding this comment.
ReviewService 에서는 Group을 찾기 위해서 사용되고 있지만,
ReviewDetailLookupService 에서는 사용되지 않고 있어요. Review 패키지의 서비스에서 group을 찾을 때 어떤 경우에는 사용하고 어떤 경우에는 사용하지 않는지 헷갈릴 것 같아요..
단순히 groupAccessCode를 검증해서 찾는 것이 Group의 책임이라 느껴져서 사용하는 것이라면 두가지 방식에 대한 제안이 있습니다!
ReviewGroup에 직접 물어보는 방식ReviewGroup패키지에ReviewGroupAuthService를 두고, 인증이 필요한 경로에서ArgumentResolver가ReviewGroupAuthService로 그룹을 검증해서 찾아서 컨트롤러에 파라미터로 전달하는 방식 (Review쪽Service에서는ReviewGroup연관이 없어짐)
There was a problem hiding this comment.
이 부분은 제가 확인을 못 했네요. 제 의도는 Review 패키지에서 ReviewGroup repository를 참조하지 않게 하는 것이 목적입니다. 다른 패키지니 API와 같이 서비스 레이어에서 주고받으면 좋을 것이라고 생각했어요.
| @@ -10,9 +10,9 @@ public interface ReviewGroupRepository extends JpaRepository<ReviewGroup, Long> | |||
|
|
|||
| Optional<ReviewGroup> findByReviewRequestCode(String reviewRequestCode); | |||
|
|
|||
| Optional<ReviewGroup> findByReviewRequestCodeAndGroupAccessCode_Code(String reviewRequestCode, String groupAccessCode); | |||
| Optional<ReviewGroup> findByReviewRequestCodeAndGroupAccessCode(String reviewRequestCode, String groupAccessCode); | |||
|
|
||
| boolean existsByReviewRequestCode(String reviewRequestCode); | ||
|
|
||
| boolean existsByReviewRequestCodeAndGroupAccessCode_Code(String reviewRequestCode, String groupAccessCode); | ||
| boolean existsByReviewRequestCodeAndGroupAccessCode(String reviewRequestCode, String groupAccessCode); |
There was a problem hiding this comment.
여기도 마찬가지로 사용하지 않는 메서드네요!
There was a problem hiding this comment.
java의 MessageDigest를 이용해서 해시코드를 생성할 수 있군요 알아갑니당😋😋
| public String encode(String groupAccessCode) { | ||
| validateGroupAccessCode(groupAccessCode); | ||
| return encoder.encode(SHA_256, groupAccessCode); | ||
| } |
There was a problem hiding this comment.
encdoe 로직 내부에서 코드의 형식 검증을 하는 것이 적절한지 고민이에요.
현재 encode는 리뷰 그룹 생성할 때, 비밀번호 check 할 때 두 곳에서 쓰입니다.
- 리뷰 그룹 생성할 때는 사용자가 입력해 저장되기 전이기때문에 형식 검증을 하는 것이 당연히 맞다고 생각해요.
- 하지만 비밀번호 check 할 때는 형식을 틀리게 입력해도 상관없습니당. 어차피 일치하는 groupAccessCode가 있는지 확인하기 때문이죠. 그렇기에 형식 검증이 이 경우에는 없어도 된다고 생각해요!
따라서, 리뷰 그룹 생성할 때만 형식 검증이 필요하니
검증 로직을 encode할 때가 아닌 리뷰 그룹 생성 로직에만 넣는 것은 어떨까요?
|
#489 으로 마무리합니다, 패키지 이야기는 의논을 꾸준히 해 볼게요 |
🚀 어떤 기능을 구현했나요 ?
🔥 어떻게 해결했나요 ?
중요하게 바뀐 사안
ReviewService에서ReviewGroupRepository를 가져오는 건 두 패키지를 나누는 기준을 흐리게 만든다고 생각해요.ReviewGroupFinder에서 진행합니다. (ReviewGroupService에서 하려고 했는데,ReviewGroup을 리턴하는ReviewGroupFinder와 Web Response를 리턴하는ReviewGroupService가 헷갈릴 것이라고 생각했어요)UnAuthorizedException이 살아났어요.📝 어떤 부분에 집중해서 리뷰해야 할까요?
📚 참고 자료, 할 말