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 week mission #20

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions 4Week_Mission/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,5 @@ application.yml
application-dev.yml
application-prod.yml
application-test.yml
application-base-addi.yml
application-API-KEY.properties
4 changes: 4 additions & 0 deletions 4Week_Mission/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ dependencies {

implementation 'org.springframework.boot:spring-boot-starter-batch'

implementation 'io.jsonwebtoken:jjwt-api:0.11.5'
runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.11.5'
runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.11.5'

testImplementation 'org.springframework.batch:spring-batch-test'
testImplementation 'org.springframework.boot:spring-boot-starter-test'
testImplementation 'org.springframework.security:spring-security-test'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package com.ll.exam.ebooks.app.api.controller;

import com.ll.exam.ebooks.app.base.dto.RsData;
import com.ll.exam.ebooks.app.base.rq.Rq;
import com.ll.exam.ebooks.app.member.entity.Member;
import com.ll.exam.ebooks.app.member.form.LoginForm;
import com.ll.exam.ebooks.app.member.service.MemberService;
import com.ll.exam.ebooks.app.myBook.entity.MyBook;
import com.ll.exam.ebooks.app.myBook.service.MyBookService;
import com.ll.exam.ebooks.app.security.dto.MemberContext;
import com.ll.exam.ebooks.util.Ut;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.ResponseEntity;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

import java.util.List;

@RestController
@RequestMapping("/api/v1")
@RequiredArgsConstructor
@Slf4j
public class ApiController {
Copy link

Choose a reason for hiding this comment

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

ApiController 를 member, myBooks 로 분리하는게 어떨까요??

Copy link
Member

Choose a reason for hiding this comment

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

동의합니다!


private final MemberService memberService;
private final MyBookService myBookService;

@PostMapping("/member/login")
public ResponseEntity<RsData> login(@RequestBody LoginForm loginForm) {
Member member = memberService.findByUsername(loginForm.getUsername());

if (member == null) {
return Ut.spring.responseEntityOf(RsData.of("F-1", "일치하는 회원이 존재하지 않습니다."));
}

Copy link

Choose a reason for hiding this comment

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

username, password 검증은 어디에서 이루어지나요???

String accessToken = memberService.genAccessToken(member);

return Ut.spring.responseEntityOf(
RsData.of(
"S-1",
"로그인 성공, Access Token을 발급합니다.",
Ut.mapOf(
"accessToken", accessToken
)
),
Ut.spring.httpHeadersOf("Authentication", accessToken)
);
}

@GetMapping("/member/me")
public ResponseEntity<RsData> me(@AuthenticationPrincipal MemberContext memberContext) {
if (memberContext == null) { // 임시코드, 나중에는 시프링 시큐리티를 이용해서 로그인을 안했다면, 아예 여기로 못 들어오도록
return Ut.spring.responseEntityOf(RsData.failOf(null));
}

return Ut.spring.responseEntityOf(RsData.successOf(memberContext.getMember()));
Copy link

Choose a reason for hiding this comment

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

REST API 로 설계할 때는 엔티티(member)객체를 넘겨주는 것보다 Response 용 DTO 를 만들어 요구사항에서 주어진 값들만 응답바디에 포함시키는게 좋을 것 같습니다!!

}

@GetMapping("/myBooks")
public ResponseEntity<RsData> myBooks(@AuthenticationPrincipal MemberContext memberContext) {
Member member = memberContext.getMember();

log.debug("member: " + member.getId());

if (member == null) {
return Ut.spring.responseEntityOf(

Choose a reason for hiding this comment

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

다음과 같이 예외가 발생했을 때? 비즈니스 로직상 문제가 생겼을 때 소망님과 같이 바로 실패 응답 메시지를 컨트롤러에서 처리하신거 예외를 빠짐없이 잘 처리하신 것 같습니다.
하지만 RestControllerAdvice를 통해 모든 예외를 공통적으로 처리할 수 있습니다. 이렇게 되면 예외만 발생시켜도 해당 예외에 대해서 응답 메시지를 만들고 처리하는 로직이 컨트롤러와 분리되어 컨트롤러를 깔끔하게 만들 수 있고, 예외가 다양하더라도 공통적으로 처리할 수 있어 간편할 것 같습니다. @RestControllerAdvice에 대해서 찾아보시면 좋을 것 같습니다.

RsData.of(
"F-1",
"조회 권한이 없습니다."
)
);
}
Comment on lines +70 to +77
Copy link

Choose a reason for hiding this comment

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

여기서 member == null 일때를 검사하는 로직이 꼭 필요할까요???
저는 수업에서처럼 해당 로직을 따로 넣진 않았습니다! JwtAuthorizationFilter 에서 accessToken 검증에 성공하고 강제 로그인처리될 때, 만약에 member 가 null 이었다면 이미 필터에서 오류가 나서 컨트롤러 단에서는 member 가 null 일 경우는 띠로 고려하지 않아도 된다고 생각했습니다. jwt나 필터에 대해서 확실하게 알지 못해서 저도 이부분이 궁금하네요! 혹시 잘 아시는 분 있으신가요???

Copy link
Member

Choose a reason for hiding this comment

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

저도 이 부분 궁금합니다! 저는 혹시 몰라서 member null체크를 해주긴 했는데 승연님 말씀처럼 세션이든 토큰이든 만료가 되면 1차적으로 필터에서(필터에 세션 정보를 업데이트하는 로직도 포함시켰다는 전제 하에) 걸러주기 때문에 컨트롤러에서 member null 체크는 필요 없는지 궁금해요!!


List<MyBook> myBooks = myBookService.findAllByOwnerId(member.getId());

log.debug("myBooks: " + myBooks);

return Ut.spring.responseEntityOf(
RsData.successOf(
Ut.mapOf(
"myBooks", myBooks
)
)
);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package com.ll.exam.ebooks.app.base.dto;

import lombok.AllArgsConstructor;
import lombok.Data;

@Data
@AllArgsConstructor
public class RsData<T> {
private String resultCode;
private String msg;
private T data;

public static <T> RsData<T> of(String resultCode, String msg, T data) {
return new RsData<>(resultCode, msg, data);
}

public static <T> RsData<T> of(String resultCode, String msg) {
return of(resultCode, msg, null);
}

public static <T> RsData<T> successOf(T data) {
return of("S-1", "성공", data);
}

public static <T> RsData<T> failOf(T data) {
return of("F-1", "실패", data);
}

public boolean isSuccess() {
return resultCode.startsWith("S-1");
}

public boolean isFail() {
return isSuccess() == false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.ll.exam.ebooks.app.base.entity.BaseEntity;
import com.ll.exam.ebooks.util.Ut;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;
Expand All @@ -16,6 +17,7 @@
import javax.persistence.Entity;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

@Entity
@Getter
Expand All @@ -36,10 +38,13 @@ public class Member extends BaseEntity {

private String nickname;

@JsonIgnore
private int authLevel;

@JsonIgnore
private long restCash; // 예치금

@JsonIgnore
Comment on lines +41 to +47
Copy link

Choose a reason for hiding this comment

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

아하 JsonIgnore 을 붙여 JSON 응답에 포함하지 않도록 하셨군요!
하지만 아까 말씀드렸던 것처럼 DTO로 반환하는게 조금 더 안전하고 좋은 방법 같습니다!

public String getName() {
if (nickname == null) {
return username;
Expand All @@ -51,6 +56,7 @@ public Member(long id) {
super(id);
}

@JsonIgnore
public List<GrantedAuthority> getAuthorities() {
List<GrantedAuthority> authorities = new ArrayList<>();
authorities.add(new SimpleGrantedAuthority("MEMBER"));
Expand All @@ -65,4 +71,17 @@ public List<GrantedAuthority> getAuthorities() {

return authorities;
}

@JsonIgnore
public Map<String, Object> getAccessTokenClaims() {
return Ut.mapOf(
"id", getId(),
"createDate", getCreateDate(),
"modifyDate", getModifyDate(),
"username", getUsername(),
"email", getEmail(),
"authLevel", getAuthLevel(),
"authorities", getAuthorities()
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.ll.exam.ebooks.app.member.form;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;

import javax.validation.constraints.NotEmpty;

@Data
@Builder
@NoArgsConstructor
@AllArgsConstructor
public class LoginForm {
@NotEmpty
private String username;

@NotEmpty
private String password;
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.ll.exam.ebooks.app.member.exception.MemberNotFoundException;
import com.ll.exam.ebooks.app.member.repository.MemberRepository;
import com.ll.exam.ebooks.app.security.dto.MemberContext;
import com.ll.exam.ebooks.app.security.jwt.JwtProvider;
import lombok.RequiredArgsConstructor;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.context.SecurityContext;
Expand All @@ -28,6 +29,7 @@ public class MemberService {
private final PasswordEncoder passwordEncoder;
private final MailService mailService;
private final CashService cashService;
private final JwtProvider jwtProvider;


@Transactional
Expand Down Expand Up @@ -74,6 +76,10 @@ public Member adminJoin(JoinForm joinForm) {
return member;
}

public String genAccessToken(Member member) {

Choose a reason for hiding this comment

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

jwt 관련 코드를 한 곳에 모으면 좋을 것 같다?라는 생각이 들었습니다!!
사실 개인적인 생각이지만, jwt관련 해서 코드의 수정이 필요하다면 memberService 내의 코드를 변경해야하는 일이 발생하기 때문에 memberService의 책임이 많다는 생각이 들었습니다.!

return jwtProvider.generatedAccessToken(member.getAccessTokenClaims(), 60 * 60 * 24 * 90);
}

public Member findByEmail(String email) {
return memberRepository.findByEmail(email).orElse(null);
}
Expand Down Expand Up @@ -132,7 +138,7 @@ public boolean beAuthor(Member member, String nickname) {
}

private void forceAuthentication(Member member) {
MemberContext memberContext = new MemberContext(member, member.getAuthorities());
MemberContext memberContext = new MemberContext(member);

UsernamePasswordAuthenticationToken authentication =
UsernamePasswordAuthenticationToken.authenticated(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
import com.ll.exam.ebooks.app.myBook.entity.MyBook;
import org.springframework.data.jpa.repository.JpaRepository;

import java.util.List;

public interface MyBookRepository extends JpaRepository<MyBook, Long> {
void deleteByProductIdAndOwnerId(Long productId, Long ownerId);

List<MyBook> findAllByOwnerId(Long ownerId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;

@Service
@Transactional(readOnly = true)
@RequiredArgsConstructor
Expand All @@ -35,4 +37,8 @@ public void remove(Order order) {
.stream()
.forEach(orderItem -> myBookRepository.deleteByProductIdAndOwnerId(orderItem.getProduct().getId(), order.getBuyer().getId()));
}

public List<MyBook> findAllByOwnerId(Long OwnerId) {
return myBookRepository.findAllByOwnerId(OwnerId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,4 @@

public interface ProductRepository extends JpaRepository<Product, Long> {
List<Product> findAllByOrderByIdDesc();

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.ll.exam.ebooks.app.security;

import com.ll.exam.ebooks.app.security.service.CustomUserDetailsService;
import com.ll.exam.ebooks.app.security.filter.JwtAuthorizationFilter;
import lombok.RequiredArgsConstructor;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
Expand All @@ -12,19 +12,19 @@
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;

@Configuration
@EnableWebSecurity
@EnableGlobalMethodSecurity(prePostEnabled = true)
@RequiredArgsConstructor
public class SecurityConfig {

private final CustomUserDetailsService customUserDetailsService;

@Bean
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
http
.csrf().disable()
.cors().disable() // 타 도메인에서 API 호출 가능
.csrf().disable() // CSRF 토큰 끄기
.formLogin(
formLogin -> formLogin
.loginPage("/member/login")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.ll.exam.ebooks.app.security.dto;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.ll.exam.ebooks.app.member.entity.Member;
import lombok.Getter;
import lombok.Setter;
Expand All @@ -8,27 +9,28 @@

import java.time.LocalDateTime;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

@Getter
@JsonIgnoreProperties({"id", "createDate", "modifyDate", "username", "email", "nickname"})

Choose a reason for hiding this comment

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

이 부분을 추가하신 이유가 궁금합니다!

public class MemberContext extends User {
private final Long id;
private final String username;
@Setter
private String email;
@Setter
private String nickname;
private final String email;
private final String nickname;
private final LocalDateTime createDate;
@Setter
private LocalDateTime modifyDate;

public MemberContext(Member member, List<GrantedAuthority> authorities) {
super(member.getUsername(), member.getPassword(), authorities);
this.id = member.getId();
this.username = member.getUsername();
this.email = member.getEmail();
this.nickname = member.getNickname();
this.createDate = member.getCreateDate();
this.modifyDate = member.getModifyDate();
private final LocalDateTime modifyDate;

public MemberContext(Member member) {
super(member.getUsername(), "", member.getAuthorities());

id = member.getId();
username = member.getUsername();
email = member.getEmail();
nickname = member.getNickname();
createDate = member.getCreateDate();
modifyDate = member.getModifyDate();
}

public Member getMember() {
Expand Down
Loading