Skip to content
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

[Docs] week6 & week7 발표자료 업로드 [Feat] 스프링 시큐리티 & MemberService 테스트코드 작성 #48

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

82everywin
Copy link
Member

@82everywin 82everywin commented May 20, 2024

📋 이슈 번호

close #62
close #63

🛠 구현 사항

  • week6 발표자료

  • service 계층 Member 테스트 코드 작성

  • week7 발표자료

  • 스프링 시큐리티 및 로그인

📚 기타

@82everywin 82everywin requested review from NARUBROWN and Juser0 May 20, 2024 09:00
@82everywin 82everywin self-assigned this May 20, 2024
@82everywin 82everywin changed the title [Feat] week5 발표자료 업로드 & MemberService 테스트코드 작성 [Feat] week6 발표자료 업로드 & MemberService 테스트코드 작성 May 21, 2024
@82everywin 82everywin changed the title [Feat] week6 발표자료 업로드 & MemberService 테스트코드 작성 [Docs] week6 & week7 발표자료 업로드 [Feat] 스프링 시큐리티 & MemberService 테스트코드 작성 May 27, 2024
Copy link
Member

@Juser0 Juser0 left a comment

Choose a reason for hiding this comment

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

수고했어!

protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
String authorizationHeader = request.getHeader(HttpHeaders.AUTHORIZATION);

// 토큰 전송 x -> 로그인 x
Copy link
Member

Choose a reason for hiding this comment

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

if문의 로직을 나눠서 적는 이유가 있을까?

.authorizeHttpRequests((authorizeRequests) ->
authorizeRequests
.requestMatchers("/","/swagger-ui/**","/v3/api-docs/**").permitAll()
.requestMatchers("/api/members","api/members/login").permitAll()
Copy link
Member

Choose a reason for hiding this comment

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

requestMatchers에 인증없이 접근 가능한 uri들이 100개면 100줄의 permitAll을 담게 되는걸까?

@EnableWebSecurity
public class SecurityConfig {

@Autowired
Copy link
Member

Choose a reason for hiding this comment

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

필드 주입을 사용한 이유가 있을까?

@@ -20,6 +25,13 @@
public class MemberController {
private final MemberService memberService;

@Value("${jwt.secret")
private String key;
Copy link
Member

Choose a reason for hiding this comment

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

해당 필드가 컨트롤러에 있는 이유가 뭐야?
tokenprovider에서 사용되는 요소 아녀?


private final JwtTokenProvider jwtTokenProvider;

private CustomUserDetailService userDetailService;
Copy link
Member

Choose a reason for hiding this comment

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

해당 서비스를 가져온 이유는 뭐야?

@@ -91,13 +96,13 @@ public void delete(Long id){
}

// 회원 조회 메서드
private Member throwFindbyId(Long id){
public Member throwFindbyId(Long id){
Copy link
Member

Choose a reason for hiding this comment

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

해당 메서드들의 접근제한자를 public으로 바꾼 이유가 뭐야?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 스프링 시큐리티와 로그인 [Docs] 7주차 발표자료 업로드
3 participants