diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/branch/Branch.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/branch/Branch.java index ec60987a8..cc73d87bd 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/branch/Branch.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/branch/Branch.java @@ -14,7 +14,6 @@ import lombok.NoArgsConstructor; import lombok.Setter; import lombok.ToString; -import org.hibernate.annotations.Filter; @Entity @IdClass(BranchId.class) @@ -22,7 +21,6 @@ @Setter @NoArgsConstructor @ToString(callSuper = true) -@Filter(name = "gitRepositoryFilter") public class Branch { @Id private String name; diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/branch/BranchRepository.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/branch/BranchRepository.java index 37c750be6..3c7cda781 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/branch/BranchRepository.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/branch/BranchRepository.java @@ -7,8 +7,6 @@ @Repository public interface BranchRepository extends JpaRepository { - Optional findByName(String name); - Optional findByNameAndRepositoryRepositoryId(String name, Long repositoryId); void deleteByNameAndRepositoryRepositoryId(String name, Long repositoryId); diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/branch/BranchService.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/branch/BranchService.java index 8e03f0de8..84d20d943 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/branch/BranchService.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/branch/BranchService.java @@ -2,6 +2,7 @@ import de.tum.cit.aet.helios.auth.AuthService; import de.tum.cit.aet.helios.commit.CommitRepository; +import de.tum.cit.aet.helios.filters.RepositoryContext; import de.tum.cit.aet.helios.releaseinfo.releasecandidate.ReleaseCandidate; import de.tum.cit.aet.helios.releaseinfo.releasecandidate.ReleaseCandidateRepository; import de.tum.cit.aet.helios.userpreference.UserPreference; @@ -17,7 +18,6 @@ import org.springframework.transaction.annotation.Transactional; @Service -@Transactional @RequiredArgsConstructor public class BranchService { @@ -32,7 +32,11 @@ public List getAllBranches(String sortField, String sortDirection ? userPreferenceRepository.findByUser(authService.getUserFromGithubId()) : Optional.empty(); Comparator comparator = buildBranchInfoDtoComparator(sortField, sortDirection); - return branchRepository.findAll().stream() + Long repositoryId = RepositoryContext.getRepositoryId(); + if (repositoryId == null) { + return List.of(); + } + return branchRepository.findByRepositoryRepositoryId(repositoryId).stream() .map((branch) -> BranchInfoDto.fromBranchAndUserPreference(branch, userPreference, commitRepository)) .sorted(comparator) diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/config/SecurityConfig.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/config/SecurityConfig.java index 11911572a..aea064f5a 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/config/SecurityConfig.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/config/SecurityConfig.java @@ -95,10 +95,7 @@ public WebMvcConfigurer corsConfigurer() { @Override public void addInterceptors(@NonNull InterceptorRegistry registry) { - // Order last (Integer.MAX_VALUE == Ordered.LOWEST_PRECEDENCE) so this runs AFTER Spring's - // Open-Session-In-View interceptor. That guarantees the request's EntityManager is already - // bound when RepositoryInterceptor enables the tenant filter on it. - registry.addWebRequestInterceptor(requestInterceptor).order(Integer.MAX_VALUE); + registry.addWebRequestInterceptor(requestInterceptor); } @Override diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/deployment/DeploymentRepository.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/deployment/DeploymentRepository.java index 1fadfc185..7cb2ebc7a 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/deployment/DeploymentRepository.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/deployment/DeploymentRepository.java @@ -18,6 +18,11 @@ public interface DeploymentRepository extends JpaRepository { List findByRepositoryRepositoryIdAndSha(Long repositoryId, String sha); + // Explicit per-repository scoped finders (Option B). + List findByRepositoryRepositoryIdOrderByCreatedAtDesc(Long repositoryId); + + Optional findByIdAndRepositoryRepositoryId(Long id, Long repositoryId); + List findByRepositoryRepositoryIdAndRefOrderByCreatedAtDesc( Long repositoryId, String ref); diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/deployment/DeploymentService.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/deployment/DeploymentService.java index 633ac47a7..5f6a98887 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/deployment/DeploymentService.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/deployment/DeploymentService.java @@ -22,7 +22,6 @@ import de.tum.cit.aet.helios.workflow.Workflow; import de.tum.cit.aet.helios.workflow.WorkflowRunRepository; import jakarta.persistence.EntityNotFoundException; -import jakarta.transaction.Transactional; import java.io.IOException; import java.time.OffsetDateTime; import java.util.ArrayList; @@ -40,7 +39,6 @@ @Log4j2 @Service -@Transactional @RequiredArgsConstructor public class DeploymentService { @@ -60,22 +58,49 @@ public class DeploymentService { private final WorkflowRunRepository workflowRunRepository; public Optional getDeploymentById(Long id) { - return deploymentRepository.findById(id).map(DeploymentDto::fromDeployment); + Long repositoryId = RepositoryContext.getRepositoryId(); + Optional deployment = + repositoryId == null + ? Optional.empty() + : deploymentRepository.findByIdAndRepositoryRepositoryId(id, repositoryId); + return deployment.map(DeploymentDto::fromDeployment); } public List getAllDeployments() { - return deploymentRepository.findAll().stream() + Long repositoryId = RepositoryContext.getRepositoryId(); + if (repositoryId == null) { + return List.of(); + } + return deploymentRepository.findByRepositoryRepositoryIdOrderByCreatedAtDesc(repositoryId) + .stream() .map(DeploymentDto::fromDeployment) .collect(Collectors.toList()); } + // Deployments/activity are keyed only by environment id; scope to the current repository so a + // foreign environment id cannot surface another repository's deployment data (the removed + // gitRepositoryFilter used to enforce this on the derived Deployment queries). + private boolean environmentInCurrentRepository(Long environmentId) { + Long repositoryId = RepositoryContext.getRepositoryId(); + return repositoryId != null + && environmentRepository + .findByIdAndRepositoryRepositoryId(environmentId, repositoryId) + .isPresent(); + } + public List getDeploymentsByEnvironmentId(Long environmentId) { + if (!environmentInCurrentRepository(environmentId)) { + return List.of(); + } return deploymentRepository.findByEnvironmentIdOrderByCreatedAtDesc(environmentId).stream() .map(DeploymentDto::fromDeployment) .collect(Collectors.toList()); } public Optional getLatestDeploymentByEnvironmentId(Long environmentId) { + if (!environmentInCurrentRepository(environmentId)) { + return Optional.empty(); + } return deploymentRepository .findFirstByEnvironmentIdOrderByCreatedAtDesc(environmentId) .map(DeploymentDto::fromDeployment); @@ -298,6 +323,9 @@ private boolean canRedeploy(Environment environment, long timeoutMinutes) { * */ public List getActivityHistoryByEnvironmentId(Long environmentId) { + if (!environmentInCurrentRepository(environmentId)) { + return List.of(); + } // 1) Real deployments List deploymentDtos = deploymentRepository.findByEnvironmentIdOrderByCreatedAtDesc(environmentId).stream() @@ -356,7 +384,9 @@ public List getActivityHistoryByEnvironmentId(Long environme * @throws EntityNotFoundException if the pull request does not exist */ public List getActivityHistoryByPullRequestId(Long pullRequestId) { - if (pullRequestRepository.findById(pullRequestId).isEmpty()) { + if (pullRequestRepository + .findByIdAndRepositoryRepositoryId(pullRequestId, RepositoryContext.getRepositoryId()) + .isEmpty()) { throw new EntityNotFoundException("Pull request not found with id: " + pullRequestId); } @@ -420,7 +450,6 @@ public List getActivityHistoryByRepositoryIdAndBranchName( * @return Basic success message * @throws DeploymentException if the deployment cannot be canceled */ - @Transactional public String cancelDeployment(CancelDeploymentRequest cancelRequest) { Long workflowRunId = cancelRequest.workflowRunId(); @@ -523,20 +552,34 @@ public WorkflowJobsResponse getWorkflowJobStatus(Long workflowRunId) { } private String resolveWorkflowRunRepositoryName(Long workflowRunId) { + // Resolve strictly within the current repository so a foreign run id can never resolve another + // repository's name (which would proxy that repo's GitHub job status back to the caller). + final Long repositoryId = RepositoryContext.getRepositoryId(); + if (repositoryId == null) { + throw new DeploymentException( + "No repository context to resolve workflow run " + workflowRunId); + } return workflowRunRepository - .findById(workflowRunId.longValue()) + .findByIdAndRepositoryRepositoryId(workflowRunId, repositoryId) .map(workflowRun -> workflowRun.getRepository().getNameWithOwner()) .or( () -> heliosDeploymentRepository .findByWorkflowRunId(workflowRunId) + .filter( + heliosDeployment -> + repositoryId.equals( + heliosDeployment + .getEnvironment() + .getRepository() + .getRepositoryId())) .map( heliosDeployment -> heliosDeployment.getEnvironment().getRepository().getNameWithOwner())) .or( () -> - Optional.ofNullable(RepositoryContext.getRepositoryId()) - .flatMap(gitRepoRepository::findById) + gitRepoRepository + .findById(repositoryId) .map(GitRepository::getNameWithOwner)) .orElseThrow( () -> diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/environment/EnvironmentRepository.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/environment/EnvironmentRepository.java index 76ab78f89..d9cbfcb70 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/environment/EnvironmentRepository.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/environment/EnvironmentRepository.java @@ -10,8 +10,6 @@ @Repository public interface EnvironmentRepository extends JpaRepository { - List findAllByOrderByNameAsc(); - List findByRepository(GitRepository repository); @Query("SELECT e FROM Environment e " @@ -22,7 +20,12 @@ public interface EnvironmentRepository extends JpaRepository List findByRepositoryRepositoryIdOrderByCreatedAtDesc(Long repositoryId); - List findByEnabledTrueOrderByNameAsc(); + // Explicit per-repository scoped finders (Option B: replaces the ambient gitRepositoryFilter). + List findByRepositoryRepositoryIdOrderByNameAsc(Long repositoryId); + + List findByEnabledTrueAndRepositoryRepositoryIdOrderByNameAsc(Long repositoryId); + + Optional findByIdAndRepositoryRepositoryId(Long id, Long repositoryId); @Query("SELECT DISTINCT e FROM Environment e " + "LEFT JOIN FETCH e.statusHistory es " diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/environment/EnvironmentService.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/environment/EnvironmentService.java index 4186a9f0a..cc8781d3b 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/environment/EnvironmentService.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/environment/EnvironmentService.java @@ -30,7 +30,6 @@ import de.tum.cit.aet.helios.workflow.WorkflowRun; import de.tum.cit.aet.helios.workflow.WorkflowRunRepository; import jakarta.persistence.EntityNotFoundException; -import jakarta.transaction.Transactional; import java.io.IOException; import java.time.Instant; import java.time.OffsetDateTime; @@ -51,7 +50,6 @@ import org.springframework.stereotype.Service; @Service -@Transactional @Log4j2 @RequiredArgsConstructor public class EnvironmentService { @@ -77,15 +75,34 @@ public class EnvironmentService { private final NatsNotificationPublisherService notificationPublisherService; public Optional getEnvironmentById(Long id) { - return environmentRepository.findById(id).map(EnvironmentDto::fromEnvironment); + return findScopedById(id).map(EnvironmentDto::fromEnvironment); } public Optional getEnvironmentTypeById(Long id) { - return environmentRepository.findById(id).map(Environment::getType); + return findScopedById(id).map(Environment::getType); + } + + /** + * Loads an environment by id, scoped to the current repository when a repository context is + * present. Explicit replacement for the ambient gitRepositoryFilter, which never applied to + * findById/PK loads — so this also closes a latent cross-repository read. + */ + private Optional findScopedById(Long id) { + Long repositoryId = RepositoryContext.getRepositoryId(); + // No repository context (missing X-REPOSITORY-ID) → no data. GET /api/** is permitAll, so a + // findById fallback here would be an anonymous cross-repo IDOR (the @Filter never covered PK + // loads); always scope, mirroring WorkflowRunService.getWorkflowRunById. + return repositoryId == null + ? Optional.empty() + : environmentRepository.findByIdAndRepositoryRepositoryId(id, repositoryId); } public List getAllEnvironments() { - return environmentRepository.findAllByOrderByNameAsc().stream() + Long repositoryId = RepositoryContext.getRepositoryId(); + if (repositoryId == null) { + return List.of(); + } + return environmentRepository.findByRepositoryRepositoryIdOrderByNameAsc(repositoryId).stream() .map( environment -> { LatestDeploymentUnion latest = findLatestDeployment(environment); @@ -98,7 +115,13 @@ public List getAllEnvironments() { } public List getAllEnabledEnvironments() { - return environmentRepository.findByEnabledTrueOrderByNameAsc().stream() + Long repositoryId = RepositoryContext.getRepositoryId(); + if (repositoryId == null) { + return List.of(); + } + return environmentRepository + .findByEnabledTrueAndRepositoryRepositoryIdOrderByNameAsc(repositoryId) + .stream() .map( environment -> { LatestDeploymentUnion latest = findLatestDeployment(environment); @@ -204,13 +227,12 @@ public LatestDeploymentUnion findLatestDeployment(Environment env) { * the environment is already locked or if an optimistic locking failure occurs * @throws EntityNotFoundException if no environment is found with the specified ID */ - @Transactional public Optional lockEnvironment(Long id) { final User currentUser = authService.getUserFromGithubId(); Environment environment = environmentRepository - .findById(id) + .findByIdAndRepositoryRepositoryId(id, RepositoryContext.getRepositoryId()) .orElseThrow(() -> new EntityNotFoundException("Environment not found with ID: " + id)); if (!environment.isEnabled()) { @@ -263,7 +285,6 @@ public Optional lockEnvironment(Long id) { * @param baseTime the base time from which to calculate expiration * @return the calculated expiration time or null if no threshold is set */ - @Transactional protected OffsetDateTime calculateLockExpiration( Environment environment, OffsetDateTime baseTime) { Long lockExpirationThreshold = @@ -285,7 +306,6 @@ protected OffsetDateTime calculateLockExpiration( * @param baseTime the base time from which to calculate expiration * @return the calculated expiration time or null if no threshold is set */ - @Transactional protected OffsetDateTime calculateReservationExpiration( Environment environment, OffsetDateTime baseTime) { Long lockReservationThreshold = @@ -300,7 +320,6 @@ protected OffsetDateTime calculateReservationExpiration( return lockReservationThreshold != -1 ? baseTime.plusMinutes(lockReservationThreshold) : null; } - @Transactional protected OffsetDateTime getLockWillExpireAt(Environment environment) { if (environment.isLocked() && environment.getLockedAt() != null) { return calculateLockExpiration(environment, environment.getLockedAt()); @@ -309,7 +328,6 @@ protected OffsetDateTime getLockWillExpireAt(Environment environment) { } } - @Transactional protected OffsetDateTime getLockReservationExpiresAt(Environment environment) { if (environment.isLocked() && environment.getLockedAt() != null) { return calculateReservationExpiration(environment, environment.getLockedAt()); @@ -334,13 +352,12 @@ protected OffsetDateTime getLockReservationExpiresAt(Environment environment) { * @throws EntityNotFoundException if no environment is found with the specified ID * @throws EnvironmentException if the environment is disabled or isn't a TEST environment */ - @Transactional public Optional extendEnvironmentLock(Long id) { final User currentUser = authService.getUserFromGithubId(); Environment environment = environmentRepository - .findById(id) + .findByIdAndRepositoryRepositoryId(id, RepositoryContext.getRepositoryId()) .orElseThrow(() -> new EntityNotFoundException("Environment not found with ID: " + id)); // Check environment status @@ -426,13 +443,12 @@ public void markStatusAsChanged(Environment environment) { * @param id the ID of the environment to unlock * @throws EntityNotFoundException if no environment is found with the specified ID */ - @Transactional public EnvironmentDto unlockEnvironment(Long id) { final User currentUser = authService.getUserFromGithubId(); Environment environment = environmentRepository - .findById(id) + .findByIdAndRepositoryRepositoryId(id, RepositoryContext.getRepositoryId()) .orElseThrow(() -> new EntityNotFoundException("Environment not found with ID: " + id)); if (!environment.isLocked()) { @@ -519,7 +535,7 @@ public EnvironmentDto unlockEnvironment(Long id) { public Optional updateEnvironment(Long id, EnvironmentDto environmentDto) throws EnvironmentException { return environmentRepository - .findById(id) + .findByIdAndRepositoryRepositoryId(id, RepositoryContext.getRepositoryId()) .map( environment -> { if (!environmentDto.enabled() && environment.isLocked()) { @@ -646,7 +662,7 @@ public EnvironmentDeploymentReadinessDto getDeploymentReadiness( Long environmentId, String branch, String sha) { Environment environment = environmentRepository - .findById(environmentId) + .findByIdAndRepositoryRepositoryId(environmentId, RepositoryContext.getRepositoryId()) .orElseThrow( () -> new EntityNotFoundException("Environment not found with ID: " + environmentId)); @@ -766,6 +782,11 @@ public EnvironmentLockHistoryDto getUsersCurrentLock() { } public List getLockHistoryByEnvironmentId(Long environmentId) { + // Scope to the current repository (lock history is keyed only by environment id, so a foreign + // id would otherwise leak its lock/unlock history). + if (findScopedById(environmentId).isEmpty()) { + return List.of(); + } return lockHistoryRepository.findLockHistoriesByEnvironment(environmentId).stream() .map( lock -> @@ -809,6 +830,10 @@ private boolean isActiveDeploymentStatus(HeliosDeployment.Status status) { } public Optional getEnvironmentReviewers(Long environmentId) { + // Scope to the current repository — protection rules are keyed only by environment id. + if (findScopedById(environmentId).isEmpty()) { + return Optional.empty(); + } return protectionRuleRepository .findByEnvironmentIdAndRuleType(environmentId, ProtectionRule.RuleType.REQUIRED_REVIEWERS) .map( @@ -860,7 +885,7 @@ public Optional getEnvironmentReviewers(Long environmen * constructing the public-facing DTO; returns the GitHub logins / team slugs directly along with * whether the rule even exists and whether self-review is prevented. * - *

An {@link Optional#empty()} result means "no {@code REQUIRED_REVIEWERS} rule on this env" — + *

An {@link Optional#empty()} result means "no {@code REQUIRED_REVIEWERS} rule here" — * the deployment is not gated on reviewers (it may still be gated on wait-timer or branch * policy, which GitHub handles itself). */ diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/filters/RepositoryFilterEntity.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/filters/RepositoryFilterEntity.java index db56ed21f..5667644c6 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/filters/RepositoryFilterEntity.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/filters/RepositoryFilterEntity.java @@ -7,11 +7,9 @@ import jakarta.persistence.MappedSuperclass; import lombok.Data; import lombok.ToString; -import org.hibernate.annotations.Filter; @MappedSuperclass @Data -@Filter(name = "gitRepositoryFilter") public abstract class RepositoryFilterEntity { @ManyToOne(fetch = FetchType.LAZY) diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/filters/RepositoryFilterTransactionConfig.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/filters/RepositoryFilterTransactionConfig.java deleted file mode 100644 index 661a9690e..000000000 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/filters/RepositoryFilterTransactionConfig.java +++ /dev/null @@ -1,64 +0,0 @@ -package de.tum.cit.aet.helios.filters; - -import org.hibernate.Session; -import org.springframework.beans.factory.config.BeanPostProcessor; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.orm.jpa.JpaTransactionManager; - -/** - * Enables Hibernate's {@code gitRepositoryFilter} on the transactional {@link - * jakarta.persistence.EntityManager} when a request-scoped repository ID is present in - * {@link RepositoryContext}, scoping all queries against tenant-filtered entities (those - * annotated with {@code @Filter(name = "gitRepositoryFilter")}) to that repository. - * - *

Wires up via {@link JpaTransactionManager#setEntityManagerInitializer} — Spring's - * purpose-built hook for "do X to each new transactional {@link - * jakarta.persistence.EntityManager}", added specifically for the Hibernate-filter use - * case. It fires inside {@code createEntityManagerForTransaction()} with a guaranteed - * non-null EM, before the JDBC connection is acquired and before the dialect begins the - * transaction. We customise Spring Boot's auto-configured {@link JpaTransactionManager} - * via a {@link BeanPostProcessor} rather than declaring a {@code @Primary} replacement, - * so we keep all of Boot's other tx-manager wiring intact. - * - *

Replaces the legacy {@code RepositoryFilterAspect} + {@code @EnableLoadTimeWeaving} - * setup, which targeted a Hibernate-internal class ({@code SessionBuilder.openSession}) - * and therefore required AspectJ load-time weaving plus the {@code aspectjweaver} and - * {@code spring-instrument} javaagents at JVM startup. The new mechanism is agent-free. - * - *

Semantic note vs the old aspect: the filter is enabled at transaction begin (per - * new transactional EM), not at every Hibernate session open. Tenant-filtered entity - * access outside a {@code @Transactional} boundary is therefore not filtered — the - * codebase consistently wraps such access in {@code @Transactional}, so this matches - * existing call patterns. - */ -@Configuration -public class RepositoryFilterTransactionConfig { - - /** - * Bean post-processor that catches Spring Boot's auto-configured {@link - * JpaTransactionManager} and installs the entity-manager initializer on it. Declared - * {@code static} so it can be created before regular singletons (the BPP contract). - */ - @Bean - public static BeanPostProcessor gitRepositoryFilterEnabler() { - return new BeanPostProcessor() { - @Override - public Object postProcessAfterInitialization(Object bean, String beanName) { - if (bean instanceof JpaTransactionManager jpaTxManager) { - jpaTxManager.setEntityManagerInitializer( - em -> { - Long repositoryId = RepositoryContext.getRepositoryId(); - if (repositoryId == null) { - return; - } - em.unwrap(Session.class) - .enableFilter("gitRepositoryFilter") - .setParameter("repository_id", repositoryId); - }); - } - return bean; - } - }; - } -} diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/filters/RepositoryInterceptor.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/filters/RepositoryInterceptor.java index 42c2c8b33..a070371d4 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/filters/RepositoryInterceptor.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/filters/RepositoryInterceptor.java @@ -1,66 +1,39 @@ package de.tum.cit.aet.helios.filters; -import jakarta.persistence.EntityManagerFactory; -import lombok.RequiredArgsConstructor; -import lombok.extern.log4j.Log4j2; -import org.hibernate.Session; import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; -import org.springframework.orm.jpa.EntityManagerHolder; import org.springframework.stereotype.Component; -import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.ui.ModelMap; import org.springframework.web.context.request.WebRequest; import org.springframework.web.context.request.WebRequestInterceptor; +/** + * Publishes the current repository id (from the {@code X-REPOSITORY-ID} header) into + * {@link RepositoryContext} for the duration of the request. Repository-scoped services read it and + * filter their queries explicitly (there is no longer any ambient Hibernate filter). + */ @Component -@RequiredArgsConstructor -@Log4j2 public class RepositoryInterceptor implements WebRequestInterceptor { public static final String X_REPOSITORY_ID = "X-REPOSITORY-ID"; - private final EntityManagerFactory entityManagerFactory; - @Override public void preHandle(@NonNull WebRequest request) { final String tenantId = request.getHeader(X_REPOSITORY_ID); - if (tenantId == null) { - return; - } - RepositoryContext.setRepositoryId(tenantId); - final Long repositoryId = RepositoryContext.getRepositoryId(); - if (repositoryId == null) { - return; - } - // Enable Hibernate's gitRepositoryFilter on this request's EntityManager so tenant-filtered - // entities are scoped to the current repository. RepositoryFilterTransactionConfig only - // enables the filter on EntityManagers created by the transaction manager, which does NOT - // happen under Open-Session-In-View (the request's transaction reuses the EM opened by OSIV). - // Without this, web reads (pull requests, environments, ...) leak across all repositories. - // This interceptor is ordered after OSIV (see SecurityConfig#corsConfigurer) so the request's - // EntityManager is already bound here. - final EntityManagerHolder holder = - (EntityManagerHolder) TransactionSynchronizationManager.getResource(entityManagerFactory); - if (holder != null) { - holder - .getEntityManager() - .unwrap(Session.class) - .enableFilter("gitRepositoryFilter") - .setParameter("repository_id", repositoryId); + if (tenantId != null) { + RepositoryContext.setRepositoryId(tenantId); } else { - log.warn( - "gitRepositoryFilter not enabled for repository {}: no EntityManager bound at preHandle", - repositoryId); + // No repository header → no repository context (also defends against a stale ThreadLocal + // left on a reused worker thread). + RepositoryContext.clear(); } } @Override public void postHandle(@NonNull WebRequest request, @Nullable ModelMap model) { - // Intentionally empty — clearing happens in afterCompletion so the ThreadLocal is wiped - // even when the handler threw (postHandle is skipped on exception, afterCompletion runs - // unconditionally). Otherwise a Tomcat worker thread keeps a stale repositoryId across - // requests, which would silently apply the previous user's tenant filter to the next. + // Intentionally empty — clearing happens in afterCompletion so the ThreadLocal is wiped even + // when the handler threw (postHandle is skipped on exception, afterCompletion runs + // unconditionally). Otherwise a Tomcat worker thread keeps a stale repositoryId across requests. } @Override diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/github/GitHubService.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/github/GitHubService.java index 4f9727347..2eaad7cc0 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/github/GitHubService.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/github/GitHubService.java @@ -14,7 +14,6 @@ import de.tum.cit.aet.helios.github.permissions.GitHubRepositoryRoleDto; import de.tum.cit.aet.helios.github.permissions.RepoPermissionType; import de.tum.cit.aet.helios.workflow.GitHubWorkflowContext; -import jakarta.transaction.Transactional; import java.io.BufferedReader; import java.io.FilterInputStream; import java.io.IOException; @@ -56,7 +55,6 @@ @Log4j2 @RequiredArgsConstructor @Service -@Transactional public class GitHubService { private static final String GITHUB_API_VERSION = "2026-03-10"; private static final int GITHUB_ENVIRONMENTS_PAGE_SIZE = 100; diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/gitrepo/GitRepository.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/gitrepo/GitRepository.java index b0f69ffe4..6c569da51 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/gitrepo/GitRepository.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/gitrepo/GitRepository.java @@ -20,7 +20,6 @@ import lombok.NoArgsConstructor; import lombok.Setter; import lombok.ToString; -import org.hibernate.annotations.Filter; import org.springframework.lang.NonNull; @Entity @@ -29,7 +28,6 @@ @Setter @NoArgsConstructor @ToString(callSuper = true) -@Filter(name = "gitRepositoryFilter") public class GitRepository { @Id diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsController.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsController.java index 9f7640c0b..482f0ae52 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsController.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsController.java @@ -33,7 +33,7 @@ public class GitRepoSettingsController { @GetMapping("/settings") public ResponseEntity getGitRepoSettings(@PathVariable Long repositoryId) { GitRepoSettingsDto gitRepoSettingsDto = - gitRepoSettingsService.getOrCreateGitRepoSettingsByRepositoryId(repositoryId).orElseThrow(); + gitRepoSettingsService.getGitRepoSettingsByRepositoryId(repositoryId); return ResponseEntity.ok(gitRepoSettingsDto); } diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsService.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsService.java index a7962580a..32807f573 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsService.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsService.java @@ -3,7 +3,6 @@ import de.tum.cit.aet.helios.environment.EnvironmentService; import de.tum.cit.aet.helios.gitrepo.GitRepoRepository; import de.tum.cit.aet.helios.gitrepo.GitRepository; -import jakarta.transaction.Transactional; import java.util.Optional; import lombok.RequiredArgsConstructor; import org.springframework.context.annotation.Lazy; @@ -13,7 +12,6 @@ @RequiredArgsConstructor @Service -@Transactional public class GitRepoSettingsService { private final GitRepoSettingsRepository gitRepoRepository; @@ -45,15 +43,51 @@ public Optional getOrCreateGitRepoSettingsByRepositoryId(Lon })); } + /** + * Read-only lookup for the settings GET endpoint: returns the persisted settings, or transient + * defaults when none exist yet — WITHOUT writing. (getOrCreate persists a row; a plain GET + * must not do; callers that need the row to exist keep using getOrCreate.) + */ + public GitRepoSettingsDto getGitRepoSettingsByRepositoryId(Long repositoryId) { + return gitRepoRepository + .findByRepositoryRepositoryId(repositoryId) + .map(GitRepoSettingsDto::fromGitRepoSettings) + .orElseGet( + () -> { + GitRepository gitRepo = + gitRepository + .findByRepositoryId(repositoryId) + .orElseThrow( + () -> + new IllegalArgumentException( + "Repository with id " + repositoryId + " not found")); + GitRepoSettings defaults = new GitRepoSettings(); + defaults.setRepository(gitRepo); + return GitRepoSettingsDto.fromGitRepoSettings(defaults); + }); + } + public Optional updateGitRepoSettings( @PathVariable Long repositoryId, @RequestBody GitRepoSettingsDto gitRepoSettingsDto) { + // Create-on-absent: GET /settings no longer persists a row as a side effect, so Save must be + // robust for a repo whose settings row was never lazily created (new repo, no locked env, no + // workflow group). The orElseThrow now only fires when the repository itself does not exist. GitRepoSettings gitRepoSettings = gitRepoRepository .findByRepositoryRepositoryId(repositoryId) - .orElseThrow( - () -> - new IllegalArgumentException( - "GitRepoSettings not found for repository with id " + repositoryId)); + .orElseGet( + () -> { + GitRepository gitRepo = + gitRepository + .findByRepositoryId(repositoryId) + .orElseThrow( + () -> + new IllegalArgumentException( + "Repository with id " + repositoryId + " not found")); + GitRepoSettings created = new GitRepoSettings(); + created.setRepository(gitRepo); + return created; + }); if (gitRepoSettingsDto.lockExpirationThreshold() != null) { gitRepoSettings.setLockExpirationThreshold(gitRepoSettingsDto.lockExpirationThreshold()); diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/issue/IssueRepository.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/issue/IssueRepository.java deleted file mode 100644 index b7612e5df..000000000 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/issue/IssueRepository.java +++ /dev/null @@ -1,5 +0,0 @@ -package de.tum.cit.aet.helios.issue; - -import org.springframework.data.jpa.repository.JpaRepository; - -public interface IssueRepository extends JpaRepository {} diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/label/Label.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/label/Label.java index cef6a3059..e12ab15d8 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/label/Label.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/label/Label.java @@ -15,14 +15,12 @@ import lombok.NonNull; import lombok.Setter; import lombok.ToString; -import org.hibernate.annotations.Filter; @Entity @Table(name = "label") @Getter @Setter @NoArgsConstructor -@Filter(name = "gitRepositoryFilter") public class Label { @Id diff --git a/server/application-server/src/main/java/de/tum/cit/aet/helios/label/LabelRepository.java b/server/application-server/src/main/java/de/tum/cit/aet/helios/label/LabelRepository.java index 4c385bf10..42ed389d8 100644 --- a/server/application-server/src/main/java/de/tum/cit/aet/helios/label/LabelRepository.java +++ b/server/application-server/src/main/java/de/tum/cit/aet/helios/label/LabelRepository.java @@ -1,7 +1,6 @@ package de.tum.cit.aet.helios.label; import java.util.Optional; -import lombok.NonNull; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; @@ -9,8 +8,6 @@ @Repository public interface LabelRepository extends JpaRepository { - Optional

This is the harness for the tenant-isolation guard tests: it exercises the same request path that + * production uses, so a scoping test written here passes both with the legacy Hibernate + * {@code gitRepositoryFilter} and with explicit per-query filtering after the migration. + * + *

External integrations that would otherwise reach out on startup are neutralised: NATS is disabled + * ({@code nats.enabled=false}), the schedulers/reconciliation/notifications/AI are turned off, and the + * GitHub clients are mocked so no credentials or network are required. + */ +@SpringBootTest +@ActiveProfiles("test") +@AutoConfigureMockMvc +@AutoConfigureEmbeddedDatabase( + type = AutoConfigureEmbeddedDatabase.DatabaseType.POSTGRES, + provider = AutoConfigureEmbeddedDatabase.DatabaseProvider.DOCKER) +public abstract class HeliosIntegrationTest { + + public static final String X_REPOSITORY_ID = "X-REPOSITORY-ID"; + + @Autowired protected MockMvc mockMvc; + @Autowired protected DataSource dataSource; + + // GitHub clients reach out to GitHub on construction/first use; mock them so the context boots + // without credentials or network. Read (GET) scoping tests do not exercise GitHub. + @MockitoBean protected GitHubClientManager gitHubClientManager; + @MockitoBean protected GitHubFacade gitHubFacade; + @MockitoBean protected GitHubService gitHubService; +} diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/branch/BranchScopingIT.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/branch/BranchScopingIT.java new file mode 100644 index 000000000..0ecd93777 --- /dev/null +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/branch/BranchScopingIT.java @@ -0,0 +1,78 @@ +package de.tum.cit.aet.helios.branch; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.everyItem; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import de.tum.cit.aet.helios.HeliosIntegrationTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.jdbc.core.JdbcTemplate; + +/** + * Cross-refactor guard: {@code GET /api/branches} must return only the current repository's + * branches (scoped by {@code X-REPOSITORY-ID}). Passes with the legacy gitRepositoryFilter and must + * keep passing after the switch to explicit per-query filtering. + */ +class BranchScopingIT extends HeliosIntegrationTest { + + private static final long REPO_A = 1L; + private static final long REPO_B = 2L; + + @BeforeEach + void seed() { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + jdbc.execute("TRUNCATE TABLE repository CASCADE"); + insertRepo(jdbc, REPO_A, "ls1intum/repo-a"); + insertRepo(jdbc, REPO_B, "ls1intum/repo-b"); + insertBranch(jdbc, REPO_A, "a-main"); + insertBranch(jdbc, REPO_A, "a-feature"); + insertBranch(jdbc, REPO_B, "b-main"); + } + + @Test + void branchesAreScopedToRepositoryA() throws Exception { + mockMvc + .perform(get("/api/branches").header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(2)) + .andExpect(jsonPath("$[*].repository.id", everyItem(equalTo((int) REPO_A)))); + } + + @Test + void branchesAreScopedToRepositoryB() throws Exception { + mockMvc + .perform(get("/api/branches").header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(1)) + .andExpect(jsonPath("$[0].repository.id").value((int) REPO_B)); + } + + @Test + void withoutRepositoryContextReturnsEmpty() throws Exception { + // No X-REPOSITORY-ID header → a tenant-scoped read has no repository → empty (never findAll). + mockMvc + .perform(get("/api/branches")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(0)); + } + + private static void insertRepo(JdbcTemplate jdbc, long id, String nameWithOwner) { + jdbc.update( + "INSERT INTO repository (repository_id, has_issues, has_projects, has_wiki, is_archived, " + + "is_disabled, is_private, stargazers_count, watchers_count, name_with_owner) " + + "VALUES (?, false, false, false, false, false, false, 0, 0, ?)", + id, + nameWithOwner); + } + + private static void insertBranch(JdbcTemplate jdbc, long repositoryId, String name) { + jdbc.update( + "INSERT INTO branch (repository_id, name, ahead_by, behind_by, is_default, protection) " + + "VALUES (?, ?, 0, 0, false, false)", + repositoryId, + name); + } +} diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/branch/BranchServiceTest.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/branch/BranchServiceTest.java index 8b208803a..cb552dc00 100644 --- a/server/application-server/src/test/java/de/tum/cit/aet/helios/branch/BranchServiceTest.java +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/branch/BranchServiceTest.java @@ -6,6 +6,7 @@ import de.tum.cit.aet.helios.auth.AuthService; import de.tum.cit.aet.helios.commit.CommitRepository; +import de.tum.cit.aet.helios.filters.RepositoryContext; import de.tum.cit.aet.helios.gitrepo.GitRepository; import de.tum.cit.aet.helios.releaseinfo.releasecandidate.ReleaseCandidateRepository; import de.tum.cit.aet.helios.userpreference.UserPreference; @@ -14,6 +15,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -44,6 +46,7 @@ public class BranchServiceTest { @BeforeEach public void setUp() { + RepositoryContext.setRepositoryId("1"); GitRepository repo = new GitRepository(); repo.setRepositoryId(1L); @@ -63,10 +66,15 @@ public void setUp() { b3.setUpdatedAt(OffsetDateTime.parse("2025-03-15T10:00:00Z")); when(commitRepository.findByShaAndRepository(any(), any())).thenReturn(Optional.empty()); - when(branchRepository.findAll()).thenReturn(List.of(b1, b2, b3)); + when(branchRepository.findByRepositoryRepositoryId(1L)).thenReturn(List.of(b1, b2, b3)); when(authService.isLoggedIn()).thenReturn(false); } + @AfterEach + public void tearDown() { + RepositoryContext.clear(); + } + @Test public void testPinnedBranchesAreShownFirst() { final UserPreference userPreference = new UserPreference(); diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/deployment/DeploymentScopingIT.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/deployment/DeploymentScopingIT.java new file mode 100644 index 000000000..ab57adeef --- /dev/null +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/deployment/DeploymentScopingIT.java @@ -0,0 +1,158 @@ +package de.tum.cit.aet.helios.deployment; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.everyItem; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import de.tum.cit.aet.helios.HeliosIntegrationTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.jdbc.core.JdbcTemplate; + +/** + * Cross-repository isolation guard for {@code /api/deployments*}. Deployments are keyed by id, + * environment id, and pull-request id; the removed Hibernate {@code gitRepositoryFilter} used to + * scope the derived {@code Deployment} queries to the current repository, so these endpoints must + * now enforce it explicitly. Because {@code GET /api/**} is unauthenticated, the no-header case is + * exercised too (a missing {@code X-REPOSITORY-ID} must not fall back to an unscoped PK load). + */ +class DeploymentScopingIT extends HeliosIntegrationTest { + + private static final long REPO_A = 1L; + private static final long REPO_B = 2L; + private static final long ENV_A = 11L; + private static final long ENV_B = 21L; + private static final long DEPLOY_A = 101L; + private static final long DEPLOY_B = 201L; + + @BeforeEach + void seed() { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + jdbc.execute("TRUNCATE TABLE repository CASCADE"); + insertRepo(jdbc, REPO_A, "ls1intum/repo-a"); + insertRepo(jdbc, REPO_B, "ls1intum/repo-b"); + insertEnv(jdbc, ENV_A, REPO_A, "a-staging"); + insertEnv(jdbc, ENV_B, REPO_B, "b-staging"); + insertDeployment(jdbc, DEPLOY_A, ENV_A, REPO_A); + insertDeployment(jdbc, DEPLOY_B, ENV_B, REPO_B); + } + + @Test + void deploymentsListIsScopedToCurrentRepository() throws Exception { + mockMvc + .perform(get("/api/deployments").header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(1)) + .andExpect(jsonPath("$[*].repository.id", everyItem(equalTo((int) REPO_A)))); + mockMvc + .perform(get("/api/deployments").header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(1)) + .andExpect(jsonPath("$[0].repository.id").value((int) REPO_B)); + } + + @Test + void deploymentsListWithoutHeaderIsEmpty() throws Exception { + mockMvc + .perform(get("/api/deployments")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(0)); + } + + @Test + void deploymentByIdIsInvisibleFromAnotherRepository() throws Exception { + mockMvc + .perform( + get("/api/deployments/{id}", DEPLOY_B).header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isNotFound()); + mockMvc + .perform( + get("/api/deployments/{id}", DEPLOY_B).header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()); + } + + @Test + void deploymentByIdWithoutHeaderIsNotFound() throws Exception { + // No header → null context → must not fall back to an unscoped findById (anonymous IDOR). + mockMvc + .perform(get("/api/deployments/{id}", DEPLOY_A)) + .andExpect(status().isNotFound()); + } + + @Test + void deploymentsByForeignEnvironmentAreEmpty() throws Exception { + // ENV_B belongs to repo B: scoping to repo A must not surface repo B's deployments. + mockMvc + .perform( + get("/api/deployments/environment/{envId}", ENV_B) + .header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(0)); + mockMvc + .perform( + get("/api/deployments/environment/{envId}", ENV_B) + .header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(1)); + } + + @Test + void latestDeploymentByForeignEnvironmentIsNotFound() throws Exception { + mockMvc + .perform( + get("/api/deployments/environment/{envId}/latest", ENV_B) + .header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isNotFound()); + mockMvc + .perform( + get("/api/deployments/environment/{envId}/latest", ENV_B) + .header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()); + } + + @Test + void activityHistoryByForeignEnvironmentIsEmpty() throws Exception { + mockMvc + .perform( + get("/api/deployments/environment/{envId}/activity-history", ENV_B) + .header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(0)); + mockMvc + .perform( + get("/api/deployments/environment/{envId}/activity-history", ENV_B) + .header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(1)); + } + + private static void insertRepo(JdbcTemplate jdbc, long id, String nameWithOwner) { + jdbc.update( + "INSERT INTO repository (repository_id, has_issues, has_projects, has_wiki, is_archived, " + + "is_disabled, is_private, stargazers_count, watchers_count, name_with_owner) " + + "VALUES (?, false, false, false, false, false, false, 0, 0, ?)", + id, + nameWithOwner); + } + + private static void insertEnv(JdbcTemplate jdbc, long id, long repositoryId, String name) { + jdbc.update( + "INSERT INTO environment (id, repository_id, enabled, locked, name) " + + "VALUES (?, ?, true, false, ?)", + id, + repositoryId, + name); + } + + private static void insertDeployment( + JdbcTemplate jdbc, long id, long environmentId, long repositoryId) { + jdbc.update( + "INSERT INTO deployment (id, environment_id, repository_id, created_at, state, ref, sha, " + + "url) VALUES (?, ?, ?, now(), 'SUCCESS', 'main', 'deadbeef', 'https://example/dep')", + id, + environmentId, + repositoryId); + } +} diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/deployment/DeploymentServiceTest.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/deployment/DeploymentServiceTest.java index f21c5581e..224bd1306 100644 --- a/server/application-server/src/test/java/de/tum/cit/aet/helios/deployment/DeploymentServiceTest.java +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/deployment/DeploymentServiceTest.java @@ -100,7 +100,8 @@ public void tearDown() { @Test public void testGetDeploymentById() { - when(deploymentRepository.findById(1L)).thenReturn(Optional.of(deployment)); + when(deploymentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(deployment)); Optional result = deploymentService.getDeploymentById(1L); @@ -109,13 +110,14 @@ public void testGetDeploymentById() { assertTrue(result.isPresent()); assertEquals(deploymentDto, result.get()); - verify(deploymentRepository, times(1)).findById(1L); + verify(deploymentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); } @Test public void testGetAllDeployments() { - when(deploymentRepository.findAll()).thenReturn(List.of(deployment, deployment)); + when(deploymentRepository.findByRepositoryRepositoryIdOrderByCreatedAtDesc(1L)) + .thenReturn(List.of(deployment, deployment)); List result = deploymentService.getAllDeployments(); @@ -127,6 +129,8 @@ public void testGetAllDeployments() { @Test public void testGetDeploymentsByEnvironmentId() { + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(deploymentRepository.findByEnvironmentIdOrderByCreatedAtDesc(1L)) .thenReturn(List.of(deployment)); @@ -140,6 +144,8 @@ public void testGetDeploymentsByEnvironmentId() { @Test public void testGetLatestDeploymentByEnvironmentId() { + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(deploymentRepository.findFirstByEnvironmentIdOrderByCreatedAtDesc(1L)) .thenReturn(Optional.of(deployment)); @@ -151,6 +157,15 @@ public void testGetLatestDeploymentByEnvironmentId() { assertEquals(deploymentDto, result.get()); } + @Test + public void getDeploymentsByEnvironmentId_forEnvironmentInAnotherRepository_returnsEmpty() { + // Env id from another repo → gate fails → empty (no cross-repo deployment leak). + when(environmentRepository.findByIdAndRepositoryRepositoryId(99L, 1L)) + .thenReturn(Optional.empty()); + + assertTrue(deploymentService.getDeploymentsByEnvironmentId(99L).isEmpty()); + } + @Test public void testDeployWithInvalidDeployRequest() { DeployRequest deployRequest = mock(DeployRequest.class); @@ -331,6 +346,8 @@ public void testGetActivityHistoryByEnvironmentId() { lockHistory.setLockedAt(now.minusMinutes(3)); lockHistory.setUnlockedAt(now); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(deploymentRepository.findByEnvironmentIdOrderByCreatedAtDesc(1L)) .thenReturn(List.of(deployment)); when(lockHistoryRepository.findLockHistoriesByEnvironment(1L)).thenReturn(List.of(lockHistory)); @@ -360,7 +377,8 @@ public void testGetActivityHistoryByPullRequestId() { heliosDeployment.setEnvironment(environment); deployment.setCreatedAt(now.minusMinutes(2)); - when(pullRequestRepository.findById(1L)).thenReturn(Optional.of(mock())); + when(pullRequestRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(mock())); when(deploymentRepository.findByPullRequest_IdOrderByCreatedAtDesc(1L)) .thenReturn(List.of(deployment)); when(heliosDeploymentRepository @@ -442,7 +460,8 @@ public void testGetWorkflowJobStatusUsesWorkflowRunRepository() throws IOExcepti workflowRun.setId(42L); workflowRun.setRepository(gitRepository); - when(workflowRunRepository.findById(42L)).thenReturn(Optional.of(workflowRun)); + when(workflowRunRepository.findByIdAndRepositoryRepositoryId(42L, 1L)) + .thenReturn(Optional.of(workflowRun)); when(gitHubService.getWorkflowJobStatus("owner/repo", 42L)) .thenReturn("{\"total_count\":0,\"jobs\":[]}"); @@ -457,12 +476,13 @@ public void testGetWorkflowJobStatusUsesWorkflowRunRepository() throws IOExcepti @Test public void testGetWorkflowJobStatusFallsBackToHeliosDeploymentRepository() throws IOException { GitRepository otherRepository = new GitRepository(); - otherRepository.setRepositoryId(2L); + otherRepository.setRepositoryId(1L); otherRepository.setNameWithOwner("other/repo"); environment.setRepository(otherRepository); heliosDeployment.setWorkflowRunId(43L); - when(workflowRunRepository.findById(43L)).thenReturn(Optional.empty()); + when(workflowRunRepository.findByIdAndRepositoryRepositoryId(43L, 1L)) + .thenReturn(Optional.empty()); when(heliosDeploymentRepository.findByWorkflowRunId(43L)) .thenReturn(Optional.of(heliosDeployment)); when(gitHubService.getWorkflowJobStatus("other/repo", 43L)) diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/environment/EnvironmentScopingIT.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/environment/EnvironmentScopingIT.java new file mode 100644 index 000000000..37aa34de6 --- /dev/null +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/environment/EnvironmentScopingIT.java @@ -0,0 +1,112 @@ +package de.tum.cit.aet.helios.environment; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.everyItem; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import de.tum.cit.aet.helios.HeliosIntegrationTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.jdbc.core.JdbcTemplate; + +/** + * Cross-refactor guard: {@code GET /api/environments*} must return only the current repository's + * environments (scoped by the {@code X-REPOSITORY-ID} header). This passes with the legacy + * Hibernate {@code gitRepositoryFilter} and must keep passing after the switch to explicit + * per-query filtering — it exercises the real request path (interceptor → OSIV → repository). + */ +class EnvironmentScopingIT extends HeliosIntegrationTest { + + private static final long REPO_A = 1L; + private static final long REPO_B = 2L; + private static final long ENV_A1 = 11L; + private static final long ENV_A2 = 12L; + private static final long ENV_B1 = 21L; + + @BeforeEach + void seed() { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + jdbc.execute("TRUNCATE TABLE repository CASCADE"); + insertRepo(jdbc, REPO_A, "ls1intum/repo-a"); + insertRepo(jdbc, REPO_B, "ls1intum/repo-b"); + insertEnabledEnv(jdbc, ENV_A1, REPO_A, "a-staging"); + insertEnabledEnv(jdbc, ENV_A2, REPO_A, "a-production"); + insertEnabledEnv(jdbc, ENV_B1, REPO_B, "b-production"); + } + + @Test + void enabledEnvironmentsAreScopedToRepositoryA() throws Exception { + mockMvc + .perform(get("/api/environments/enabled").header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(2)) + .andExpect(jsonPath("$[*].repository.id", everyItem(equalTo((int) REPO_A)))); + } + + @Test + void enabledEnvironmentsAreScopedToRepositoryB() throws Exception { + mockMvc + .perform(get("/api/environments/enabled").header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(1)) + .andExpect(jsonPath("$[0].repository.id").value((int) REPO_B)); + } + + @Test + void allEnvironmentsAreScopedToCurrentRepository() throws Exception { + mockMvc + .perform(get("/api/environments").header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(2)) + .andExpect(jsonPath("$[*].repository.id", everyItem(equalTo((int) REPO_A)))); + } + + @Test + void environmentByIdIsInvisibleFromAnotherRepository() throws Exception { + // ENV_B1 belongs to repo B: invisible when scoped to repo A, visible when scoped to repo B. + mockMvc + .perform( + get("/api/environments/{id}", ENV_B1).header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isNotFound()); + mockMvc + .perform( + get("/api/environments/{id}", ENV_B1).header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()); + } + + @Test + void withoutRepositoryContextReturnsEmpty() throws Exception { + // No X-REPOSITORY-ID header → tenant-scoped read has no repository → empty (never findAll). + mockMvc + .perform(get("/api/environments/enabled")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(0)); + mockMvc + .perform(get("/api/environments")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(0)); + // Detail-by-id with no header must not fall back to an unscoped findById (anonymous IDOR): + // GET /api/** is permitAll, so a missing header must yield 404, not another repo's environment. + mockMvc.perform(get("/api/environments/{id}", ENV_A1)).andExpect(status().isNotFound()); + } + + private static void insertRepo(JdbcTemplate jdbc, long id, String nameWithOwner) { + jdbc.update( + "INSERT INTO repository (repository_id, has_issues, has_projects, has_wiki, is_archived, " + + "is_disabled, is_private, stargazers_count, watchers_count, name_with_owner) " + + "VALUES (?, false, false, false, false, false, false, 0, 0, ?)", + id, + nameWithOwner); + } + + private static void insertEnabledEnv(JdbcTemplate jdbc, long id, long repositoryId, String name) { + jdbc.update( + "INSERT INTO environment (id, repository_id, enabled, locked, name) " + + "VALUES (?, ?, true, false, ?)", + id, + repositoryId, + name); + } +} diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/environment/EnvironmentServiceTest.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/environment/EnvironmentServiceTest.java index d662f2a54..0f667e3a6 100644 --- a/server/application-server/src/test/java/de/tum/cit/aet/helios/environment/EnvironmentServiceTest.java +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/environment/EnvironmentServiceTest.java @@ -36,6 +36,7 @@ import java.time.OffsetDateTime; import java.util.List; import java.util.Optional; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -73,6 +74,8 @@ public class EnvironmentServiceTest { @BeforeEach public void setUp() { + RepositoryContext.setRepositoryId("1"); + gitRepository = new GitRepository(); gitRepository.setRepositoryId(1L); @@ -86,30 +89,59 @@ public void setUp() { user.setId(1L); } + @AfterEach + public void tearDown() { + RepositoryContext.clear(); + } + @Test public void testGetEnvironmentById() { - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); Optional result = environmentService.getEnvironmentById(1L); assertTrue(result.isPresent()); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); + } + + @Test + public void getLockHistoryByEnvironmentId_forEnvironmentInAnotherRepository_returnsEmpty() { + // Env id from another repo → gate fails → empty (lock history is keyed only by env id). + when(environmentRepository.findByIdAndRepositoryRepositoryId(99L, 1L)) + .thenReturn(Optional.empty()); + + assertTrue(environmentService.getLockHistoryByEnvironmentId(99L).isEmpty()); + verify(lockHistoryRepository, never()).findLockHistoriesByEnvironment(99L); + } + + @Test + public void lockEnvironment_forEnvironmentInAnotherRepository_throwsNotFound() { + // Cross-repo write must not reach a foreign environment by id. + when(environmentRepository.findByIdAndRepositoryRepositoryId(99L, 1L)) + .thenReturn(Optional.empty()); + + assertThrows( + jakarta.persistence.EntityNotFoundException.class, + () -> environmentService.lockEnvironment(99L)); } @Test public void testGetEnvironmentTypeById() { - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); Optional result = environmentService.getEnvironmentTypeById(1L); assertTrue(result.isPresent()); assertEquals(Environment.Type.TEST, result.get()); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); } @Test public void testGetAllEnvironments() { - when(environmentRepository.findAllByOrderByNameAsc()) + RepositoryContext.setRepositoryId("1"); + when(environmentRepository.findByRepositoryRepositoryIdOrderByNameAsc(1L)) .thenReturn(List.of(environment, environment)); List result = environmentService.getAllEnvironments(); @@ -118,12 +150,13 @@ public void testGetAllEnvironments() { assertEquals(2, result.size()); assertEquals(List.of(dto, dto), result); - verify(environmentRepository, times(1)).findAllByOrderByNameAsc(); + verify(environmentRepository, times(1)).findByRepositoryRepositoryIdOrderByNameAsc(1L); } @Test public void testGetAllEnabledEnvironments() { - when(environmentRepository.findByEnabledTrueOrderByNameAsc()) + RepositoryContext.setRepositoryId("1"); + when(environmentRepository.findByEnabledTrueAndRepositoryRepositoryIdOrderByNameAsc(1L)) .thenReturn(List.of(environment, environment)); List result = environmentService.getAllEnabledEnvironments(); @@ -132,13 +165,15 @@ public void testGetAllEnabledEnvironments() { assertEquals(2, result.size()); assertEquals(List.of(dto, dto), result); - verify(environmentRepository, times(1)).findByEnabledTrueOrderByNameAsc(); + verify(environmentRepository, times(1)) + .findByEnabledTrueAndRepositoryRepositoryIdOrderByNameAsc(1L); } @Test public void testLockEnvironment() { when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(lockHistoryRepository.saveAndFlush(any(EnvironmentLockHistory.class))) .thenReturn(new EnvironmentLockHistory()); when(environmentRepository.save(any(Environment.class))).thenReturn(environment); @@ -147,7 +182,7 @@ public void testLockEnvironment() { assertTrue(result.isPresent()); assertTrue(result.get().isLocked()); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); verify(environmentRepository, times(1)).save(any(Environment.class)); } @@ -158,7 +193,8 @@ public void testLockEnvironmentAlreadyLocked() { environment.setLockedBy(user); when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); Optional result = environmentService.lockEnvironment(1L); @@ -166,7 +202,7 @@ public void testLockEnvironmentAlreadyLocked() { assertTrue(result.get().isLocked()); // Should not modify lockedAt assertTrue(result.get().getLockedAt().isEqual(environment.getLockedAt())); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); } @Test @@ -177,7 +213,8 @@ public void testLockEnvironmentAlreadyLockedByOtherUser() { environment.setLockedBy(otherUser); when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); Exception exception = assertThrows( @@ -187,7 +224,7 @@ public void testLockEnvironmentAlreadyLockedByOtherUser() { }); assertTrue(exception.getMessage().contains("Environment is locked by another user")); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); } @Test @@ -202,7 +239,8 @@ public void testExtendEnvironmentLock() { environment.setLockReservationThreshold(lockReservationExpirationThreshold); when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(environmentRepository.save(any(Environment.class))).thenReturn(environment); Optional result = environmentService.extendEnvironmentLock(1L); @@ -218,14 +256,15 @@ public void testExtendEnvironmentLock() { .get() .getLockReservationExpiresAt() .isAfter(dateTimeNow.plusMinutes(lockReservationExpirationThreshold))); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); verify(environmentRepository, times(1)).save(any(Environment.class)); } @Test public void testExtendEnvironmentLockNotLockedFails() { when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); Exception exception = assertThrows( @@ -235,7 +274,7 @@ public void testExtendEnvironmentLockNotLockedFails() { }); assertEquals("Environment is not locked. Cannot extend lock.", exception.getMessage()); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); } @Test @@ -243,7 +282,8 @@ public void testExtendProductionEnvironmentLockFails() { environment.setType(Environment.Type.PRODUCTION); when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); Exception exception = assertThrows( @@ -253,7 +293,7 @@ public void testExtendProductionEnvironmentLockFails() { }); assertEquals("Only TEST environments can have their locks extended", exception.getMessage()); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); } @Test @@ -261,7 +301,8 @@ public void testExtendDisabledEnvironmentLockFails() { environment.setEnabled(false); when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); Exception exception = assertThrows( @@ -271,7 +312,7 @@ public void testExtendDisabledEnvironmentLockFails() { }); assertEquals("Environment is disabled", exception.getMessage()); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); } @Test @@ -281,7 +322,8 @@ public void testUnlockEnvironment() { environment.setLockedAt(OffsetDateTime.now()); when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(environmentRepository.save(any(Environment.class))).thenReturn(environment); EnvironmentDto result = environmentService.unlockEnvironment(1L); @@ -293,7 +335,7 @@ public void testUnlockEnvironment() { assertNull(result.lockedAt()); assertNull(result.lockWillExpireAt()); assertNull(result.lockReservationWillExpireAt()); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); verify(environmentRepository, times(1)).save(any(Environment.class)); } @@ -306,7 +348,8 @@ public void testLockOwnerCanNotUnlockWithActiveDeployment() { createHeliosDeployment(HeliosDeployment.Status.IN_PROGRESS, OffsetDateTime.now()); when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(heliosDeploymentRepository.findTopByEnvironmentOrderByCreatedAtDesc(environment)) .thenReturn(Optional.of(deployment)); @@ -330,7 +373,8 @@ public void testMaintainerCanNotUnlockWithActiveDeploymentBeforeTimeout() { when(authService.getUserFromGithubId()).thenReturn(user); when(authService.isAtLeastMaintainer()).thenReturn(true); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(heliosDeploymentRepository.findTopByEnvironmentOrderByCreatedAtDesc(environment)) .thenReturn(Optional.of(deployment)); @@ -355,7 +399,8 @@ public void testMaintainerCanUnlockWithStaleActiveDeployment() { when(authService.getUserFromGithubId()).thenReturn(user); when(authService.isAtLeastMaintainer()).thenReturn(true); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(environmentRepository.save(any(Environment.class))).thenReturn(environment); when(heliosDeploymentRepository.findTopByEnvironmentOrderByCreatedAtDesc(environment)) .thenReturn(Optional.of(deployment)); @@ -379,7 +424,8 @@ public void testLockOwnerCanUnlockWithStaleActiveDeployment() { HeliosDeployment.Status.IN_PROGRESS, OffsetDateTime.now().minusMinutes(21)); when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(environmentRepository.save(any(Environment.class))).thenReturn(environment); when(heliosDeploymentRepository.findTopByEnvironmentOrderByCreatedAtDesc(environment)) .thenReturn(Optional.of(deployment)); @@ -408,7 +454,8 @@ public void testUnlockEnvironmentWithTerminalDeployments() { HeliosDeployment deployment = createHeliosDeployment(status, OffsetDateTime.now()); when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(environmentRepository.save(any(Environment.class))).thenReturn(environment); when(heliosDeploymentRepository.findTopByEnvironmentOrderByCreatedAtDesc(environment)) .thenReturn(Optional.of(deployment)); @@ -424,7 +471,8 @@ public void testUnlockEnvironmentWithTerminalDeployments() { @Test public void testUnlockEnvironmentNotLocked() { when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); Exception exception = assertThrows( @@ -434,7 +482,7 @@ public void testUnlockEnvironmentNotLocked() { }); assertEquals("Environment is not locked", exception.getMessage()); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); } @Test @@ -446,7 +494,8 @@ public void testMaintainerCanUnlockOtherUsersLock() { when(authService.isAtLeastMaintainer()).thenReturn(true); when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); EnvironmentDto result = environmentService.unlockEnvironment(1L); @@ -457,7 +506,7 @@ public void testMaintainerCanUnlockOtherUsersLock() { assertNull(result.lockedAt()); assertNull(result.lockWillExpireAt()); assertNull(result.lockReservationWillExpireAt()); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); verify(environmentRepository, times(1)).save(any(Environment.class)); } @@ -470,7 +519,8 @@ public void testUserCanNotUnlockOtherUsersLock() { when(authService.isAtLeastMaintainer()).thenReturn(false); when(authService.getUserFromGithubId()).thenReturn(user); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); Exception exception = assertThrows( @@ -481,7 +531,7 @@ public void testUserCanNotUnlockOtherUsersLock() { assertTrue( exception.getMessage().contains("You do not have permission to unlock this environment")); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); } @Test @@ -505,7 +555,8 @@ public void testUpdateLockExpirationAndReservation() { public void testSetLockedEnvironmentAsDisabled() { environment.setLocked(true); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); environment.setEnabled(false); @@ -517,7 +568,7 @@ public void testSetLockedEnvironmentAsDisabled() { }); assertTrue(exception.getMessage().contains("Environment is locked and can not be disabled")); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); } @Test @@ -527,7 +578,8 @@ public void testUpdateEnvironment() { wf.setRepository(gitRepository); final Long threshold = 10L; - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(workflowRepository.findById(1L)).thenReturn(Optional.of(wf)); environment.setEnabled(false); @@ -554,7 +606,7 @@ public void testUpdateEnvironment() { assertEquals(threshold, environmentDto.get().lockReservationThreshold()); assertFalse(environmentDto.get().enabled()); - verify(environmentRepository, times(1)).findById(1L); + verify(environmentRepository, times(1)).findByIdAndRepositoryRepositoryId(1L, 1L); verify(workflowRepository, times(1)).findById(1L); } @@ -591,7 +643,8 @@ public void testUpdateEnvironmentWithRequiredPreDeploymentWorkflows() { requiredWorkflow.setName("Build"); requiredWorkflow.setRepository(gitRepository); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(workflowRepository.findById(1L)).thenReturn(Optional.of(deploymentWorkflow)); when(workflowRepository.findById(2L)).thenReturn(Optional.of(requiredWorkflow)); @@ -615,7 +668,8 @@ public void testUpdateEnvironmentRejectsDeploymentWorkflowAsRequiredPreDeploymen deploymentWorkflow.setName("Deploy"); deploymentWorkflow.setRepository(gitRepository); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when(workflowRepository.findById(1L)).thenReturn(Optional.of(deploymentWorkflow)); environment.setDeploymentWorkflow(deploymentWorkflow); @@ -646,7 +700,8 @@ public void testGetDeploymentReadinessReturnsReadyForSuccessfulRequiredWorkflow( workflowRun.setStatus(WorkflowRun.Status.COMPLETED); workflowRun.setConclusion(Optional.of(WorkflowRun.Conclusion.SUCCESS)); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when( workflowRunRepository .findLatestForDeploymentReadiness( @@ -676,7 +731,8 @@ public void testGetDeploymentReadinessReturnsWaitingForRunningRequiredWorkflow() workflowRun.setHtmlUrl("https://example.com/run/100"); workflowRun.setStatus(WorkflowRun.Status.IN_PROGRESS); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when( workflowRunRepository .findLatestForDeploymentReadiness( @@ -700,7 +756,8 @@ public void testGetDeploymentReadinessReturnsMissingRunWhenNoMatchingWorkflowRun requiredWorkflow.setRepository(gitRepository); environment.setRequiredPreDeploymentWorkflows(List.of(requiredWorkflow)); - when(environmentRepository.findById(1L)).thenReturn(Optional.of(environment)); + when(environmentRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(environment)); when( workflowRunRepository .findLatestForDeploymentReadiness( diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/filters/RepositoryTenantFilterIntegrationTest.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/filters/RepositoryTenantFilterIntegrationTest.java deleted file mode 100644 index e9cf56d54..000000000 --- a/server/application-server/src/test/java/de/tum/cit/aet/helios/filters/RepositoryTenantFilterIntegrationTest.java +++ /dev/null @@ -1,149 +0,0 @@ -package de.tum.cit.aet.helios.filters; - -import static org.assertj.core.api.Assertions.assertThat; - -import de.tum.cit.aet.helios.environment.Environment; -import de.tum.cit.aet.helios.environment.EnvironmentRepository; -import io.zonky.test.db.AutoConfigureEmbeddedDatabase; -import jakarta.persistence.EntityManagerFactory; -import java.util.List; -import javax.sql.DataSource; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.data.jpa.test.autoconfigure.DataJpaTest; -import org.springframework.boot.jdbc.test.autoconfigure.AutoConfigureTestDatabase; -import org.springframework.boot.test.context.TestConfiguration; -import org.springframework.cache.CacheManager; -import org.springframework.cache.concurrent.ConcurrentMapCacheManager; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Import; -import org.springframework.jdbc.core.JdbcTemplate; -import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.web.context.request.ServletWebRequest; - -/** - * Verifies Helios's per-repository tenant isolation: the Hibernate {@code gitRepositoryFilter} - * must actually scope a web-request query to the current repository. - * - *

This guards the regression that shipped in 1.7.0 and was fixed in 1.7.2: after the - * {@code RepositoryFilterAspect -> RepositoryFilterTransactionConfig} refactor, the filter was no - * longer enabled for web reads (under Open-Session-In-View the request transaction reuses the - * pre-opened EntityManager, so the transaction-manager initializer never fired), and every tenant - * query leaked across all repositories. {@link RepositoryInterceptor} now enables the filter on - * the request's EntityManager; this test drives that exact path against a real embedded Postgres - * and the real Flyway schema, so a future change that stops the filter from being enabled — or - * breaks the {@code @Filter} definition — fails here instead of silently leaking data. - * - *

Scope note: the servlet-level ordering that guarantees the EntityManager is bound before the - * interceptor runs (i.e. that OSIV runs first) is a wiring concern verified in staging/prod, not - * reproduced here; this test binds the EntityManager via the surrounding {@code @DataJpaTest} - * transaction, exactly as OSIV would at request time. - */ -@DataJpaTest(properties = {"spring.flyway.enabled=true", "spring.jpa.hibernate.ddl-auto=none"}) -@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) -@AutoConfigureEmbeddedDatabase( - type = AutoConfigureEmbeddedDatabase.DatabaseType.POSTGRES, - provider = AutoConfigureEmbeddedDatabase.DatabaseProvider.DOCKER) -@Import(RepositoryTenantFilterIntegrationTest.CacheTestConfig.class) -class RepositoryTenantFilterIntegrationTest { - - /** The @DataJpaTest slice does not load cache auto-configuration; provide a simple one. */ - @TestConfiguration - static class CacheTestConfig { - @Bean - CacheManager cacheManager() { - return new ConcurrentMapCacheManager(); - } - } - - private static final long REPO_A = 1L; - private static final long REPO_B = 2L; - - @Autowired private EnvironmentRepository environmentRepository; - @Autowired private EntityManagerFactory entityManagerFactory; - @Autowired private DataSource dataSource; - - private RepositoryInterceptor interceptor; - - @BeforeEach - void seed() { - JdbcTemplate jdbc = new JdbcTemplate(dataSource); - insertRepository(jdbc, REPO_A, "ls1intum/repo-a"); - insertRepository(jdbc, REPO_B, "ls1intum/repo-b"); - // Two enabled environments in repo A, one in repo B. - insertEnabledEnvironment(jdbc, 1L, REPO_A, "repo-a-staging"); - insertEnabledEnvironment(jdbc, 2L, REPO_A, "repo-a-production"); - insertEnabledEnvironment(jdbc, 3L, REPO_B, "repo-b-production"); - interceptor = new RepositoryInterceptor(entityManagerFactory); - } - - @AfterEach - void clearContext() { - RepositoryContext.clear(); - } - - @Test - void scopesEnabledEnvironmentsToRepositoryA() { - enterWebRequestForRepository(REPO_A); - - List environments = environmentRepository.findByEnabledTrueOrderByNameAsc(); - - assertThat(environments) - .extracting(env -> env.getRepository().getRepositoryId()) - .as("only repo A's environments must be visible while scoped to repo A") - .containsExactly(REPO_A, REPO_A); - } - - @Test - void scopesEnabledEnvironmentsToRepositoryB() { - enterWebRequestForRepository(REPO_B); - - List environments = environmentRepository.findByEnabledTrueOrderByNameAsc(); - - assertThat(environments) - .extracting(env -> env.getRepository().getRepositoryId()) - .as("only repo B's environment must be visible while scoped to repo B") - .containsExactly(REPO_B); - } - - @Test - void withoutRepositoryContextTheFilterIsNotEnabled() { - // No X-REPOSITORY-ID header -> no repository context -> filter stays off, matching how - // global (non-repo-scoped) requests behave. Proves the filter — not some other mechanism — is - // what scopes the tenant queries; if this returned only one repo, scoping would be an - // accident of the data rather than the filter. - List environments = environmentRepository.findByEnabledTrueOrderByNameAsc(); - - assertThat(environments) - .extracting(env -> env.getRepository().getRepositoryId()) - .containsExactlyInAnyOrder(REPO_A, REPO_A, REPO_B); - } - - /** Reproduces what {@link RepositoryInterceptor} does at the start of a web request. */ - private void enterWebRequestForRepository(long repositoryId) { - MockHttpServletRequest request = new MockHttpServletRequest(); - request.addHeader(RepositoryInterceptor.X_REPOSITORY_ID, String.valueOf(repositoryId)); - interceptor.preHandle(new ServletWebRequest(request)); - } - - private static void insertRepository(JdbcTemplate jdbc, long id, String nameWithOwner) { - jdbc.update( - "INSERT INTO repository (repository_id, has_issues, has_projects, has_wiki, is_archived, " - + "is_disabled, is_private, stargazers_count, watchers_count, name_with_owner) " - + "VALUES (?, false, false, false, false, false, false, 0, 0, ?)", - id, - nameWithOwner); - } - - private static void insertEnabledEnvironment( - JdbcTemplate jdbc, long id, long repositoryId, String name) { - jdbc.update( - "INSERT INTO environment (id, repository_id, enabled, locked, name) " - + "VALUES (?, ?, true, false, ?)", - id, - repositoryId, - name); - } -} diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsIT.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsIT.java new file mode 100644 index 000000000..f075bd272 --- /dev/null +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsIT.java @@ -0,0 +1,67 @@ +package de.tum.cit.aet.helios.gitreposettings; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import de.tum.cit.aet.helios.HeliosIntegrationTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.jdbc.core.JdbcTemplate; + +/** + * Guards the write-on-GET fix: {@code GET /api/settings/{id}/settings} must be a pure read. For a + * repository with no settings row it returns transient defaults and persists nothing. Previously a + * GET INSERTed a row as a side effect, and a plain GET must never write. + */ +class GitRepoSettingsIT extends HeliosIntegrationTest { + + private static final long REPO = 1L; + + @BeforeEach + void seed() { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + jdbc.execute("TRUNCATE TABLE repository CASCADE"); + insertRepo(jdbc, REPO, "ls1intum/repo-a"); + } + + @Test + void getSettingsReturnsDefaultsAndDoesNotPersistARow() throws Exception { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + assertSettingsRowCount(jdbc, 0); + + mockMvc + .perform(get("/api/settings/{id}/settings", REPO)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.lockExpirationThreshold").value(60)) + .andExpect(jsonPath("$.lockReservationThreshold").value(30)); + + // The GET must not have created a settings row (write-on-GET regression). + assertSettingsRowCount(jdbc, 0); + + // A second GET is still a pure read. + mockMvc.perform(get("/api/settings/{id}/settings", REPO)).andExpect(status().isOk()); + assertSettingsRowCount(jdbc, 0); + } + + private static void assertSettingsRowCount(JdbcTemplate jdbc, int expected) { + Integer count = + jdbc.queryForObject( + "SELECT count(*) FROM repository_settings WHERE repository_id = ?", + Integer.class, + REPO); + if (count == null || count != expected) { + throw new AssertionError( + "expected " + expected + " repository_settings rows but found " + count); + } + } + + private static void insertRepo(JdbcTemplate jdbc, long id, String nameWithOwner) { + jdbc.update( + "INSERT INTO repository (repository_id, has_issues, has_projects, has_wiki, is_archived, " + + "is_disabled, is_private, stargazers_count, watchers_count, name_with_owner) " + + "VALUES (?, false, false, false, false, false, false, 0, 0, ?)", + id, + nameWithOwner); + } +} diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsServiceTest.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsServiceTest.java index 00cbe0489..553c8006f 100644 --- a/server/application-server/src/test/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsServiceTest.java +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/gitreposettings/GitRepoSettingsServiceTest.java @@ -112,6 +112,18 @@ public void testCreateNewGitRepoSettingsRepositoryNotFound() { verify(gitRepository, times(1)).findByRepositoryId(1L); } + @Test + public void getGitRepoSettingsByRepositoryId_returnsDefaultsWithoutWriting() { + when(gitRepoRepository.findByRepositoryRepositoryId(1L)).thenReturn(Optional.empty()); + when(gitRepository.findByRepositoryId(1L)).thenReturn(Optional.of(testGitRepository)); + + GitRepoSettingsDto result = gitRepoSettingsService.getGitRepoSettingsByRepositoryId(1L); + + assertNotNull(result); + // A plain GET must not persist a settings row. + verify(gitRepoRepository, times(0)).save(any(GitRepoSettings.class)); + } + @Test public void testUpdateGitRepoSettings() { when(gitRepoRepository.findByRepositoryRepositoryId(1L)) @@ -132,6 +144,26 @@ public void testUpdateGitRepoSettings() { verify(environmentService, times(1)).updateLockExpirationAndReservation(1L); } + @Test + public void updateGitRepoSettings_createsRowWhenAbsent() { + // GET /settings no longer persists a row, so Save must create-on-absent instead of 400ing. + when(gitRepoRepository.findByRepositoryRepositoryId(1L)).thenReturn(Optional.empty()); + when(gitRepository.findByRepositoryId(1L)).thenReturn(Optional.of(testGitRepository)); + when(gitRepoRepository.save(any(GitRepoSettings.class))) + .thenAnswer(invocation -> invocation.getArgument(0)); + + GitRepoSettingsDto updateDto = new GitRepoSettingsDto(null, 120L, 60L, "com.example.created"); + + Optional result = + gitRepoSettingsService.updateGitRepoSettings(1L, updateDto); + + assertTrue(result.isPresent()); + assertEquals(120L, result.get().lockExpirationThreshold()); + assertEquals("com.example.created", result.get().packageName()); + verify(gitRepoRepository, times(1)).save(any(GitRepoSettings.class)); + verify(environmentService, times(1)).updateLockExpirationAndReservation(1L); + } + @Test public void testUpdateGitRepoSettingsNotFound() { when(gitRepoRepository.findByRepositoryRepositoryId(1L)).thenReturn(Optional.empty()); diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/pullrequest/PullRequestScopingIT.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/pullrequest/PullRequestScopingIT.java new file mode 100644 index 000000000..d7809d6ec --- /dev/null +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/pullrequest/PullRequestScopingIT.java @@ -0,0 +1,131 @@ +package de.tum.cit.aet.helios.pullrequest; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.everyItem; +import static org.hamcrest.Matchers.hasItems; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import de.tum.cit.aet.helios.HeliosIntegrationTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.jdbc.core.JdbcTemplate; + +/** + * Cross-repository isolation guard for {@code /api/pullrequests*}. Detail-by-id scopes explicitly; + * with {@code GET /api/**} unauthenticated, a missing {@code X-REPOSITORY-ID} must 404, not leak. + */ +class PullRequestScopingIT extends HeliosIntegrationTest { + + private static final long REPO_A = 1L; + private static final long REPO_B = 2L; + private static final long PR_A1 = 31L; + private static final long PR_A2 = 32L; + private static final long PR_B1 = 41L; + + @BeforeEach + void seed() { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + jdbc.execute("TRUNCATE TABLE repository CASCADE"); + insertRepo(jdbc, REPO_A, "ls1intum/repo-a"); + insertRepo(jdbc, REPO_B, "ls1intum/repo-b"); + insertPr(jdbc, PR_A1, REPO_A, 1, "A-first"); + insertPr(jdbc, PR_A2, REPO_A, 2, "A-second"); + insertPr(jdbc, PR_B1, REPO_B, 1, "B-first"); + } + + @Test + void pullRequestByIdIsInvisibleFromAnotherRepository() throws Exception { + // PR_B1 belongs to repo B. + mockMvc + .perform( + get("/api/pullrequests/{id}", PR_B1).header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.id").value((int) PR_B1)) + .andExpect(jsonPath("$.repository.id").value((int) REPO_B)); + mockMvc + .perform( + get("/api/pullrequests/{id}", PR_B1).header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isNotFound()); + } + + @Test + void pullRequestByIdWithoutHeaderIsNotFound() throws Exception { + // No header → null context → must not fall back to an unscoped findById (anonymous IDOR). + mockMvc.perform(get("/api/pullrequests/{id}", PR_A1)).andExpect(status().isNotFound()); + } + + @Test + void pullRequestsByRepositoryIdReturnOnlyThatRepository() throws Exception { + mockMvc + .perform(get("/api/pullrequests/repository/{id}", REPO_A)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(2)) + .andExpect(jsonPath("$[*].repository.id", everyItem(equalTo((int) REPO_A)))); + mockMvc + .perform(get("/api/pullrequests/repository/{id}", REPO_B)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(1)) + .andExpect(jsonPath("$[0].id").value((int) PR_B1)) + .andExpect(jsonPath("$[0].repository.id").value((int) REPO_B)); + } + + @Test + void paginatedPullRequestsAreScopedToCurrentRepository() throws Exception { + mockMvc + .perform( + get("/api/pullrequests") + .param("page", "1") + .param("size", "50") + .header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.totalNonPinned").value(2)) + .andExpect(jsonPath("$.page.length()").value(2)) + .andExpect(jsonPath("$.page[*].id", hasItems((int) PR_A1, (int) PR_A2))); + mockMvc + .perform( + get("/api/pullrequests") + .param("page", "1") + .param("size", "50") + .header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.totalNonPinned").value(1)) + .andExpect(jsonPath("$.page.length()").value(1)) + .andExpect(jsonPath("$.page[0].id").value((int) PR_B1)); + } + + @Test + void paginatedPullRequestsWithoutHeaderAreEmpty() throws Exception { + mockMvc + .perform(get("/api/pullrequests").param("page", "1").param("size", "50")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.totalNonPinned").value(0)) + .andExpect(jsonPath("$.page.length()").value(0)); + } + + private static void insertRepo(JdbcTemplate jdbc, long id, String nameWithOwner) { + jdbc.update( + "INSERT INTO repository (repository_id, has_issues, has_projects, has_wiki, is_archived, " + + "is_disabled, is_private, stargazers_count, watchers_count, name_with_owner) " + + "VALUES (?, false, false, false, false, false, false, 0, 0, ?)", + id, + nameWithOwner); + } + + private static void insertPr(JdbcTemplate jdbc, long id, long repositoryId, int number, + String title) { + jdbc.update( + "INSERT INTO issue (id, repository_id, issue_type, number, additions, deletions, " + + "changed_files, commits, comments_count, is_locked, is_draft, is_merged, " + + "is_mergeable, maintainer_can_modify, state, title, html_url, created_at, " + + "updated_at) VALUES " + + "(?, ?, 'PULL_REQUEST', ?, 0, 0, 0, 0, 0, false, false, false, false, false, " + + "'OPEN', ?, ?, now(), now())", + id, + repositoryId, + number, + title, + "https://example/pr/" + id); + } +} diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/pullrequest/PullRequestServiceTest.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/pullrequest/PullRequestServiceTest.java index e3d4a3c8e..f1eaa11d2 100644 --- a/server/application-server/src/test/java/de/tum/cit/aet/helios/pullrequest/PullRequestServiceTest.java +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/pullrequest/PullRequestServiceTest.java @@ -8,6 +8,7 @@ import static org.mockito.Mockito.when; import de.tum.cit.aet.helios.auth.AuthService; +import de.tum.cit.aet.helios.filters.RepositoryContext; import de.tum.cit.aet.helios.gitrepo.GitRepository; import de.tum.cit.aet.helios.gitrepo.GitRepository.Visibility; import de.tum.cit.aet.helios.label.Label; @@ -19,6 +20,8 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; @@ -39,6 +42,16 @@ public class PullRequestServiceTest { @Mock private UserPreferenceRepository userPreferenceRepository; @Mock private AuthService authService; + @BeforeEach + public void setUp() { + RepositoryContext.setRepositoryId("1"); + } + + @AfterEach + public void tearDown() { + RepositoryContext.clear(); + } + @Test public void testPinnedBranchesAreShownFirst() { final GitRepository repo = new GitRepository(); @@ -59,7 +72,8 @@ public void testPinnedBranchesAreShownFirst() { final UserPreference userPreference = new UserPreference(); userPreference.setFavouritePullRequests(Set.of(pr2)); - when(pullRequestsRepository.findAllByOrderByUpdatedAtDesc()).thenReturn(prs); + when(pullRequestsRepository.findByRepositoryRepositoryIdOrderByUpdatedAtDesc(1L)) + .thenReturn(prs); when(authService.isLoggedIn()).thenReturn(true); when(authService.getUserFromGithubId()).thenReturn(null); when(userPreferenceRepository.findByUser(null)).thenReturn(Optional.of(userPreference)); diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/releaseinfo/ReleaseInfoScopingIT.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/releaseinfo/ReleaseInfoScopingIT.java new file mode 100644 index 000000000..d74eed30c --- /dev/null +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/releaseinfo/ReleaseInfoScopingIT.java @@ -0,0 +1,86 @@ +package de.tum.cit.aet.helios.releaseinfo; + +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import de.tum.cit.aet.helios.HeliosIntegrationTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.jdbc.core.JdbcTemplate; + +/** + * Cross-repository isolation guard for {@code GET /api/release-info}. Release candidates are scoped + * by the current repository; with {@code GET /api/**} unauthenticated, a missing + * {@code X-REPOSITORY-ID} must return an empty list (never a cross-repo or unscoped read). + */ +class ReleaseInfoScopingIT extends HeliosIntegrationTest { + + private static final long REPO_A = 1L; + private static final long REPO_B = 2L; + + @BeforeEach + void seed() { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + jdbc.execute("TRUNCATE TABLE repository CASCADE"); + insertRepo(jdbc, REPO_A, "ls1intum/repo-a"); + insertRepo(jdbc, REPO_B, "ls1intum/repo-b"); + insertCommit(jdbc, "shaA", REPO_A); + insertCommit(jdbc, "shaB", REPO_B); + insertReleaseCandidate(jdbc, 81L, REPO_A, "rc-a1", "shaA"); + insertReleaseCandidate(jdbc, 82L, REPO_A, "rc-a2", "shaA"); + insertReleaseCandidate(jdbc, 91L, REPO_B, "rc-b1", "shaB"); + } + + @Test + void releaseInfosAreScopedToRepositoryA() throws Exception { + mockMvc + .perform(get("/api/release-info").header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(2)) + .andExpect(jsonPath("$[*].name", containsInAnyOrder("rc-a1", "rc-a2"))); + } + + @Test + void releaseInfosAreScopedToRepositoryB() throws Exception { + mockMvc + .perform(get("/api/release-info").header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(1)) + .andExpect(jsonPath("$[0].name").value("rc-b1")); + } + + @Test + void releaseInfosWithoutHeaderAreEmpty() throws Exception { + mockMvc + .perform(get("/api/release-info")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(0)); + } + + private static void insertRepo(JdbcTemplate jdbc, long id, String nameWithOwner) { + jdbc.update( + "INSERT INTO repository (repository_id, has_issues, has_projects, has_wiki, is_archived, " + + "is_disabled, is_private, stargazers_count, watchers_count, name_with_owner) " + + "VALUES (?, false, false, false, false, false, false, 0, 0, ?)", + id, + nameWithOwner); + } + + private static void insertCommit(JdbcTemplate jdbc, String sha, long repositoryId) { + jdbc.update("INSERT INTO commit (sha, repository_id) VALUES (?, ?)", sha, repositoryId); + } + + private static void insertReleaseCandidate( + JdbcTemplate jdbc, long id, long repositoryId, String name, String commitSha) { + jdbc.update( + "INSERT INTO release_candidate (id, repository_id, name, created_at, commit_repository_id, " + + "commit_sha) VALUES (?, ?, ?, now(), ?, ?)", + id, + repositoryId, + name, + repositoryId, + commitSha); + } +} diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/releaseinfo/ReleaseInfoServiceTest.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/releaseinfo/ReleaseInfoServiceTest.java index b70deb381..ceee40cb5 100644 --- a/server/application-server/src/test/java/de/tum/cit/aet/helios/releaseinfo/ReleaseInfoServiceTest.java +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/releaseinfo/ReleaseInfoServiceTest.java @@ -459,7 +459,8 @@ void getAllReleaseInfos_returnsMappedList() { commit2.setSha("dummySha2"); rc2.setCommit(commit2); - when(releaseCandidateRepository.findAllByOrderByCreatedAtDesc()).thenReturn(List.of(rc2, rc1)); + when(releaseCandidateRepository.findByRepositoryRepositoryIdOrderByCreatedAtDesc(repoId)) + .thenReturn(List.of(rc2, rc1)); // Act List result = service.getAllReleaseInfos(); diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/tests/TestResultScopingIT.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/tests/TestResultScopingIT.java new file mode 100644 index 000000000..2d98cb62b --- /dev/null +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/tests/TestResultScopingIT.java @@ -0,0 +1,115 @@ +package de.tum.cit.aet.helios.tests; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import de.tum.cit.aet.helios.HeliosIntegrationTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.jdbc.core.JdbcTemplate; + +/** + * Cross-repository isolation guard for {@code /api/tests/*}. Test results are reached via a pull + * request or workflow-run id; a foreign id must not surface another repository's test results. The + * entry lookups are scoped, so cross-repo and no-header (unauthenticated) requests must 404. + */ +class TestResultScopingIT extends HeliosIntegrationTest { + + private static final long REPO_A = 1L; + private static final long REPO_B = 2L; + private static final long PR_B = 41L; + private static final long WF_B = 52L; + private static final long RUN_B = 71L; + + @BeforeEach + void seed() { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + jdbc.execute("TRUNCATE TABLE repository CASCADE"); + insertRepo(jdbc, REPO_A, "ls1intum/repo-a"); + insertRepo(jdbc, REPO_B, "ls1intum/repo-b"); + insertDefaultBranch(jdbc, REPO_B, "main"); + insertPr(jdbc, PR_B, REPO_B, 1, "b-pr"); + insertWorkflow(jdbc, WF_B, REPO_B); + insertRun(jdbc, RUN_B, REPO_B, WF_B); + } + + @Test + void testResultsForPullRequestAreScopedByRepository() throws Exception { + // Own repo → 200 (well-formed, empty result set — no linked runs). + mockMvc + .perform(get("/api/tests/pr/{id}", PR_B).header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.testResults.length()").value(0)); + // A PR id from another repository must not surface its test results. + mockMvc + .perform(get("/api/tests/pr/{id}", PR_B).header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isNotFound()); + // No header → null context → 404 (anonymous, since GET /api/** is permitAll). + mockMvc.perform(get("/api/tests/pr/{id}", PR_B)).andExpect(status().isNotFound()); + } + + @Test + void testResultsForWorkflowRunAreScopedByRepository() throws Exception { + mockMvc + .perform(get("/api/tests/run/{id}", RUN_B).header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.testResults.length()").value(0)); + mockMvc + .perform(get("/api/tests/run/{id}", RUN_B).header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isNotFound()); + mockMvc.perform(get("/api/tests/run/{id}", RUN_B)).andExpect(status().isNotFound()); + } + + private static void insertRepo(JdbcTemplate jdbc, long id, String nameWithOwner) { + jdbc.update( + "INSERT INTO repository (repository_id, has_issues, has_projects, has_wiki, is_archived, " + + "is_disabled, is_private, stargazers_count, watchers_count, name_with_owner) " + + "VALUES (?, false, false, false, false, false, false, 0, 0, ?)", + id, + nameWithOwner); + } + + private static void insertDefaultBranch(JdbcTemplate jdbc, long repositoryId, String name) { + jdbc.update( + "INSERT INTO branch (name, repository_id, ahead_by, behind_by, is_default, protection) " + + "VALUES (?, ?, 0, 0, true, false)", + name, + repositoryId); + } + + private static void insertPr(JdbcTemplate jdbc, long id, long repositoryId, int number, + String title) { + jdbc.update( + "INSERT INTO issue (id, repository_id, issue_type, number, additions, deletions, " + + "changed_files, commits, comments_count, is_locked, is_draft, is_merged, " + + "is_mergeable, maintainer_can_modify, state, title, html_url, head_sha, created_at, " + + "updated_at) VALUES (?, ?, 'PULL_REQUEST', ?, 0, 0, 0, 0, 0, false, false, false, " + + "false, false, 'OPEN', ?, ?, 'prsha', now(), now())", + id, + repositoryId, + number, + title, + "https://example/pr/" + id); + } + + private static void insertWorkflow(JdbcTemplate jdbc, long id, long repositoryId) { + jdbc.update( + "INSERT INTO workflow (id, repository_id, state, label, name) " + + "VALUES (?, ?, 'ACTIVE', 'NONE', ?)", + id, + repositoryId, + "wf-" + id); + } + + private static void insertRun(JdbcTemplate jdbc, long id, long repositoryId, long workflowId) { + jdbc.update( + "INSERT INTO workflow_run (id, repository_id, workflow_id, run_attempt, run_number, " + + "status, head_branch, head_sha, created_at, updated_at) " + + "VALUES (?, ?, ?, 1, ?, 'COMPLETED', 'main', 'deadbeef', now(), now())", + id, + repositoryId, + workflowId, + id); + } +} diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/tests/TestResultServiceTest.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/tests/TestResultServiceTest.java index fa13dff83..8c295e5e9 100644 --- a/server/application-server/src/test/java/de/tum/cit/aet/helios/tests/TestResultServiceTest.java +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/tests/TestResultServiceTest.java @@ -189,7 +189,8 @@ void getLatestTestResultsForPr_shouldReturnResults() { pullRequest.setHeadSha("prSha"); pullRequest.setRepository(gitRepository); - when(pullRequestRepository.findById(1L)).thenReturn(Optional.of(pullRequest)); + when(pullRequestRepository.findByIdAndRepositoryRepositoryId(1L, 1L)) + .thenReturn(Optional.of(pullRequest)); when(branchRepository.findFirstByRepositoryRepositoryIdAndIsDefaultTrue(anyLong())) .thenReturn(Optional.of(defaultBranch)); when(workflowRunRepository.findByHeadBranchAndHeadShaAndRepositoryRepositoryId( @@ -236,6 +237,20 @@ void getLatestTestResultsForPr_shouldReturnResults() { assertEquals("test1", testTypeResults.testSuites().get(0).testCases().get(0).name()); } + @Test + void getLatestTestResultsForPr_forAnotherRepository_throwsNotFound() { + // A PR id from another repository must not surface its test results (scoped, like the + // run/branch variants). Null/foreign context → not found, not a leak. + when(pullRequestRepository.findByIdAndRepositoryRepositoryId(99L, 1L)) + .thenReturn(java.util.Optional.empty()); + + org.junit.jupiter.api.Assertions.assertThrows( + jakarta.persistence.EntityNotFoundException.class, + () -> + testResultService.getLatestTestResultsForPr( + 99L, new TestResultService.TestSearchCriteria(0, 10, null, false))); + } + @Test void getLatestTestResultsForWorkflowRun_shouldReturnResults() { final TestResultService.TestSearchCriteria criteria = diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/workflow/WorkflowRunScopingIT.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/workflow/WorkflowRunScopingIT.java new file mode 100644 index 000000000..d99b3a65d --- /dev/null +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/workflow/WorkflowRunScopingIT.java @@ -0,0 +1,117 @@ +package de.tum.cit.aet.helios.workflow; + +import static org.hamcrest.Matchers.hasItems; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import de.tum.cit.aet.helios.HeliosIntegrationTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.jdbc.core.JdbcTemplate; + +/** + * Cross-repository isolation guard for {@code /api/workflows/runs*}. Detail-by-id scopes with no + * findById fallback and the paginated query carries a repository predicate; a missing + * {@code X-REPOSITORY-ID} ({@code GET /api/**} is unauthenticated) must 404 / return empty. + */ +class WorkflowRunScopingIT extends HeliosIntegrationTest { + + private static final long REPO_A = 1L; + private static final long REPO_B = 2L; + private static final long WF_A = 51L; + private static final long WF_B = 52L; + private static final long RUN_A1 = 61L; + private static final long RUN_A2 = 62L; + private static final long RUN_B1 = 71L; + + @BeforeEach + void seed() { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + jdbc.execute("TRUNCATE TABLE repository CASCADE"); + insertRepo(jdbc, REPO_A, "ls1intum/repo-a"); + insertRepo(jdbc, REPO_B, "ls1intum/repo-b"); + insertWorkflow(jdbc, WF_A, REPO_A); + insertWorkflow(jdbc, WF_B, REPO_B); + insertRun(jdbc, RUN_A1, REPO_A, WF_A); + insertRun(jdbc, RUN_A2, REPO_A, WF_A); + insertRun(jdbc, RUN_B1, REPO_B, WF_B); + } + + @Test + void workflowRunByIdIsInvisibleFromAnotherRepository() throws Exception { + mockMvc + .perform(get("/api/workflows/runs/{id}", RUN_B1).header(X_REPOSITORY_ID, str(REPO_B))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.id").value((int) RUN_B1)); + mockMvc + .perform(get("/api/workflows/runs/{id}", RUN_B1).header(X_REPOSITORY_ID, str(REPO_A))) + .andExpect(status().isNotFound()); + } + + @Test + void workflowRunByIdWithoutHeaderIsNotFound() throws Exception { + // No header → null context → must not fall back to an unscoped findById (anonymous IDOR). + mockMvc.perform(get("/api/workflows/runs/{id}", RUN_A1)).andExpect(status().isNotFound()); + } + + @Test + void paginatedWorkflowRunsAreScopedToCurrentRepository() throws Exception { + mockMvc + .perform(get("/api/workflows/runs").param("page", "1").param("size", "50") + .header(X_REPOSITORY_ID, str(REPO_A))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.totalElements").value(2)) + .andExpect(jsonPath("$.runs.length()").value(2)) + .andExpect(jsonPath("$.runs[*].id", hasItems((int) RUN_A1, (int) RUN_A2))); + mockMvc + .perform(get("/api/workflows/runs").param("page", "1").param("size", "50") + .header(X_REPOSITORY_ID, str(REPO_B))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.totalElements").value(1)) + .andExpect(jsonPath("$.runs.length()").value(1)) + .andExpect(jsonPath("$.runs[0].id").value((int) RUN_B1)); + } + + @Test + void paginatedWorkflowRunsWithoutHeaderAreEmpty() throws Exception { + mockMvc + .perform(get("/api/workflows/runs").param("page", "1").param("size", "50")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.totalElements").value(0)) + .andExpect(jsonPath("$.runs.length()").value(0)); + } + + private static String str(long id) { + return String.valueOf(id); + } + + private static void insertRepo(JdbcTemplate jdbc, long id, String nameWithOwner) { + jdbc.update( + "INSERT INTO repository (repository_id, has_issues, has_projects, has_wiki, is_archived, " + + "is_disabled, is_private, stargazers_count, watchers_count, name_with_owner) " + + "VALUES (?, false, false, false, false, false, false, 0, 0, ?)", + id, + nameWithOwner); + } + + private static void insertWorkflow(JdbcTemplate jdbc, long id, long repositoryId) { + jdbc.update( + "INSERT INTO workflow (id, repository_id, state, label, name) " + + "VALUES (?, ?, 'ACTIVE', 'NONE', ?)", + id, + repositoryId, + "wf-" + id); + } + + private static void insertRun(JdbcTemplate jdbc, long id, long repositoryId, long workflowId) { + jdbc.update( + "INSERT INTO workflow_run (id, repository_id, workflow_id, run_attempt, run_number, " + + "status, head_branch, head_sha, created_at, updated_at) " + + "VALUES (?, ?, ?, 1, ?, 'COMPLETED', 'main', 'deadbeef', now(), now())", + id, + repositoryId, + workflowId, + id); + } +} diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/workflow/WorkflowRunServiceTest.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/workflow/WorkflowRunServiceTest.java index f5718ac1c..dc92a335f 100644 --- a/server/application-server/src/test/java/de/tum/cit/aet/helios/workflow/WorkflowRunServiceTest.java +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/workflow/WorkflowRunServiceTest.java @@ -33,6 +33,7 @@ import java.util.HashSet; import java.util.List; import java.util.Optional; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -64,6 +65,11 @@ public void setUp() { RepositoryContext.setRepositoryId("1"); } + @AfterEach + public void tearDown() { + RepositoryContext.clear(); + } + @Test public void getPaginatedWorkflowRuns_returnsMappedRunsAndPageMetadata() { WorkflowRun run1 = createWorkflowRun( diff --git a/server/application-server/src/test/java/de/tum/cit/aet/helios/workflow/WorkflowScopingIT.java b/server/application-server/src/test/java/de/tum/cit/aet/helios/workflow/WorkflowScopingIT.java new file mode 100644 index 000000000..00c249f7d --- /dev/null +++ b/server/application-server/src/test/java/de/tum/cit/aet/helios/workflow/WorkflowScopingIT.java @@ -0,0 +1,91 @@ +package de.tum.cit.aet.helios.workflow; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.everyItem; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import de.tum.cit.aet.helios.HeliosIntegrationTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.jdbc.core.JdbcTemplate; + +/** + * Cross-refactor guard: {@code GET /api/workflows*} must return only the current repository's + * workflows (scoped by {@code X-REPOSITORY-ID}). Passes with the legacy gitRepositoryFilter and + * after the switch to explicit per-query filtering; also asserts the {@code findById} path no longer + * leaks cross-repo and that a missing repository context yields an empty list (never findAll). + */ +class WorkflowScopingIT extends HeliosIntegrationTest { + + private static final long REPO_A = 1L; + private static final long REPO_B = 2L; + private static final long WF_A1 = 11L; + private static final long WF_A2 = 12L; + private static final long WF_B1 = 21L; + + @BeforeEach + void seed() { + JdbcTemplate jdbc = new JdbcTemplate(dataSource); + jdbc.execute("TRUNCATE TABLE repository CASCADE"); + insertRepo(jdbc, REPO_A, "ls1intum/repo-a"); + insertRepo(jdbc, REPO_B, "ls1intum/repo-b"); + insertWorkflow(jdbc, WF_A1, REPO_A, "CI"); + insertWorkflow(jdbc, WF_A2, REPO_A, "Deploy"); + insertWorkflow(jdbc, WF_B1, REPO_B, "CI"); + } + + @Test + void workflowsAreScopedToRepositoryA() throws Exception { + mockMvc + .perform(get("/api/workflows").header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(2)) + .andExpect(jsonPath("$[*].repository.id", everyItem(equalTo((int) REPO_A)))); + } + + @Test + void workflowsByStateAreScopedToRepositoryA() throws Exception { + mockMvc + .perform(get("/api/workflows/state/ACTIVE").header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(2)) + .andExpect(jsonPath("$[*].repository.id", everyItem(equalTo((int) REPO_A)))); + } + + @Test + void workflowByIdIsInvisibleFromAnotherRepository() throws Exception { + mockMvc + .perform(get("/api/workflows/{id}", WF_B1).header(X_REPOSITORY_ID, String.valueOf(REPO_A))) + .andExpect(status().isNotFound()); + mockMvc + .perform(get("/api/workflows/{id}", WF_B1).header(X_REPOSITORY_ID, String.valueOf(REPO_B))) + .andExpect(status().isOk()); + } + + @Test + void withoutRepositoryContextReturnsEmpty() throws Exception { + mockMvc + .perform(get("/api/workflows")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value(0)); + } + + private static void insertRepo(JdbcTemplate jdbc, long id, String nameWithOwner) { + jdbc.update( + "INSERT INTO repository (repository_id, has_issues, has_projects, has_wiki, is_archived, " + + "is_disabled, is_private, stargazers_count, watchers_count, name_with_owner) " + + "VALUES (?, false, false, false, false, false, false, 0, 0, ?)", + id, + nameWithOwner); + } + + private static void insertWorkflow(JdbcTemplate jdbc, long id, long repositoryId, String name) { + jdbc.update( + "INSERT INTO workflow (id, repository_id, name, state) VALUES (?, ?, ?, 'ACTIVE')", + id, + repositoryId, + name); + } +}