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

[Spring 지하철 경로 조회 - 1,2단계] 아마찌(신지혜) 미션 제출합니다. #66

Merged
merged 22 commits into from
May 16, 2021

Conversation

NewWisdom
Copy link

안녕하세요 재연링! 아마찌입니다 👻
이번 지하철 경로 조회 미션을 함께하게 되어 정말 감사하게 생각합니다 🙌

미션을 구현하기 전 Spring MVC Config와 Auth 학습테스트를 먼저 진행했는데
비슷한 맥락이라 미션을 진행하기 조금 더 수월했던 것 같아요!
이번 미션은 인증과 인가를 하기 위해 토큰을 사용하였는데요,
인가에 관한 토큰 검증 처리를 HandlerMethodArgumentResolver를 통해
공통된 로직을 분리하면서 스프링의 다양한 방법들을 경험해 볼 수 있어 정말 즐겁게 진행했습니다!

또 발급한 토큰을 저장하는 방법도 다양했는데,
로컬 스토리지에도 저장해보았다가 이 글을 참고하여
vue-cookie를 이용해 cookie에 저장하도록 변경도 해보았습니다.

이번 미션 잘 부탁드려요 재연링 👏
경로 조회 미션동안 많이 괴롭혀주세요!!! ㅎㅎㅎㅎ
감사합니다 🙇‍♂️

Copy link

@jaeyeonling jaeyeonling left a comment

Choose a reason for hiding this comment

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

안녕하세요 아마찌! 지하철 경로 조회 미션을 함께 할 재연링입니다 😄
요구사항에 맞게 잘 구현해주셨습니다 💯
소소한 피드백 남겨두었으니 확인 부탁드려요!

@@ -0,0 +1,41 @@
## 기능 목록

Choose a reason for hiding this comment

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

요구사항 정리 👍

@@ -3,6 +3,12 @@ import App from "./App.vue";
import vuetify from "./plugins/vuetify";
import router from "./router";
import store from './store'
import VueCookies from "vue-cookies";
//쿠키를 사용한다.
Vue.use(VueCookies);

Choose a reason for hiding this comment

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

쿠키를 사용할 경우 XSS 공격을 막을 수 있지만, CSRF 공격에 취약해지기 때문에 해당 방법도 처리해주실 필요가 있을 것 같아요 ㅎㅎ
CSRF Token 또는 SameSite에 대해 알아보아요!

Copy link
Author

Choose a reason for hiding this comment

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

재연링이 던져주신 키워드들을 먼저 살펴보고 정말 간략히 기록해보았어요!!!
이 덕분에 쿠키를 좀 더 파볼수 있었습니다 ㅎㅎㅎ
학습하는데 많은 도움이 되었어요 감사합니다!

Comment on lines 56 to 58
if(!response1.ok){
throw new Error(`${response1.status}`);
}

Choose a reason for hiding this comment

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

Suggested change
if(!response1.ok){
throw new Error(`${response1.status}`);
}
if (!response1.ok) {
throw new Error(`${response1.status}`);
}

매의 눈 @_@

Copy link
Author

Choose a reason for hiding this comment

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

프론트엔드 쪽은 리포맷을 안돌렸었네요 ㅎㅎㅎㅎ
전반적으로 반영하였습니다!

}
const stations = await response1.json()
this.setStations([...stations])
// TODO 초기 노선 데이터를 불러오는 API를 추가해주세요...

Choose a reason for hiding this comment

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

구현한 TODO는 제거해보면 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

반영했습니다!!ㅎㅎ

Comment on lines 157 to 169
const response = await fetch("http://localhost:8080/lines", {
method:"POST",
headers: {
"Content-Type": "application/json",
},
body : JSON.stringify({
name : this.lineForm.name,
color : this.lineForm.color,
upStationId : this.lineForm.upStationId,
downStationId: this.lineForm.downStationId,
distance: this.lineForm.distance
})
})

Choose a reason for hiding this comment

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

스키마, 도메인, 포트(http://localhost:8080)를 변경할 경우 모든 url을 수정해줘야 하는 문제가 있네요!
또한 API 요청 코드에 중복되는 정보가 많은 것 같은데, 해당 기능을 메서드로 분리해보면 좋을 것 같아요~

Copy link
Author

@NewWisdom NewWisdom May 15, 2021

Choose a reason for hiding this comment

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

vue.config를 통해 중복되는 URL에 대한 설정을 해주었어요!
또한 중복되는 fetch는 모듈로 따로 만들어서(fetch.js) 사용하도록 변경했습니다!
감사합니다!

fdcbfa1

Comment on lines 84 to 85
email: email,
password: password

Choose a reason for hiding this comment

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

Suggested change
email: email,
password: password
email,
password

변수의 이름과 값이 동일할 경우 위와 같이 적용 가능합니다 : )

Copy link
Author

Choose a reason for hiding this comment

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

오오 이부분을 놓치고 있었네요!! 감사합니다 :)

public static final String ACCESS_TOKEN_TYPE = AuthorizationExtractor.class.getSimpleName() + ".ACCESS_TOKEN_TYPE";
public static String BEARER_TYPE = "Bearer";

Choose a reason for hiding this comment

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

Suggested change
public static String BEARER_TYPE = "Bearer";
public static final String BEARER_TYPE = "Bearer";

기본 코드에 final이 빠져있군요 ㅎㅎ

Copy link
Author

Choose a reason for hiding this comment

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

ㅎㅎ 수정했습니다 감사합니다!!


public TokenResponse createToken(TokenRequest tokenRequest) {
Member member = memberDao.findByEmailAndPassword(tokenRequest.getEmail(), tokenRequest.getPassword())
.orElseThrow(() -> new AuthorizationException("[ERROR] 로그인 실패입니다."));

Choose a reason for hiding this comment

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

예외처리 메서드에 [ERROR]를 붙여주시는 이유가 있을까요!?

Copy link
Author

Choose a reason for hiding this comment

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

오 이 부분은 콘솔에서 뷰를 띄울 때는 에러 메시지 출력을 위해 저렇게 [ERROR]를 붙여주었어요
여기서도 그냥 습관처럼 붙이고 있었네요!!! ㅎㅎㅎ

로그로 남기고 있으니 빼는 방향으로 수정했습니다:)

}

public void deleteById(Long id) {
String sql = "delete from MEMBER where id = ?";
jdbcTemplate.update(sql, id);
}

public Member findById(Long id) {
public Optional<Member> findById(Long id) {

Choose a reason for hiding this comment

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

Optional 👍

@NewWisdom
Copy link
Author

NewWisdom commented May 15, 2021

재연링의 말씀대로 쿠키를 사용했을 경우의 보안 문제들을 먼저 살펴보았어요!!
DM으로 나눈 이야기 대로 제가 구현한 로직에서의 쿠키는 XSS 공격을 막을 수 없었어요 🥲
그러다가 이 공격에 안전한 쿠키 설정을 찾아보게 되었고,
그러면서 자연스럽게 http-only와 SameSite에 대해서 학습할 수 있었습니다!!
하지만 이렇게 만든 쿠키는 JS에서 꺼내 쓸 수 없어 헤더에 토큰을 실어 보낼 수 없는 이슈가 있었네요 ㅎㅎㅎ
사실 오직 쿠키만 사용해 토큰을 주고 받는 방향으로 리팩토링을 진행하고 있었는데
말씀 나눈 대로 미션 요구사항과 여러 이유들로 다시 OAuth 표준을 지키고, 헤더로 토큰을 주고 받고 하도록 했어요!
이렇게되니 사실 로그인을 할 때 클라이언트에게 쿠키로 토큰을 실어 보낼 필요가 없었고
결국 다시 로컬 스토리지에 토큰을 저장하게 되었습니다 ㅎㅎㅎ

재연링이 던져주신 보안 키워드들을 고민하고 여러 방안을 생각하다보니
인증과 인가의 보안 이슈에 대해 알아보고 학습할 수 있었어요!
또한 덕분에 쿠키와 스토리지로 토큰을 관리하는 것의 차이가 명확해지는 것 같아 흥미로웠습니다!!!
사실 쿠키로 삽질을 하다가 코드를 다시 돌이키면서 토큰을 주고 받는데 결과물만 보면 큰 리팩토링은 없는 것 같지만...ㅎㅎ(?)
요런 저런 시도를 하면서 많은 것을 배울 수 있었습니다
조금 더 정리해서 학습 로그로 남기려 해요! ㅎㅎ 정말 감사합니다 🙇‍♂️

리뷰와 질의응답에 시간 내주셔서 너무나도 감사합니다 🔥
정말 큰 도움이 되어요!!! 이번에도 잘 부탁드립니다 👏

Copy link

@jaeyeonling jaeyeonling left a comment

Choose a reason for hiding this comment

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

피드백을 잘 반영해주셨어요 💯
코멘트 남겨두었으니 확인해주시고! 다음 단계 진행해주세요 😄


function getFetch(url) {
return fetch(`${url}`).then(data => {
if (data.status === 400) {

Choose a reason for hiding this comment

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

status가 20x 이 아니라면 exceptionHandling하는 것이 좋지 않을까요!?

public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, NativeWebRequest webRequest, WebDataBinderFactory binderFactory) {
// TODO: 유효한 로그인인 경우 LoginMember 만들어서 응답하기
return null;
public Member resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, NativeWebRequest webRequest, WebDataBinderFactory binderFactory) {

Choose a reason for hiding this comment

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

인증/인가에 대해 공부하셨다 하셔서 추가적으로 키워드를 드리자면..!!

ArgumentResolver는 말 그대로 매개변수에 대해 리졸빙을 담당합니다
지금의 구조에선 로그인한 유저만 사용할 수 있는 기능을 AuthenticationPrincipal를 통해서만 인증이 가능한 구조인데요!
AuthenticationPrincipal가 필요 없는 경우에도 AuthenticationPrincipal를 통해 검증을 해야하는 문제도 있고, 추후 인가를 처리하는 기능이 추가되어 순수 Member가 존재하는지가 아닌 어떠한 권한을 가지고 있는지를 검증해야 할 수도 있을 것 같아요 ㅎㅎ

요약한다면 ArgumentResolver의 역할은 매개변수의 리졸빙이고, AuthInterceptor와 같이 인증/인가에 대한 역할을 가지는 클래스를 나눌 수 있을 것 같은데 어떻게 생각하시나요!?

@jaeyeonling jaeyeonling merged commit 26b33d1 into woowacourse:newwisdom May 16, 2021
@NewWisdom
Copy link
Author

NewWisdom commented May 20, 2021

학습 로그

쿠키와 localstorage

  • JWT토큰을 저장하는 곳에 대한 여러 고민을 하면서 각각의 장, 단점과 보안 이슈에 대해 학습

정리

태그

  • Network

HandlerInterceptor와 ArgumentResolver

  • 스프링 부트에서 토큰 검증에 대한 처리를 HandlerInterceptor를 통해 구현
  • 토큰 검증은 HandlerInterceptor, 매개변수 리졸빙 역할은 HandlerMethodArgumentResolver가 담당하도록 구현

정리

태그

  • springboot

Cors간 쿠키

문제

  • 브라우저는 same origin이 아닌 경우 CORS 정책에 의해 헤더에 set-cookie를 해주어도 쿠키 설정이 되지 않았음

해결

서버 단에서는 자격 증명이 담긴 헤더를 명시적으로 허용하겠다는 세팅을 해준다.
해당 origin에 대해 Access-Control-Allow-Origin를 설정해주고
Access-Control-Allow-Credentials 설정을 true로 바꿔준다.

@Override
public void addCorsMappings(CorsRegistry registry) {
    registry.addMapping("/**").allowedMethods("*").allowedOriginPatterns("https://localhost:8081").allowCredentials(true);
}

프론트 단에서는 쿠키를 사용하는 요청에 credentials: "include"를 추가한다.

fetch('http://another.com', {
  credentials: "include"
});

이는 'http://another.com'에 인증정보를 포함한 요청을 보내겠다는 의미이다.
자격 증명 정보가 담긴 요청을 서버에서 받아들이기로 동의했으니 same origin이 아니어도 자격 증명을 허용한다.
이때 서버는 응답에 Access-Control-Allow-Origin 헤더와 함께 Access-Control-Allow-Credentials: true 헤더를 추가해서 보내는데,
이는 현재 개발자가 직접 설정하지 않아도 자동으로 되고 있다.

태그

  • cookie

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.

None yet

3 participants