-
Notifications
You must be signed in to change notification settings - Fork 2
[Feat] 사용자 위치 정보 접근 로그 저장 및 관리자 페이지(위치정보시스템) 구현 #244
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
[Feat] 사용자 위치 정보 접근 로그 저장 및 관리자 페이지(위치정보시스템) 구현 #244
Conversation
- 폼 로그인 시 JWT토큰 생성, JWT토큰에 role추가 - JWT토큰을 쿠키에 담아 브라우저에 전송 - JwtAuthenticationFilter에서 쿠키에 담겨오는 JWT토큰도 검증하도록 추가 구현, 검증 후 Security Context에 role도 함께 저장하여 관리자 페이지에 SUPER_ADMIN과 ADMIN역할의 계정만 접근할 수 있도록 함. - User엔티티에 role을 추가함에 따라 기존 따따 서비스 회원가입 시 role을 USER로 설정하고 로그인 시 role을 JWT토큰에 넣도록 구현
- 위치정보로그 종류 1. 일기 저장 시 2. 지도 일기 페이징 조회 시 3. 위치기반리마인드 알림 on/off시 4. 위치기반리마인드 알림 전송시
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
이번 PR은 사용자 위치 정보 접근에 대한 로그를 기록하고, 이를 조회할 수 있는 관리자 페이지를 구현하는 중요한 변경사항을 담고 있습니다. 전반적으로 기능 구현의 완성도가 높지만, 몇 가지 보안 및 안정성 관련 개선점을 발견했습니다.
가장 중요한 점은 JwtAuthenticationFilter에서 쿠키 기반 인증 시 발생할 수 있는 보안 취약점입니다. 로그아웃된 사용자의 접근을 막지 못할 수 있으므로 반드시 수정이 필요합니다. 또한, 관리자 접속 로그가 권한 없는 사용자에게도 기록되는 로직 오류, JWT 토큰과 쿠키의 만료 시간 불일치 문제 등도 함께 지적했습니다.
그 외에 코드의 명확성과 유지보수성을 높이기 위한 몇 가지 제안(로그인 파라미터 명칭 통일, 불필요한 코드 제거 등)을 포함했으니 리뷰 코멘트를 확인해주시기 바랍니다.
| // 타임리프 페이지 인가 처리 | ||
| String accessToken = resolveToken(request); | ||
| jwtUtils.validateToken(accessToken); // 토큰 검증 | ||
| jwtUtils.isTokenBlacklisted(authHeader); // 🚨 블랙리스트 확인 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isTokenBlacklisted 메소드에 authHeader 변수를 전달하고 있습니다. resolveToken을 통해 쿠키에서 토큰을 가져온 경우 authHeader는 null이 되어 isTokenBlacklisted 내부에서 NullPointerException이 발생합니다. 이 예외는 catch 블록에 잡혀 사용자에게 오류 응답을 보내게 되므로, 쿠키를 통한 모든 인증이 실패하게 됩니다. 이는 심각한 서비스 거부(Denial of Service) 문제입니다. authHeader 대신 resolveToken으로 얻은 accessToken을 사용해야 합니다.
| jwtUtils.isTokenBlacklisted(authHeader); // 🚨 블랙리스트 확인 | |
| jwtUtils.isTokenBlacklisted("Bearer " + accessToken); // 🚨 블랙리스트 확인 |
| if (getUser.getRole().equals(UserRole.ADMIN)) response.sendRedirect("/admin/location-log"); | ||
| else if (getUser.getRole().equals(UserRole.SUPER_ADMIN)) response.sendRedirect("/super/admin/home"); | ||
| else response.sendRedirect("/admin/login?error=insufficient_role"); // 관리자 권한이 없는 경우: 로그인 페이지로 재이동 (사유 전달) | ||
| // 관리자 접속 로그 저장 | ||
| locationAccessLogRepository.save(LocationAccessLogs.builder() | ||
| .adminId(getUser.getId().toString()) | ||
| .build()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
관리자 접속 로그가 사용자의 역할(Role)과 관계없이 저장되고 있습니다. ADMIN이나 SUPER_ADMIN이 아닌 사용자가 로그인을 시도하고 성공하면, 권한 부족으로 리다이렉트되지만 접속 로그는 남게 됩니다. 이는 부정확한 로그를 생성할 수 있습니다. 관리자 접속 로그는 실제 관리자 권한을 가진 사용자가 성공적으로 로그인했을 때만 저장되어야 합니다.
if (getUser.getRole().equals(UserRole.ADMIN) || getUser.getRole().equals(UserRole.SUPER_ADMIN)) {
// 관리자 접속 로그 저장
locationAccessLogRepository.save(LocationAccessLogs.builder()
.adminId(getUser.getId().toString())
.build());
if (getUser.getRole().equals(UserRole.ADMIN)) {
response.sendRedirect("/admin/location-log");
} else { // SUPER_ADMIN
response.sendRedirect("/super/admin/home");
}
} else {
response.sendRedirect("/admin/login?error=insufficient_role"); // 관리자 권ahan이 없는 경우: 로그인 페이지로 재이동 (사유 전달)
}| String accessToken = jwtUtils.generateToken(valueMap, 15); | ||
| // JWT 토큰을 HTTP 응답 헤더에 추가 | ||
| var cookie = org.springframework.http.ResponseCookie.from("ACCESS_TOKEN", accessToken) | ||
| .httpOnly(true).secure(true).sameSite("Lax").path("/") | ||
| .maxAge(java.time.Duration.ofMinutes(30)).build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JWT 토큰의 만료 시간(15분)과 쿠키의 maxAge(30분)가 일치하지 않습니다. 이로 인해 클라이언트는 유효한 쿠키를 가지고 있지만, 서버에서는 만료된 토큰으로 인해 인증에 실패하는 상황이 발생할 수 있습니다. 사용자 경험에 혼란을 줄 수 있으므로 토큰의 만료 시간과 쿠키의 maxAge를 일치시키는 것이 좋습니다. 또한, '15'와 같은 매직 넘버 대신 설정 파일(application.yml)에서 값을 관리하고 @Value 어노테이션으로 주입받아 사용하는 것을 권장합니다.
| String accessToken = jwtUtils.generateToken(valueMap, 15); | |
| // JWT 토큰을 HTTP 응답 헤더에 추가 | |
| var cookie = org.springframework.http.ResponseCookie.from("ACCESS_TOKEN", accessToken) | |
| .httpOnly(true).secure(true).sameSite("Lax").path("/") | |
| .maxAge(java.time.Duration.ofMinutes(30)).build(); | |
| String accessToken = jwtUtils.generateToken(valueMap, 15); | |
| // JWT 토큰을 HTTP 응답 헤더에 추가 | |
| var cookie = org.springframework.http.ResponseCookie.from("ACCESS_TOKEN", accessToken) | |
| .httpOnly(true).secure(true).sameSite("Lax").path("/") | |
| .maxAge(java.time.Duration.ofMinutes(15)).build(); |
| .formLogin( (formLogin) -> formLogin | ||
| .loginPage("/admin/login") | ||
| .loginProcessingUrl("/loginProc") | ||
| .usernameParameter("email") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로그인 폼에서 사용하는 파라미터 이름이 email로 설정되어 있지만, 실제로는 사용자 ID(username)를 받는 것으로 보입니다. loginForm.html에서도 <label for="email">ID</label>로 되어 있어 혼란을 야기합니다. UserDetailsService 구현에서도 username을 기준으로 사용자를 조회하고 있으므로, 파라미터 이름을 username으로 통일하여 코드의 명확성을 높이는 것이 좋겠습니다. SecurityConfig와 함께 loginForm.html의 <input> 태그 name 속성도 username으로 변경해야 합니다.
| .usernameParameter("email") | |
| .usernameParameter("username") |
| LocalDateTime safeMin = LocalDateTime.of(1000, 1, 1, 0, 0); | ||
| LocalDateTime safeMax = LocalDateTime.of(9999, 12, 31, 23, 59, 59); | ||
| LocalDateTime from = (fromDate != null) ? fromDate.atStartOfDay() : safeMin; | ||
| LocalDateTime toEx = (toDate != null) ? toDate.plusDays(1).atStartOfDay() : safeMax; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
날짜 검색 조건이 null일 경우, 1000-01-01, 9999-12-31과 같은 특정 날짜(매직 넘버)를 기본값으로 사용하고 있습니다. LocationLogRepository의 searchWithKeywordAndDate 쿼리는 이미 null 값을 안전하게 처리하도록 설계되어 있으므로, 이 기본값들은 불필요합니다. 코드를 단순화하고 가독성을 높이기 위해 fromDate나 toDate가 null일 경우, null을 그대로 리포지토리 메소드에 전달하는 것이 좋습니다.
| LocalDateTime safeMin = LocalDateTime.of(1000, 1, 1, 0, 0); | |
| LocalDateTime safeMax = LocalDateTime.of(9999, 12, 31, 23, 59, 59); | |
| LocalDateTime from = (fromDate != null) ? fromDate.atStartOfDay() : safeMin; | |
| LocalDateTime toEx = (toDate != null) ? toDate.plusDays(1).atStartOfDay() : safeMax; | |
| LocalDateTime from = (fromDate != null) ? fromDate.atStartOfDay() : null; | |
| LocalDateTime toEx = (toDate != null) ? toDate.plusDays(1).atStartOfDay() : null; |
| return viewOnMapDiaries; | ||
| } | ||
|
|
||
| private void saveLocationLog(Users user, String provisionalService, String Receipient) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| LocalDateTime safeMin = LocalDateTime.of(1000, 1, 1, 0, 0); | ||
| LocalDateTime safeMax = LocalDateTime.of(9999, 12, 31, 23, 59, 59); | ||
|
|
||
| LocalDateTime from = (fromDate != null) ? fromDate.atStartOfDay() : safeMin; | ||
| LocalDateTime toEx = (toDate != null) ? toDate.plusDays(1).atStartOfDay() : safeMax; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
날짜 검색 조건이 null일 경우, 1000-01-01, 9999-12-31과 같은 특정 날짜(매직 넘버)를 기본값으로 사용하고 있습니다. LocationAccessLogRepository의 searchWithKeywordAndDate 쿼리는 이미 null 값을 안전하게 처리하도록 설계되어 있으므로, 이 기본값들은 불필요합니다. 코드를 단순화하고 가독성을 높이기 위해 fromDate나 toDate가 null일 경우, null을 그대로 리포지토리 메소드에 전달하는 것이 좋습니다.
| LocalDateTime safeMin = LocalDateTime.of(1000, 1, 1, 0, 0); | |
| LocalDateTime safeMax = LocalDateTime.of(9999, 12, 31, 23, 59, 59); | |
| LocalDateTime from = (fromDate != null) ? fromDate.atStartOfDay() : safeMin; | |
| LocalDateTime toEx = (toDate != null) ? toDate.plusDays(1).atStartOfDay() : safeMax; | |
| LocalDateTime from = (fromDate != null) ? fromDate.atStartOfDay() : null; | |
| LocalDateTime toEx = (toDate != null) ? toDate.plusDays(1).atStartOfDay() : null; |
| System.out.println("page: " + page); | ||
|
|
||
| // 1) 조회 | ||
| var resultPage = superAdminPageQueryService.search(page, pageSize, keyword, fromDate, toDate); | ||
|
|
||
| System.out.println(resultPage.getContent()); | ||
| for (LocationAccessLogs log : resultPage.getContent()) { | ||
| System.out.println("Log ID: " + log.getId()); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#️⃣연관된 이슈
📝작업 내용
사용자 위치 정보 접근 로그 저장 및 관리자 페이지(위치정보시스템) 구현
위치정보로그 종류
🔎코드 설명
💬고민사항 및 리뷰 요구사항 (Optional)
비고 (Optional)