[Feature] - 인가 구현 #79

eunjungL · 2024-07-21T14:26:54Z

✅ 작업 내용

Filter를 통한 Authorization Header 여부 검사 및 JWT 해독
ArgumentResolver를 사용해 현재 로그인한 사용자 조회

🙈 참고 사항

인증이 필요한 곳이 필요 없는 곳보다 훨씬 많을 것이라 생각해 filter를 도입해봤는데 지금 상태에서 오히려 복잡해지기만 한건지 확신이 안서네요. 차라리 인터셉터로 구현하는게 나았을지 의견 부탁드립니다.
filter는 dispatcherServlet에 들어가기 이전에 동작하는거라 발생하는 에러에 대해 ControllerAdvice로 잡을 수가 없습니다. 이 부분에 대해서는 개선점을 좀 찾아보겠습니다. 있으면 제보 바람 😢
~~filter는 return도 때릴 수가 없어서 if-else 구문이 생겼습니다. 여기서 개선점 좀 찾아보겠슴다. 있으면 제보 바람 22~~ early return 되네요 허위사실 유포 죄송합니다 여러분

github-actions · 2024-07-21T14:28:06Z

Unit Test Results

0 tests 0 ✔️ 0s ⏱️
0 suites 0 💤
0 files 0 ❌

Results for commit f769c2e.

♻️ This comment has been updated with latest results.

Libienz

안녕하세요 클로버 🍀

주말인데 고생많으셨습니다.

인가 방식에 대한 것과 나머지 간단한 코멘트 남겨드렸어요
방식을 정하고 넘어가면 좋을 것 같아 우선은 RC 남겨드립니다.
회의가 필요할 수도 있을 것 같군요 🤔

ps. 인가를 구현하신 것으로 확인했는데 PR제목과 코드에서 사용되는 여러 용어 검토 부탁드립니다

Libienz · 2024-07-21T14:35:04Z

backend/src/main/java/woowacourse/touroot/authentication/infrastructure/JwtTokenProvider.java

+        try {
+            return Jwts.parserBuilder()
+                    .setSigningKey(secretKey.getBytes(StandardCharsets.UTF_8))
+                    .build()
+                    .parseClaimsJws(token)
+                    .getBody()
+                    .get(MEMBER_ID_KEY)
+                    .toString();
+        } catch (ExpiredJwtException exception) {
+            throw new UnauthorizedException("이미 만료된 토큰입니다.");
+        } catch (Exception exception) {
+            throw new UnauthorizedException("유효하지 않은 토큰입니다.");
+        }


try catch 사용하는 만큼 stackTrace를 확인할 수 있게 cause를 예외 생성시에 추가시켜 주세용

반영 완료했심다 👍

Libienz · 2024-07-21T14:44:43Z

backend/src/main/java/woowacourse/touroot/global/auth/JwtAuthFilter.java

+@RequiredArgsConstructor
+@Slf4j
+@Component
+public class JwtAuthFilter extends OncePerRequestFilter {
+
+    private final ObjectMapper objectMapper;
+    private final JwtTokenProvider tokenProvider;


필터를 운용할 필요가 있나 싶습니다.

ArgumentResolver를 운용하는 만큼 인가 과정을 ArgumentResolver에 맞기시는 것은 어떨까요?
이와 같이 구현하면 관리하기 힘든 화이트 리스트를 운용할 필요도 없고 서블릿 필터라는 굉장히 Low한 계층에 의존하고 있는 문제점을 제거할 수 있습니다!

ArgumentResolver가 JwtTokenProvider를 의존하도록 하고 인자가 resolve 될 때마다 인가하도록 처리하면 제가 말씀드린 방식의 구현이 됩니닷
인자가 붙어있는 컨트롤러 메서드에서만 인가가 이루어지도록 처리할 수 있쥬

클로버의 의견 들려주세요!

Interceptor로 변경하자는 의견이 아니라 ArgumentResolver에게 인가를 맡기자는거라면 반대입니당
인증되지 않은 사용자의 요청은 Handler까지 흘러들어와야 할 필요가 없다고 생각해요. 적어도 Interceptor를 통해 Handler가 불리기 이전에 blocking 되는게 리소스 낭비도 덜하고 보안적인 측면에서도 더 안전할 것 같습니다.
위의 이유로 MethodArguementResolver는 모든 메서드의 파라미터를 처리할 때 공통적으로 필요할 때 사용되는 기능이지 인증/인가를 위한 기능이라고 생각되지 않습니다. 🙇

+) 추가로 화이트 리스트는 이후 더 늘어날 곳은 없어보입니다. 유지보수가 힘들 것 같지는 않아용!

오 .. 몰랐던 시각이에요
클로버의 말 이해했고 충분히 합리적이라는 생각이 드네요.

다만 인터셉터 혹은 필터로 분리할 경우 인가과정이 두개의 스텝으로 진행되면서 한눈에 들어오지 않는 것도 사실 인 것 같아요.
그리고 리소스 낭비를 이야기 하셨는데 트랜잭션이 열리기 전에 요청이 반려된다면 리소스 낭비가 그렇게 심할지도 의문이긴 합니다.

저는 개인적으로 argumentResolver에서 한큐에 처리하고 싶다는 생각이기는 한데요, 제가 일반적인 방법을 모르고 있어서 저도 제 방식에 확신을 하지는 못합니다.

다 같이 얘기해보면 좋을 것 같아유!

다만 인터셉터 혹은 필터로 분리할 경우 인가과정이 두개의 스텝으로 진행되면서 한눈에 들어오지 않는 것도 사실 인 것 같아요.

요 부분에 대해서는 filter에서는 JWT를 해독하는 인증 + 해당 요청은 로그인 된 유저만 접근할 수 있다는 인가를 진행하고 MethodArgumentResolver는 개발 편의를 위한 MemberAuth의 객체를 만드는 역할만 하기 때문에 저는 인증/인가가 두 가지 스텝이라고 생각하고 있지 않습니다!

다른 부분에 대해서는 다 같이 얘기해보면 좋을 것 같아요 좋은 의견 감삼둥

저는 Filter를 활용하지 않는다면 Interceptor가 좀 더 합리적이라고 생각됩니다. ArgumentResolver의 역할은 인증/인가라기 보단 인증/인가된 대상을 메소드 인자로 할당하는 것이라고 생각합니다. 클로버가 적어주신 Filter의 여러 단점(그 중에서도 전역 예외처리 불가)를 생각해보면 여러모로 굳이 도입할 이유가 없어보이긴 합니다.

전 Interceptor로의 리팩토링에 찬성합니다!

filter가 early return이 안된다는건 저의 허위사실 유포였습니다 사죄드립니다 리건 :cry ealry return으로 수정해뒀습니다,,,

Libienz · 2024-07-21T14:48:13Z

backend/src/main/java/woowacourse/touroot/global/auth/JwtAuthFilter.java

+        if (token == null) {
+            sendUnauthorizedResponse(response, "인증이 필요한 요청입니다.");
+        } else {


인증, 요청 이라는 워딩보다는 로그인등의 표현이 유저에게 가까울 것 같아요!

굳 바로 반영 고고링했심다

eunjungL

리비 코멘트 우선적으로 반영했습니당
주말인데도 빠른 코멘트 감동..

eunjungL · 2024-07-21T14:55:23Z

backend/src/main/java/woowacourse/touroot/global/auth/JwtAuthFilter.java

+@RequiredArgsConstructor
+@Slf4j
+@Component
+public class JwtAuthFilter extends OncePerRequestFilter {
+
+    private final ObjectMapper objectMapper;
+    private final JwtTokenProvider tokenProvider;


Interceptor로 변경하자는 의견이 아니라 ArgumentResolver에게 인가를 맡기자는거라면 반대입니당
인증되지 않은 사용자의 요청은 Handler까지 흘러들어와야 할 필요가 없다고 생각해요. 적어도 Interceptor를 통해 Handler가 불리기 이전에 blocking 되는게 리소스 낭비도 덜하고 보안적인 측면에서도 더 안전할 것 같습니다.
위의 이유로 MethodArguementResolver는 모든 메서드의 파라미터를 처리할 때 공통적으로 필요할 때 사용되는 기능이지 인증/인가를 위한 기능이라고 생각되지 않습니다. 🙇

eunjungL · 2024-07-21T14:57:53Z

backend/src/main/java/woowacourse/touroot/global/auth/JwtAuthFilter.java

+        if (token == null) {
+            sendUnauthorizedResponse(response, "인증이 필요한 요청입니다.");
+        } else {


굳 바로 반영 고고링했심다

eunjungL · 2024-07-21T15:00:27Z

backend/src/main/java/woowacourse/touroot/authentication/infrastructure/JwtTokenProvider.java

+        try {
+            return Jwts.parserBuilder()
+                    .setSigningKey(secretKey.getBytes(StandardCharsets.UTF_8))
+                    .build()
+                    .parseClaimsJws(token)
+                    .getBody()
+                    .get(MEMBER_ID_KEY)
+                    .toString();
+        } catch (ExpiredJwtException exception) {
+            throw new UnauthorizedException("이미 만료된 토큰입니다.");
+        } catch (Exception exception) {
+            throw new UnauthorizedException("유효하지 않은 토큰입니다.");
+        }


반영 완료했심다 👍

hangillee

클로버 고생 많으셨습니다!
제 생각에도 Filter를 도입했을 때의 이점보다 단점이 더 많은 것 같습니다.
if-else 구문의 등장만으로 저희 레벨 1, 2를 완전 부정하는 것 같아서 꺼려지네요.

저는 Interceptor로의 리팩토링을 제안해봅니다!

hangillee · 2024-07-22T04:42:04Z

backend/src/main/java/woowacourse/touroot/global/auth/JwtAuthFilter.java

+@RequiredArgsConstructor
+@Slf4j
+@Component
+public class JwtAuthFilter extends OncePerRequestFilter {
+
+    private final ObjectMapper objectMapper;
+    private final JwtTokenProvider tokenProvider;


저는 Filter를 활용하지 않는다면 Interceptor가 좀 더 합리적이라고 생각됩니다. ArgumentResolver의 역할은 인증/인가라기 보단 인증/인가된 대상을 메소드 인자로 할당하는 것이라고 생각합니다. 클로버가 적어주신 Filter의 여러 단점(그 중에서도 전역 예외처리 불가)를 생각해보면 여러모로 굳이 도입할 이유가 없어보이긴 합니다.

전 Interceptor로의 리팩토링에 찬성합니다!

github-actions · 2024-07-22T11:50:11Z

Test Results

12 files 12 suites 4s ⏱️
33 tests 32 ✅ 1 💤 0 ❌
34 runs 33 ✅ 1 💤 0 ❌

Results for commit ef20383.

♻️ This comment has been updated with latest results.

Libienz

작성하신 auth filter 우선 운용하는걸로 하시죠!

백로그에 남겨두었습니다. 후에 리팩토링 토의 하면 좋을 것 같습니다 👍

hangillee

Filter에서 early return이 가능하다면 굳이 사용하지 않을 이유도 없을 것 같네요.

고생하셨습니다 클로버!

* feat: UnauthorizedException 추가 * feat: Swagger에 Authorization 추가 * feat: Authorization Header 검증용 filter 구현 * feat: 로그인된 사요앚 정보 얻기 위한 ArgumentResolver 구현 * test: TravelPlanController 테스트에 accessToken 추가 * refactor: 필터에서 token이 없을 경우의 에러메시지 수정 * refactor: JwtTokenProvider 예외 발생 시 cause 포함되게 수정 * refactor: JwtAuthFilter else문 제거

eunjungL added 4 commits July 21, 2024 22:22

feat: UnauthorizedException 추가

d98379d

feat: Swagger에 Authorization 추가

21d01fd

feat: Authorization Header 검증용 filter 구현

46cf15c

feat: 로그인된 사요앚 정보 얻기 위한 ArgumentResolver 구현

875a908

eunjungL added the BE label Jul 21, 2024

eunjungL added this to the sprint 2 milestone Jul 21, 2024

eunjungL requested review from hangillee, Libienz, nak-honest and slimsha2dy July 21, 2024 14:26

eunjungL self-assigned this Jul 21, 2024

test: TravelPlanController 테스트에 accessToken 추가

c03c0ea

Libienz requested changes Jul 21, 2024

View reviewed changes

eunjungL added 2 commits July 21, 2024 23:58

refactor: 필터에서 token이 없을 경우의 에러메시지 수정

80e0340

refactor: JwtTokenProvider 예외 발생 시 cause 포함되게 수정

f769c2e

eunjungL commented Jul 21, 2024

View reviewed changes

eunjungL requested a review from Libienz July 21, 2024 15:00

eunjungL changed the title ~~[Feature] - 인증 구현~~ [Feature] - 인가 구현 Jul 21, 2024

hangillee requested changes Jul 22, 2024

View reviewed changes

refactor: JwtAuthFilter else문 제거

c4150d1

eunjungL linked an issue Jul 24, 2024 that may be closed by this pull request

[Feature] - 인가 구현 #69

Closed

2 tasks

Libienz approved these changes Jul 25, 2024

View reviewed changes

hangillee approved these changes Jul 25, 2024

View reviewed changes

fix: develop/be와 충돌 해결

ef20383

eunjungL merged commit 06d7eb3 into develop/be Jul 30, 2024
3 checks passed

eunjungL deleted the feature/be/#69 branch July 30, 2024 04:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] - 인가 구현 #79

[Feature] - 인가 구현 #79

eunjungL commented Jul 21, 2024 •

edited

Loading

github-actions bot commented Jul 21, 2024 •

edited

Loading

Libienz left a comment

Libienz Jul 21, 2024

eunjungL Jul 21, 2024

Libienz Jul 21, 2024

eunjungL Jul 21, 2024

eunjungL Jul 21, 2024

Libienz Jul 21, 2024

eunjungL Jul 21, 2024

hangillee Jul 22, 2024

eunjungL Jul 22, 2024 •

edited

Loading

Libienz Jul 21, 2024

eunjungL Jul 21, 2024

eunjungL left a comment

eunjungL Jul 21, 2024

eunjungL Jul 21, 2024

eunjungL Jul 21, 2024

hangillee left a comment

hangillee Jul 22, 2024

github-actions bot commented Jul 22, 2024 •

edited

Loading

Libienz left a comment •

edited

Loading

hangillee left a comment

[Feature] - 인가 구현 #79

[Feature] - 인가 구현 #79

Conversation

eunjungL commented Jul 21, 2024 • edited Loading

✅ 작업 내용

🙈 참고 사항

github-actions bot commented Jul 21, 2024 • edited Loading

Unit Test Results

Libienz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eunjungL Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eunjungL left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hangillee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 22, 2024 • edited Loading

Test Results

Libienz left a comment • edited Loading

Choose a reason for hiding this comment

hangillee left a comment

Choose a reason for hiding this comment

eunjungL commented Jul 21, 2024 •

edited

Loading

github-actions bot commented Jul 21, 2024 •

edited

Loading

eunjungL Jul 22, 2024 •

edited

Loading

github-actions bot commented Jul 22, 2024 •

edited

Loading

Libienz left a comment •

edited

Loading