diff --git a/README.md b/README.md index 5baa3f26b152..6f7eefdc4050 100644 --- a/README.md +++ b/README.md @@ -205,7 +205,7 @@ Refer to [Using JHipster in production](http://www.jhipster.tech/production) for The following command can automate the deployment to a server. The example shows the deployment to the main Artemis test server (which runs a virtual machine): ```shell -./artemis-server-cli deploy username@artemis-test0.artemis.in.tum.de -w build/libs/Artemis-8.7.4.war +./artemis-server-cli deploy username@artemis-test0.artemis.in.tum.de -w build/libs/Artemis-8.7.5.war ``` ## Architecture diff --git a/build.gradle b/build.gradle index 140c05b9abbe..1ec486f2bc41 100644 --- a/build.gradle +++ b/build.gradle @@ -33,7 +33,7 @@ plugins { } group = "de.tum.cit.aet.artemis" -version = "8.7.4" +version = "8.7.5" description = "Interactive Learning with Individual Feedback" java { @@ -43,7 +43,7 @@ java { } wrapper { - gradleVersion = "9.3.0" + gradleVersion = "9.4.1" } node { @@ -215,7 +215,7 @@ dependencies { implementation "de.jplag:typescript:${jplag_version}" // Align all Spring AI modules to the same version - implementation platform("org.springframework.ai:spring-ai-bom:1.1.2") + implementation platform("org.springframework.ai:spring-ai-bom:1.1.3") // Azure OpenAI (prod, or when you want Azure) implementation "org.springframework.ai:spring-ai-starter-model-azure-openai" // can also use non-azure models implementation "org.springframework.ai:spring-ai-starter-model-chat-memory-repository-jdbc" @@ -298,7 +298,7 @@ dependencies { implementation "io.opentelemetry:opentelemetry-exporter-otlp" // Prometheus requires the protobuf-java dependency, but we explicitly use the latest version to avoid security vulnerabilities - implementation "com.google.protobuf:protobuf-java:4.33.4" + implementation "com.google.protobuf:protobuf-java:4.34.1" implementation "tech.jhipster:jhipster-framework:${jhipster_dependencies_version}" @@ -356,16 +356,16 @@ dependencies { implementation "org.springframework.boot:spring-boot-starter-aop" implementation "org.springframework.boot:spring-boot-starter-data-jpa" implementation "org.springframework.boot:spring-boot-starter-security" - implementation "org.springframework.boot:spring-boot-starter-web" + implementation "org.springframework.boot:spring-boot-starter-web:3.5.13" implementation "org.springframework.boot:spring-boot-starter-tomcat" implementation "org.springframework.boot:spring-boot-starter-websocket" - implementation "org.springframework.boot:spring-boot-starter-thymeleaf" + implementation "org.springframework.boot:spring-boot-starter-thymeleaf:3.5.13" implementation "org.springframework.boot:spring-boot-starter-oauth2-resource-server" implementation "org.springframework.boot:spring-boot-starter-oauth2-client" - implementation "org.springframework.data:spring-data-jpa:3.5.8" - implementation "org.springframework.data:spring-data-ldap:3.5.8" - implementation "org.springframework.ldap:spring-ldap-core:3.3.5" + implementation "org.springframework.data:spring-data-jpa:3.5.10" + implementation "org.springframework.data:spring-data-ldap:3.5.10" + implementation "org.springframework.ldap:spring-ldap-core:3.3.6" implementation "org.springframework.cloud:spring-cloud-starter-netflix-eureka-client:${spring_cloud_version}" implementation "org.springframework.cloud:spring-cloud-starter-config:${spring_cloud_version}" @@ -416,9 +416,9 @@ dependencies { implementation "org.zalando:problem-spring-web:0.29.1" implementation "org.zalando:jackson-datatype-problem:0.27.1" // Required by JPlag - implementation "com.ibm.icu:icu4j-charset:78.2" + implementation "com.ibm.icu:icu4j-charset:78.3" // Required by exam session service - implementation "com.github.seancfoley:ipaddress:5.5.1" + implementation "com.github.seancfoley:ipaddress:5.6.2" // used for testing and Java Template Upgrade Service implementation "org.apache.maven:maven-model:3.9.12" @@ -442,7 +442,7 @@ dependencies { implementation "com.google.errorprone:error_prone_annotations:2.46.0" // needed for OpenAPI generation - implementation "org.springdoc:springdoc-openapi-starter-webmvc-ui:2.8.15" + implementation "org.springdoc:springdoc-openapi-starter-webmvc-ui:2.8.16" // NOTE: we want to keep the same unique version for all configurations, implementation and annotationProcessor @@ -457,7 +457,7 @@ dependencies { annotationProcessor "org.glassfish.jaxb:jaxb-runtime:${jaxb_runtime_version}" annotationProcessor "org.springframework.boot:spring-boot-configuration-processor" - implementation "org.mnode.ical4j:ical4j:4.2.3" + implementation "org.mnode.ical4j:ical4j:4.2.4" // ---- CHECKSTYLE DEPENDENCIES ---- @@ -485,9 +485,9 @@ dependencies { testImplementation "io.github.classgraph:classgraph:4.8.184" testImplementation "org.awaitility:awaitility:4.3.0" testImplementation "org.apache.maven.shared:maven-invoker:3.3.0" - testImplementation "org.gradle:gradle-tooling-api:9.3.0" - testImplementation "org.apache.maven.surefire:surefire-report-parser:3.5.4" - testImplementation "io.zonky.test:embedded-database-spring-test:2.7.1" + testImplementation "org.gradle:gradle-tooling-api:9.4.1" + testImplementation "org.apache.maven.surefire:surefire-report-parser:3.5.5" + testImplementation "io.zonky.test:embedded-database-spring-test:2.8.0" testImplementation "com.redis:testcontainers-redis:2.2.4" testImplementation "com.tngtech.archunit:archunit:1.4.1" testImplementation "org.skyscreamer:jsonassert:1.5.3" @@ -556,13 +556,15 @@ def isNonStable = { String version -> return !stableKeyword && !(version ==~ regex) } -tasks.named("dependencyUpdates").configure { - rejectVersionIf { - isNonStable(it.candidate.version) - } +def getMajorVersion = { String version -> + version.tokenize('.')[0] +} +tasks.named("dependencyUpdates").configure { + def rawSkipMajor = System.getProperty("skipMajorUpdates") + def skipMajor = rawSkipMajor != null && (rawSkipMajor.isBlank() || rawSkipMajor.toBoolean()) rejectVersionIf { - isNonStable(it.candidate.version) && !isNonStable(it.currentVersion) + isNonStable(it.candidate.version) || (skipMajor && getMajorVersion(it.candidate.version) != getMajorVersion(it.currentVersion)) } } @@ -578,7 +580,8 @@ tasks.named("dependencyUpdates").configure { // 3) Verify code coverage (after tests): ./gradlew jacocoTestCoverageVerification -x webapp // 4) Check Java code format: ./gradlew spotlessCheck -x webapp // 5) Apply Java code formatter: ./gradlew spotlessApply -x webapp -// 6) Find dependency updates: ./gradlew dependencyUpdates -Drevision=release +// 6) Find dependency updates: ./gradlew dependencyUpdates -Drevision=release -x webapp +// 6a) Find only minor/patch dependency updates: ./gradlew dependencyUpdates -Drevision=release -DskipMajorUpdates -x webapp // 7) Check JavaDoc: ./gradlew checkstyleMain -x webapp // 8) Detects uses of legacy code: ./gradlew modernizer -x webapp // 9) Check for vulnerabilities in dependencies ./gradlew dependencyCheckAnalyze -x webapp diff --git a/gradle.properties b/gradle.properties index 47bf591d3386..b3798139f86f 100644 --- a/gradle.properties +++ b/gradle.properties @@ -7,20 +7,20 @@ npm_version=11.5.1 # Dependency versions jhipster_dependencies_version=8.12.0 -spring_boot_version=3.5.10 +spring_boot_version=3.5.12 tomcat_version=10.1.49 spring_cloud_version=4.3.1 # TODO: upgrading to 6.6.x currently leads to issues due to internal changes in Hibernate and potentially wrong use in Artemis server code. See https://hibernate.atlassian.net/browse/HHH-19249 hibernate_version=6.5.3.Final opensaml_version=5.1.6 jwt_version=0.13.0 -jaxb_runtime_version=4.0.6 +jaxb_runtime_version=4.0.7 hazelcast_version=5.5.0 fasterxml_version=2.20.1 netty_version=4.2.7.Final -jgit_version=7.5.0.202512021534-r +jgit_version=7.6.0.202603022253-r sshd_version=2.16.0 -checkstyle_version=13.0.0 +checkstyle_version=13.3.0 jplag_version=6.3.0 # not really used in Artemis, nor JPlag, nor the used version of Stanford CoreNLP, but we use the latest to avoid security vulnerability warnings # NOTE: we cannot need to use the latest version 9.x or 10.x here as long as Stanford CoreNLP does not reference it @@ -28,17 +28,17 @@ lucene_version=8.11.4 slf4j_version=2.0.17 sentry_version=8.31.0 liquibase_version=5.0.1 -docker_java_version=3.7.0 +docker_java_version=3.7.1 logback_version=1.5.21 java_parser_version=3.26.2 -byte_buddy_version=1.18.4 +byte_buddy_version=1.18.7 mysql_version=9.5.0 micrometer_version=1.16.2 # micrometer-tracing follows a different versioning schema than micrometer-core micrometer_tracing_version=1.6.2 snakeyaml_version=2.5 helios_status_version=1.1.0 -bucket4j_version=8.16.0 +bucket4j_version=8.17.0 commons_lang3_version=3.20.0 commons_text_version=1.15.0 redisson_version=3.52.0 @@ -46,16 +46,18 @@ redisson_version=3.52.0 # testing # make sure both versions are compatible -junit_version=6.0.2 -mockito_version=5.21.0 +junit_version=6.0.3 +mockito_version=5.23.0 +testcontainers_version=2.0.4 +ryuk_version=0.14.0 # gradle plugin version gradle_node_plugin_version=7.1.0 apt_plugin_version=0.21 liquibase_plugin_version=3.1.0 -modernizer_plugin_version=1.12.0 -spotless_plugin_version=8.2.0 +modernizer_plugin_version=1.13.0 +spotless_plugin_version=8.4.0 org.gradle.jvmargs=-Xmx4g -XX:+HeapDumpOnOutOfMemoryError -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en \ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \ diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 19a6bdeb848a..8e61ef12590c 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,7 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-9.3.0-bin.zip +distributionSha256Sum=2ab2958f2a1e51120c326cad6f385153bb11ee93b3c216c5fccebfdfbb7ec6cb +distributionUrl=https\://services.gradle.org/distributions/gradle-9.4.1-bin.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME diff --git a/package.json b/package.json index 42c9a3bc9efc..d4598fda29e3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "artemis", - "version": "8.7.4", + "version": "8.7.5", "description": "Interactive Learning with Individual Feedback", "private": true, "license": "MIT", diff --git a/src/main/java/de/tum/cit/aet/artemis/core/exception/localvc/LocalVCForbiddenException.java b/src/main/java/de/tum/cit/aet/artemis/core/exception/localvc/LocalVCForbiddenException.java index 82acc72a8169..79d2a5e094f6 100644 --- a/src/main/java/de/tum/cit/aet/artemis/core/exception/localvc/LocalVCForbiddenException.java +++ b/src/main/java/de/tum/cit/aet/artemis/core/exception/localvc/LocalVCForbiddenException.java @@ -10,6 +10,10 @@ public LocalVCForbiddenException() { // empty constructor } + public LocalVCForbiddenException(String message) { + super(message); + } + public LocalVCForbiddenException(Throwable cause) { super(cause); } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/ArtemisGitServletService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/ArtemisGitServletService.java index f221cb4ba471..8760d4cfe38f 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/ArtemisGitServletService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/ArtemisGitServletService.java @@ -7,6 +7,7 @@ import jakarta.servlet.http.HttpServletRequest; import org.eclipse.jgit.http.server.GitServlet; +import org.eclipse.jgit.http.server.resolver.AsIsFileService; import org.eclipse.jgit.transport.ReceivePack; import org.eclipse.jgit.transport.UploadPack; import org.slf4j.Logger; @@ -71,6 +72,11 @@ public ArtemisGitServletService(LocalVCServletService localVCServletService) { @Override public void init() throws ServletException { super.init(); + // Disable dumb HTTP protocol (serving raw files like /HEAD, /objects/). + // Only the smart HTTP protocol (info/refs + git-upload-pack/git-receive-pack) is allowed, + // which goes through our authentication and authorization filters. + this.setAsIsFileService(AsIsFileService.DISABLED); + this.setRepositoryResolver((request, name) -> { // request – the current request, may be used to inspect session state including cookies or user authentication. // name – name of the repository, as parsed out of the URL (everything after /git/). diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java index b20c17391d3f..16e7f1be55ea 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java @@ -20,7 +20,6 @@ import jakarta.servlet.http.HttpServletRequest; -import org.apache.commons.lang3.StringUtils; import org.apache.sshd.server.session.ServerSession; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; @@ -53,6 +52,7 @@ import de.tum.cit.aet.artemis.core.repository.UserRepository; import de.tum.cit.aet.artemis.core.security.RateLimitType; import de.tum.cit.aet.artemis.core.security.SecurityUtils; +import de.tum.cit.aet.artemis.core.service.AuthorizationCheckService; import de.tum.cit.aet.artemis.core.service.RateLimitService; import de.tum.cit.aet.artemis.core.util.TimeLogUtil; import de.tum.cit.aet.artemis.exercise.domain.Exercise; @@ -118,6 +118,8 @@ public class LocalVCServletService { private final ParticipationVCSAccessTokenRepository participationVCSAccessTokenRepository; + private final AuthorizationCheckService authorizationCheckService; + private final RateLimitService rateLimitService; private final ExerciseVersionService exerciseVersionService; @@ -140,8 +142,8 @@ public LocalVCServletService(AuthenticationManager authenticationManager, UserRe RepositoryAccessService repositoryAccessService, ProgrammingExerciseParticipationService programmingExerciseParticipationService, AuxiliaryRepositoryService auxiliaryRepositoryService, ContinuousIntegrationTriggerService ciTriggerService, ProgrammingSubmissionService programmingSubmissionService, ProgrammingSubmissionMessagingService programmingSubmissionMessagingService, ProgrammingExerciseTestCaseChangedService programmingExerciseTestCaseChangedService, - ParticipationVCSAccessTokenRepository participationVCSAccessTokenRepository, Optional vcsAccessLogService, RateLimitService rateLimitService, - ExerciseVersionService exerciseVersionService) { + ParticipationVCSAccessTokenRepository participationVCSAccessTokenRepository, Optional vcsAccessLogService, + AuthorizationCheckService authorizationCheckService, RateLimitService rateLimitService, ExerciseVersionService exerciseVersionService) { this.authenticationManager = authenticationManager; this.userRepository = userRepository; this.programmingExerciseRepository = programmingExerciseRepository; @@ -154,6 +156,7 @@ public LocalVCServletService(AuthenticationManager authenticationManager, UserRe this.programmingExerciseTestCaseChangedService = programmingExerciseTestCaseChangedService; this.participationVCSAccessTokenRepository = participationVCSAccessTokenRepository; this.vcsAccessLogService = vcsAccessLogService; + this.authorizationCheckService = authorizationCheckService; this.rateLimitService = rateLimitService; this.exerciseVersionService = exerciseVersionService; } @@ -227,19 +230,15 @@ public void authenticateAndAuthorizeGitRequest(HttpServletRequest request, Repos } } - // Optimization. - // For each git command (i.e. 'git fetch' or 'git push'), the git client sends three requests. - // The URLs of the first two requests end on '[repository URI]/info/refs'. The third one ends on '[repository URI]/git-receive-pack' (for push) and '[repository - // URL]/git-upload-pack' (for fetch). - // The following checks will only be conducted for the second request, so we do not have to access the database too often. - if (!request.getRequestURI().endsWith("/info/refs")) { - return; + // Only count rate limit on /info/refs (the initial handshake request per git operation). + // The data transfer requests (git-upload-pack, git-receive-pack) reuse the same credentials + // and should not consume additional rate limit budget. + if (request.getRequestURI().endsWith("/info/refs")) { + String ipString = getIpStringFromRequest(request); + final IPAddress ipAddress = new IPAddressString(ipString).getAddress(); + rateLimitService.enforcePerMinute(ipAddress, RateLimitType.AUTHENTICATION); } - String ipString = getIpStringFromRequest(request); - final IPAddress ipAddress = new IPAddressString(ipString).getAddress(); - rateLimitService.enforcePerMinute(ipAddress, RateLimitType.AUTHENTICATION); - LocalVCRepositoryUri localVCRepositoryUri = parseRepositoryUri(request); log.debug("Parsed repository URI from request: {}", localVCRepositoryUri); String projectKey = localVCRepositoryUri.getProjectKey(); @@ -259,7 +258,12 @@ public void authenticateAndAuthorizeGitRequest(HttpServletRequest request, Repos try { var optionalParticipation = authorizeUser(repositoryTypeOrUserName, user, exercise, repositoryAction, localVCRepositoryUri, false); - savePreliminaryVcsAccessLogForHTTPs(request, localVCRepositoryUri, user, repositoryAction, optionalParticipation); + // Only create the preliminary access log on /info/refs requests. + // The data transfer requests (git-upload-pack, git-receive-pack) will update this log entry + // via PreUploadHook / processNewPush rather than creating a duplicate. + if (request.getRequestURI().endsWith("/info/refs")) { + savePreliminaryVcsAccessLogForHTTPs(request, localVCRepositoryUri, user, repositoryAction, optionalParticipation); + } } catch (LocalVCForbiddenException e) { log.error("User {} does not have access to the repository {}", user.getLogin(), localVCRepositoryUri); @@ -305,12 +309,17 @@ private void savePreliminaryVcsAccessLogForHTTPs(HttpServletRequest request, Loc */ public void saveFailedAccessVcsAccessLog(AuthenticationContext context, String repositoryTypeOrUserName, Exercise exercise, LocalVCRepositoryUri localVCRepositoryUri, User user, RepositoryActionType repositoryAction) { - var participation = tryToLoadParticipation(false, repositoryTypeOrUserName, localVCRepositoryUri, (ProgrammingExercise) exercise); - var commitHash = getCommitHash(localVCRepositoryUri); - var authenticationMechanism = resolveAuthenticationMechanismFromSessionOrRequest(context, user); - var action = repositoryAction == RepositoryActionType.WRITE ? RepositoryActionType.PUSH_FAIL : RepositoryActionType.CLONE_FAIL; - var ipAddress = context.getIpAddress(); - vcsAccessLogService.ifPresent(service -> service.saveAccessLog(user, participation, action, authenticationMechanism, commitHash, ipAddress)); + try { + var participation = tryToLoadParticipation(false, repositoryTypeOrUserName, localVCRepositoryUri, (ProgrammingExercise) exercise); + var commitHash = getCommitHash(localVCRepositoryUri); + var authenticationMechanism = resolveAuthenticationMechanismFromSessionOrRequest(context, user); + var action = repositoryAction == RepositoryActionType.WRITE ? RepositoryActionType.PUSH_FAIL : RepositoryActionType.CLONE_FAIL; + var ipAddress = context.getIpAddress(); + vcsAccessLogService.ifPresent(service -> service.saveAccessLog(user, participation, action, authenticationMechanism, commitHash, ipAddress)); + } + catch (Exception e) { + log.warn("Failed to save VCS access log for failed access attempt by user {} to repository {}: {}", user.getLogin(), localVCRepositoryUri, e.getMessage()); + } } /** @@ -411,9 +420,7 @@ private User authenticateUser(String authorizationHeader, ProgrammingExercise ex SecurityUtils.checkUsernameAndPasswordValidity(username, passwordOrToken); } catch (AccessForbiddenException | AuthenticationException e) { - if (StringUtils.isNotEmpty(passwordOrToken)) { - log.warn("Failed login attempt for user {} with password {} due to issue: {}", username, passwordOrToken, e.getMessage()); - } + log.warn("Failed login attempt for user {} due to issue: {}", username, e.getMessage()); throw new LocalVCAuthException(e.getMessage()); } @@ -576,12 +583,18 @@ private UsernameAndPassword extractUsernameAndPassword(String authorizationHeade } String[] basicAuthCredentialsEncoded = authorizationHeader.split(" "); - if (!("Basic".equals(basicAuthCredentialsEncoded[0]))) { - throw new LocalVCAuthException("Non basic authorization header provided"); + if (basicAuthCredentialsEncoded.length < 2 || !("Basic".equals(basicAuthCredentialsEncoded[0]))) { + throw new LocalVCAuthException("Invalid authorization header format"); } - // Return decoded basic auth credentials which contain the username and the password. - String basicAuthCredentials = new String(Base64.getDecoder().decode(basicAuthCredentialsEncoded[1])); + // Decode the Base64-encoded credentials (username:password). + String basicAuthCredentials; + try { + basicAuthCredentials = new String(Base64.getDecoder().decode(basicAuthCredentialsEncoded[1])); + } + catch (IllegalArgumentException e) { + throw new LocalVCAuthException("Invalid Base64 encoding in authorization header"); + } int separatorIndex = basicAuthCredentials.indexOf(":"); @@ -609,7 +622,19 @@ private UsernameAndPassword extractUsernameAndPassword(String authorizationHeade public Optional authorizeUser(String repositoryTypeOrUserName, User user, ProgrammingExercise exercise, RepositoryActionType repositoryActionType, LocalVCRepositoryUri localVCRepositoryUri, boolean usingSSH) throws LocalVCForbiddenException { - if (checkIfRepositoryIsAuxiliaryOrTestRepository(exercise, repositoryTypeOrUserName, repositoryActionType, user)) { + if (checkAccessToStaffRepository(exercise, repositoryTypeOrUserName, repositoryActionType, user)) { + // For tests and auxiliary repos, no participation is needed (they don't have dedicated participations). + // For template and solution repos, load the participation so callers can use it for access logging. + if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString()) || repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString())) { + try { + return Optional.of(tryToLoadParticipation(usingSSH, repositoryTypeOrUserName, localVCRepositoryUri, exercise)); + } + catch (LocalVCInternalException e) { + log.warn("Missing participation for staff repository {} in exercise {}. Continuing without participation-based logging.", localVCRepositoryUri, + exercise.getId(), e); + return Optional.empty(); + } + } return Optional.empty(); } @@ -677,51 +702,60 @@ private void checkAccessForRepository(ProgrammingExerciseParticipation participa } /** - * Checks if the provided repository is an auxiliary or test repository. - * But: for students it only checks for test repository, and assumes the requested repository is not an auxiliary repository. - * This avoids an unnecessary database call, and postpones the actual check to - * {@link LocalVCServletService#tryToLoadParticipation(boolean, String, LocalVCRepositoryUri, ProgrammingExercise)} - * and only checks it if it is really needed. + * Checks if the repository is a staff-only repository (template, solution, tests, or auxiliary) and whether the user has access. + *

+ * Students are denied access to template, solution, tests, and auxiliary repositories. + * TAs can read but not write to these repositories. Editors and above have full access. + *

+ * For auxiliary repositories, the check is only performed for users who are at least TA, + * to avoid an unnecessary database query for students (since loading auxiliary repositories requires a DB call). + * If a student requests an auxiliary repository, this method returns {@code false} and the check is deferred to + * {@link LocalVCServletService#tryToLoadParticipation(boolean, String, LocalVCRepositoryUri, ProgrammingExercise)}. * - * @param exercise the exercise, where the repository belongs to - * @param repositoryTypeOrUserName the type or username of the repository - * @param repositoryActionType the action that should be performed on of the repository - * @param user the user who tries to access the repository - * @return true if the repository is an Auxiliary or Test repository, and the user has access to it. - * false for students if the repository is possibly an auxiliary repository, or - * false for TAs if the repository is neither auxiliary nor test - * @throws LocalVCForbiddenException if the user has no access rights for the requested repository + * @param exercise the exercise the repository belongs to + * @param repositoryTypeOrUserName the repository type name (e.g. "exercise", "solution", "tests") or the username for student repos + * @param repositoryActionType the action to be performed (READ or WRITE) + * @param user the user requesting access + * @return {@code true} if the repository is a staff-only repository and the user has access (caller can skip further checks). + * {@code false} if the repository is not a known staff-only type (caller should proceed with student participation checks). + * @throws LocalVCForbiddenException if the user does not have the required permissions for the requested repository */ - private boolean checkIfRepositoryIsAuxiliaryOrTestRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName, RepositoryActionType repositoryActionType, - User user) throws LocalVCForbiddenException { + private boolean checkAccessToStaffRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName, RepositoryActionType repositoryActionType, User user) + throws LocalVCForbiddenException { - // Students are not able to access Test or Aux repositories. - // To save on db queries we do not check whether it is an Aux repo here, as we would need to fetch them first. - try { - repositoryAccessService.checkAccessTestOrAuxRepositoryElseThrow(false, exercise, user, repositoryTypeOrUserName); - } - catch (AccessForbiddenException e) { - if (repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { - throw new LocalVCForbiddenException(e); - } - // The user is a student, and the repository is not a test repository - return false; - } + boolean isTemplateOrSolutionOrTestsRepo = repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString()) + || repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString()) || repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString()); - // Here we only check if the repository is an auxiliary repository if the user is at least TA. - // Why? If the requested repository is not an auxiliary repo, we do not need to load auxiliary repositories - if (auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise) || repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { - try { - repositoryAccessService.checkAccessTestOrAuxRepositoryElseThrow(repositoryActionType == RepositoryActionType.WRITE, exercise, user, repositoryTypeOrUserName); + var course = exercise.getCourseViaExerciseGroupOrCourseMember(); + + if (isTemplateOrSolutionOrTestsRepo) { + // For WRITE operations, check editor permission first (avoids a second role check later) + if (repositoryActionType == RepositoryActionType.WRITE) { + if (!authorizationCheckService.isAtLeastEditorInCourse(course, user)) { + throw new LocalVCForbiddenException("You are not allowed to push to the " + repositoryTypeOrUserName + " repository of this programming exercise."); + } } - catch (AccessForbiddenException e) { - throw new LocalVCForbiddenException(e); + else if (!authorizationCheckService.isAtLeastTeachingAssistantInCourse(course, user)) { + throw new LocalVCForbiddenException("You are not allowed to access the " + repositoryTypeOrUserName + " repository of this programming exercise."); } - // The user is at least TA, it is either an Auxiliary repository or a Test repository, and the user has access to it return true; } - // The repository is neither an Auxiliary repository nor a Test repository - return false; + + // For auxiliary repositories, only check if the user is at least TA (avoids unnecessary DB query for students) + boolean isAtLeastTA = authorizationCheckService.isAtLeastTeachingAssistantInCourse(course, user); + boolean isAuxiliaryRepo = isAtLeastTA && auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise); + + if (!isAuxiliaryRepo) { + // Not a staff-only repository — proceed with student participation checks + return false; + } + + // Auxiliary repository: TAs can read; writing requires at least editor permissions. + if (repositoryActionType == RepositoryActionType.WRITE && !authorizationCheckService.isAtLeastEditorInCourse(course, user)) { + throw new LocalVCForbiddenException("You are not allowed to push to the " + repositoryTypeOrUserName + " repository of this programming exercise."); + } + + return true; } /** @@ -948,13 +982,13 @@ private ProgrammingSubmission getProgrammingSubmission(ProgrammingExercise exerc } private RepositoryType getRepositoryType(String repositoryTypeOrUserName, ProgrammingExercise exercise) { - if (repositoryTypeOrUserName.equals("exercise")) { + if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { return RepositoryType.TEMPLATE; } - else if (repositoryTypeOrUserName.equals("solution")) { + else if (repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString())) { return RepositoryType.SOLUTION; } - else if (repositoryTypeOrUserName.equals("tests")) { + else if (repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { return RepositoryType.TESTS; } else if (auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise)) { @@ -966,12 +1000,16 @@ else if (auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTy } private RepositoryType getRepositoryTypeWithoutAuxiliary(String repositoryTypeOrUserName) { - return switch (repositoryTypeOrUserName) { - case "exercise" -> RepositoryType.TEMPLATE; - case "solution" -> RepositoryType.SOLUTION; - case "tests" -> RepositoryType.TESTS; - default -> RepositoryType.USER; - }; + if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { + return RepositoryType.TEMPLATE; + } + else if (repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString())) { + return RepositoryType.SOLUTION; + } + else if (repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { + return RepositoryType.TESTS; + } + return RepositoryType.USER; } /** @@ -1057,7 +1095,8 @@ public void updateAndStoreVCSAccessLogForCloneAndPullHTTPS(HttpServletRequest re vcsAccessLogService.ifPresent(service -> service.updateRepositoryActionType(localVCRepositoryUri, repositoryActionType)); } - catch (Exception ignored) { + catch (Exception e) { + log.debug("Could not update VCS access log for HTTPS clone/pull: {}", e.getMessage()); } } @@ -1082,7 +1121,8 @@ public void updateAndStoreVCSAccessLogForCloneAndPullSSH(ServerSession session, accessLog.setRepositoryActionType(repositoryActionType); vcsAccessLogService.ifPresent(service -> service.saveVcsAccesslog(accessLog)); } - catch (Exception ignored) { + catch (Exception e) { + log.debug("Could not update VCS access log for SSH clone/pull: {}", e.getMessage()); } } diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCIntegrationTest.java index 48ea4d123a2b..591096809940 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCIntegrationTest.java @@ -2,6 +2,7 @@ import static de.tum.cit.aet.artemis.core.user.util.UserFactory.USER_PASSWORD; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.awaitility.Awaitility.await; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -11,6 +12,7 @@ import java.net.URISyntaxException; import java.nio.file.Path; import java.time.ZonedDateTime; +import java.util.Base64; import java.util.Optional; import javax.naming.InvalidNameException; @@ -27,8 +29,12 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpHeaders; +import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.test.context.support.WithMockUser; +import de.tum.cit.aet.artemis.core.exception.localvc.LocalVCAuthException; +import de.tum.cit.aet.artemis.core.exception.localvc.LocalVCForbiddenException; import de.tum.cit.aet.artemis.core.service.TempFileUtilService; import de.tum.cit.aet.artemis.core.service.ldap.LdapUserDto; import de.tum.cit.aet.artemis.programming.AbstractProgrammingIntegrationLocalCILocalVCTestBase; @@ -37,6 +43,7 @@ import de.tum.cit.aet.artemis.programming.service.localvc.LocalVCRepositoryUri; import de.tum.cit.aet.artemis.programming.util.LocalRepository; import de.tum.cit.aet.artemis.programming.util.RepositoryExportTestUtil; +import de.tum.cit.aet.artemis.programming.web.repository.RepositoryActionType; /** * This class contains integration tests for edge cases pertaining to the local VC system. @@ -220,8 +227,10 @@ void testFetchPush_templateRepository_noParticipation() { programmingExerciseRepository.save(programmingExercise); templateProgrammingExerciseParticipationRepository.delete(templateParticipation); - localVCLocalCITestService.testFetchReturnsError(templateRepository.workingCopyGitRepo, instructor1Login, projectKey1, templateRepositorySlug, INTERNAL_SERVER_ERROR); - localVCLocalCITestService.testPushReturnsError(templateRepository.workingCopyGitRepo, instructor1Login, projectKey1, templateRepositorySlug, INTERNAL_SERVER_ERROR); + // Instructors should still be able to access the template repository even if the participation record is missing. + // Authorization is based on the user's course role, not on the existence of a participation. + localVCLocalCITestService.testFetchSuccessful(templateRepository.workingCopyGitRepo, instructor1Login, projectKey1, templateRepositorySlug); + localVCLocalCITestService.testPushSuccessful(templateRepository.workingCopyGitRepo, instructor1Login, projectKey1, templateRepositorySlug); } @Test @@ -231,8 +240,10 @@ void testFetchPush_solutionRepository_noParticipation() { programmingExerciseRepository.save(programmingExercise); solutionProgrammingExerciseParticipationRepository.delete(solutionParticipation); - localVCLocalCITestService.testFetchReturnsError(solutionRepository.workingCopyGitRepo, instructor1Login, projectKey1, solutionRepositorySlug, INTERNAL_SERVER_ERROR); - localVCLocalCITestService.testPushReturnsError(solutionRepository.workingCopyGitRepo, instructor1Login, projectKey1, solutionRepositorySlug, INTERNAL_SERVER_ERROR); + // Instructors should still be able to access the solution repository even if the participation record is missing. + // Authorization is based on the user's course role, not on the existence of a participation. + localVCLocalCITestService.testFetchSuccessful(solutionRepository.workingCopyGitRepo, instructor1Login, projectKey1, solutionRepositorySlug); + localVCLocalCITestService.testPushSuccessful(solutionRepository.workingCopyGitRepo, instructor1Login, projectKey1, solutionRepositorySlug); } @Test @@ -399,6 +410,273 @@ void testRepositoryFolderName() { assertThat(studentAssignmentRepositoryUri2.folderNameForRepositoryUri()).isEqualTo(projectKey1 + "/" + projectKey1.toLowerCase() + "-" + login2); } + // --- Security tests: authentication and authorization for git operations --- + // These tests directly exercise LocalVCServletService.authenticateAndAuthorizeGitRequest + // with MockHttpServletRequest to verify every branch in the authentication and authorization flow. + + // == Authentication tests: verifying credential validation == + + @Test + void testFetch_nonExistentUser_isRejected() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/git-upload-pack", "hacker", "randompassword"); + + assertThatExceptionOfType(LocalVCAuthException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ)); + } + + @Test + void testPush_nonExistentUser_isRejected() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/git-receive-pack", "hacker", "randompassword"); + + assertThatExceptionOfType(LocalVCAuthException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.WRITE)); + } + + @Test + void testFetch_noAuthorizationHeader_isRejected() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/git/" + projectKey1 + "/" + solutionRepositorySlug + ".git/git-upload-pack"); + request.setRemoteAddr("127.0.0.1"); + + assertThatExceptionOfType(LocalVCAuthException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ)); + } + + /** + * Verifies that the dumb HTTP protocol is disabled at the JGit servlet level by sending + * real HTTP requests to dumb-protocol endpoints (/HEAD, /objects/info/packs). + * These paths bypass our authentication filters entirely (they are served by JGit's + * AsIsFileService), so we disable them via {@code setAsIsFileService(AsIsFileService.DISABLED)} + * in ArtemisGitServletService. Without this, anyone could clone repositories anonymously. + */ + @Test + void testDumbHttpProtocol_isDisabledInServlet() throws Exception { + // Send real HTTP requests to dumb-protocol endpoints on the running server. + // These must return non-200 (JGit returns 403 when AsIsFileService is DISABLED). + var headUrl = new java.net.URI("http://localhost:" + port + "/git/" + projectKey1 + "/" + solutionRepositorySlug + ".git/HEAD").toURL(); + var headConnection = (java.net.HttpURLConnection) headUrl.openConnection(); + headConnection.setRequestMethod("GET"); + assertThat(headConnection.getResponseCode()).as("Dumb HTTP /HEAD endpoint should be blocked").isGreaterThanOrEqualTo(400); + headConnection.disconnect(); + + var packsUrl = new java.net.URI("http://localhost:" + port + "/git/" + projectKey1 + "/" + solutionRepositorySlug + ".git/objects/info/packs").toURL(); + var packsConnection = (java.net.HttpURLConnection) packsUrl.openConnection(); + packsConnection.setRequestMethod("GET"); + assertThat(packsConnection.getResponseCode()).as("Dumb HTTP /objects/info/packs endpoint should be blocked").isGreaterThanOrEqualTo(400); + packsConnection.disconnect(); + } + + /** + * Build agent credentials should allow READ (fetch/clone) operations + * without going through normal user authentication. + */ + @Test + void testFetch_buildAgentCredentials_succeeds() throws Exception { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/info/refs", "buildjob_user", "buildjob_password"); + + // Build agent bypass only applies to READ — should succeed without normal user auth + localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ); + } + + /** + * Build agent credentials must NOT bypass authentication for WRITE (push) operations. + * The build agent check only applies to RepositoryActionType.READ. + */ + @Test + void testPush_buildAgentCredentials_isRejected() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/git-receive-pack", "buildjob_user", "buildjob_password"); + + // Build agent bypass does NOT apply to WRITE — "buildjob_user" is not a real user, so auth fails + assertThatExceptionOfType(LocalVCAuthException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.WRITE)); + } + + // == Authorization tests: student access to staff repositories == + // Covers checkAccessToStaffRepository: student (not TA) + staff repo type → throws + + @Test + void testFetch_studentAccessesSolutionRepo_isForbidden() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + solutionRepositorySlug + ".git/git-upload-pack", student1Login, USER_PASSWORD); + + assertThatExceptionOfType(LocalVCForbiddenException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ)) + .withMessageContaining("solution"); + } + + @Test + void testFetch_studentAccessesTemplateRepo_isForbidden() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/git-upload-pack", student1Login, USER_PASSWORD); + + assertThatExceptionOfType(LocalVCForbiddenException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ)) + .withMessageContaining("exercise"); + } + + @Test + void testFetch_studentAccessesTestsRepo_isForbidden() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + testsRepositorySlug + ".git/git-upload-pack", student1Login, USER_PASSWORD); + + assertThatExceptionOfType(LocalVCForbiddenException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ)) + .withMessageContaining("tests"); + } + + @Test + void testPush_studentAccessesSolutionRepo_isForbidden() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + solutionRepositorySlug + ".git/git-receive-pack", student1Login, USER_PASSWORD); + + assertThatExceptionOfType(LocalVCForbiddenException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.WRITE)) + .withMessageContaining("solution"); + } + + @Test + void testPush_studentAccessesTemplateRepo_isForbidden() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/git-receive-pack", student1Login, USER_PASSWORD); + + assertThatExceptionOfType(LocalVCForbiddenException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.WRITE)) + .withMessageContaining("exercise"); + } + + @Test + void testPush_studentAccessesTestsRepo_isForbidden() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + testsRepositorySlug + ".git/git-receive-pack", student1Login, USER_PASSWORD); + + assertThatExceptionOfType(LocalVCForbiddenException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.WRITE)) + .withMessageContaining("tests"); + } + + // Verify /info/refs endpoint applies the same rules (consistent across all URL paths) + + @Test + void testFetch_studentAccessesSolutionRepoViaInfoRefs_isForbidden() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + solutionRepositorySlug + ".git/info/refs", student1Login, USER_PASSWORD); + + assertThatExceptionOfType(LocalVCForbiddenException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ)) + .withMessageContaining("solution"); + } + + @Test + void testFetch_studentAccessesTemplateRepoViaInfoRefs_isForbidden() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/info/refs", student1Login, USER_PASSWORD); + + assertThatExceptionOfType(LocalVCForbiddenException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ)) + .withMessageContaining("exercise"); + } + + // == Authorization tests: TA access to staff repositories == + // Covers checkAccessToStaffRepository: TA + READ → succeeds, TA + WRITE → throws + + @Test + void testFetch_tutorAccessesSolutionRepo_succeeds() throws Exception { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + solutionRepositorySlug + ".git/git-upload-pack", tutor1Login, USER_PASSWORD); + + localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ); + } + + @Test + void testFetch_tutorAccessesTemplateRepo_succeeds() throws Exception { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/git-upload-pack", tutor1Login, USER_PASSWORD); + + localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ); + } + + @Test + void testFetch_tutorAccessesTestsRepo_succeeds() throws Exception { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + testsRepositorySlug + ".git/git-upload-pack", tutor1Login, USER_PASSWORD); + + localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ); + } + + @Test + void testPush_tutorAccessesSolutionRepo_isForbidden() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + solutionRepositorySlug + ".git/git-receive-pack", tutor1Login, USER_PASSWORD); + + assertThatExceptionOfType(LocalVCForbiddenException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.WRITE)) + .withMessageContaining("push"); + } + + @Test + void testPush_tutorAccessesTemplateRepo_isForbidden() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/git-receive-pack", tutor1Login, USER_PASSWORD); + + assertThatExceptionOfType(LocalVCForbiddenException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.WRITE)) + .withMessageContaining("push"); + } + + @Test + void testPush_tutorAccessesTestsRepo_isForbidden() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + testsRepositorySlug + ".git/git-receive-pack", tutor1Login, USER_PASSWORD); + + assertThatExceptionOfType(LocalVCForbiddenException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.WRITE)) + .withMessageContaining("push"); + } + + // == Authorization tests: instructor/editor access to staff repositories == + // Covers checkAccessToStaffRepository: editor+ → succeeds for both READ and WRITE + + @Test + void testFetch_instructorAccessesSolutionRepo_succeeds() throws Exception { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + solutionRepositorySlug + ".git/git-upload-pack", instructor1Login, USER_PASSWORD); + + localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ); + } + + @Test + void testPush_instructorAccessesSolutionRepo_succeeds() throws Exception { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + solutionRepositorySlug + ".git/git-receive-pack", instructor1Login, USER_PASSWORD); + + localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.WRITE); + } + + @Test + void testFetch_instructorAccessesTemplateRepo_succeeds() throws Exception { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/git-upload-pack", instructor1Login, USER_PASSWORD); + + localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ); + } + + @Test + void testPush_instructorAccessesTemplateRepo_succeeds() throws Exception { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/git-receive-pack", instructor1Login, USER_PASSWORD); + + localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.WRITE); + } + + @Test + void testFetch_instructorAccessesTestsRepo_succeeds() throws Exception { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + testsRepositorySlug + ".git/info/refs", instructor1Login, USER_PASSWORD); + + localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ); + } + + @Test + void testPush_instructorAccessesTestsRepo_succeeds() throws Exception { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + testsRepositorySlug + ".git/git-receive-pack", instructor1Login, USER_PASSWORD); + + localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.WRITE); + } + + // == Consistency: all URL patterns are authenticated the same way == + + @Test + void testFetch_nonExistentUser_isRejectedOnInfoRefs() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/info/refs", "hacker", "randompassword"); + + assertThatExceptionOfType(LocalVCAuthException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.READ)); + } + + @Test + void testPush_nonExistentUser_isRejectedOnInfoRefs() { + MockHttpServletRequest request = createGitRequest("/git/" + projectKey1 + "/" + templateRepositorySlug + ".git/info/refs", "hacker", "randompassword"); + + assertThatExceptionOfType(LocalVCAuthException.class).isThrownBy(() -> localVCServletService.authenticateAndAuthorizeGitRequest(request, RepositoryActionType.WRITE)); + } + + /** + * Creates a MockHttpServletRequest with Basic authentication for git endpoints. + */ + private MockHttpServletRequest createGitRequest(String requestUri, String username, String password) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI(requestUri); + request.setRemoteAddr("127.0.0.1"); + String credentials = Base64.getEncoder().encodeToString((username + ":" + password).getBytes()); + request.addHeader(HttpHeaders.AUTHORIZATION, "Basic " + credentials); + return request; + } + @Test void testFilesLargerThan10MbAreRejected() throws Exception { localVCLocalCITestService.createParticipation(programmingExercise, student1Login);