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

코드 리뷰~ #4

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
251 changes: 251 additions & 0 deletions src/main/java/kr/where/backend/admin/AdminController.java

Large diffs are not rendered by default.

16 changes: 16 additions & 0 deletions src/main/java/kr/where/backend/admin/dto/AdminInfo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package kr.where.backend.admin.dto;

import io.swagger.v3.oas.annotations.media.Schema;
import lombok.Getter;
import lombok.Setter;
@Getter
@Setter
@Schema(description = "사용자 정보")
public class AdminInfo {

@Schema(description = "이름", example = "홍길동")
private String name;

@Schema(description = "비밀번호", example = "****")
private String passwd;
}
14 changes: 14 additions & 0 deletions src/main/java/kr/where/backend/admin/dto/KeyValueInfo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package kr.where.backend.admin.dto;

import io.swagger.v3.oas.annotations.media.Schema;
import lombok.Getter;
import lombok.Setter;

@Getter
@Setter
@Schema(description = "JSON 형식의 key:value")
public class KeyValueInfo {

@Schema(description = "{'key':'value'}", example = "value")
private String key;
}
49 changes: 49 additions & 0 deletions src/main/java/kr/where/backend/auth/authUser/AuthUser.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package kr.where.backend.auth.authUser;

import kr.where.backend.auth.authUser.exception.AuthUserException;
import lombok.AccessLevel;
import lombok.Builder;
import lombok.NoArgsConstructor;
import lombok.Setter;
import org.springframework.security.core.context.SecurityContextHolder;

@Setter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class AuthUser {
private Integer intraId;
private String intraName;
private Long defaultGroupId;

@Builder
public AuthUser(final Integer intraId, final String intraName, final Long defaultGroupId) {
this.intraId = intraId;
this.intraName = intraName;
this.defaultGroupId = defaultGroupId;
}

public Integer getIntraId() {
return intraId;
}

public String getIntraName() {
return intraName;
}

public Long getDefaultGroupId() {
if (defaultGroupId != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

defaultGroupId가 널일 경우가 언제인가요?? where42 서비스 동의를 안 한 경우인가요??
그리고 유효할 경우 return 정상, 아닌경우 자연스럽게 예외를 throw하는 방법이 너무 코드가 깔끔하고 좋은 방법이라 생각하는데 이런 플로우를 짜게 되면 일관성 있게 다른 코드들도 이렇게 구현하는게 나을까요??

Copy link
Collaborator

Choose a reason for hiding this comment

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

예를 들어 if 유효하지 않은 경우 {throw}랑 위와 같은 코드랑 고민이 될 때가 있어서 남깁니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

defaultGroup id가 없는경우는 where42 동의를 하지 않은 경우 입니다
-> 비동의 맴버로 남는다면, defaultId 없이 Member 엔티티만 생성 됩니다

음 마지막 질문이 이해가 잘 안되는데 혹시 어떤 부분일까요?
저희 컨벤션은 if (유효한 경우, 유효한 값을 리턴) 그 외 throw 하는 방식을 선택하고 있습니다.
그리고 만약 자료구조를 순회하거나, Optional<>을 사용할때는 람다식으로 orElseThrow()를 사용해서 예외 처리하고 있습니다

컨벤션은 정의하고 팀 끼리 지켜나가면 좋을거같습니다
다른 질문이었다면 편하게 남겨주세용

Copy link
Collaborator

Choose a reason for hiding this comment

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

"저희 컨벤션은 if (유효한 경우, 유효한 값을 리턴) 그 외 throw 하는 방식을 선택하고 있습니다." 로 이해 했습니다 ! 딱 제가 원하던 답변을 주셨어요. 감사합니다.

Optional<> 을 좀 많이 사용해보고 람다식도 얼른 연습해보겠습니다.

return defaultGroupId;
}
throw new AuthUserException.ForbiddenUserException();
}

public void setDefaultGroupId(final Long groupId) {
this.defaultGroupId = groupId;
}
public static AuthUser of() {
final Object principle = SecurityContextHolder.getContext().getAuthentication().getPrincipal();
if (principle.equals("anonymousUser")) {
throw new AuthUserException.AnonymousUserException();
}
return (AuthUser) principle;
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.

저희 v2의 컨벤션이에요

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.

음 저희는 만나서 코드 작성해서 정해진건 없지만 네이버를 기준으로 합니다

}
}
13 changes: 13 additions & 0 deletions src/main/java/kr/where/backend/auth/authUser/AuthUserInfo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package kr.where.backend.auth.authUser;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import org.springframework.security.core.annotation.AuthenticationPrincipal;

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.PARAMETER)
@AuthenticationPrincipal
public @interface AuthUserInfo {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package kr.where.backend.auth.authUser.exception;

import kr.where.backend.exception.ErrorCode;
import lombok.AllArgsConstructor;
import lombok.Getter;

@Getter
@AllArgsConstructor
public enum AuthUserErrorCode implements ErrorCode {
FORBIDDEN_USER(1800, "Where42 서비스에 동의한 user가 아닙니다."),
ANONYMOUS_USER(1801, "인가 인증을 받지 않은 user 입니다");

private final int errorCode;
private final String errorMessage;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package kr.where.backend.auth.authUser.exception;

import kr.where.backend.exception.CustomException;

public class AuthUserException extends CustomException {
public AuthUserException(final AuthUserErrorCode authUserErrorCode) {
super(authUserErrorCode);
}

public static class ForbiddenUserException extends AuthUserException {
public ForbiddenUserException() {
super(AuthUserErrorCode.FORBIDDEN_USER);
}
}

public static class AnonymousUserException extends AuthUserException {
public AnonymousUserException() {
super(AuthUserErrorCode.ANONYMOUS_USER);
}
}
}
24 changes: 24 additions & 0 deletions src/main/java/kr/where/backend/auth/filter/JwtConstants.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package kr.where.backend.auth.filter;

public enum JwtConstants {
HEADER_TYPE("Bearer"),
ACCESS("accessToken"),
REFRESH("refreshToken"),
USER_SUBJECTS("User"),
USER_ID("intraId"),
USER_NAME("intraName"),
USER_ROLE("Cadet"),
TOKEN_TYPE("type"),
ROLE_LEVEL("roles"),
REISSUE_URI("/v3/jwt/reissue");

private final String value;

JwtConstants(final String value) {
this.value = value;
}

public String getValue() {
return this.value;
}
}
49 changes: 49 additions & 0 deletions src/main/java/kr/where/backend/auth/filter/JwtExceptionFilter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package kr.where.backend.auth.filter;

import static com.fasterxml.jackson.core.JsonEncoding.UTF8;

import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import kr.where.backend.jwt.exception.JwtException;
import kr.where.backend.member.exception.MemberException;
import lombok.RequiredArgsConstructor;
import org.springframework.http.MediaType;
import org.springframework.stereotype.Component;
import org.springframework.web.filter.OncePerRequestFilter;

@Component
@RequiredArgsConstructor
public class JwtExceptionFilter extends OncePerRequestFilter {
private final ObjectMapper objectMapper;

@Override
protected void doFilterInternal(
final HttpServletRequest request,
final HttpServletResponse response,
final FilterChain filterChain
) throws ServletException, IOException {
try {
filterChain.doFilter(request, response);
} catch (final JwtException e) {
sendRequest(response, HttpServletResponse.SC_UNAUTHORIZED, e.toString());
} catch (final MemberException e) {
sendRequest(response, HttpServletResponse.SC_NOT_FOUND, e.toString());
}
}

private void sendRequest(
final HttpServletResponse response,
final int errorCode,
final String error
) throws IOException {
response.setStatus(errorCode);
response.setCharacterEncoding(UTF8.getJavaName());
response.setContentType(MediaType.APPLICATION_JSON_VALUE);

response.getWriter().write(objectMapper.writeValueAsString(error));
}
}
37 changes: 37 additions & 0 deletions src/main/java/kr/where/backend/auth/filter/JwtFilter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package kr.where.backend.auth.filter;

import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import kr.where.backend.jwt.JwtService;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.stereotype.Component;
import org.springframework.web.filter.OncePerRequestFilter;

import java.io.IOException;

@Component
@RequiredArgsConstructor
@Slf4j
public class JwtFilter extends OncePerRequestFilter {
private final JwtService jwtService;

@Override
protected void doFilterInternal(
final HttpServletRequest request,
final HttpServletResponse response,
final FilterChain filterChain)
throws ServletException, IOException {

final String token = jwtService.extractToken(request).orElse(null);
if (token != null) {
final Authentication auth = jwtService.getAuthentication(request, token);
SecurityContextHolder.getContext().setAuthentication(auth);
}
filterChain.doFilter(request, response);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package kr.where.backend.auth.filter.exception;

import static com.fasterxml.jackson.core.JsonEncoding.UTF8;

import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.web.access.AccessDeniedHandler;
import org.springframework.stereotype.Component;

import java.io.IOException;

@Component
@RequiredArgsConstructor
public class CustomAccessDeniedHandler implements AccessDeniedHandler {
private final ObjectMapper objectMapper;

@Override
public void handle(final HttpServletRequest request,
final HttpServletResponse response,
final AccessDeniedException accessDeniedException) throws IOException {
//필요한 권한이 없이 접근하려 할때 403
sendErrorResponse(response);
// response.sendError(HttpServletResponse.SC_FORBIDDEN);
}

private void sendErrorResponse(final HttpServletResponse response) throws IOException{
response.setStatus(HttpServletResponse.SC_FORBIDDEN);
response.setCharacterEncoding(UTF8.getJavaName());
response.setContentType(MediaType.APPLICATION_JSON_VALUE);

response.getWriter().write(objectMapper.writeValueAsString("접근 권한을 확인하세요."));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package kr.where.backend.auth.filter.exception;

import static com.fasterxml.jackson.core.JsonEncoding.UTF8;

import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import kr.where.backend.exception.CustomException;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.web.AuthenticationEntryPoint;
import org.springframework.stereotype.Component;
import org.springframework.web.servlet.HandlerExceptionResolver;

@Slf4j
@Component
public class JwtAuthenticationEntryPoint implements AuthenticationEntryPoint {
private final HandlerExceptionResolver exceptionResolver;

public JwtAuthenticationEntryPoint(@Qualifier("handlerExceptionResolver") final HandlerExceptionResolver exceptionResolver) {
this.exceptionResolver = exceptionResolver;
}
@Override
public void commence(final HttpServletRequest request, final HttpServletResponse response,
final AuthenticationException authException) throws IOException, ServletException {
String error = (String) request.getAttribute("exception");
response.setContentType("application/json;charset=UTF-8");
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);

response.setCharacterEncoding(UTF8.getJavaName());
response.setContentType(MediaType.APPLICATION_JSON_VALUE);
ObjectMapper objectMapper = new ObjectMapper();
response.getWriter().write(objectMapper.writeValueAsString(error));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package kr.where.backend.auth.oauth2login;

import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService;
import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest;
import org.springframework.security.oauth2.client.userinfo.OAuth2UserService;
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
import org.springframework.security.oauth2.core.user.OAuth2User;
import org.springframework.stereotype.Service;

import java.util.Collections;
import java.util.Map;

@RequiredArgsConstructor
@Service
@Slf4j
public class CustomOauth2UserService implements OAuth2UserService<OAuth2UserRequest, OAuth2User> {

@Override
public UserProfile loadUser(final OAuth2UserRequest userRequest) throws OAuth2AuthenticationException {
final OAuth2UserService<OAuth2UserRequest, OAuth2User> oAuth2UserService = new DefaultOAuth2UserService();
final OAuth2User oAuth2User = oAuth2UserService.loadUser(userRequest);

final Map<String, Object> attributes = oAuth2User.getAttributes();
final String registrationId = userRequest.getClientRegistration().getRegistrationId();

return new UserProfile(
registrationId,
Collections.singleton(new SimpleGrantedAuthority("ROLE_USER")),
attributes
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package kr.where.backend.auth.oauth2login;

import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.web.authentication.SimpleUrlAuthenticationFailureHandler;
import org.springframework.stereotype.Component;
import org.springframework.web.util.UriComponentsBuilder;

import java.io.IOException;
import java.rmi.RemoteException;

@Component
public class OAuth2FailureHandler extends SimpleUrlAuthenticationFailureHandler {

@Override
public void onAuthenticationFailure(
HttpServletRequest request, HttpServletResponse response, AuthenticationException exception)
throws IOException, ServletException {
response.sendRedirect("https://where42.kr/login-fail");
}
}
Loading