[Spring 장바구니 - 1단계] 더즈(이동규) 미션 제출합니다. #18
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ksy90101
reviewed
Jun 3, 2022
Comment on lines
+30
to
+33
| @ExceptionHandler | ||
| public ResponseEntity<String> loginExceptionHandler(RuntimeException exception) { | ||
| return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(exception.getMessage()); | ||
| } |
There was a problem hiding this comment.
ExceptionHandler를 따로 만들어서 처리하는게 좋을거 같습니다!
아울러 ExptionResponse DTO를 따로 만들어도 좋을거 같아요!
또한 RuntimeException은 너무 포괄적일거 같은데 좀더 타입을 좀히면 좋을거 같네요. 사이드 이펙트가 발생할듯 해서요.
ksy90101
reviewed
Jun 3, 2022
| @Getter | ||
| public class Email { | ||
|
|
||
| private static final String regex = "^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+@[a-zA-Z0-9.-]+$"; |
ksy90101
approved these changes
Jun 3, 2022
|
|
||
| public Customer signUp(CustomerRequest request) { | ||
| validateEmailDuplicated(request.getEmail()); | ||
| Password password = new Password(request.getPassword()); |
There was a problem hiding this comment.
요청이 들어왔을때 password를 암호화해서 하면 어떨까요?
지금은 개발자가 로그를 찍으면 해당 사용자의 패스워드를 볼 수 잇는 상태인데요.
필터 단에서 하면 되지 않을까 합니다.
엄청 중요한건 아니니 다음 미션에 시간 나시면 적용해주셔도 됩니다 :)
| @Getter | ||
| @RequiredArgsConstructor | ||
| @EqualsAndHashCode | ||
| public class EncryptedPassword { |
There was a problem hiding this comment.
Password를 받아서 이 객체가 생성돌때 Encrypted가 되는것도 좋을거 같은데 혹시 밖에서 하는 이유가 있나요?
There was a problem hiding this comment.
DB에서 가져올 때는 암호화된 비밀번호를 가져오는데 이 때는 다시 한 번 더 암호화를 하는 게 아니라 그대로 넣어서 Customer를 생성해야 합니다. 그래서 밖에서 암호화해서 넣어주거나 그대로 넣어주거나 2가지 생성 방법을 선택했었습니다.
Comment on lines
+10
to
+12
| private static final int MIN_LENGTH = 2; | ||
| private static final int MAX_LENGTH = 10; | ||
| private final String value; |
Comment on lines
+19
to
+24
| MessageDigest digest = MessageDigest.getInstance("SHA"); | ||
| digest.update(input.getBytes(UTF_8)); | ||
|
|
||
| StringBuilder encrypted = new StringBuilder(); | ||
| for (byte each : digest.digest()) { | ||
| encrypted.append(String.format("%02X", each)); |
| assertAll( | ||
| () -> assertThat(response.getNickname()).isEqualTo("does"), | ||
| () -> assertThat(response.getAccessToken()).isEqualTo("access-token"), | ||
| () -> verify(customerService).findByEmail(email), |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요 럿고! 4기 더즈입니다. 레벨 2 마지막 미션에서 만나게 되었네요 잘 부탁드려요ㅎㅎ
프론트와 협의한 API는 docs 하위 리드미에 있습니다.
그리고 이번 미션은 레거시 코드를 제공받고 시작했는데요. 1단계에선
auth패키지 하위의Customer관련 기능과 로그인 기능만 손대고shoppingcart패키지 부분은 깨졌던 테스트만 통과시키도록 수정만 하였습니다. 1단계에선auth패키지 중심으로 피드백 주시면 감사하겠습니다!!