-
Notifications
You must be signed in to change notification settings - Fork 0
[feat] #9 - 구글 소셜 로그인 기능 구현 #14
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
base: develop
Are you sure you want to change the base?
Changes from all commits
65b8073
f3443e2
c16a213
3e85bde
e52c701
012c19e
fdb720e
2a6faf7
3f2bda6
3e99a23
2f01497
1447905
b7dce5a
b1440b0
a0529cd
d5cea0e
bc564f4
f8fd683
f531a63
bef91e4
b6cb9b5
505eb23
011a7a5
1f44da3
cd4ca03
91eb023
e90308d
5da973e
9d40673
bf208c3
22a203a
5de9d41
eabe1d3
dcc9ce2
faa733c
c1db021
f3afcb7
88edebb
d97344d
02a1696
548ef81
9c32021
58197dc
33f38dd
52671a5
ecb5853
8d5761c
770cfa0
18d23a3
56893ba
f2adfd0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| package com.Timo.Timo.domain.user.entity; | ||
|
|
||
| import com.Timo.Timo.domain.user.enums.Provider; | ||
| import jakarta.persistence.Column; | ||
| import jakarta.persistence.Entity; | ||
| import jakarta.persistence.EnumType; | ||
| import jakarta.persistence.Enumerated; | ||
| import jakarta.persistence.GeneratedValue; | ||
| import jakarta.persistence.GenerationType; | ||
| import jakarta.persistence.Id; | ||
| import jakarta.persistence.Table; | ||
| import lombok.AccessLevel; | ||
| import lombok.Builder; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| @Entity | ||
| @Table(name = "users") | ||
| @Getter | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class User { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @Column(nullable = false, unique = true) | ||
| private String email; | ||
|
|
||
| private String name; | ||
| private String imageUrl; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| @Column(nullable = false) | ||
| private Provider provider; | ||
|
|
||
| @Builder | ||
| private User(String email, String name, String imageUrl, Provider provider) { | ||
| this.email = email; | ||
| this.name = name; | ||
| this.imageUrl = imageUrl; | ||
| this.provider = provider; | ||
| } | ||
|
|
||
| public void update(String name, String imageUrl){ | ||
| this.name = name; | ||
| this.imageUrl = imageUrl; | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| package com.Timo.Timo.domain.user.enums; | ||
|
|
||
| public enum Provider { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| package com.Timo.Timo.domain.user.repository; | ||
|
|
||
| import com.Timo.Timo.domain.user.entity.User; | ||
| import java.util.Optional; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
|
|
||
| public interface UserRepository extends JpaRepository<User, Long> { | ||
| Optional<User> findByEmail(String email); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| package com.Timo.Timo.global.auth.controller; | ||
|
|
||
| import com.Timo.Timo.global.auth.handler.AuthErrorResponseWriter; | ||
| import com.Timo.Timo.global.auth.service.AuthCodeService; | ||
| import com.Timo.Timo.global.exception.code.ErrorCode; | ||
| import com.Timo.Timo.global.jwt.provider.JwtTokenProvider; | ||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import java.io.IOException; | ||
| import java.util.Map; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.http.MediaType; | ||
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.RequestMapping; | ||
| import org.springframework.web.bind.annotation.RequestParam; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
| @RestController | ||
| @RequestMapping("/api/auth") | ||
| @RequiredArgsConstructor | ||
| public class AuthController { | ||
|
|
||
| private final AuthCodeService authCodeService; | ||
| private final JwtTokenProvider jwtTokenProvider; | ||
| private final AuthErrorResponseWriter authErrorResponseWriter; | ||
| private final ObjectMapper objectMapper; | ||
|
|
||
| @GetMapping("/token") | ||
| public void token( | ||
| @RequestParam String code, | ||
| HttpServletRequest request, | ||
| HttpServletResponse response | ||
| ) throws IOException { | ||
|
|
||
| String userId = authCodeService.getAndDelete(code); | ||
|
|
||
| if (userId == null) { | ||
| authErrorResponseWriter.write(response, ErrorCode.INVALID_AUTH_CODE, request.getRequestURI()); | ||
| return; | ||
| } | ||
|
|
||
| String accessToken = jwtTokenProvider.generateAccessToken(Long.parseLong(userId)); | ||
|
|
||
| response.setContentType(MediaType.APPLICATION_JSON_VALUE); | ||
| response.setCharacterEncoding("UTF-8"); | ||
| response.getWriter().write( | ||
| objectMapper.writeValueAsString(Map.of("accessToken", accessToken)) | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| package com.Timo.Timo.global.auth.dto; | ||
|
|
||
| import com.Timo.Timo.domain.user.entity.User; | ||
| import java.util.Collection; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import lombok.Getter; | ||
| import org.springframework.security.core.GrantedAuthority; | ||
| import org.springframework.security.core.authority.SimpleGrantedAuthority; | ||
| import org.springframework.security.core.userdetails.UserDetails; | ||
| import org.springframework.security.oauth2.core.user.OAuth2User; | ||
|
|
||
| @Getter | ||
| public class CustomUserDetails implements OAuth2User, UserDetails { | ||
|
|
||
| private final User user; | ||
| private final Map<String, Object> attributes; | ||
|
|
||
| public CustomUserDetails(User user, Map<String, Object> attributes){ | ||
| this.user = user; | ||
| this.attributes = attributes; | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, Object> getAttributes() { | ||
| return attributes; | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return user.getEmail(); | ||
| } | ||
|
|
||
| @Override | ||
| public Collection<? extends GrantedAuthority> getAuthorities() { | ||
| return List.of(new SimpleGrantedAuthority("ROLE_USER")); | ||
| } | ||
|
|
||
| @Override | ||
| public String getPassword(){ | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public String getUsername(){ | ||
| return user.getEmail(); | ||
| } | ||
|
Comment on lines
+44
to
+47
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. getUsername과 getName모두 getEmail을 가져오는 것 같은데 두개가 뭐가 다른건가요?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 이 부분 정말 헷갈렸는데요,, 둘 다 사용자를 식별하는 용도이긴 하지만, 역할이 조금 다르다고 합니다! 현재 해당 클래스는 구글 로그인으로 받아온 사용자를 Spring Security가 이해할 수 있는 로그인 객체로 변환해 주는 어댑터 역할을 하며,
이었는데 아래 코드리뷰와 관련해서 식별자 값을
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 완전 이해되었습니다 꼼꼼한 설명 감사합니다. |
||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package com.Timo.Timo.global.auth.handler; | ||
|
|
||
| import com.Timo.Timo.global.exception.code.ErrorCode; | ||
| import com.Timo.Timo.global.exception.dto.ErrorDto; | ||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import java.io.IOException; | ||
| import java.time.LocalDateTime; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.http.MediaType; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class AuthErrorResponseWriter { | ||
|
|
||
| private final ObjectMapper objectMapper; | ||
|
|
||
| public void write(HttpServletResponse response, ErrorCode errorCode, String path) | ||
| throws IOException { | ||
|
|
||
| ErrorDto errorDto = new ErrorDto( | ||
| LocalDateTime.now(), | ||
| errorCode.getHttpStatus().value(), | ||
| errorCode.getCode(), | ||
| errorCode.getMessage(), | ||
| path | ||
| ); | ||
|
|
||
| response.setStatus(errorCode.getHttpStatus().value()); | ||
| response.setContentType(MediaType.APPLICATION_JSON_VALUE); | ||
| response.setCharacterEncoding("UTF-8"); | ||
| response.getWriter().write(objectMapper.writeValueAsString(errorDto)); | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| package com.Timo.Timo.global.auth.handler; | ||
|
|
||
| import com.Timo.Timo.global.exception.code.ErrorCode; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import java.io.IOException; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.security.core.AuthenticationException; | ||
| import org.springframework.security.web.AuthenticationEntryPoint; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class JwtAuthenticationEntryPoint implements AuthenticationEntryPoint { | ||
|
|
||
| private final AuthErrorResponseWriter authErrorResponseWriter; | ||
|
|
||
| @Override | ||
| public void commence( | ||
| HttpServletRequest request, | ||
| HttpServletResponse response, | ||
| AuthenticationException authException | ||
| ) throws IOException { | ||
| authErrorResponseWriter.write(response, ErrorCode.UNAUTHORIZED, request.getRequestURI()); | ||
| } | ||
| } |
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. p3) 지금 보았을때 위의 jwtAuthenticationEntryPoint와 oauthFailureHandler의 코드가 겹치는 부분이 많은 것 같아요! 따로 writer로 분리해서 작성하면 중복코드 사용을 줄일 수 있지 않을까하는 생각입니다!
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오홍 인지하지 못하고 있었는데 ErrorDto 생성 → status 설정 → contentType 설정 → write 하는 로직이 동일하게 반복되고 있었네요! AuthErrorResponseWriter로 분리하여 중복을 제거하고 이후 에러 응답 형식이 바뀌더라도 한 곳만 수정하면 되도록 개선해야겠습니다-!! |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| package com.Timo.Timo.global.auth.handler; | ||
|
|
||
| import com.Timo.Timo.global.exception.code.ErrorCode; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import java.io.IOException; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.security.core.AuthenticationException; | ||
| import org.springframework.security.web.authentication.SimpleUrlAuthenticationFailureHandler; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class OAuthFailureHandler extends SimpleUrlAuthenticationFailureHandler { | ||
|
|
||
| private final AuthErrorResponseWriter authErrorResponseWriter; | ||
|
|
||
| @Override | ||
| public void onAuthenticationFailure( | ||
| HttpServletRequest request, | ||
| HttpServletResponse response, | ||
| AuthenticationException exception | ||
| ) throws IOException { | ||
| authErrorResponseWriter.write(response, ErrorCode.OAUTH_LOGIN_FAILED, request.getRequestURI()); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package com.Timo.Timo.global.auth.handler; | ||
|
|
||
| import com.Timo.Timo.global.auth.dto.CustomUserDetails; | ||
| import com.Timo.Timo.global.auth.service.AuthCodeService; | ||
| import com.Timo.Timo.global.auth.service.RefreshTokenService; | ||
| import com.Timo.Timo.global.jwt.provider.JwtTokenProvider; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import java.io.IOException; | ||
| import java.time.Duration; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.beans.factory.annotation.Value; | ||
| import org.springframework.http.HttpHeaders; | ||
| import org.springframework.http.ResponseCookie; | ||
| import org.springframework.security.core.Authentication; | ||
| import org.springframework.security.web.authentication.SimpleUrlAuthenticationSuccessHandler; | ||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.web.util.UriComponentsBuilder; | ||
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class OAuthSuccessHandler extends SimpleUrlAuthenticationSuccessHandler { | ||
|
|
||
| private final JwtTokenProvider jwtTokenProvider; | ||
| private final RefreshTokenService refreshTokenService; | ||
| private final AuthCodeService authCodeService; | ||
|
|
||
| @Value("${app.oauth2.redirect-uri}") | ||
| private String redirectUri; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이부분은 아직 구현되지 않아서 따로 사용되지 않는거겠죠...?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. redirectUri 선언만 해두고 실제로 사용하는 코드가 누락되어 있었네요.. 테스트용 코드 작성하느라 수정하면서 실수로 지운 것 같습니다.. getRedirectStrategy().sendRedirect()로 리다이렉트 처리 다시 추가해놓겠습니다! |
||
|
|
||
| @Value("${app.auth.cookie-secure:false}") | ||
| private boolean cookieSecure; | ||
|
|
||
| @Override | ||
| public void onAuthenticationSuccess( | ||
| HttpServletRequest request, | ||
| HttpServletResponse response, | ||
| Authentication authentication | ||
| ) throws IOException { | ||
|
|
||
| CustomUserDetails userDetails = (CustomUserDetails) authentication.getPrincipal(); | ||
| Long userId = userDetails.getUser().getId(); | ||
|
|
||
| String refreshToken = jwtTokenProvider.generateRefreshToken(userId); | ||
| refreshTokenService.save(String.valueOf(userId), refreshToken); | ||
|
|
||
| ResponseCookie refreshCookie = ResponseCookie.from("refreshToken", refreshToken) | ||
| .httpOnly(true) | ||
| .secure(cookieSecure) | ||
| .path("/") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. p4) Refresh Token 쿠키가 path("/")로 설정되어 모든 API 요청에 함께 전송되는것 같아요. 실제 사용되는 재발급·로그아웃 API로 경로를 제한하면 토큰의 불필요한 전송과 노출 범위를 줄일 수 있을 것 같습니다. /api/auth 정도로 제한하는 것은 어떨까요? |
||
| .maxAge(Duration.ofSeconds(jwtTokenProvider.getRefreshTokenExpiry())) | ||
| .sameSite("Strict") | ||
| .build(); | ||
|
Comment on lines
+47
to
+53
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. p2) refreshCookie를 생성하고 있지만 응답 헤더에 추가하는 코드가 없어 실제 클라이언트로 쿠키가 내려가지 않을 것 같습니다.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 헉 맞습니다! 마지막에 accessToken 전달 방식을 쿠키로 잘못 전달하고 있었어서 Body 응답으로 변경하는 과정에서 refreshToken 쿠키 헤더 추가 코드가 누락되었던 것 같습니다. 꼼꼼하게 봐주셔서 감사합니다! 바로 수정하겠습니다~ |
||
| response.addHeader(HttpHeaders.SET_COOKIE, refreshCookie.toString()); | ||
|
|
||
| String code = authCodeService.generateAndSave(String.valueOf(userId)); | ||
|
|
||
| String redirectUrl = UriComponentsBuilder.fromUriString(redirectUri) | ||
| .queryParam("code", code) | ||
| .build().toUriString(); | ||
|
|
||
| getRedirectStrategy().sendRedirect(request, response, redirectUrl); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package com.Timo.Timo.global.auth.service; | ||
|
|
||
| import java.util.UUID; | ||
| import java.util.concurrent.TimeUnit; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.data.redis.core.RedisTemplate; | ||
| import org.springframework.stereotype.Service; | ||
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class AuthCodeService { | ||
|
|
||
| private final RedisTemplate<String, String> redisTemplate; | ||
|
|
||
| private static final String KEY_PREFIX = "auth:code:"; | ||
| private static final long CODE_EXPIRY_SECONDS = 30L; | ||
|
|
||
| public String generateAndSave(String userId) { | ||
| String code = UUID.randomUUID().toString(); | ||
| redisTemplate.opsForValue().set( | ||
| KEY_PREFIX + code, | ||
| userId, | ||
| CODE_EXPIRY_SECONDS, | ||
| TimeUnit.SECONDS | ||
| ); | ||
| return code; | ||
| } | ||
|
|
||
| public String getAndDelete(String code) { | ||
| return redisTemplate.opsForValue().getAndDelete(KEY_PREFIX + code); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: Team-Timo/Timo-Server
Length of output: 4087
🏁 Script executed:
Repository: Team-Timo/Timo-Server
Length of output: 7970
토큰 교환 엔드포인트는 POST로 바꾸고 응답 캐시를 막아 주세요.
현재
GET으로 인증 코드를 소모한 뒤accessToken을 내려서, 브라우저 히스토리/캐시/프리페치/접근 로그에 값이 남을 수 있습니다. 응답에는Cache-Control: no-store(필요하면Pragma: no-cache도) 를 추가해 주세요.🤖 Prompt for AI Agents
Source: Path instructions
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.
p2) accessToken을 발급하는 엔드포인트라서 코드래빗 리뷰처럼 post 메소드가 더 어울릴 것 같네요.
get은 히스토리가 남아서 보안상 위험할 수도 있을 것 같아요!!
그리고 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.
🧩 Analysis chain
🏁 Script executed:
Length of output: 4615
@laura-jung맞습니다! 아래처럼 반영하는 걸 제안드립니다.GET→POST로 변경 (히스토리/캐시 노출 방지)Cache-Control: no-store헤더 추가springdoc-openapi가 이미 의존성에 포함되어 있으니 (build.gradle)@Tag,@Operation,@Parameter애노테이션으로 Swagger 문서화 추가SecurityConfig의requestMatchers("/api/auth/token")은 경로 기준이라 메서드를 바꿔도 별도 설정 변경은 필요 없습니다.Task: AuthController
/api/auth/token을 POST로 변경 + 캐시 방지 헤더 + Swagger 문서화The requested changes have been implemented and a pull request has been created: View PR