diff --git a/readme.md b/readme.md index 54117c7..df28e92 100644 --- a/readme.md +++ b/readme.md @@ -70,7 +70,9 @@ To Be Played is split into three repositories: Core never issues tokens. It verifies Auth's EdDSA (Ed25519) JWTs with public keys only. It loads them from Auth's JWKS by the token's `kid` and loads again when a new `kid` shows up (at most every 10 seconds). Loaded keys stay until Core restarts. `JWT_PUBLIC_KEY` is used only until the first JWKS load works. The token comes from Auth's -`tbp_access` cookie or a `Bearer` header, whichever verifies first. Core reads `userId` and `steamId` from the claims. +`tbp_access` cookie or a `Bearer` header, whichever verifies first. Core reads `userId`, `steamId` and the active +`role` (missing or unknown = USER) from the claims and grants that role and every lower one +(`USER < AUDITOR < PLATFORM_ADMIN`). `POST`, `PUT` and `DELETE` need an `X-TBP-Client` or `Authorization` header (CSRF guard), and CORS allows only the frontend origin. A token with a `typ` claim (for example a profile ticket) is rejected on normal endpoints; the profile ticket is @@ -79,6 +81,14 @@ accepted only by `GET /api/public/profiles/library` in the `X-Profile-Ticket` he as `Bearer`), never a login, also not in the dev profile. Traefik exposes the path, so the signed token is its only guard. +Every service method a controller calls carries one role marker, on the method or on its class (the method wins): +`@RequiresRole(Role.X)`, `@Public` (no login, e.g. landing stats; the public profile checks its ticket) or +`@Internal` (only Auth's service token). A Spring Security method interceptor checks it while a request is served; +scheduled jobs and `@Async` work run without a user and are not checked. A denied call answers `401` without a +login, else `403`. The URL rules in `SecurityConfig` stay as the outer guard. ArchUnit fails the build for an +unmarked method, two markers, a controller calling an `@Async` method (the check would never run), and a service +method calling another service's method that needs more. + ## API | Method | Path | Auth | diff --git a/src/main/java/core_service/configuration/security/AccessTokens.java b/src/main/java/core_service/configuration/security/AccessTokens.java index 16207da..05bd7b2 100644 --- a/src/main/java/core_service/configuration/security/AccessTokens.java +++ b/src/main/java/core_service/configuration/security/AccessTokens.java @@ -1,9 +1,12 @@ package core_service.configuration.security; +import core_service.shared.security.Role; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; +import org.springframework.security.core.authority.SimpleGrantedAuthority; import java.util.Arrays; +import java.util.List; import java.util.Optional; import java.util.stream.Stream; @@ -19,6 +22,14 @@ final class AccessTokens { private AccessTokens() { } + /** + * The active role and every role below it, so a check for USER also lets an admin in. + */ + static List authorities(Role role) { + return Arrays.stream(Role.values()).filter(role::atLeast) + .map(each -> new SimpleGrantedAuthority("ROLE_" + each.name())).toList(); + } + static Stream candidates(HttpServletRequest request) { return Stream.of(cookie(request), bearer(request)).flatMap(Optional::stream); } diff --git a/src/main/java/core_service/configuration/security/DevJwtAuthenticationFilter.java b/src/main/java/core_service/configuration/security/DevJwtAuthenticationFilter.java index 3d8b851..94d5a5d 100644 --- a/src/main/java/core_service/configuration/security/DevJwtAuthenticationFilter.java +++ b/src/main/java/core_service/configuration/security/DevJwtAuthenticationFilter.java @@ -16,7 +16,6 @@ import org.springframework.stereotype.Component; import org.springframework.web.filter.OncePerRequestFilter; import java.io.IOException; -import java.util.Collections; import java.util.Optional; /** @@ -49,7 +48,7 @@ public class DevJwtAuthenticationFilter extends OncePerRequestFilter { UsernamePasswordAuthenticationToken authToken = new UsernamePasswordAuthenticationToken( principal, null, - Collections.emptyList() + AccessTokens.authorities(principal.role()) ); authToken.setDetails(new WebAuthenticationDetailsSource().buildDetails(request)); SecurityContextHolder.getContext().setAuthentication(authToken); diff --git a/src/main/java/core_service/configuration/security/JwtAuthenticationFilter.java b/src/main/java/core_service/configuration/security/JwtAuthenticationFilter.java index 0daac99..6b828eb 100644 --- a/src/main/java/core_service/configuration/security/JwtAuthenticationFilter.java +++ b/src/main/java/core_service/configuration/security/JwtAuthenticationFilter.java @@ -15,7 +15,6 @@ import org.springframework.stereotype.Component; import org.springframework.web.filter.OncePerRequestFilter; import java.io.IOException; -import java.util.Collections; import java.util.Optional; @Component("jwtAuthenticationFilter") @@ -40,7 +39,7 @@ public class JwtAuthenticationFilter extends OncePerRequestFilter { UsernamePasswordAuthenticationToken authToken = new UsernamePasswordAuthenticationToken( principal.get(), null, - Collections.emptyList() + AccessTokens.authorities(principal.get().role()) ); authToken.setDetails(new WebAuthenticationDetailsSource().buildDetails(request)); SecurityContextHolder.getContext().setAuthentication(authToken); diff --git a/src/main/java/core_service/configuration/security/JwtService.java b/src/main/java/core_service/configuration/security/JwtService.java index 89d9875..de42277 100644 --- a/src/main/java/core_service/configuration/security/JwtService.java +++ b/src/main/java/core_service/configuration/security/JwtService.java @@ -2,6 +2,7 @@ package core_service.configuration.security; import core_service.shared.security.ProfileTicket; import core_service.shared.security.ProfileTicketVerifier; +import core_service.shared.security.Role; import core_service.shared.security.UserPrincipal; import io.jsonwebtoken.Claims; import io.jsonwebtoken.JwtParser; @@ -23,6 +24,7 @@ public class JwtService implements ProfileTicketVerifier { private static final String TYPE_CLAIM = "typ"; private static final String PROFILE_TICKET_TYPE = "public-profile"; private static final String SERVICE_TOKEN_TYPE = "service"; + private static final String ROLE_CLAIM = "role"; private final JwtParser parser; @@ -45,7 +47,8 @@ public class JwtService implements ProfileTicketVerifier { if (claims.get(TYPE_CLAIM) != null || claims.getSubject() == null || userId == null) { return Optional.empty(); } - return Optional.of(new UserPrincipal(userId, claims.getSubject(), claims.get("steamId", Long.class))); + return Optional.of(new UserPrincipal(userId, claims.getSubject(), claims.get("steamId", Long.class), + Role.parse(claims.get(ROLE_CLAIM) instanceof String role ? role : null))); } catch (Exception e) { return Optional.empty(); } diff --git a/src/main/java/core_service/configuration/security/MethodSecurityConfig.java b/src/main/java/core_service/configuration/security/MethodSecurityConfig.java new file mode 100644 index 0000000..55a7d8e --- /dev/null +++ b/src/main/java/core_service/configuration/security/MethodSecurityConfig.java @@ -0,0 +1,34 @@ +package core_service.configuration.security; + +import core_service.shared.security.Internal; +import core_service.shared.security.RequiresRole; +import org.springframework.aop.Advisor; +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.Pointcuts; +import org.springframework.aop.support.annotation.AnnotationMatchingPointcut; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Role; +import org.springframework.security.authorization.method.AuthorizationManagerBeforeMethodInterceptor; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; + +/** + * Runs {@link RoleAuthorizationManager} before every method with a role marker, or of a class with one. A method + * that is only {@code @Public} needs no check. + */ +@Configuration +@EnableMethodSecurity(prePostEnabled = false) +public class MethodSecurityConfig { + + @Bean + @Role(BeanDefinition.ROLE_INFRASTRUCTURE) + static Advisor roleAuthorization() { + Pointcut marked = Pointcuts.union( + Pointcuts.union(new AnnotationMatchingPointcut(RequiresRole.class, true), + new AnnotationMatchingPointcut(null, RequiresRole.class, true)), + Pointcuts.union(new AnnotationMatchingPointcut(Internal.class, true), + new AnnotationMatchingPointcut(null, Internal.class, true))); + return new AuthorizationManagerBeforeMethodInterceptor(marked, new RoleAuthorizationManager()); + } +} diff --git a/src/main/java/core_service/configuration/security/RoleAuthorizationManager.java b/src/main/java/core_service/configuration/security/RoleAuthorizationManager.java new file mode 100644 index 0000000..3c086a1 --- /dev/null +++ b/src/main/java/core_service/configuration/security/RoleAuthorizationManager.java @@ -0,0 +1,57 @@ +package core_service.configuration.security; + +import core_service.shared.security.Internal; +import core_service.shared.security.Public; +import core_service.shared.security.RequiresRole; +import org.aopalliance.intercept.MethodInvocation; +import org.springframework.aop.support.AopUtils; +import org.springframework.core.annotation.AnnotatedElementUtils; +import org.springframework.security.authorization.AuthorizationDecision; +import org.springframework.security.authorization.AuthorizationManager; +import org.springframework.security.authorization.AuthorizationResult; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.web.context.request.RequestContextHolder; + +import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Method; +import java.util.function.Supplier; + +class RoleAuthorizationManager implements AuthorizationManager { + + private static final AuthorizationDecision GRANTED = new AuthorizationDecision(true); + private static final AuthorizationDecision DENIED = new AuthorizationDecision(false); + + @Override + public AuthorizationResult authorize(Supplier authentication, MethodInvocation invocation) { + if (RequestContextHolder.getRequestAttributes() == null) { + return GRANTED; + } + Class targetClass = invocation.getThis() != null ? AopUtils.getTargetClass(invocation.getThis()) + : invocation.getMethod().getDeclaringClass(); + Method method = AopUtils.getMostSpecificMethod(invocation.getMethod(), targetClass); + AnnotatedElement marked = isMarked(method) ? method : targetClass; + if (AnnotatedElementUtils.hasAnnotation(marked, Public.class)) { + return GRANTED; + } + if (AnnotatedElementUtils.hasAnnotation(marked, Internal.class)) { + return hasAuthority(authentication.get(), ServiceTokenFilter.SERVICE_AUTHORITY) ? GRANTED : DENIED; + } + RequiresRole required = AnnotatedElementUtils.findMergedAnnotation(marked, RequiresRole.class); + if (required == null) { + return DENIED; + } + return hasAuthority(authentication.get(), "ROLE_" + required.value().name()) ? GRANTED : DENIED; + } + + private static boolean isMarked(Method method) { + return AnnotatedElementUtils.hasAnnotation(method, RequiresRole.class) + || AnnotatedElementUtils.hasAnnotation(method, Public.class) + || AnnotatedElementUtils.hasAnnotation(method, Internal.class); + } + + private static boolean hasAuthority(Authentication authentication, String authority) { + return authentication != null && authentication.getAuthorities().stream() + .map(GrantedAuthority::getAuthority).anyMatch(authority::equals); + } +} diff --git a/src/main/java/core_service/modules/account/service/AccountCleanupService.java b/src/main/java/core_service/modules/account/service/AccountCleanupService.java index 558c125..580e163 100644 --- a/src/main/java/core_service/modules/account/service/AccountCleanupService.java +++ b/src/main/java/core_service/modules/account/service/AccountCleanupService.java @@ -2,6 +2,7 @@ package core_service.modules.account.service; import core_service.modules.account.domain.CleanupResult; import core_service.modules.account.repo.AccountCleanupRepository; +import core_service.shared.security.Internal; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Value; import org.springframework.cache.annotation.CacheEvict; @@ -16,6 +17,7 @@ import java.util.Map; * Auth decides who is deleted and when; Core only checks that a purged user is tombstoned. */ @Service +@Internal @Slf4j public class AccountCleanupService { diff --git a/src/main/java/core_service/modules/library/api/LibraryController.java b/src/main/java/core_service/modules/library/api/LibraryController.java index 2f11a2b..e8ced5f 100644 --- a/src/main/java/core_service/modules/library/api/LibraryController.java +++ b/src/main/java/core_service/modules/library/api/LibraryController.java @@ -23,8 +23,6 @@ import core_service.modules.library.service.GameDetailService; import core_service.modules.library.service.ImageRepairService; import core_service.modules.library.service.LibraryQueryService; import core_service.modules.library.service.LibrarySyncService; -import core_service.modules.library.service.MoodService; -import core_service.modules.library.service.SpeedrunService; import core_service.modules.library.service.SteamProfileService; import core_service.modules.library.service.SyncCooldownException; import core_service.modules.library.service.UserStatsService; @@ -70,8 +68,6 @@ public class LibraryController { private final GameDetailService gameDetailService; private final BacklogService backlogService; private final SteamProfileService steamProfileService; - private final MoodService moodService; - private final SpeedrunService speedrunService; private final ImageRepairService imageRepairService; private final UserStatsService userStatsService; @@ -241,11 +237,6 @@ public class LibraryController { if (principal.steamId() != null) { syncService.syncIfStale(userId, String.valueOf(principal.steamId())); } - if (mood == Mood.SPEEDRUN) { - speedrunService.refreshBacklogInBackground(userId); - } else if (mood != null) { - moodService.refreshBacklogInBackground(userId); - } return ResponseEntity.ok(queryService.pickBacklogGames(userId, budget, mood, excluded, safeCount)); } @@ -257,10 +248,7 @@ public class LibraryController { @RequestParam(required = false) Set groups) { Long userId = principal.getIdAsLong(); Set asked = groups == null || groups.isEmpty() ? EnumSet.allOf(StatGroup.class) : groups; - if (asked.contains(StatGroup.TASTE)) { - moodService.refreshListedInBackground(userId); - } - return ResponseEntity.ok(userStatsService.getStats(userId, asked)); + return ResponseEntity.ok(userStatsService.getOwnStats(userId, asked)); } @GetMapping("/sync-status") diff --git a/src/main/java/core_service/modules/library/service/BacklogService.java b/src/main/java/core_service/modules/library/service/BacklogService.java index 77171ed..0a144b5 100644 --- a/src/main/java/core_service/modules/library/service/BacklogService.java +++ b/src/main/java/core_service/modules/library/service/BacklogService.java @@ -7,6 +7,8 @@ import core_service.modules.library.domain.GameStatus; import core_service.modules.library.domain.UserGame; import core_service.modules.library.domain.UserGameId; import core_service.modules.library.repo.UserGameRepository; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -15,6 +17,7 @@ import java.time.Instant; import java.util.Optional; @Service +@RequiresRole(Role.USER) @RequiredArgsConstructor public class BacklogService { diff --git a/src/main/java/core_service/modules/library/service/GameDetailService.java b/src/main/java/core_service/modules/library/service/GameDetailService.java index 659eb41..bbcc742 100644 --- a/src/main/java/core_service/modules/library/service/GameDetailService.java +++ b/src/main/java/core_service/modules/library/service/GameDetailService.java @@ -25,6 +25,8 @@ import core_service.modules.library.repo.SteamGameRepository; import core_service.modules.library.repo.UserGameAchievementMetaRepository; import core_service.modules.library.repo.UserGameAchievementRepository; import core_service.modules.library.repo.UserGameRepository; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Value; @@ -39,6 +41,7 @@ import java.util.Set; import java.util.stream.Collectors; @Service +@RequiresRole(Role.USER) @RequiredArgsConstructor @Slf4j public class GameDetailService { diff --git a/src/main/java/core_service/modules/library/service/ImageRepairService.java b/src/main/java/core_service/modules/library/service/ImageRepairService.java index 9bb1728..c9bc3e3 100644 --- a/src/main/java/core_service/modules/library/service/ImageRepairService.java +++ b/src/main/java/core_service/modules/library/service/ImageRepairService.java @@ -13,6 +13,8 @@ import core_service.modules.library.repo.SteamGameRepository; import core_service.modules.library.repo.UserGameRepository; import core_service.modules.steam_adapter.service.SteamApiService; import core_service.modules.steam_adapter.service.SteamCdnService; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Value; import org.springframework.scheduling.annotation.Scheduled; @@ -35,6 +37,7 @@ import java.util.stream.Collectors; * the job checks the picture itself first, so a false report (ad blocker, bad network) changes nothing. */ @Service +@RequiresRole(Role.USER) @Slf4j public class ImageRepairService { diff --git a/src/main/java/core_service/modules/library/service/LibraryQueryService.java b/src/main/java/core_service/modules/library/service/LibraryQueryService.java index 3b941d3..55be522 100644 --- a/src/main/java/core_service/modules/library/service/LibraryQueryService.java +++ b/src/main/java/core_service/modules/library/service/LibraryQueryService.java @@ -28,6 +28,9 @@ import core_service.modules.library.repo.UserGameAchievementMetaRepository; import core_service.modules.library.repo.UserGameRepository; import core_service.modules.library.repo.UserSteamSyncRepository; import core_service.modules.library.repo.WishlistGameRepository; +import core_service.shared.security.Public; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; import lombok.RequiredArgsConstructor; import org.springframework.beans.factory.annotation.Value; import org.springframework.data.domain.Page; @@ -50,6 +53,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; @Service +@RequiresRole(Role.USER) @RequiredArgsConstructor @Transactional(readOnly = true) public class LibraryQueryService { @@ -68,6 +72,8 @@ public class LibraryQueryService { private final GameAchievementSchemaMetaRepository schemaMetaRepository; private final UserGameAchievementMetaRepository userAchievementMetaRepository; private final UserStatsService userStatsService; + private final MoodService moodService; + private final SpeedrunService speedrunService; @Value("${app.library.zone:Europe/Berlin}") private ZoneId zone; @@ -86,7 +92,15 @@ public class LibraryQueryService { return new PageImpl<>(games, page.getPageable(), page.getTotalElements()); } + /** + * With a mood, also refreshes the backlog's moods or speedrun links in the background for the next pick. + */ public PickerDto pickBacklogGames(Long userId, Integer maxMinutes, Mood mood, List excludeAppids, int count) { + if (mood == Mood.SPEEDRUN) { + speedrunService.refreshBacklogInBackground(userId); + } else if (mood != null) { + moodService.refreshBacklogInBackground(userId); + } return new PickerDto(toDtos(userGameRepository.findPickerGames(userId, maxMinutes, mood, excludeAppids, count))); } @@ -197,6 +211,7 @@ public class LibraryQueryService { /** * @param profileStats the stat group names of the profile ticket */ + @Public public PublicLibraryDto getPublicLibrary(Long userId, Collection profileStats) { Map byStatus = userGameRepository.countEveryStatus(userId); List playing = userGameRepository.findListedByStatusIn(userId, diff --git a/src/main/java/core_service/modules/library/service/LibrarySyncService.java b/src/main/java/core_service/modules/library/service/LibrarySyncService.java index d149e56..55cbd58 100644 --- a/src/main/java/core_service/modules/library/service/LibrarySyncService.java +++ b/src/main/java/core_service/modules/library/service/LibrarySyncService.java @@ -3,6 +3,8 @@ package core_service.modules.library.service; import core_service.modules.library.domain.UserSteamSync; import core_service.modules.library.repo.BacklogSnapshotRepository; import core_service.modules.library.repo.UserSteamSyncRepository; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Value; @@ -16,6 +18,7 @@ import java.time.ZoneId; import java.util.List; @Service +@RequiresRole(Role.USER) @RequiredArgsConstructor @Slf4j public class LibrarySyncService { diff --git a/src/main/java/core_service/modules/library/service/ManualGameService.java b/src/main/java/core_service/modules/library/service/ManualGameService.java index 210285e..df6d6e6 100644 --- a/src/main/java/core_service/modules/library/service/ManualGameService.java +++ b/src/main/java/core_service/modules/library/service/ManualGameService.java @@ -11,6 +11,8 @@ import core_service.modules.library.domain.UserGame; import core_service.modules.library.domain.UserGameId; import core_service.modules.library.repo.SteamGameRepository; import core_service.modules.library.repo.UserGameRepository; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; import lombok.extern.slf4j.Slf4j; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; @@ -32,6 +34,7 @@ import java.util.stream.Stream; * to the Steam sync. */ @Service +@RequiresRole(Role.USER) @Slf4j public class ManualGameService { diff --git a/src/main/java/core_service/modules/library/service/SteamProfileService.java b/src/main/java/core_service/modules/library/service/SteamProfileService.java index b6e628e..edf4d83 100644 --- a/src/main/java/core_service/modules/library/service/SteamProfileService.java +++ b/src/main/java/core_service/modules/library/service/SteamProfileService.java @@ -2,10 +2,13 @@ package core_service.modules.library.service; import core_service.modules.library.api.dto.SteamProfileDto; import core_service.modules.steam_adapter.service.SteamApiService; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Service; @Service +@RequiresRole(Role.USER) @RequiredArgsConstructor public class SteamProfileService { diff --git a/src/main/java/core_service/modules/library/service/UserStatsService.java b/src/main/java/core_service/modules/library/service/UserStatsService.java index 926ed5d..3bf7fd9 100644 --- a/src/main/java/core_service/modules/library/service/UserStatsService.java +++ b/src/main/java/core_service/modules/library/service/UserStatsService.java @@ -8,6 +8,8 @@ import core_service.modules.library.domain.UserStats; import core_service.modules.library.repo.BacklogSnapshotRepository; import core_service.modules.library.repo.UserGameRepository; import core_service.modules.library.repo.UserStatsRepository; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; import lombok.RequiredArgsConstructor; import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Service; @@ -33,10 +35,23 @@ public class UserStatsService { private final UserGameRepository userGameRepository; private final UserStatsRepository userStatsRepository; private final BacklogSnapshotRepository backlogSnapshotRepository; + private final MoodService moodService; @Value("${app.library.zone:Europe/Berlin}") private ZoneId zone; + /** + * The stats page of the user: {@link #getStats}, and for TASTE also a refresh of the listed games' moods in the + * background, not done for a profile visitor. + */ + @RequiresRole(Role.USER) + public UserStatsDto getOwnStats(long userId, Set groups) { + if (groups.contains(StatGroup.TASTE)) { + moodService.refreshListedInBackground(userId); + } + return getStats(userId, groups); + } + /** * Runs only the queries of the asked groups; the others stay null. */ diff --git a/src/main/java/core_service/modules/stats/service/PublicStatsService.java b/src/main/java/core_service/modules/stats/service/PublicStatsService.java index 9ceaf28..2b5fb90 100644 --- a/src/main/java/core_service/modules/stats/service/PublicStatsService.java +++ b/src/main/java/core_service/modules/stats/service/PublicStatsService.java @@ -2,6 +2,7 @@ package core_service.modules.stats.service; import core_service.modules.stats.api.dto.PublicStatsDto; import core_service.modules.stats.repo.StatsRepository; +import core_service.shared.security.Public; import lombok.RequiredArgsConstructor; import org.springframework.cache.annotation.Cacheable; import org.springframework.stereotype.Service; @@ -10,6 +11,7 @@ import java.util.List; import java.util.Map; @Service +@Public @RequiredArgsConstructor public class PublicStatsService { diff --git a/src/main/java/core_service/modules/steam_adapter/service/SteamApiService.java b/src/main/java/core_service/modules/steam_adapter/service/SteamApiService.java index f6c75fb..b7de47b 100644 --- a/src/main/java/core_service/modules/steam_adapter/service/SteamApiService.java +++ b/src/main/java/core_service/modules/steam_adapter/service/SteamApiService.java @@ -8,6 +8,8 @@ import core_service.modules.steam_adapter.api.dto.response.SteamProfileAndGamesR import core_service.modules.steam_adapter.api.dto.response.SteamSchemaForGameResponse; import core_service.modules.steam_adapter.api.dto.response.SteamStoreItemsResponse; import core_service.modules.steam_adapter.api.dto.response.SteamWishlistResponse; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; import org.springframework.web.client.HttpClientErrorException; import io.github.resilience4j.ratelimiter.annotation.RateLimiter; import io.github.resilience4j.retry.annotation.Retry; @@ -27,6 +29,7 @@ import java.util.Optional; import java.util.stream.Collectors; @Service +@RequiresRole(Role.USER) @RequiredArgsConstructor @Slf4j public class SteamApiService { diff --git a/src/main/java/core_service/shared/security/Internal.java b/src/main/java/core_service/shared/security/Internal.java new file mode 100644 index 0000000..75576c7 --- /dev/null +++ b/src/main/java/core_service/shared/security/Internal.java @@ -0,0 +1,16 @@ +package core_service.shared.security; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Callable only with Auth's service token (the /internal calls). See {@link RequiresRole}. + */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE, ElementType.METHOD}) +public @interface Internal { +} diff --git a/src/main/java/core_service/shared/security/Public.java b/src/main/java/core_service/shared/security/Public.java new file mode 100644 index 0000000..417eab3 --- /dev/null +++ b/src/main/java/core_service/shared/security/Public.java @@ -0,0 +1,16 @@ +package core_service.shared.security; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Callable without a login, e.g. the landing stats. See {@link RequiresRole}. + */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE, ElementType.METHOD}) +public @interface Public { +} diff --git a/src/main/java/core_service/shared/security/RequiresRole.java b/src/main/java/core_service/shared/security/RequiresRole.java new file mode 100644 index 0000000..3adb3fa --- /dev/null +++ b/src/main/java/core_service/shared/security/RequiresRole.java @@ -0,0 +1,20 @@ +package core_service.shared.security; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * The lowest active role that may call this service method, or every method of this class. A method marker + * replaces the class marker. Every service method a controller calls carries one of {@link RequiresRole}, + * {@link Public} or {@link Internal} (ArchitectureTest). Checked only while an HTTP request is served: scheduled + * jobs and {@code @Async} work run without a user. + */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE, ElementType.METHOD}) +public @interface RequiresRole { + Role value(); +} diff --git a/src/main/java/core_service/shared/security/Role.java b/src/main/java/core_service/shared/security/Role.java new file mode 100644 index 0000000..5caa5df --- /dev/null +++ b/src/main/java/core_service/shared/security/Role.java @@ -0,0 +1,26 @@ +package core_service.shared.security; + +/** + * A ladder: each role has every right of the roles before it. Auth owns the roles; the same enum lives there. + */ +public enum Role { + USER, AUDITOR, PLATFORM_ADMIN; + + public boolean atLeast(Role other) { + return compareTo(other) >= 0; + } + + /** + * USER for null or an unknown name, e.g. a token from before roles. + */ + public static Role parse(String name) { + if (name == null) { + return USER; + } + try { + return valueOf(name); + } catch (IllegalArgumentException _) { + return USER; + } + } +} diff --git a/src/main/java/core_service/shared/security/UserPrincipal.java b/src/main/java/core_service/shared/security/UserPrincipal.java index 4eabd73..cd62358 100644 --- a/src/main/java/core_service/shared/security/UserPrincipal.java +++ b/src/main/java/core_service/shared/security/UserPrincipal.java @@ -2,12 +2,20 @@ package core_service.shared.security; import java.security.Principal; +/** + * @param role the active role of this login ("view as"), set by Auth + */ public record UserPrincipal( String id, String email, - Long steamId + Long steamId, + Role role ) implements Principal { + public UserPrincipal(String id, String email, Long steamId) { + this(id, email, steamId, Role.USER); + } + @Override public String getName() { return id; diff --git a/src/test/java/core_service/architecture/ArchitectureTest.java b/src/test/java/core_service/architecture/ArchitectureTest.java index 4e4d1ef..d35450d 100644 --- a/src/test/java/core_service/architecture/ArchitectureTest.java +++ b/src/test/java/core_service/architecture/ArchitectureTest.java @@ -100,4 +100,17 @@ class ArchitectureTest extends AbstractArchitectureTest { .orShould().beAnnotatedWith(org.springframework.web.bind.annotation.ControllerAdvice.class) .orShould().beAnnotatedWith(org.springframework.web.bind.annotation.RestControllerAdvice.class) .because("Classes in 'api' package named *Controller should be Spring REST controllers."); + + // =================================================================================== + // 5. ROLE MARKERS (see RoleAnnotationRules) + // =================================================================================== + + @ArchTest + static final ArchRule controllers_call_only_marked_service_methods = RoleAnnotationRules.controllers_call_only_marked_service_methods; + + @ArchTest + static final ArchRule one_role_marker_per_element = RoleAnnotationRules.one_role_marker_per_element; + + @ArchTest + static final ArchRule services_never_call_a_method_that_needs_more = RoleAnnotationRules.services_never_call_a_method_that_needs_more; } diff --git a/src/test/java/core_service/architecture/RoleAnnotationRules.java b/src/test/java/core_service/architecture/RoleAnnotationRules.java new file mode 100644 index 0000000..dd36be4 --- /dev/null +++ b/src/test/java/core_service/architecture/RoleAnnotationRules.java @@ -0,0 +1,198 @@ +package core_service.architecture; + +import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.core.domain.JavaClass; +import com.tngtech.archunit.core.domain.JavaCodeUnit; +import com.tngtech.archunit.core.domain.JavaCodeUnitAccess; +import com.tngtech.archunit.core.domain.JavaMethod; +import com.tngtech.archunit.core.domain.JavaModifier; +import com.tngtech.archunit.core.domain.properties.HasAnnotations; +import com.tngtech.archunit.lang.ArchCondition; +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.ConditionEvents; +import com.tngtech.archunit.lang.SimpleConditionEvent; +import core_service.shared.security.Internal; +import core_service.shared.security.Public; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; +import org.springframework.scheduling.annotation.Async; + +import java.lang.annotation.Annotation; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Stream; + +import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes; + +/** + * The rules for {@link RequiresRole}, {@link Public} and {@link Internal}. Auth has the same rules without + * {@code Internal}. + */ +final class RoleAnnotationRules { + + private static final List> MARKERS = List.of(RequiresRole.class, Public.class, + Internal.class); + + private static final String APP_PACKAGE = "core_service."; + + private RoleAnnotationRules() { + } + + /** + * What a marked method needs from its caller. {@code role} is set only for {@link RequiresRole}. + */ + private record Need(Class marker, Role role) { + + boolean isMetBy(Need caller) { + if (marker == Public.class) { + return true; + } + if (marker == Internal.class) { + return caller.marker == Internal.class; + } + return caller.marker == RequiresRole.class && caller.role.atLeast(role); + } + + @Override + public String toString() { + return marker == RequiresRole.class ? "@RequiresRole(" + role + ")" : "@" + marker.getSimpleName(); + } + } + + private static long markerCount(HasAnnotations element) { + return MARKERS.stream().filter(element::isAnnotatedWith).count(); + } + + private static Optional need(HasAnnotations element) { + for (Class marker : MARKERS) { + if (element.isAnnotatedWith(marker)) { + Role role = marker == RequiresRole.class + ? element.getAnnotationOfType(RequiresRole.class).value() : null; + return Optional.of(new Need(marker, role)); + } + } + return Optional.empty(); + } + + private static Optional effectiveNeed(JavaMethod method) { + Optional own = need(method); + return own.isPresent() ? own : need(method.getOwner()); + } + + private record MethodAccess(JavaCodeUnitAccess access, JavaMethod target) { + + @Override + public String toString() { + return target.getOwner().getSimpleName() + "." + target.getName() + " called in " + + access.getSourceCodeLocation(); + } + } + + /** + * Calls and method references ({@code service::method}) to non-static methods: a proxy never sees a static call, + * so a marker there would promise a check that does not run. + */ + private static List methodAccesses(JavaCodeUnit from) { + return Stream.>concat(from.getMethodCallsFromSelf().stream(), + from.getMethodReferencesFromSelf().stream()) + .flatMap(access -> access.getTarget().resolveMember().stream() + .filter(JavaMethod.class::isInstance).map(JavaMethod.class::cast) + .filter(method -> !method.getModifiers().contains(JavaModifier.STATIC)) + .map(method -> new MethodAccess(access, method))) + .toList(); + } + + private static boolean isService(JavaClass javaClass) { + return javaClass.getSimpleName().endsWith("Service") && javaClass.getPackageName().contains(".service"); + } + + private static boolean isAsync(JavaMethod method) { + return method.isAnnotatedWith(Async.class) || method.getOwner().isAnnotatedWith(Async.class); + } + + private static final DescribedPredicate SERVICE_CLASSES = + DescribedPredicate.describe("are services", RoleAnnotationRules::isService); + + /** + * Every service method a controller calls is marked, and is not {@code @Async}: there the check would never run. + */ + static final ArchRule controllers_call_only_marked_service_methods = classes() + .that().resideInAPackage("..api..") + .should(new ArchCondition<>("call only service methods that carry a role marker and are not @Async") { + @Override + public void check(JavaClass controller, ConditionEvents events) { + for (JavaCodeUnit codeUnit : controller.getCodeUnits()) { + for (MethodAccess access : methodAccesses(codeUnit)) { + if (!isService(access.target().getOwner())) { + continue; + } + if (effectiveNeed(access.target()).isEmpty()) { + events.add(SimpleConditionEvent.violated(access.access(), "no role marker on " + access)); + } + if (isAsync(access.target())) { + events.add(SimpleConditionEvent.violated(access.access(), "@Async method " + access)); + } + } + } + } + }) + .because("a new endpoint must not forget the role check"); + + static final ArchRule one_role_marker_per_element = classes() + .should(new ArchCondition<>("carry at most one role marker on the class and on each method") { + @Override + public void check(JavaClass javaClass, ConditionEvents events) { + if (markerCount(javaClass) > 1) { + events.add(SimpleConditionEvent.violated(javaClass, javaClass.getName() + " has two role markers")); + } + for (JavaMethod method : javaClass.getMethods()) { + if (markerCount(method) > 1) { + events.add(SimpleConditionEvent.violated(method, method.getFullName() + " has two role markers")); + } + } + } + }) + .because("the marker must say one thing"); + + /** + * Starts at every method the proxy checks (not private, not static) and follows every call that is not checked + * again: self calls and calls of unmarked methods. Calls through an interface (a port) end the walk: the + * implementation is not known here. + */ + static final ArchRule services_never_call_a_method_that_needs_more = classes() + .that(SERVICE_CLASSES) + .should(new ArchCondition<>("never call a service method that needs more than the caller") { + @Override + public void check(JavaClass service, ConditionEvents events) { + for (JavaMethod entry : service.getMethods()) { + Optional caller = effectiveNeed(entry); + if (caller.isEmpty() || entry.getModifiers().contains(JavaModifier.PRIVATE) + || entry.getModifiers().contains(JavaModifier.STATIC)) { + continue; + } + Set seen = new HashSet<>(List.of(entry)); + Deque toVisit = new ArrayDeque<>(seen); + while (!toVisit.isEmpty()) { + for (MethodAccess access : methodAccesses(toVisit.pop())) { + JavaMethod target = access.target(); + Optional callee = target.getOwner().equals(service) ? Optional.empty() + : effectiveNeed(target); + if (callee.isEmpty()) { + if (target.getOwner().getPackageName().startsWith(APP_PACKAGE) && seen.add(target)) { + toVisit.push(target); + } + } else if (!callee.get().isMetBy(caller.get())) { + events.add(SimpleConditionEvent.violated(access.access(), entry.getName() + " (" + + caller.get() + ") calls " + access + ", which needs " + callee.get())); + } + } + } + } + } + }) + .because("the inner check would refuse a caller the outer check let in"); +} diff --git a/src/test/java/core_service/architecture/RoleAnnotationRulesTest.java b/src/test/java/core_service/architecture/RoleAnnotationRulesTest.java new file mode 100644 index 0000000..790c84b --- /dev/null +++ b/src/test/java/core_service/architecture/RoleAnnotationRulesTest.java @@ -0,0 +1,113 @@ +package core_service.architecture; + +import com.tngtech.archunit.core.domain.JavaClasses; +import com.tngtech.archunit.core.importer.ClassFileImporter; +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.EvaluationResult; +import core_service.architecture.fixture.api.BadController; +import core_service.architecture.fixture.api.GoodController; +import core_service.architecture.fixture.api.MethodReferenceController; +import core_service.architecture.fixture.service.ForwardingService; +import core_service.architecture.fixture.service.InternalCallerService; +import core_service.architecture.fixture.service.InternalMarkedService; +import core_service.architecture.fixture.service.MarkedService; +import core_service.architecture.fixture.service.PublicCallerService; +import core_service.architecture.fixture.service.PublicViaUnmarkedCallerService; +import core_service.architecture.fixture.service.PublicHelperCallerService; +import core_service.architecture.fixture.service.PublicMethodReferenceCallerService; +import core_service.architecture.fixture.service.TwoMarkersService; +import core_service.architecture.fixture.service.UnmarkedService; +import core_service.architecture.fixture.service.UserCallerService; +import core_service.shared.test.AbstractArchitectureTest; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +class RoleAnnotationRulesTest extends AbstractArchitectureTest { + + private static List violations(ArchRule rule, Class... classes) { + JavaClasses imported = new ClassFileImporter().importClasses(classes); + EvaluationResult result = rule.evaluate(imported); + return result.getFailureReport().getDetails(); + } + + @Test + void controllerRule_PassesMarkedMethodsTheMethodMarkerAndStaticHelpers() { + assertThat(violations(RoleAnnotationRules.controllers_call_only_marked_service_methods, + GoodController.class, MarkedService.class, UnmarkedService.class)).isEmpty(); + } + + @Test + void controllerRule_FindsAnUnmarkedAndAnAsyncMethod() { + List found = violations(RoleAnnotationRules.controllers_call_only_marked_service_methods, + BadController.class, MarkedService.class, UnmarkedService.class); + + assertThat(found).hasSize(2); + assertThat(found).anyMatch(line -> line.contains("no role marker on UnmarkedService.bare")); + assertThat(found).anyMatch(line -> line.contains("@Async method MarkedService.later")); + } + + @Test + void markerRule_FindsTwoMarkersOnOneMethod() { + assertThat(violations(RoleAnnotationRules.one_role_marker_per_element, TwoMarkersService.class)) + .singleElement().asString().contains("both"); + } + + @Test + void escalationRule_FindsAPublicMethodCallingAUserMethod() { + List found = violations(RoleAnnotationRules.services_never_call_a_method_that_needs_more, + PublicCallerService.class, MarkedService.class); + + assertThat(found).singleElement().asString() + .contains("callsUser (@Public)", "MarkedService.plain", "needs @RequiresRole(USER)"); + } + + @Test + void escalationRule_FindsAUserMethodCallingAnAuditorMethod() { + assertThat(violations(RoleAnnotationRules.services_never_call_a_method_that_needs_more, + UserCallerService.class, MarkedService.class)) + .singleElement().asString().contains("needs @RequiresRole(AUDITOR)"); + } + + @Test + void controllerRule_FindsAnUnmarkedMethodCalledByMethodReference() { + assertThat(violations(RoleAnnotationRules.controllers_call_only_marked_service_methods, + MethodReferenceController.class, UnmarkedService.class)) + .anyMatch(line -> line.contains("no role marker on UnmarkedService.bare")); + } + + @Test + void escalationRule_FindsAPublicMethodCallingAUserMethodThroughAPrivateHelper() { + assertThat(violations(RoleAnnotationRules.services_never_call_a_method_that_needs_more, + PublicHelperCallerService.class, MarkedService.class)) + .anyMatch(line -> line.contains("MarkedService.plain") && line.contains("needs @RequiresRole(USER)")); + } + + @Test + void escalationRule_FindsAPublicMethodCallingAUserMethodByMethodReference() { + assertThat(violations(RoleAnnotationRules.services_never_call_a_method_that_needs_more, + PublicMethodReferenceCallerService.class, MarkedService.class)) + .anyMatch(line -> line.contains("MarkedService.plain") && line.contains("needs @RequiresRole(USER)")); + } + + @Test + void escalationRule_FindsAPublicMethodReachingAUserMethodThroughAnUnmarkedService() { + assertThat(violations(RoleAnnotationRules.services_never_call_a_method_that_needs_more, + PublicViaUnmarkedCallerService.class, ForwardingService.class, MarkedService.class)) + .singleElement().asString() + .contains("callsThroughAnUnmarkedService (@Public)", "MarkedService.plain", "needs @RequiresRole(USER)"); + } + + @Test + void escalationRule_KeepsInternalAndRolesApart() { + List found = violations(RoleAnnotationRules.services_never_call_a_method_that_needs_more, + InternalCallerService.class, InternalMarkedService.class, MarkedService.class); + + assertThat(found).hasSize(2); + assertThat(found).anyMatch(line -> line.contains("callsUser (@Internal)") && line.contains("needs @RequiresRole(USER)")); + assertThat(found).anyMatch(line -> line.contains("adminCallsInternal (@RequiresRole(PLATFORM_ADMIN))") + && line.contains("needs @Internal")); + } +} diff --git a/src/test/java/core_service/architecture/fixture/api/BadController.java b/src/test/java/core_service/architecture/fixture/api/BadController.java new file mode 100644 index 0000000..63f5158 --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/api/BadController.java @@ -0,0 +1,15 @@ +package core_service.architecture.fixture.api; + +import core_service.architecture.fixture.service.MarkedService; +import core_service.architecture.fixture.service.UnmarkedService; + +public class BadController { + + private final UnmarkedService unmarkedService = new UnmarkedService(); + private final MarkedService markedService = new MarkedService(); + + public void handle() { + unmarkedService.bare(); + markedService.later(); + } +} diff --git a/src/test/java/core_service/architecture/fixture/api/GoodController.java b/src/test/java/core_service/architecture/fixture/api/GoodController.java new file mode 100644 index 0000000..9977c0b --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/api/GoodController.java @@ -0,0 +1,15 @@ +package core_service.architecture.fixture.api; + +import core_service.architecture.fixture.service.MarkedService; +import core_service.architecture.fixture.service.UnmarkedService; + +public class GoodController { + + private final MarkedService markedService = new MarkedService(); + + public void handle() { + markedService.plain(); + markedService.open(); + UnmarkedService.helper(); + } +} diff --git a/src/test/java/core_service/architecture/fixture/api/MethodReferenceController.java b/src/test/java/core_service/architecture/fixture/api/MethodReferenceController.java new file mode 100644 index 0000000..949c712 --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/api/MethodReferenceController.java @@ -0,0 +1,13 @@ +package core_service.architecture.fixture.api; + +import core_service.architecture.fixture.service.UnmarkedService; + +public class MethodReferenceController { + + private final UnmarkedService unmarkedService = new UnmarkedService(); + + public void handle() { + Runnable call = unmarkedService::bare; + call.run(); + } +} diff --git a/src/test/java/core_service/architecture/fixture/service/ForwardingService.java b/src/test/java/core_service/architecture/fixture/service/ForwardingService.java new file mode 100644 index 0000000..8d277c8 --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/service/ForwardingService.java @@ -0,0 +1,10 @@ +package core_service.architecture.fixture.service; + +public class ForwardingService { + + private final MarkedService markedService = new MarkedService(); + + public void forward() { + markedService.plain(); + } +} diff --git a/src/test/java/core_service/architecture/fixture/service/InternalCallerService.java b/src/test/java/core_service/architecture/fixture/service/InternalCallerService.java new file mode 100644 index 0000000..32fe6a9 --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/service/InternalCallerService.java @@ -0,0 +1,25 @@ +package core_service.architecture.fixture.service; + +import core_service.shared.security.Internal; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; + +@Internal +public class InternalCallerService { + + private final MarkedService markedService = new MarkedService(); + private final InternalMarkedService internalMarkedService = new InternalMarkedService(); + + public void callsInternal() { + internalMarkedService.call(); + } + + public void callsUser() { + markedService.plain(); + } + + @RequiresRole(Role.PLATFORM_ADMIN) + public void adminCallsInternal() { + internalMarkedService.call(); + } +} diff --git a/src/test/java/core_service/architecture/fixture/service/InternalMarkedService.java b/src/test/java/core_service/architecture/fixture/service/InternalMarkedService.java new file mode 100644 index 0000000..3982988 --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/service/InternalMarkedService.java @@ -0,0 +1,10 @@ +package core_service.architecture.fixture.service; + +import core_service.shared.security.Internal; + +@Internal +public class InternalMarkedService { + + public void call() { + } +} diff --git a/src/test/java/core_service/architecture/fixture/service/MarkedService.java b/src/test/java/core_service/architecture/fixture/service/MarkedService.java new file mode 100644 index 0000000..b195773 --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/service/MarkedService.java @@ -0,0 +1,25 @@ +package core_service.architecture.fixture.service; + +import core_service.shared.security.Public; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; +import org.springframework.scheduling.annotation.Async; + +@RequiresRole(Role.USER) +public class MarkedService { + + public void plain() { + } + + @Public + public void open() { + } + + @RequiresRole(Role.AUDITOR) + public void audit() { + } + + @Async + public void later() { + } +} diff --git a/src/test/java/core_service/architecture/fixture/service/PublicCallerService.java b/src/test/java/core_service/architecture/fixture/service/PublicCallerService.java new file mode 100644 index 0000000..096f080 --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/service/PublicCallerService.java @@ -0,0 +1,17 @@ +package core_service.architecture.fixture.service; + +import core_service.shared.security.Public; + +@Public +public class PublicCallerService { + + private final MarkedService markedService = new MarkedService(); + + public void callsOpen() { + markedService.open(); + } + + public void callsUser() { + markedService.plain(); + } +} diff --git a/src/test/java/core_service/architecture/fixture/service/PublicHelperCallerService.java b/src/test/java/core_service/architecture/fixture/service/PublicHelperCallerService.java new file mode 100644 index 0000000..9a13047 --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/service/PublicHelperCallerService.java @@ -0,0 +1,20 @@ +package core_service.architecture.fixture.service; + +import core_service.shared.security.Public; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; + +@RequiresRole(Role.USER) +public class PublicHelperCallerService { + + private final MarkedService markedService = new MarkedService(); + + @Public + public void open() { + helper(); + } + + private void helper() { + markedService.plain(); + } +} diff --git a/src/test/java/core_service/architecture/fixture/service/PublicMethodReferenceCallerService.java b/src/test/java/core_service/architecture/fixture/service/PublicMethodReferenceCallerService.java new file mode 100644 index 0000000..f4d941c --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/service/PublicMethodReferenceCallerService.java @@ -0,0 +1,17 @@ +package core_service.architecture.fixture.service; + +import core_service.shared.security.Public; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; + +@RequiresRole(Role.USER) +public class PublicMethodReferenceCallerService { + + private final MarkedService markedService = new MarkedService(); + + @Public + public void open() { + Runnable call = markedService::plain; + call.run(); + } +} diff --git a/src/test/java/core_service/architecture/fixture/service/PublicViaUnmarkedCallerService.java b/src/test/java/core_service/architecture/fixture/service/PublicViaUnmarkedCallerService.java new file mode 100644 index 0000000..462cdf9 --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/service/PublicViaUnmarkedCallerService.java @@ -0,0 +1,13 @@ +package core_service.architecture.fixture.service; + +import core_service.shared.security.Public; + +@Public +public class PublicViaUnmarkedCallerService { + + private final ForwardingService forwardingService = new ForwardingService(); + + public void callsThroughAnUnmarkedService() { + forwardingService.forward(); + } +} diff --git a/src/test/java/core_service/architecture/fixture/service/TwoMarkersService.java b/src/test/java/core_service/architecture/fixture/service/TwoMarkersService.java new file mode 100644 index 0000000..a911d84 --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/service/TwoMarkersService.java @@ -0,0 +1,12 @@ +package core_service.architecture.fixture.service; + +import core_service.shared.security.Internal; +import core_service.shared.security.Public; + +public class TwoMarkersService { + + @Public + @Internal + public void both() { + } +} diff --git a/src/test/java/core_service/architecture/fixture/service/UnmarkedService.java b/src/test/java/core_service/architecture/fixture/service/UnmarkedService.java new file mode 100644 index 0000000..541fab9 --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/service/UnmarkedService.java @@ -0,0 +1,11 @@ +package core_service.architecture.fixture.service; + +public class UnmarkedService { + + public void bare() { + } + + public static boolean helper() { + return true; + } +} diff --git a/src/test/java/core_service/architecture/fixture/service/UserCallerService.java b/src/test/java/core_service/architecture/fixture/service/UserCallerService.java new file mode 100644 index 0000000..6dc48a2 --- /dev/null +++ b/src/test/java/core_service/architecture/fixture/service/UserCallerService.java @@ -0,0 +1,14 @@ +package core_service.architecture.fixture.service; + +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; + +@RequiresRole(Role.USER) +public class UserCallerService { + + private final MarkedService markedService = new MarkedService(); + + public void callsAuditor() { + markedService.audit(); + } +} diff --git a/src/test/java/core_service/configuration/security/JwtServiceTest.java b/src/test/java/core_service/configuration/security/JwtServiceTest.java index e7455f5..c6e0953 100644 --- a/src/test/java/core_service/configuration/security/JwtServiceTest.java +++ b/src/test/java/core_service/configuration/security/JwtServiceTest.java @@ -1,6 +1,7 @@ package core_service.configuration.security; import core_service.shared.security.ProfileTicket; +import core_service.shared.security.Role; import core_service.shared.security.UserPrincipal; import core_service.shared.test.AbstractUnitTest; import io.jsonwebtoken.Jwts; @@ -166,4 +167,33 @@ class JwtServiceTest extends AbstractUnitTest { void parseLoginToken_ServiceTokenIsNoLogin() { assertEquals(Optional.empty(), jwtService.parseLoginToken(signServiceToken(signingKey, "service", 60_000L))); } + + private String signTokenWithRole(String role) { + return Jwts.builder() + .subject("test@example.com") + .claim("userId", "0815") + .claim("role", role) + .expiration(new Date(System.currentTimeMillis() + 60_000)) + .signWith(signingKey, Jwts.SIG.EdDSA) + .compact(); + } + + @Test + void parseLoginToken_ReadsTheActiveRole() { + assertEquals(Role.AUDITOR, jwtService.parseLoginToken(signTokenWithRole("AUDITOR")).orElseThrow().role()); + } + + @Test + void parseLoginToken_WithoutOrWithAnUnknownRole_IsUser() { + assertEquals(Role.USER, jwtService.parseLoginToken(signToken("0815", "test@example.com", null)).orElseThrow().role()); + assertEquals(Role.USER, jwtService.parseLoginToken(signTokenWithRole("GOD")).orElseThrow().role()); + } + + @Test + void parseLoginToken_WithARoleThatIsNoString_IsStillALoginAsUser() { + String token = Jwts.builder().subject("test@example.com").claim("userId", "0815").claim("role", List.of("AUDITOR")) + .expiration(new Date(System.currentTimeMillis() + 60_000)).signWith(signingKey, Jwts.SIG.EdDSA).compact(); + + assertEquals(Role.USER, jwtService.parseLoginToken(token).orElseThrow().role()); + } } diff --git a/src/test/java/core_service/configuration/security/PublicEndpointsTest.java b/src/test/java/core_service/configuration/security/PublicEndpointsTest.java index 7a647a0..b474a94 100644 --- a/src/test/java/core_service/configuration/security/PublicEndpointsTest.java +++ b/src/test/java/core_service/configuration/security/PublicEndpointsTest.java @@ -20,6 +20,7 @@ import java.security.KeyPair; import java.security.KeyPairGenerator; import java.util.Base64; import java.util.Date; +import java.util.List; import java.util.Set; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -91,6 +92,23 @@ class PublicEndpointsTest { .andExpect(jsonPath("$.code").value("INVALID_TICKET")); } + // Without a login: a class role marker on the way would answer 401. + @Test + void publicProfileWithSharedStats_WorksWithoutALogin() throws Exception { + String ticket = Jwts.builder().subject("public-profile").claim("typ", "public-profile").claim("profileUserId", "1") + .claim("profileStats", List.of("TOP_GAMES", "OVERVIEW")).expiration(new Date(System.currentTimeMillis() + 60_000)) + .signWith(KEYS.getPrivate(), Jwts.SIG.EdDSA).compact(); + + mockMvc.perform(get("/api/public/profiles/library").header("X-Profile-Ticket", ticket)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.stats.topGames").exists()); + } + + @Test + void landingStatsArePublic() throws Exception { + mockMvc.perform(get("/api/stats/public")).andExpect(status().isOk()); + } + @Test void versionIsPublic() throws Exception { mockMvc.perform(get("/api/version")) diff --git a/src/test/java/core_service/configuration/security/RoleAuthorizationManagerTest.java b/src/test/java/core_service/configuration/security/RoleAuthorizationManagerTest.java new file mode 100644 index 0000000..188398e --- /dev/null +++ b/src/test/java/core_service/configuration/security/RoleAuthorizationManagerTest.java @@ -0,0 +1,119 @@ +package core_service.configuration.security; + +import core_service.shared.security.Internal; +import core_service.shared.security.Public; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; +import core_service.shared.test.AbstractUnitTest; +import org.aopalliance.intercept.MethodInvocation; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.security.authentication.AnonymousAuthenticationToken; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.web.context.request.RequestContextHolder; +import org.springframework.web.context.request.ServletRequestAttributes; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; + +class RoleAuthorizationManagerTest extends AbstractUnitTest { + + private final RoleAuthorizationManager manager = new RoleAuthorizationManager(); + + @RequiresRole(Role.USER) + static class UserMarked { + public void plain() { + } + + @RequiresRole(Role.AUDITOR) + public void audit() { + } + + @Public + public void open() { + } + } + + @Internal + static class InternalMarked { + public void call() { + } + } + + @AfterEach + void clearRequest() { + RequestContextHolder.resetRequestAttributes(); + } + + private static void inRequest() { + RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(new MockHttpServletRequest())); + } + + private static Authentication login(Role role) { + return new UsernamePasswordAuthenticationToken("42", null, AccessTokens.authorities(role)); + } + + private static Authentication anonymous() { + return new AnonymousAuthenticationToken("key", "anonymousUser", List.of(new SimpleGrantedAuthority("ROLE_ANONYMOUS"))); + } + + private static Authentication service() { + return new UsernamePasswordAuthenticationToken("tbp-auth", null, + List.of(new SimpleGrantedAuthority(ServiceTokenFilter.SERVICE_AUTHORITY))); + } + + private boolean granted(Authentication authentication, Object target, String method) throws Exception { + MethodInvocation invocation = mock(MethodInvocation.class); + // Lenient: without a request the manager does not look at the invocation. + lenient().when(invocation.getThis()).thenReturn(target); + lenient().when(invocation.getMethod()).thenReturn(target.getClass().getMethod(method)); + return manager.authorize(() -> authentication, invocation).isGranted(); + } + + @Test + void classMarker_CoversTheMethodsWithoutOwnMarker() throws Exception { + inRequest(); + + assertTrue(granted(login(Role.USER), new UserMarked(), "plain")); + assertFalse(granted(anonymous(), new UserMarked(), "plain")); + } + + @Test + void methodMarker_ReplacesTheClassMarker() throws Exception { + inRequest(); + + assertFalse(granted(login(Role.USER), new UserMarked(), "audit")); + assertTrue(granted(anonymous(), new UserMarked(), "open")); + } + + @Test + void higherRoles_PassLowerChecks() throws Exception { + inRequest(); + + assertTrue(granted(login(Role.AUDITOR), new UserMarked(), "audit")); + assertTrue(granted(login(Role.PLATFORM_ADMIN), new UserMarked(), "audit")); + assertTrue(granted(login(Role.PLATFORM_ADMIN), new UserMarked(), "plain")); + } + + @Test + void internal_NeedsTheServiceToken() throws Exception { + inRequest(); + + assertTrue(granted(service(), new InternalMarked(), "call")); + assertFalse(granted(login(Role.PLATFORM_ADMIN), new InternalMarked(), "call")); + assertFalse(granted(service(), new UserMarked(), "plain")); + } + + @Test + void withoutARequest_EverythingRuns() throws Exception { + assertTrue(granted(null, new UserMarked(), "audit")); + assertTrue(granted(null, new InternalMarked(), "call")); + } +} diff --git a/src/test/java/core_service/configuration/security/RoleCheckFlowTest.java b/src/test/java/core_service/configuration/security/RoleCheckFlowTest.java new file mode 100644 index 0000000..94ad95e --- /dev/null +++ b/src/test/java/core_service/configuration/security/RoleCheckFlowTest.java @@ -0,0 +1,137 @@ +package core_service.configuration.security; + +import core_service.shared.security.Public; +import core_service.shared.security.RequiresRole; +import core_service.shared.security.Role; +import core_service.shared.test.AbstractIntegrationTest; +import io.jsonwebtoken.Jwts; +import jakarta.servlet.http.Cookie; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; +import org.springframework.context.annotation.Import; +import org.springframework.stereotype.Service; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.ResultActions; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +import java.util.Date; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +/** + * The role markers through the real (non-dev) filter chain, on a probe that only this test sees. + */ +@AutoConfigureMockMvc +@ActiveProfiles(value = "prod", inheritProfiles = false) +@Import({RoleCheckFlowTest.ProbeService.class, RoleCheckFlowTest.ProbeController.class}) +class RoleCheckFlowTest extends AbstractIntegrationTest { + + @Service + @RequiresRole(Role.USER) + static class ProbeService { + public String user() { + return "user"; + } + + @RequiresRole(Role.AUDITOR) + public String auditor() { + return "auditor"; + } + + @Public + public String open() { + return "open"; + } + } + + @RestController + static class ProbeController { + private final ProbeService probeService; + + ProbeController(ProbeService probeService) { + this.probeService = probeService; + } + + @GetMapping("/api/role-probe/user") + String user() { + return probeService.user(); + } + + @GetMapping("/api/role-probe/auditor") + String auditor() { + return probeService.auditor(); + } + + // Authenticated URL: the marker opens nothing the URL rules close. + @GetMapping("/api/role-probe/open") + String open() { + return probeService.open(); + } + + // Public URL: only the marker stops an anonymous caller. + @GetMapping("/api/public/role-probe/user") + String publicUser() { + return probeService.user(); + } + + @GetMapping("/api/public/role-probe/open") + String publicOpen() { + return probeService.open(); + } + } + + @Autowired + private MockMvc mockMvc; + + @Autowired + private ProbeService probeService; + + private static String token(Role role) { + return Jwts.builder() + .subject("probe@example.com") + .claim("userId", "4711") + .claim("role", role.name()) + .expiration(new Date(System.currentTimeMillis() + 60_000)) + .signWith(TEST_KEYS.getPrivate(), Jwts.SIG.EdDSA) + .compact(); + } + + private ResultActions call(String path, Role role) throws Exception { + MockHttpServletRequestBuilder request = get("/api/role-probe/" + path); + if (role != null) { + request.cookie(new Cookie(AccessTokens.ACCESS_COOKIE, token(role))); + } + return mockMvc.perform(request); + } + + @Test + void user_PassesUserAndIsRefusedAuditor() throws Exception { + call("user", Role.USER).andExpect(status().isOk()); + call("open", Role.USER).andExpect(status().isOk()); + call("auditor", Role.USER).andExpect(status().isForbidden()); + } + + @Test + void higherRoles_PassTheLowerChecks() throws Exception { + call("auditor", Role.AUDITOR).andExpect(status().isOk()); + call("auditor", Role.PLATFORM_ADMIN).andExpect(status().isOk()); + call("user", Role.PLATFORM_ADMIN).andExpect(status().isOk()); + } + + @Test + void noLogin_OnAPublicUrl_IsStoppedByTheMarker() throws Exception { + mockMvc.perform(get("/api/public/role-probe/user")).andExpect(status().isUnauthorized()); + mockMvc.perform(get("/api/public/role-probe/open")).andExpect(status().isOk()); + } + + @Test + void outsideARequest_TheServiceRuns() { + assertEquals("auditor", probeService.auditor()); + } +} diff --git a/src/test/java/core_service/modules/library/service/UserStatsServiceTrendTest.java b/src/test/java/core_service/modules/library/service/UserStatsServiceTrendTest.java index 63eeb60..91138ce 100644 --- a/src/test/java/core_service/modules/library/service/UserStatsServiceTrendTest.java +++ b/src/test/java/core_service/modules/library/service/UserStatsServiceTrendTest.java @@ -14,6 +14,7 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Import; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.test.context.bean.override.mockito.MockitoBean; import java.time.Instant; import java.time.LocalDate; @@ -29,6 +30,9 @@ import static org.assertj.core.api.Assertions.assertThat; @Import({UserStatsService.class, UserStatsRepository.class, BacklogSnapshotRepository.class}) class UserStatsServiceTrendTest extends AbstractPersistenceTest { + @MockitoBean + private MoodService moodService; + private static final long USER_ID = 1L; @Autowired diff --git a/src/test/java/core_service/shared/test/AbstractIntegrationTest.java b/src/test/java/core_service/shared/test/AbstractIntegrationTest.java index ac366ac..42cf2e9 100644 --- a/src/test/java/core_service/shared/test/AbstractIntegrationTest.java +++ b/src/test/java/core_service/shared/test/AbstractIntegrationTest.java @@ -19,8 +19,10 @@ import java.util.Base64; */ @Tag("IntegrationTest") // The nightly sync would call Steam if a test ran at 23:55; speedrun.com needs no key, so it is switched off. +// Every cached context keeps its own pool: small pools keep them all below Postgres' 100 clients. @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, - properties = {"app.library.nightly-sync-cron=-", "app.library.image-repair-cron=-", "app.speedrun.enabled=false"}) + properties = {"app.library.nightly-sync-cron=-", "app.library.image-repair-cron=-", "app.speedrun.enabled=false", + "spring.datasource.hikari.maximum-pool-size=4"}) @ActiveProfiles("dev") public abstract class AbstractIntegrationTest {