Conversation
truongbt-0855
commented
Mar 29, 2026
- Implemented `getCategoryForEdit` method in `CategoryService` to return category details as a Map for editing. - Added `getDetail` and `getTourId` methods in `TourScheduleService` for fetching schedule details and associated tour ID. - Introduced admin API methods in `TourService` for listing tours and fetching detailed tour information including all schedules. - Created Thymeleaf templates for managing categories (`form.html`, `list.html`) and tours (`detail.html`, `form.html`, `list.html`, `schedule_form.html`). - Updated admin sidebar to include links for categories and tours management. - Added filtering options in the tours list page and implemented pagination.
…out functionality
… and scheduled cleanup
There was a problem hiding this comment.
Pull request overview
Adds a Thymeleaf-based Admin UI (login + dashboard + management screens) and wires up cookie-based admin authentication with JWT revocation on logout.
Changes:
- Added Admin UI pages (Users, Tours, Categories, Dashboard) with shared layout fragments and pagination/filtering.
- Implemented admin web controllers + admin-specific service methods for listing/detail/edit flows (including tour schedules).
- Added admin cookie auth filter, CSRF support for admin routes, and token revocation storage + scheduled cleanup.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/templates/admin/users/list.html | Admin users list view with filters + pagination |
| src/main/resources/templates/admin/users/detail.html | Admin user detail view with activation/deactivation actions |
| src/main/resources/templates/admin/tours/schedule_form.html | Create/edit tour schedule form |
| src/main/resources/templates/admin/tours/list.html | Admin tours list view with status filter + pagination |
| src/main/resources/templates/admin/tours/form.html | Admin tour create/edit form |
| src/main/resources/templates/admin/tours/detail.html | Admin tour detail view with status change + schedules table |
| src/main/resources/templates/admin/login.html | Admin login page |
| src/main/resources/templates/admin/dashboard.html | Admin dashboard placeholder page |
| src/main/resources/templates/admin/categories/list.html | Admin categories list (tree flattened with depth indent) |
| src/main/resources/templates/admin/categories/form.html | Admin category create/edit form |
| src/main/resources/templates/admin/_fragments.html | Shared head/sidebar/scripts/logout fragments for admin pages |
| src/main/resources/db/migration/V7__create_revoked_access_tokens.sql | Migration for revoked access token tracking table + index |
| src/main/java/com/sun/bookingtours/service/TourService.java | Added admin list/detail methods (incl. schedules) |
| src/main/java/com/sun/bookingtours/service/TourScheduleService.java | Added schedule detail/update helpers for admin UI with tour ownership validation |
| src/main/java/com/sun/bookingtours/service/RevokedAccessTokenService.java | Token revocation + scheduled cleanup |
| src/main/java/com/sun/bookingtours/service/CategoryService.java | Added Map-based category edit projection for Thymeleaf compatibility |
| src/main/java/com/sun/bookingtours/security/JwtTokenProvider.java | Added jti generation and claim helpers (email/jti/expiration) |
| src/main/java/com/sun/bookingtours/security/AdminCookieAuthFilter.java | Cookie-based admin JWT authentication + revocation check |
| src/main/java/com/sun/bookingtours/repository/RevokedAccessTokenRepository.java | JPA repo for revoked tokens + cleanup delete method |
| src/main/java/com/sun/bookingtours/entity/RevokedAccessToken.java | Entity mapping for revoked token records |
| src/main/java/com/sun/bookingtours/controller/web/AdminUserWebController.java | Admin web controller for users list/detail + activate/deactivate |
| src/main/java/com/sun/bookingtours/controller/web/AdminTourWebController.java | Admin web controller for tours CRUD + schedule management |
| src/main/java/com/sun/bookingtours/controller/web/AdminLoginWebController.java | Admin login/logout controller using cookie token |
| src/main/java/com/sun/bookingtours/controller/web/AdminCategoryWebController.java | Admin web controller for categories CRUD + tree flattening |
| src/main/java/com/sun/bookingtours/config/SecurityConfig.java | Secures /admin/**, enables CSRF for admin, adds admin cookie filter + login redirect entrypoint |
| src/main/java/com/sun/bookingtours/Application.java | Enabled scheduling for revoked-token cleanup job |
| pom.xml | Added Thymeleaf starter dependency |
| docs/technical.md | Documented Spring Framework 7 SpEL restrictions + Map workaround |
| TASKS.md | Marked admin UI tasks as completed/in progress |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Cookie cookie = new Cookie("admin_token", auth.getAccessToken()); | ||
| cookie.setHttpOnly(true); | ||
| cookie.setSecure(true); | ||
| cookie.setPath("/admin"); | ||
| cookie.setMaxAge(3600); // 1 giờ |
There was a problem hiding this comment.
cookie.setSecure(true) will prevent the browser from sending admin_token over plain HTTP, which makes admin login unusable in local/dev environments unless HTTPS is always enabled in front of the app. Consider making this conditional (e.g., based on request.isSecure() / a config property) or documenting that admin UI requires HTTPS everywhere.
| // Gọi khi admin logout: lưu jti vào bảng để filter từ chối token này về sau | ||
| @Transactional | ||
| public void revoke(String token) { | ||
| String jti = jwtTokenProvider.getJtiFromToken(token); |
There was a problem hiding this comment.
revoke() assumes the token contains a non-null jti, but refresh tokens (and older access tokens) may not. If jti is null/blank, persisting RevokedAccessToken will fail because the PK is null. Add a guard (treat as no-op / throw a controlled exception) and consider parsing claims once to avoid repeated JWT parsing for jti+expiration.
| String jti = jwtTokenProvider.getJtiFromToken(token); | |
| String jti = jwtTokenProvider.getJtiFromToken(token); | |
| if (jti == null || jti.isBlank()) { | |
| log.warn("Attempted to revoke a token without a valid jti; skipping revoke operation"); | |
| return; | |
| } |
|
|
||
| @Transactional(readOnly = true) | ||
| public TourResponse getAdminDetail(UUID id) { | ||
| Tour tour = findById(id); |
There was a problem hiding this comment.
adminList() filters out soft-deleted tours (deletedAt IS NULL), but getAdminDetail() uses findByIdWithDetails() (native query) which can return soft-deleted tours. This makes deleted tours accessible via direct URL even though they never appear in the admin list. Either enforce the same deletedAt IS NULL constraint here, or update the method/comment/UI to clearly support viewing deleted tours.
| Tour tour = findById(id); | |
| Tour tour = findById(id); | |
| // Không cho truy cập chi tiết tour đã bị xóa mềm | |
| if (tour.getDeletedAt() != null) { | |
| throw new ResourceNotFoundException("Tour not found with id: " + id); | |
| } |
| // REST API dùng JWT nên không cần CSRF. Admin UI dùng cookie → cần CSRF. | ||
| // CookieCsrfTokenRepository hoạt động với STATELESS session (không cần HttpSession). | ||
| // Thymeleaf tự inject _csrf hidden field vào mọi form có th:action. | ||
| .csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse()) |
There was a problem hiding this comment.
CookieCsrfTokenRepository.withHttpOnlyFalse() makes the CSRF token cookie readable by JavaScript. If the admin UI doesn’t need to read CSRF tokens client-side (no JS usage found), consider keeping this cookie HttpOnly (default) to reduce exposure surface.
| .csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse()) | |
| .csrfTokenRepository(new CookieCsrfTokenRepository()) |
|
|
||
| <div class="p-4" style="max-width:680px"> | ||
| <div class="d-flex align-items-center gap-3 mb-4"> | ||
| <a th:href="@{/admin/categories}" class="btn btn-sm btn-outline-secondary"> |
There was a problem hiding this comment.
This back link is icon-only, which isn’t announced well by screen readers. Add an accessible name (e.g., aria-label or visually-hidden text) so the control has a label.
| <a th:href="@{/admin/categories}" class="btn btn-sm btn-outline-secondary"> | |
| <a th:href="@{/admin/categories}" class="btn btn-sm btn-outline-secondary" aria-label="Quay lại danh sách Categories"> |
| cookie.setHttpOnly(true); | ||
| cookie.setSecure(true); | ||
| cookie.setPath("/admin"); | ||
| cookie.setMaxAge(3600); // 1 giờ | ||
| response.addCookie(cookie); |
There was a problem hiding this comment.
admin_token cookie is set with Max-Age=3600 (1 hour), but default app.jwt.expiration-ms is 900000 (15 minutes). This mismatch can leave a cookie present long after the JWT expires (and users being forced to re-login sooner than the cookie lifetime suggests). Consider aligning cookie lifetime with the access token expiration (or deriving Max-Age from the token’s exp).
| <div class="p-4" style="max-width:760px"> | ||
| <div class="d-flex align-items-center gap-3 mb-4"> | ||
| <a th:href="${isEdit} ? @{/admin/tours/{id}(id=${tour.id})} : @{/admin/tours}" | ||
| class="btn btn-sm btn-outline-secondary"> |
There was a problem hiding this comment.
This back link is icon-only, which isn’t announced well by screen readers. Add an accessible name (e.g., aria-label or visually-hidden text) so the control has a label.
| class="btn btn-sm btn-outline-secondary"> | |
| class="btn btn-sm btn-outline-secondary" | |
| aria-label="Quay lại"> |
| <a th:href="@{/admin/tours}" class="btn btn-sm btn-outline-secondary"> | ||
| <i class="bi bi-arrow-left"></i> |
There was a problem hiding this comment.
This back button is icon-only, which is not announced well by screen readers. Add an accessible name (e.g., aria-label or visually-hidden text) so the control has a label.
| <a th:href="@{/admin/tours}" class="btn btn-sm btn-outline-secondary"> | |
| <i class="bi bi-arrow-left"></i> | |
| <a th:href="@{/admin/tours}" class="btn btn-sm btn-outline-secondary" aria-label="Quay lại"> | |
| <i class="bi bi-arrow-left"></i> | |
| <span class="visually-hidden">Quay lại</span> |
| <a th:href="@{/admin/tours/{id}(id=${tourId})}" class="btn btn-sm btn-outline-secondary"> | ||
| <i class="bi bi-arrow-left"></i> |
There was a problem hiding this comment.
This back link is icon-only, which isn’t announced well by screen readers. Add an accessible name (e.g., aria-label or visually-hidden text) so the control has a label.
| <a th:href="@{/admin/tours/{id}(id=${tourId})}" class="btn btn-sm btn-outline-secondary"> | |
| <i class="bi bi-arrow-left"></i> | |
| <a th:href="@{/admin/tours/{id}(id=${tourId})}" class="btn btn-sm btn-outline-secondary" aria-label="Quay lại trang chi tiết tour"> | |
| <i class="bi bi-arrow-left" aria-hidden="true"></i> |
| if (token != null && jwtTokenProvider.validateToken(token) | ||
| && !revokedAccessTokenService.isRevoked(jwtTokenProvider.getJtiFromToken(token))) { | ||
| try { | ||
| String email = jwtTokenProvider.getEmailFromToken(token); | ||
| UserDetails userDetails = userDetailsService.loadUserByUsername(email); | ||
|
|
||
| UsernamePasswordAuthenticationToken authentication = | ||
| new UsernamePasswordAuthenticationToken(userDetails, null, userDetails.getAuthorities()); | ||
| authentication.setDetails(new WebAuthenticationDetailsSource().buildDetails(request)); | ||
|
|
||
| SecurityContextHolder.getContext().setAuthentication(authentication); |
There was a problem hiding this comment.
getJtiFromToken(token) can return null for tokens that don’t have a JWT ID (e.g., refresh tokens or older access tokens issued before .id(...) was added). Because the revocation check runs outside the try/catch, a null jti can throw (e.g., existsById(null)) and break all /admin requests with a 500. Treat missing/blank jti as invalid (skip auth) and move claim parsing + revocation check inside the try/catch after parsing claims once.
| if (token != null && jwtTokenProvider.validateToken(token) | |
| && !revokedAccessTokenService.isRevoked(jwtTokenProvider.getJtiFromToken(token))) { | |
| try { | |
| String email = jwtTokenProvider.getEmailFromToken(token); | |
| UserDetails userDetails = userDetailsService.loadUserByUsername(email); | |
| UsernamePasswordAuthenticationToken authentication = | |
| new UsernamePasswordAuthenticationToken(userDetails, null, userDetails.getAuthorities()); | |
| authentication.setDetails(new WebAuthenticationDetailsSource().buildDetails(request)); | |
| SecurityContextHolder.getContext().setAuthentication(authentication); | |
| if (token != null && jwtTokenProvider.validateToken(token)) { | |
| try { | |
| String jti = jwtTokenProvider.getJtiFromToken(token); | |
| // Treat missing or blank JTI as invalid: skip authentication in that case | |
| if (jti != null && !jti.trim().isEmpty() && !revokedAccessTokenService.isRevoked(jti)) { | |
| String email = jwtTokenProvider.getEmailFromToken(token); | |
| UserDetails userDetails = userDetailsService.loadUserByUsername(email); | |
| UsernamePasswordAuthenticationToken authentication = | |
| new UsernamePasswordAuthenticationToken(userDetails, null, userDetails.getAuthorities()); | |
| authentication.setDetails(new WebAuthenticationDetailsSource().buildDetails(request)); | |
| SecurityContextHolder.getContext().setAuthentication(authentication); | |
| } |
| @GetMapping | ||
| public String list(Model model) { | ||
| List<CategoryResponse> tree = categoryService.getTree(); | ||
| List<Map<String, Object>> flatList = new ArrayList<>(); |
There was a problem hiding this comment.
Truyền List<Map<String, Object>> thay vì dùng một DTO riêng. Việc dùng Map<String, Object> thô khiến code thiếu type-safety, dễ gây lỗi typo khi đặt key (ví dụ: "hasChildren", "depth"),
có thể tạo một DTO nhỏ như CategoryFlatItem với các field rõ ràng
|
|
||
| @GetMapping("/new") | ||
| public String newForm(Model model) { | ||
| List<Map<String, Object>> parentOptions = buildParentOptions(null); |
There was a problem hiding this comment.
buildParentOptions luôn gọi thêm categoryService.getTree() bên trong, trong khi editForm đã có tree sẵn nhưng lại không dùng buildParentOptions() mà gọi buildOptions() trực tiếp → code không nhất quán, và newForm sẽ gọi getTree() một lần dư thừa nếu logic thay đổi
| bindingResult.getFieldErrors().stream() | ||
| .map(e -> e.getField() + ": " + e.getDefaultMessage()) | ||
| .findFirst().orElse("Dữ liệu không hợp lệ.")); | ||
| return "redirect:/admin/categories/new"; |
There was a problem hiding this comment.
Khi có lỗi validation, code redirect về /new mà chỉ truyền errorMessage. Dữ liệu người dùng đã nhập bị mất, trải nghiệm UX kém.
| } | ||
|
|
||
| @PostMapping("/login") | ||
| public String login(@RequestParam String email, |
There was a problem hiding this comment.
Bỏ qua validation (email format, password độ dài...). Nếu email hoặc password rỗng/null, lỗi sẽ bị catch (Exception e) nuốt mất mà không có thông báo rõ ràng
…, and user experience updates