-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FEAT] 스프링 시큐리티 & 로그인 구현 #17
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
각 API에 관한 swagger를 작성해야할 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그 저희 그러면 API 하나 만든다는 거에 스웨거까지 포함으로 하는 걸까요? 그냥 의견이 궁금해서 여쭤보는 것입니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 그렇게 하는게 좋을것 같습니다!
} | ||
|
||
|
||
@GetMapping("/authTest") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 엔드포인트는 임시인가요? 이후 어떻게 바꾸는게 좋을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 배포 후에는 필요 없는(지워야 하는) API 입니다! 저희 개발할 때만 필요할 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어떤 용도인가요? 코드만 봐서는 잘 와닿지 않아서 여쭙습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인가코드 받을 때 사용할 수 있습니다!
} | ||
|
||
@GetMapping("/verify") | ||
public ResponseEntity<MemberAuthResponseDTO> verify( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹시 verify에 email을 붙여서 eamil-verify라고 하는 것은 별로일까요? 한순간이지만 무엇을 인증하는건지 헷갈렸어서요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네네 좋아요!
return userRepository.saveAndFlush(newUser); | ||
} | ||
|
||
private static User createSocialMember(String email, String name, PlatformType platformType, String id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
함수명을 createSocialUser로 변경하면 좋을듯 합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 수정하겟슴다
Optional<University> findByUniversityName(String universityName); | ||
|
||
default University findByUniversityNameOrElseThrowException(String universityName) { | ||
return findByUniversityName(universityName).orElseThrow(() -> new AuthException(NOT_FOUND_EMAIL_REGEX)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exception 타입이 잘못된것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
엇 그러네요 감삼다!
if(!foundUniversity.getEmailRegex().equals(regex)){ | ||
return false; | ||
} | ||
|
||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 코드를
return foundUniversity.getEmailRegex().equals(regex);
이렇게 바꾸는건 어떠신가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아하 좋네요! 변경하겟슴다!
|
||
@RestController | ||
@RequiredArgsConstructor | ||
public class ServerProfileController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 컨트롤러는 applicaion.yml 파일의 profile에 대해 알려주는 용도인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
서버 health check 용도로 생각했습니다!
} | ||
|
||
public NaverMemberVO getNaverUserInfo(String accessToken) { | ||
WebClient webClient = WebClient.builder().build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
별도의 설정을 추가로 하지 않으니 builder보다는
WebClient.create();
로 변경하는것이 좋을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
으흠 넵 감사합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹시 달리 의도하신 부분이 있었나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아뇨 없었습니다! 잘 작동만 하면 그게 좋을거 같아요!
frameOptionsConfig.disable() | ||
) | ||
) | ||
// .userDetailsService(memberAuthService) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹시 여기 주석은 어떤 걸 위한 용도인가요? 추후 추가될 부분인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아마 추가되지 않을 것 같습니다! 다만 조금만 더 고민해볼게요!
.headers((headerConfig) -> | ||
headerConfig.frameOptions(frameOptionsConfig -> | ||
frameOptionsConfig.disable() | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 코드도 위의 다른 코드들과 동일하게
.headers((headerConfig) ->
headerConfig.frameOptions(FrameOptionsConfig::disable)
)
이렇게 적는게 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앗 그게 좋겠네요 감삼다~!
* [FEAT] cors 설정 * [FEAT] User, University 엔티티 생성 * [ADD] validate 의존성 추가 * [DEL] 더미데이터 삭제 * [ADD] 더미데이터 추가 * [DEL] 더미 데이터 삭제 * [ADD] 라이브러리 추가 * [FEAT] Oauth2와 관련된 AuthController 구현 * [ADD] Oauth2와 관련된 AuthException 추가 * [ADD] Oauth2와 관련된 AuthException enum 추가 * [FEAT] Oauth2와 관련된 service단 로직 구현 * [FEAT] Naver provider 구현 * [ADD] AuthType enum 추가 * [ADD] 인증된 객체 AuthUser 추가 * [FEAT] UserDetails 커스텀한 CustomAuthUser 구현 * [FEAT] 토큰 관련 JwtAuthenticationFilter 구현 * [FEAT] 토큰 관련 예외 처리를 위한 필터 구현 * [FEAT] Jwt 관련 Service단 구현 * [FEAT] Jwt 관련 util 역할하는 Provider 구현 * [FEAT] Mail 인증을 위한 config 구현 * [FEAT] Mail 인증을 위한 service단 구현 * [ADD] Mail 객체를 나타내기 위한 VO 생성 * [ADD] Auth 관련 response DTO 추가 * [FEAT] UserDetail 생성을 위해 커스텀한 MemberAuthService 구현 * [ADD] 토큰 재발행에 쓰이는 DTO 추가 * [FEAT] Naver 로그인 관련 login 로직 구현 및 Authorization-server와의 통신 메서드 구현 * [ADD] Naver 유저를 표현하기 위한 VO 추가 * [ADD] Naver의 Authorization-server와 통신을 위한 NaverTokenVO 추가 * [FEAT] 임의의 비밀번호 생성을 위한 utils 구현 * [ADD] redis 사용을 위한 설정파일 추가 * [ADD] 플랫폼타입 enum 추가 * [FEAT] token 관리하는 redis 저장소 구현 * [FEAT] 이메일 인증 관리하는 redis 저장소 구현 * [FEAT] 액세스 토큰 관련 utils 구현 * [ADD] 시큐리티 config 파일 생성 * [DEL] 필요없는 파일 삭제 * [FEAT] 서버 상태 확인을 위한 글로벌 컨트롤러 구현 * [ADD] 액세스 및 리프레시 토큰을 표현하기 위한 VO 생성 * [ADD] university 엔티티 생성 * [FEAT] university Repository 구현 * [FEAT] university 도메인 확인 로직 구현 * [ADD] user 엔티티 생성 * [ADD] Auth 관련 request DTO 생성 * [ADD] User 관련 예외 enum 추가 * [FEAT] User Repository 구현 * [FEAT] 학교 인증 후 정보 갱신하는 로직 구현 * [ADD] 학교 인증 관련된 request DTO 생성 * [ADD] 학교 인증 관련된 response DTO 생성 * [ADD] 회원가입과 관련된 유저를 나타내기 위한 VO 추가 * [ADD] 이메일 인증을 위한 request DTO 추가 * [FIX] 오타 수정 * [CHORE] 변수 이름 변경 * [CHORE] Builder 추가 * [REFACTOR] uri 경로 변경 (코드리뷰) * [REFACTOR] 예외 enum 변경 (코드리뷰) * [FIX] 메서드 이름 변경 (코드리뷰) * [DEL] 필요없는 코드 삭제 (코드리뷰) * [FIX] DTO명 변경 (코드리뷰) * [DEL] 필요없는 코드 삭제 (코드리뷰) * [DEL] 필요없는 코드 삭제 (코드리뷰) * [DEL] 필요없는 코드 삭제 (코드리뷰) * [CHORE] 변수명 변경 (코드리뷰) * [FEAT] 대학 예외 enum 추가 (코드리뷰) * [FIX] 대학 예외 클래스 변경 (코드리뷰) * [DEL] 불필요 코드 삭제 (코드리뷰) * [ADD] DTO 명 변경 및 추가 (코드리뷰) * [FIX] userTable 테이블 명 수정 --------- Co-authored-by: goder-0 <goder0@naver.com>
브랜치
구현 내용/방법
리뷰 필요