스프링부트 게시판 API 서버 만들기 (30) - 코드 리팩토링

이번 시간에는 개선점이 필요한 코드를 살펴보고, 리팩토링하는 시간을 가져보겠습니다.

 

 

우리 코드에서는 중복이 나타나고 있습니다.

 

먼저 Security 설정을 확인해봅시다.

// SecurityConfig.java
.antMatchers(HttpMethod.DELETE, "/api/members/{id}/**").access("@memberGuard.check(#id)")
...
.antMatchers(HttpMethod.PUT, "/api/posts/{id}").access("@postGuard.check(#id)")
.antMatchers(HttpMethod.DELETE, "/api/posts/{id}").access("@postGuard.check(#id)")
...
.antMatchers(HttpMethod.DELETE, "/api/comments/{id}").access("@commentGuard.check(#id)")

config.security.SecurityConfig에서는, 직접 정의한 Guard들을 이용해서 API에 따른 접근을 제어하고 있습니다.

스프링 빈으로 등록된 Guard들의 check 메소드를 호출하면서, 요청자를 인가해주기 위해 path parameter를 넘겨주는 것입니다.

 

 

이제 각각의 Guard 들을 살펴봅시다.

package kukekyakya.kukemarket.config.security.guard;

import ...

@Component
@RequiredArgsConstructor
@Slf4j
public class MemberGuard {

    private final AuthHelper authHelper;

    public boolean check(Long id) {
        return authHelper.isAuthenticated() && hasAuthority(id);
    }

    private boolean hasAuthority(Long id) {
        Long memberId = authHelper.extractMemberId();
        Set<RoleType> memberRoles = authHelper.extractMemberRoles();
        return id.equals(memberId) || memberRoles.contains(RoleType.ROLE_ADMIN);
    }
}

package kukekyakya.kukemarket.config.security.guard;

import ...

@Component
@RequiredArgsConstructor
@Slf4j
public class CommentGuard {
    private final AuthHelper authHelper;
    private final CommentRepository commentRepository;

    public boolean check(Long id) {
        return authHelper.isAuthenticated() && hasAuthority(id);
    }

    private boolean hasAuthority(Long id) {
        return hasAdminRole() || isResourceOwner(id);
    }

    private boolean isResourceOwner(Long id) {
        Comment comment = commentRepository.findById(id).orElseThrow(() -> { throw new AccessDeniedException(""); });
        Long memberId = authHelper.extractMemberId();
        return comment.getMember().getId().equals(memberId);
    }

    private boolean hasAdminRole() {
        return authHelper.extractMemberRoles().contains(RoleType.ROLE_ADMIN);
    }
}

package kukekyakya.kukemarket.config.security.guard;

import ...

@Component
@RequiredArgsConstructor
@Slf4j
public class PostGuard {
    private final AuthHelper authHelper;
    private final PostRepository postRepository;

    public boolean check(Long id) {
        return authHelper.isAuthenticated() && hasAuthority(id);
    }

    private boolean hasAuthority(Long id) {
        return hasAdminRole() || isResourceOwner(id);
    }

    private boolean isResourceOwner(Long id) {
        Post post = postRepository.findById(id).orElseThrow(() -> { throw new AccessDeniedException(""); });
        Long memberId = authHelper.extractMemberId();
        return post.getMember().getId().equals(memberId);
    }

    private boolean hasAdminRole() {
        return authHelper.extractMemberRoles().contains(RoleType.ROLE_ADMIN);
    }
}

 

코드를 살펴보면, 유사한 로직이 반복되고 있습니다.

1. AuthHelper.isAuthenticated - 인증된 사용자인지 확인

2. hasAdminRole - 관리자인지 확인

3. isResourceOwner - 자원의 소유주인지 확인

결국 위 세 단계의, ((1번) AND (2번 || 3번)) 조건식을 거쳐서 검증하고 있는 것입니다.

 

* 물론, MemberGuard에는 isResourceOwner가 명시되어있진 않지만, 요청자 id와 요청하는 id가 일치하는지 확인하면서 자원의 소유주인지 검증하고 있습니다.

 

위 코드를 살펴보면, 1번과 2번 작업은 모두 완전히 동일하고, 결국 달라지는건 3번 뿐입니다.

중복이 나타나고 있는 것이며, 모두 동일한 메소드 시그니처를 가지고 있음에도 불구하고, 직접 정의해줘야하는 번거로움이 있습니다.

이를 해결해봅시다.

 

 

추상 클래스 Guard를 작성해주겠습니다.

동일한 알고리즘은 템플릿 메소드에 정의해두고, 변화가 필요한 작업은 추상 메소드를 호출하겠습니다.

package kukekyakya.kukemarket.config.security.guard;

import ...

public abstract class Guard {
    public final boolean check(Long id) {
        return AuthHelper.isAuthenticated() && (hasRole(getRoleTypes()) || isResourceOwner(id));
    }

    abstract protected List<RoleType> getRoleTypes();
    abstract protected boolean isResourceOwner(Long id);

    private boolean hasRole(List<RoleType> roleTypes) {
        return roleTypes.stream().allMatch(roleType -> AuthHelper.extractMemberRoles().contains(roleType));
    }
}

 

허용되는 권한을 가져오는 메소드와 자원의 소유주인지 판별하는 메소드는, 추상 메소드로 선언하여 하위 클래스에서 구현할 수 있도록 하겠습니다.

템플릿이 되는 check 메소드는, 재정의할 수 없도록 final로 지정하였습니다.

 

* 기존의 코드에서는 모두 관리자 권한인지만 검사하였지만, 유연함을 위해 권한 정보도 전달받도록 하겠습니다. 전달받은 모든 권한 정보를 가지고 있어야할 것입니다.

 

코드를 살펴보면, AuthHelper.isAuthenticated()는 스태틱 메소드로 호출되고 있는 것을 확인할 수 있습니다.

헬퍼 클래스로 사용되고 있는 AuthHelper의 모든 메소드는 스태틱으로 바꿔주고, 이제는 빈으로 등록해두지 않겠습니다.

하위 클래스에서 의존성을 주입 받은 뒤, 상위 클래스의 생성자에 명시해주는 번거로움을 방지하기 위함입니다.

package kukekyakya.kukemarket.config.security.guard;

import ...

@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class AuthHelper {

    public static boolean isAuthenticated() {
        return ...;
    }

    public static Long extractMemberId() {
        return ...;
    }

    public static Set<RoleType> extractMemberRoles() {
        return ...;
    }

    private static CustomUserDetails getUserDetails() {
        return ...;
    }

    private static Authentication getAuthentication() {
        return ...;
    }
}

코드는 다음과 같습니다.

직접 인스턴스를 생성하여 사용하지 않도록, private 생성자를 만들어주었습니다.

 

 

이제 기존의 Guard 클래스들이, 추상 클래스 Guard를 상속받도록 하겠습니다.

package kukekyakya.kukemarket.config.security.guard;

import ...

@Component
@RequiredArgsConstructor
public class MemberGuard extends Guard {
    private List<RoleType> roleTypes = List.of(RoleType.ROLE_ADMIN);

    @Override
    protected List<RoleType> getRoleTypes() {
        return roleTypes;
    }

    @Override
    protected boolean isResourceOwner(Long id) {
        return id.equals(AuthHelper.extractMemberId());
    }
}

package kukekyakya.kukemarket.config.security.guard;

import ...

@Component
@RequiredArgsConstructor
public class CommentGuard extends Guard {
    private final CommentRepository commentRepository;
    private List<RoleType> roleTypes = List.of(RoleType.ROLE_ADMIN);

    @Override
    protected List<RoleType> getRoleTypes() {
        return roleTypes;
    }

    @Override
    protected boolean isResourceOwner(Long id) {
        Comment comment = commentRepository.findById(id).orElseThrow(() -> { throw new AccessDeniedException(""); });
        Long memberId = AuthHelper.extractMemberId();
        return comment.getMember().getId().equals(memberId);
    }
}

package kukekyakya.kukemarket.config.security.guard;

import ...

@Component
@RequiredArgsConstructor
public class PostGuard extends Guard {
    private final PostRepository postRepository;
    private List<RoleType> roleTypes = List.of(RoleType.ROLE_ADMIN);

    @Override
    protected List<RoleType> getRoleTypes() {
        return roleTypes;
    }

    @Override
    protected boolean isResourceOwner(Long id) {
        Post post = postRepository.findById(id).orElseThrow(() -> { throw new AccessDeniedException(""); });
        Long memberId = AuthHelper.extractMemberId();
        return post.getMember().getId().equals(memberId);
    }
}

 

추상 메소드를 구현해주었을 뿐입니다.

 

 

리팩토링으로 인해 다른 문제가 발생하진 않았는지 테스트를 수행해봅시다.

 

 

이번 시간에는 코드에서 나타나는 문제점을 살펴보고, 개선하는 시간을 가져보았습니다.

다음 시간에는 사용자 간에 쪽지를 전송할 수 있도록 하고, 쪽지 송수신 내역은 무한 스크롤을 이용하여 페이징 처리해보겠습니다.

 

 

* 질문 및 피드백은 환영입니다.

* 전체 소스코드에서는 여기에서 확인해볼 수 있습니다.

https://github.com/SongHeeJae/kuke-market