Skip to content

선택 - RoleHierarchy 리뷰 부탁드립니다. #26

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

Open
wants to merge 2 commits into
base: jukekxm
Choose a base branch
from

Conversation

jukekxm
Copy link

@jukekxm jukekxm commented Feb 15, 2025

Spring Security의 코드를 가져와서 필요없는 부분은 제거하고, 수정할 부분은 수정하는 방식으로 구현했습니다.
리뷰 부탁드립니다!

Copy link

@parkeeseul parkeeseul left a comment

Choose a reason for hiding this comment

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

선택 미션도 빠르게 구현해 주셨네요! 👍
코멘트 남겨 두었으니 확인 부탁드리겠습니다 😄
화이팅 입니다 🔥


import java.util.*;

public class RoleHierarchyImpl implements RoleHierarchy {

Choose a reason for hiding this comment

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

만약 다음과 같이 권한의 계층 구조를 설정할 경우에는 어떻게 처리되어야 할까요?

RoleHierarchyImpl.with()
  .role("ADMIN").implies("USER")
  .role("USER").implies("GUEST")
  .build();

현재 구조에서는 직접적인 하위 역할만 추가하고 있어서, ADMIN이 USER만 포함하고 GUEST까지 확장되지 않는 문제가 발생할 수 있습니다.

Spring Security의 RoleHierarchyImpl은 buildRolesReachableInOneOrMoreStepsMap()을 활용하여 모든 계층 관계를 반영하는데, 현재 코드에서는 해당 로직이 없어 다단계 계층 탐색이 누락될 가능성이 있습니다 😢

개선해보면 어떨까요? 😄


@DisplayName("상위 권한의 reachable authorities는 사용자의 권한과 하위 권한을 같이 반환한다.")
@Test
public void roleHierarchyTest() {

Choose a reason for hiding this comment

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

테스트 코드 👍

코드 개선해 보시면서 아래 케이스에 대한 테스트 코드도 작성해 보면 좋을 것 같네요 😄

RoleHierarchyImpl.with()
  .role("ADMIN").implies("USER")
  .role("USER").implies("GUEST")
  .build();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants