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

[Feat] controller & service 테스트 코드 작성, [Feat] 시큐리티 & jwt 구현, 예외 필터 구현, [Docs] 7주차 발표자료 업로드 #54

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

Conversation

kangwook1
Copy link
Contributor

@kangwook1 kangwook1 commented May 20, 2024

📋 이슈 번호

close #53
close #58
close #59

🛠 구현 사항

  • service 계층 Member 테스트 코드 작성
  • 시큐리티 & jwt 구현
  • jwt 인증, 예외 필터 구현
  • 7주차 발표자료 업로드

📚 기타

@kangwook1 kangwook1 self-assigned this May 20, 2024
@kangwook1 kangwook1 changed the title [Feat] controller & service 테스트 코드 작성 [Feat] controller & service 테스트 코드 작성, [Feat] 시큐리티 & jwt 구현, 예외 필터 구현 May 23, 2024
@kangwook1 kangwook1 changed the title [Feat] controller & service 테스트 코드 작성, [Feat] 시큐리티 & jwt 구현, 예외 필터 구현 [Feat] controller & service 테스트 코드 작성, [Feat] 시큐리티 & jwt 구현, 예외 필터 구현, [Docs] 7주차 발표자료 업로드 May 23, 2024
@Juser0 Juser0 requested review from Juser0 and NARUBROWN May 26, 2024 16:22
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.

수고했어!
토큰 기반 로그인 구현이 어려웠을텐데 어떤 거 참고해서 구현했는 지 알 수 있을까??

@@ -26,6 +26,11 @@ dependencies {
implementation 'org.springframework.boot:spring-boot-starter-validation'
implementation 'org.springframework.boot:spring-boot-starter-web'
implementation 'org.springdoc:springdoc-openapi-starter-webmvc-ui:2.2.0' // 접속 url: localhost:8080/swagger-ui/index.html
implementation 'com.google.code.gson:gson' // 객체를 json으로 변환해주는 구글의 라이브러리
Copy link
Member

Choose a reason for hiding this comment

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

해당 의존성은 테스트코드만을 위해서 사용되는거야?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

맞아. http 요청을 시뮬레이션해야하니까, json 데이터를 본문에 넣어야하기 떄문에 json으로 변환해주는 클래스를 썼어.

implementation 'org.springframework.boot:spring-boot-starter-security'
// JWT
implementation 'io.jsonwebtoken:jjwt:0.9.1'
implementation 'javax.xml.bind:jaxb-api:2.3.1' //추가하지 않으면 tokenProvider에서 에러 남
Copy link
Member

Choose a reason for hiding this comment

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

나는 이런 의존성을 넣은적이 없어서 그런데, 오류가 났을 때 해당 의존성을 넣으라고 한 레퍼런스를 알려줄 수 있어? 그리고 해당 의존성이 어떤 역할을 수행해?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이게 저 패키지안에 있는 javax.xml.bind.DatatypeConverter 클래스를 이용해서,
jwt토큰을 생성할 때 서명에 쓸 secret key가 base64로 인코딩된 문자열로 넣어도 내부적으로 디코딩해서 알맞는 값으로 넣어주는 역할을 하더라고.

저게 없으면 직접 base64로 secretkey를 디코딩하고 서명에 쓸 키를 따로 만들어줘야하더라.

Copy link
Member

Choose a reason for hiding this comment

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

나는 저 의존성을 넣지 않고 base64로 시크릿 키를 인코딩해도 잘 동작했어서 질문한 거였어
당연히 직접 디코딩하고 하는 작업도 안했었구

Copy link
Contributor Author

Choose a reason for hiding this comment

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

음.. 버전이 낮아서 그런건가?

Copy link
Member

Choose a reason for hiding this comment

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

지금 requestMatchers로 로그인하지 않아도 접근할 수 있는 엔드포인트에 대한 정의를 하는데, 만약 이러한 엔드포인트가 여럿 생긴다고 한다면 코드에 몇줄씩 추가하는 방향으로 밖에 구현이 안되는걸까?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그러게. 이건 고쳐봐야겠다.

Copy link
Member

Choose a reason for hiding this comment

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

엔드포인트 형식을 고치면서 prefix (comments)를 requestMapping으로 빼주면 좋을 것 같아!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

협업 투두리스트 만들 때 이건 고쳤어! 같은 테이블의 id를 사용할 땐 pathvariable로 얻어오고, 다른 테이블의 id가 필요하면 requestparam으로 받아오게끔 했어. 이러면 uri가 좀 깔끔해지더라고.

@@ -1,17 +1,21 @@
package com.appcenter.practice.controller;

import com.appcenter.practice.common.StatusCode;
import com.appcenter.practice.dto.reqeust.member.LoginMemberReq;
Copy link
Member

Choose a reason for hiding this comment

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

패키지 경로에 request가 오타가 났네
이 부분은 수정해줘야 될 것 같아

Copy link
Contributor Author

Choose a reason for hiding this comment

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

땡큐 ㅋㅋ 아예 몰랐네.

@@ -24,8 +25,14 @@ public enum StatusCode {
COMMENT_DELETE(OK,"할 일 삭제 완료"),

/* 400 BAD_REQUEST : 잘못된 요청 */
LOGIN_ID_INVALID(BAD_REQUEST,"아이디가 틀렸습니다."),
EMAIL_INVALID(BAD_REQUEST,"이메일이 틀렸습니다."),
Copy link
Member

Choose a reason for hiding this comment

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

아이디 / 비밀번호가 틀렸다고 했을 때 어떤 부분이 틀렸다고 알려주는 게 적합할까? 한 번 고민해봐

Copy link
Contributor Author

Choose a reason for hiding this comment

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

맞아. 이게 내가 프론트를 모르다보니까 주는 정보를 어디까지 한정해야하는지 고민이 되더라고.
그래서 jwt를 만들 때도 시큐리티의 인증 예외를 처리하는 클래스에서 그냥 인증이 실패했다고 정보를 줄 지, 따로 jwt 인증 예외 필터를 만들어서 jwt 토큰이 유효하지않다고 처리할 지 고민했어서 일단은 후자를 선택했는데, 전자가 더 나은 거같아.

Copy link
Member

Choose a reason for hiding this comment

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

토큰을 활용한 인증에서의 문제와 내가 여기서 잡은 문제는 약간 다르다고 생각해.
형이 답변해준 내용처럼 jwt를 활용한 시큐리티의 인증 예외에서 주는 정보의 범위는 형이 알아서 판단하는 게 맞는데,

여기 내용은 그 내용이 아니라 틀린 부분을 알려주면 안된다는 의도에서 질문을 한거였어
만약 악의적인 사용자가 로그인을 시도했을 때, 아이디가 틀렸습니다 / 비밀번호가 틀렸습니다 라고 틀린부분을 정확하게 알게 된다면 그것으로 공격이 진행될 수 있거든.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

글쿤..

토큰의 조작여부와 유효여부를 판단하는 메소드.
claims.getBody().getExpiration().before(new Date())를 통해 유효여부를 판단한다.
Jwts.parserBuilder().setSigningKey(secretKey).build().parseClaimsJws(token)를 통해 조작여부를 판단한다.
만약 조작이 의심되면 예외를 던진다.
Copy link
Member

Choose a reason for hiding this comment

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

그렇다면 조작이 의심되는 지 어떻게 확인하는 걸까?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그냥 조작이 되면 예외를 던진다를 의심이 된다고 말한거같아 ㅋㅋ..

Copy link
Member

Choose a reason for hiding this comment

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

내가 질문을 한 의도는 조작이 의심되는 상황, 즉 예외가 던져지는 상황이 어떨 때 발생하는 지 형이 알아야 해서 질문을 한거야.
jwt를 사용하는데 어떻게 코드에서 조작된 토큰인지 판별을 형도 알고서 활용하는 게 맞으니깐!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

음 해시 알고리즘을 통해 서명을 만드니까 똑같이 받은 토큰의 헤더와 페이로드를 시크릿키로 암호화해서 대조해보지 않을까? 그렇게해서 틀리면 조작된거라고 판단하는거지.

import java.io.IOException;

@Component
public class JwtExceptionFilter extends OncePerRequestFilter {
Copy link
Member

Choose a reason for hiding this comment

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

AuthenticationFilter의 doFilterInternal과 ExceptionFilter의 doFilterInternal의 차이는 뭐고, 형의 security config에서의 필터 구조는 어떻게 되는거야? 두 필터를 다 거치는거야?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AuthenticationFilter는 토큰을 받아서 유효성 검증을 하고 인증객체를 만들어서 세션 저장소에 저장을 하는거고,
ExceptionFiter는 AuthenticationFilter 앞에 위치시켜놓고 AuthenticationFilter에서 일어나는 예외를 처리하는 필터야.

//subject는 주제로 제목이라고 보면 된다.제목에 memberId를 넣는다.
Claims claims = Jwts.claims().setSubject(memberId.toString());
claims.put("role", role);
Date now = new Date();
Copy link
Member

Choose a reason for hiding this comment

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

Date 타입을 사용한 이유가 있어?
Date 타입은 Java 8 부터 Deprecated 된 걸로 알아

Copy link
Contributor Author

@kangwook1 kangwook1 May 27, 2024

Choose a reason for hiding this comment

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

내가 사용하는 jwt 라이브러리 버전이 좀 오래된건가봐.. Date 형식을 사용하네


@Transactional
public Long signup(SignupMemberReq reqDto){
if(memberRepository.existsByEmail(reqDto.getEmail()))
throw new CustomException(StatusCode.EMAIL_DUPLICATED);
Member member=reqDto.toEntity();
member.passwordEncode(passwordEncoder);
Copy link
Member

Choose a reason for hiding this comment

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

요청 dto의 요소들을 기반으로 인스턴스화를 진행했는데 추가로 변경하는 메서드를 불러온다면 인스턴스화 메서드의 의미가 있을까?

member를 생성하고 setter로 일부 값을 넣어주는 방식이 약간 부자연스러워보여

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이것도 고쳐야겠다!

Comment on lines +6 to +7
@Configuration
@EnableJpaAuditing
Copy link
Collaborator

Choose a reason for hiding this comment

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

따로 Class 정의해서 뺐구나 잘했어

Copy link
Contributor Author

Choose a reason for hiding this comment

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

테스트할 때 오류가 발생하더라고 ㅋㅋ

Comment on lines 35 to 36
//response.getWriter().write(...)는 내부적으로 flush()가 발생해서 따로 호출할 필요 없음.
response.getWriter().write(String.format("{\"message\": \"%s\"}", ex.getStatusCode().getMessage()));
Copy link
Collaborator

Choose a reason for hiding this comment

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

호출할 필요는 없지만 호출한 이유는 뭘 까..??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아 저게 flush를 따로 호출할 필요가 없다는 뜻이었어. 고쳐야겠다.

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