Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/java/Gotcha/common/config/SecurityFilterConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@

import Gotcha.common.jwt.BlackListTokenService;
import Gotcha.common.jwt.TokenProvider;
import Gotcha.common.jwt.UserDetailsServiceImpl;
import Gotcha.common.jwt.filter.JwtAuthenticationFilter;
import Gotcha.common.jwt.filter.JwtExceptionFilter;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.RequiredArgsConstructor;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.core.userdetails.UserDetailsService;

@Configuration
@RequiredArgsConstructor
public class SecurityFilterConfig {
private final UserDetailsService userDetailsService;
private final UserDetailsServiceImpl userDetailsService;
private final TokenProvider tokenProvider;
private final ObjectMapper objectMapper;
private final BlackListTokenService blackListTokenService;
Expand Down
26 changes: 18 additions & 8 deletions src/main/java/Gotcha/common/jwt/JwtHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,29 +26,39 @@ public class JwtHelper {
public TokenDto createToken(User user) {
Long userId = user.getId();
String email = user.getEmail();
return getTokenDto(user, userId, email);
}

public TokenDto createGuestToken(User guest){
Long userId = guest.getId();
String username = guest.getNickname();
return getTokenDto(guest, userId, username);
}

private TokenDto getTokenDto(User user, Long userId, String username) {
String role = String.valueOf(user.getRole());

String accessToken = TOKEN_PREFIX + tokenProvider.createAccessToken(role, userId, email);
String refreshToken = tokenProvider.createRefreshToken(role, userId, email);
String accessToken = TOKEN_PREFIX + tokenProvider.createAccessToken(role, userId, username);
String refreshToken = tokenProvider.createRefreshToken(role, userId, username);

refreshTokenService.saveRefreshToken(email, refreshToken);
refreshTokenService.saveRefreshToken(username, refreshToken);
return new TokenDto(accessToken, refreshToken);
}

public TokenDto reissueToken(String refreshToken) {
String email = tokenProvider.getEmail(refreshToken);
String username = tokenProvider.getUsername(refreshToken);

if (!refreshTokenService.existedRefreshToken(email, refreshToken))
if (!refreshTokenService.existedRefreshToken(username, refreshToken))
throw new CustomException(JwtExceptionCode.REFRESH_TOKEN_NOT_FOUND);

Long userId = tokenProvider.getUserId(refreshToken);
String role = tokenProvider.getRole(refreshToken);

String newAccessToken = TOKEN_PREFIX + tokenProvider.createAccessToken(role, userId, email);
String newRefreshToken = tokenProvider.createRefreshToken(role, userId, email);
String newAccessToken = TOKEN_PREFIX + tokenProvider.createAccessToken(role, userId, username);
String newRefreshToken = tokenProvider.createRefreshToken(role, userId, username);

refreshTokenService.deleteRefreshToken(refreshToken);
refreshTokenService.saveRefreshToken(email, newRefreshToken);
refreshTokenService.saveRefreshToken(username, newRefreshToken);

return new TokenDto(newAccessToken, newRefreshToken);
}
Expand Down
12 changes: 6 additions & 6 deletions src/main/java/Gotcha/common/jwt/RefreshTokenService.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,20 @@ public class RefreshTokenService {
@Value("${token.refresh.in-redis}")
private long REFRESH_EXPIRATION;

public void saveRefreshToken(String email, String refreshToken) {
String key = REFRESH_TOKEN_KEY_PREFIX + email;
public void saveRefreshToken(String username, String refreshToken) {
String key = REFRESH_TOKEN_KEY_PREFIX + username;
redisUtil.setData(key, refreshToken);
redisUtil.setDataExpire(key, REFRESH_EXPIRATION);
}

public void deleteRefreshToken(String refreshToken) {
String email = tokenProvider.getEmail(refreshToken);
String key = REFRESH_TOKEN_KEY_PREFIX + email;
String username = tokenProvider.getUsername(refreshToken);
String key = REFRESH_TOKEN_KEY_PREFIX + username;
redisUtil.deleteData(key);
}

public boolean existedRefreshToken(String email, String requestRefreshToken) {
String key = REFRESH_TOKEN_KEY_PREFIX + email;
public boolean existedRefreshToken(String username, String requestRefreshToken) {
String key = REFRESH_TOKEN_KEY_PREFIX + username;

String storedRefreshToken = (String) redisUtil.getData(key);

Expand Down
4 changes: 4 additions & 0 deletions src/main/java/Gotcha/common/jwt/SecurityUserDetails.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ public String getUsername() {
return user.getEmail();
}

public String getNickname(){
return user.getNickname();
}

public Long getId(){
return user.getId();
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/Gotcha/common/jwt/TokenProvider.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ private Claims getClaims(String token) {
.getPayload();
}

public String getEmail(String token) {
public String getUsername(String token) {
return getClaims(token).getSubject();
}

Expand Down
15 changes: 15 additions & 0 deletions src/main/java/Gotcha/common/jwt/UserDetailsServiceImpl.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package Gotcha.common.jwt;

import Gotcha.common.util.RedisUtil;
import Gotcha.domain.user.entity.User;
import Gotcha.domain.user.repository.UserRepository;
import lombok.RequiredArgsConstructor;
Expand All @@ -9,10 +10,15 @@
import org.springframework.security.core.userdetails.UsernameNotFoundException;
import org.springframework.stereotype.Service;

import java.util.Optional;

import static Gotcha.common.redis.RedisProperties.GUEST_TTL_SECONDS;

@Service
@RequiredArgsConstructor
public class UserDetailsServiceImpl implements UserDetailsService {
private final UserRepository userRepository;
private final RedisUtil redisUtil;

@Override
@Cacheable(value = "users", key = "#username")
Expand All @@ -22,4 +28,13 @@ public UserDetails loadUserByUsername(String username) throws UsernameNotFoundEx

return new SecurityUserDetails(user);
}

public UserDetails loadGuestByUserId(Long guestId) throws UsernameNotFoundException{
User guest = Optional.ofNullable((User) redisUtil.getData("guest:" + guestId))
.orElseThrow(()-> new UsernameNotFoundException("Guest not found : " + guestId));

redisUtil.setDataExpire("guest:" + guestId, GUEST_TTL_SECONDS);

return new SecurityUserDetails(guest);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

import Gotcha.common.constants.SecurityConstants;
import Gotcha.common.jwt.BlackListTokenService;
import Gotcha.common.jwt.UserDetailsServiceImpl;
import Gotcha.common.jwt.exception.JwtExceptionCode;
import Gotcha.common.jwt.TokenProvider;
import Gotcha.domain.user.entity.Role;
import io.jsonwebtoken.ExpiredJwtException;
import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
Expand All @@ -16,7 +18,6 @@
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.web.filter.OncePerRequestFilter;
import org.springframework.util.AntPathMatcher;

Expand All @@ -29,7 +30,7 @@
@Slf4j
@RequiredArgsConstructor
public class JwtAuthenticationFilter extends OncePerRequestFilter {
private final UserDetailsService userDetailsService;
private final UserDetailsServiceImpl userDetailsService;
Copy link
Contributor

Choose a reason for hiding this comment

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

UserDetailsService를 상속받는 GuestDetailsService Interface를 작성 후에

GuestDetailsService guestDetailsService;
UserDetailsService userDetailsService;


if (role.equals(String.valueOf(Role.GUEST))) {
    Long guestId = tokenProvider.getUserId(accessToken);
    userDetails = guestDetailsService.loadGuestByUserId(guestId);
}
else{
    String username = tokenProvider.getUsername(accessToken);
    userDetails = userDetailsService.loadUserByUsername(username);
}

요런 식으로 하는게 책임분리도 되고 좋지 않을까용? 아직 서비스 크기가 크진 않으니까 UserDetailsServiceImpl에 몰아도 상관 없을 것 같지만, Guest와 User가 성격 자체가 조금은 다르니 별도의 DetailsService에서 처리해주어도 좋다고 생각합니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

Guest와 User가 완전히 다른 엔터티가 아니고 같은 엔터티니까 UserDetailsService에서 전부 처리하는 것도 맞는 것 같지만, 그렇다고 loadUserByUsername() 함수 안에 Guest, User에 대한 로직을 합쳐서 username 조건에 따라 분기하는 방법을 쓰기에는 Guest는 unique한 email이 저장되지 않으니 좋은 방법은 아닌 것 같습니다.

하나의 UserDetailsService.loadUserByUsername() 에서 전부 로직을 처리하려면

    public TokenDto createGuestToken(User guest){
        Long userId = guest.getId();
        String username = guest.getNickname(); // "guest"
        return getTokenDto(guest, userId, username);
    }

여기서 username 대신에 "guest" 를 넣어서 loadUserByUsername에서 username.equals("guest") 로 분기를 하는 방법도 가능할 것 같습니당!!

Copy link
Contributor

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
Contributor

Choose a reason for hiding this comment

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

게스트 토큰을 만들 때, usetName에 게스트임을 구분할 수 있게 "guest"라는 고정된 값을 넣자는 게 좋은 의견인 것 같아! 로컬에서 한 번 구현을 해보았습니다. 가능할 것 이라고 생각하나, 직접 구현을 해보니 제가 생각하기에 해당 로직은 다음과 같은 단점을 지녔던 것 같습니다.

단순 저의 생각으로 구현을 해본 것임을 감안해주세요!

현재 방법 : userDetailsService 함수 하나로 전부 처리해보자!

1. token 생성 단계

    public TokenDto createGuestToken(User guest){
        Long userId = guest.getId();
        String username = GUEST + userId // GUEST =  "guest"; 
        return getTokenDto(guest, userId, username);
    }

2. JwtAuthenticationFilter는 코드 변경 X

  UserDetails userDetails = userDetailsService.loadUserByUsername(username);

3. UserDetailsServiceImpl

@Service
@RequiredArgsConstructor
public class UserDetailsServiceImpl implements UserDetailsService {
    private final UserRepository userRepository;
    private final RedisUtil redisUtil;

    @Override
    @Cacheable(value = "users", key = "#username")
    public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
        if(username.startsWith(GUEST)) { //만약 게스트 로그인인 경우, 현재 해당 값은 GUEST+uuid(guestId)
            return loadGuestByGuestId (guesetId); //userName에서 prefix GUEST 떼고 UUID 만 넣음
        }

        User user = userRepository.findByEmail(username)
                .orElseThrow(() -> new UsernameNotFoundException("User not found : " + username));

        return new SecurityUserDetails(user);
    }

    private UserDetails loadGuestByGuestId(String id){
        User guest = Optional.ofNullable((User) redisUtil.getData("guest:" + guestId))
                .orElseThrow(()-> new UsernameNotFoundException("Guest not found : " + guestId));

        redisUtil.setDataExpire("guest:" + guestId, GUEST_TTL_SECONDS);

        return new SecurityUserDetails(guest);
    }
}

loadUserByUsername은 제공되는 함수라 인자를 userName밖에 못 받아서 String 파싱을 통해 구현함.


다음과 같이 코드를 변경 했을 때 다음과 같은 장점을 지니는 것 같습니다.

  1. JwtAuthenticationFilter가 간단함.
  2. 회원 / 비회원 별 클래스를 따로 구현하지 않아 클래스 구조가 단순
  3. 게스트와 일반 유저 구분이 username으로 드러나 직관적

그러나 단점이 더 큽니다.

  1. Spring Security의 원칙 및 장점은 권한 기반 인가 시스템임.
    -> 그러나 String 파싱을 통한 코드 구현을 하면 우리만의 새로운 인가 흐름이 생기는 것.
    -> 본인 외 다른 개발자들이 보기에는 자연스럽지 못한 코드 흐름으로 가독성 측면에서 좋지 않음.
  2. srp를 지키지 못함. 유저용 UserDetails를 반환하던 함수에서 더 추가되어 유저용/비유저용 객체 반환해야 함
    -> 각 케이스 별 로직 처리도 달라, 사실상 하나의 함수가 너무 많은 일을 하는 중
    -> 만약 여기서 권한이 더 늘어나야 한다면 하드코딩 + 가독성 최악 + srp 파괴 의 함수가 될 것으로 예상

결론 : JwtAuthentication에서 토큰 내 권한으로 분기처리를 하는 방법 추천.

형준 선배의 첫 번째 의견처럼
GuestDetailsService guestDetailsService;
UserDetailsService userDetailsService;

이 srp 원칙 준수 + 권한 기반 인가 시스템 로직 유지 + 가독성 최고

일 것 같습니다.

private final TokenProvider tokenProvider;
private final BlackListTokenService blackListTokenService;

Expand All @@ -52,8 +53,16 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse

String accessToken = resolveAccessToken(response, accessTokenHeader);

String username = tokenProvider.getEmail(accessToken);
UserDetails userDetails = userDetailsService.loadUserByUsername(username);
String role = tokenProvider.getRole(accessToken);
UserDetails userDetails;
if (role.equals(String.valueOf(Role.GUEST))) {
Long guestId = tokenProvider.getUserId(accessToken);
userDetails = userDetailsService.loadGuestByUserId(guestId);
}
else{
String username = tokenProvider.getUsername(accessToken);
userDetails = userDetailsService.loadUserByUsername(username);
}

Authentication auth = new UsernamePasswordAuthenticationToken(userDetails, null, userDetails.getAuthorities());
SecurityContextHolder.getContext().setAuthentication(auth);
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/Gotcha/common/redis/RedisProperties.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,6 @@ public interface RedisProperties {
String NICKNAME_VERIFY_KEY_PREFIX = "nicknameVerify::";

long CODE_EXPIRATION_TIME = 5*60;

long GUEST_TTL_SECONDS = 30 * 60;
}
2 changes: 1 addition & 1 deletion src/main/java/Gotcha/common/util/RedisUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public Object getData(String key) {
return redisTemplate.opsForValue().get(key);
}

public void setData(String key, String value) {
public void setData(String key, Object value) {
redisTemplate.opsForValue().set(key, value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ public ResponseEntity<?> signIn(@Valid @RequestBody SignInReq signInReq) {
return createTokenRes(tokenDto);
}

@PostMapping("/guest/sign-in")
public ResponseEntity<?> guestSignIn(){
TokenDto tokenDto = authService.guestSignIn();

return createTokenRes(tokenDto);
}

@PostMapping("/token-reissue")
public ResponseEntity<?> reIssueToken(@CookieValue(name = REFRESH_COOKIE_VALUE, required = false) String refreshToken) {
if (refreshToken == null) {
Expand Down
28 changes: 28 additions & 0 deletions src/main/java/Gotcha/domain/auth/service/AuthService.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import Gotcha.domain.auth.dto.SignUpReq;
import Gotcha.domain.auth.dto.TokenDto;
import Gotcha.domain.auth.exception.AuthExceptionCode;
import Gotcha.domain.auth.util.RandomNicknameGenerator;
import Gotcha.domain.user.entity.Role;
import Gotcha.domain.user.entity.User;
import Gotcha.domain.user.repository.UserRepository;
import jakarta.servlet.http.HttpServletResponse;
Expand All @@ -18,9 +20,11 @@

import java.util.HashMap;
import java.util.Map;
import java.util.UUID;

import static Gotcha.common.jwt.JwtProperties.TOKEN_PREFIX;
import static Gotcha.common.redis.RedisProperties.EMAIL_VERIFY_KEY_PREFIX;
import static Gotcha.common.redis.RedisProperties.GUEST_TTL_SECONDS;
import static Gotcha.common.redis.RedisProperties.NICKNAME_VERIFY_KEY_PREFIX;

@Service
Expand Down Expand Up @@ -77,6 +81,30 @@ public void signOut(String HeaderAccessToken, String refreshToken, HttpServletRe
jwtHelper.removeToken(accessToken, refreshToken, response);
}

public TokenDto guestSignIn(){
//무작위 아이디 값 생성
Long guestId;
do{
guestId = UUID.randomUUID().getMostSignificantBits() & Long.MAX_VALUE;
}while(redisUtil.existed("guest:" + guestId));
//무작위 닉네임 생성
String nickname = RandomNicknameGenerator.generateNickname();

//게스트 유저 생성
User guestUser = User.builder()
.id(guestId)
.nickname(nickname)
.role(Role.GUEST)
.build();

//게스트 유저 정보를 Redis에 저장
redisUtil.setData("guest:" + guestId, guestUser);
redisUtil.setDataExpire("guest:" + guestId, GUEST_TTL_SECONDS);

//게스트 유저 토큰 생성
return jwtHelper.createGuestToken(guestUser);
}

public TokenDto reissueAccessToken(String refreshToken) {
return jwtHelper.reissueToken(refreshToken);
}
Expand Down
29 changes: 29 additions & 0 deletions src/main/java/Gotcha/domain/auth/util/RandomNicknameGenerator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package Gotcha.domain.auth.util;

import java.util.Arrays;
import java.util.List;
import java.util.Random;

public class RandomNicknameGenerator {
private static final List<String> adjectives = Arrays.asList(
"귀여운", "무서운", "배고픈", "졸린", "쓸쓸한", "취한", "웃긴", "멋진", "시끄러운", "느린"
);

private static final List<String> nouns = Arrays.asList(
"고양이", "강아지", "곰", "너구리", "햄스터", "호랑이", "펭귄", "낙타", "토끼", "벌새"
);

private static final Random random = new Random();

public static String generateNickname() {
String nickname;

do {
String adj = adjectives.get(random.nextInt(adjectives.size()));
String noun = nouns.get(random.nextInt(nouns.size()));
nickname = adj + noun;
} while (nickname.length() > 6);

return nickname;
}
}
2 changes: 1 addition & 1 deletion src/main/java/Gotcha/domain/user/entity/Role.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
@RequiredArgsConstructor
@Getter
public enum Role {
USER("ROLE_USER"), ADMIN("ROLE_ADMIN");
USER("ROLE_USER"), ADMIN("ROLE_ADMIN"), GUEST("ROLE_GUEST");

private final String value;
}
3 changes: 2 additions & 1 deletion src/main/java/Gotcha/domain/user/entity/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ public class User extends BaseTimeEntity {
private List<BugReport> bugReports = new ArrayList<>();

@Builder
public User(String email, String password, String nickname, Role role){
public User(Long id, String email, String password, String nickname, Role role){
this.id = id;
this.email = email;
this.password = password;
this.nickname = nickname;
Expand Down