From bb4e6c8bb537f43adc9ab4fc0a7471a24eea802a Mon Sep 17 00:00:00 2001 From: entholzer Date: Sun, 17 Nov 2024 22:40:41 +0100 Subject: [PATCH 01/14] refactored query usage, SSH works nicely, for HTTPS state keeping is still a challenge --- ...xerciseStudentParticipationRepository.java | 11 +- ...ammingExerciseParticipationRepository.java | 7 +- ...ammingExerciseParticipationRepository.java | 7 +- ...ogrammingExerciseParticipationService.java | 69 ++++------- .../service/RepositoryService.java | 7 +- .../localvc/ArtemisGitServletService.java | 17 ++- .../service/localvc/HttpsConstants.java | 12 ++ .../localvc/LocalVCFetchPreUploadHook.java | 8 +- .../localvc/LocalVCFetchPreUploadHookSSH.java | 8 +- .../service/localvc/LocalVCPostPushHook.java | 28 ++++- .../service/localvc/LocalVCPushFilter.java | 17 ++- .../localvc/LocalVCServletService.java | 115 +++++++++++------- .../SshGitLocationResolverService.java | 4 +- .../service/localvc/VcsAccessLogService.java | 14 ++- .../service/localvc/ssh/SshConstants.java | 9 ++ .../service/localvc/ssh/SshGitCommand.java | 4 +- .../web/repository/RepositoryResource.java | 4 +- 17 files changed, 204 insertions(+), 137 deletions(-) create mode 100644 src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/HttpsConstants.java diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java index cc1f57c533fa..6323ff4d68ab 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java @@ -78,14 +78,11 @@ Optional findByIdWithAllResultsAndRelat List findAllByExerciseIdAndStudentLogin(long exerciseId, String username); - default ProgrammingExerciseStudentParticipation findByExerciseIdAndStudentLoginOrThrow(long exerciseId, String username) { - return getValueElseThrow(findByExerciseIdAndStudentLogin(exerciseId, username)); - } - - Optional findByRepositoryUri(String repositoryUri); + @EntityGraph(type = LOAD, attributePaths = { "submissions" }) + Optional findWithSubmissionsByRepositoryUri(String repositoryUri); - default ProgrammingExerciseStudentParticipation findByRepositoryUriElseThrow(String repositoryUri) { - return getValueElseThrow(findByRepositoryUri(repositoryUri)); + default ProgrammingExerciseStudentParticipation findWithSubmissionsByRepositoryUriElseThrow(String repositoryUri) { + return getValueElseThrow(findWithSubmissionsByRepositoryUri(repositoryUri)); } @EntityGraph(type = LOAD, attributePaths = { "submissions" }) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/SolutionProgrammingExerciseParticipationRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/SolutionProgrammingExerciseParticipationRepository.java index 4297b39f9ea9..716c0b6301d9 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/SolutionProgrammingExerciseParticipationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/SolutionProgrammingExerciseParticipationRepository.java @@ -43,10 +43,11 @@ public interface SolutionProgrammingExerciseParticipationRepository """) Optional findByBuildPlanIdWithResults(@Param("buildPlanId") String buildPlanId); - Optional findByRepositoryUri(String repositoryUri); + @EntityGraph(type = LOAD, attributePaths = { "submissions" }) + Optional findWithSubmissionsByRepositoryUri(String repositoryUri); - default SolutionProgrammingExerciseParticipation findByRepositoryUriElseThrow(String repositoryUri) { - return getValueElseThrow(findByRepositoryUri(repositoryUri)); + default SolutionProgrammingExerciseParticipation findWithSubmissionsByRepositoryUriElseThrow(String repositoryUri) { + return getValueElseThrow(findWithSubmissionsByRepositoryUri(repositoryUri)); } @EntityGraph(type = LOAD, attributePaths = { "results", "submissions", "submissions.results" }) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/TemplateProgrammingExerciseParticipationRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/TemplateProgrammingExerciseParticipationRepository.java index 67795b58ae54..b661c03c00f2 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/TemplateProgrammingExerciseParticipationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/TemplateProgrammingExerciseParticipationRepository.java @@ -48,10 +48,11 @@ default TemplateProgrammingExerciseParticipation findWithEagerResultsAndSubmissi return getValueElseThrow(findWithEagerResultsAndSubmissionsByProgrammingExerciseId(exerciseId)); } - Optional findByRepositoryUri(String repositoryUri); + @EntityGraph(type = LOAD, attributePaths = { "submissions" }) + Optional findWithSubmissionsByRepositoryUri(String repositoryUri); - default TemplateProgrammingExerciseParticipation findByRepositoryUriElseThrow(String repositoryUri) { - return getValueElseThrow(findByRepositoryUri(repositoryUri)); + default TemplateProgrammingExerciseParticipation findWithSubmissionsByRepositoryUriElseThrow(String repositoryUri) { + return getValueElseThrow(findWithSubmissionsByRepositoryUri(repositoryUri)); } @EntityGraph(type = LOAD, attributePaths = { "results", "results.feedbacks", "results.feedbacks.testCase", "submissions" }) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java index bd8db678b5a2..7a4ec1c8e1a0 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java @@ -127,25 +127,18 @@ public TemplateProgrammingExerciseParticipation findTemplateParticipationByProgr /** * Tries to retrieve a team participation for the given exercise and team short name. * - * @param exercise the exercise for which to find a participation. - * @param teamShortName of the team to which the participation belongs. - * @param withSubmissions true if the participation should be fetched with its submissions. + * @param exercise the exercise for which to find a participation. + * @param teamShortName of the team to which the participation belongs. * @return the participation for the given exercise and team. * @throws EntityNotFoundException if the team participation was not found. */ - public ProgrammingExerciseStudentParticipation findTeamParticipationByExerciseAndTeamShortNameOrThrow(ProgrammingExercise exercise, String teamShortName, - boolean withSubmissions) { + public ProgrammingExerciseStudentParticipation findTeamParticipationByExerciseAndTeamShortNameOrThrow(ProgrammingExercise exercise, String teamShortName) { Optional participationOptional; // It is important to fetch all students of the team here, because the local VC and local CI system use this participation to check if the authenticated user is part of the // team. - if (withSubmissions) { - participationOptional = studentParticipationRepository.findWithSubmissionsAndEagerStudentsByExerciseIdAndTeamShortName(exercise.getId(), teamShortName); - } - else { - participationOptional = studentParticipationRepository.findWithEagerStudentsByExerciseIdAndTeamShortName(exercise.getId(), teamShortName); - } + participationOptional = studentParticipationRepository.findWithSubmissionsAndEagerStudentsByExerciseIdAndTeamShortName(exercise.getId(), teamShortName); if (participationOptional.isEmpty()) { throw new EntityNotFoundException("Participation could not be found by exerciseId " + exercise.getId() + " and team short name " + teamShortName); @@ -169,25 +162,19 @@ public Optional findTeamParticipationBy /** * Tries to retrieve a student participation for the given exercise and username and test run flag. * - * @param exercise the exercise for which to find a participation. - * @param username of the user to which the participation belongs. - * @param isTestRun true if the participation is a test run participation. - * @param withSubmissions true if the participation should be loaded with its submissions. + * @param exercise the exercise for which to find a participation. + * @param username of the user to which the participation belongs. + * @param isTestRun true if the participation is a test run participation. * @return the participation for the given exercise and user. * @throws EntityNotFoundException if there is no participation for the given exercise and user. */ @NotNull public ProgrammingExerciseStudentParticipation findStudentParticipationByExerciseAndStudentLoginAndTestRunOrThrow(ProgrammingExercise exercise, String username, - boolean isTestRun, boolean withSubmissions) { + boolean isTestRun) { Optional participationOptional; - if (withSubmissions) { - participationOptional = studentParticipationRepository.findWithSubmissionsByExerciseIdAndStudentLoginAndTestRun(exercise.getId(), username, isTestRun); - } - else { - participationOptional = studentParticipationRepository.findByExerciseIdAndStudentLoginAndTestRun(exercise.getId(), username, isTestRun); - } + participationOptional = studentParticipationRepository.findWithSubmissionsByExerciseIdAndStudentLoginAndTestRun(exercise.getId(), username, isTestRun); if (participationOptional.isEmpty()) { throw new EntityNotFoundException("Participation could not be found by exerciseId " + exercise.getId() + " and user " + username); @@ -449,39 +436,29 @@ public void resetRepository(VcsRepositoryUri targetURL, VcsRepositoryUri sourceU * @param exercise the exercise for which to get the participation. * @param repositoryTypeOrUserName the repository type ("exercise", "solution", or "tests") or username (e.g. "artemis_test_user_1") as extracted from the repository URI. * @param isPracticeRepository whether the repository is a practice repository, i.e. the repository URI contains "-practice-". - * @param withSubmissions whether submissions should be loaded with the participation. This is needed for the local CI system. + * * @return the participation. * @throws EntityNotFoundException if the participation could not be found. */ - public ProgrammingExerciseParticipation retrieveParticipationForRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName, boolean isPracticeRepository, - boolean withSubmissions) { + public ProgrammingExerciseParticipation retrieveParticipationWithSubmissionsByRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName, + boolean isPracticeRepository) { boolean isAuxiliaryRepository = auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise); // For pushes to the tests repository, the solution repository is built first, and thus we need the solution participation. // Can possibly be used by auxiliary repositories if (repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString()) || repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString()) || isAuxiliaryRepository) { - if (withSubmissions) { - return solutionParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); - } - else { - return solutionParticipationRepository.findByProgrammingExerciseIdElseThrow(exercise.getId()); - } + return solutionParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); } if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { - if (withSubmissions) { - return templateParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); - } - else { - return templateParticipationRepository.findByProgrammingExerciseIdElseThrow(exercise.getId()); - } + return templateParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); } if (exercise.isTeamMode()) { // The repositoryTypeOrUserName is the team short name. // For teams, there is no practice participation. - return findTeamParticipationByExerciseAndTeamShortNameOrThrow(exercise, repositoryTypeOrUserName, withSubmissions); + return findTeamParticipationByExerciseAndTeamShortNameOrThrow(exercise, repositoryTypeOrUserName); } // If the exercise is an exam exercise and the repository's owner is at least an editor, the repository could be a test run repository, or it could be the instructor's @@ -492,14 +469,10 @@ public ProgrammingExerciseParticipation retrieveParticipationForRepository(Progr boolean isExamEditorRepository = exercise.isExamExercise() && authorizationCheckService.isAtLeastEditorForExercise(exercise, userRepository.getUserByLoginElseThrow(repositoryTypeOrUserName)); if (isExamEditorRepository) { - if (withSubmissions) { - return studentParticipationRepository.findWithSubmissionsByExerciseIdAndStudentLoginOrThrow(exercise.getId(), repositoryTypeOrUserName); - } - - return studentParticipationRepository.findByExerciseIdAndStudentLoginOrThrow(exercise.getId(), repositoryTypeOrUserName); + return studentParticipationRepository.findWithSubmissionsByExerciseIdAndStudentLoginOrThrow(exercise.getId(), repositoryTypeOrUserName); } - return findStudentParticipationByExerciseAndStudentLoginAndTestRunOrThrow(exercise, repositoryTypeOrUserName, isPracticeRepository, withSubmissions); + return findStudentParticipationByExerciseAndStudentLoginAndTestRunOrThrow(exercise, repositoryTypeOrUserName, isPracticeRepository); } /** @@ -510,17 +483,17 @@ public ProgrammingExerciseParticipation retrieveParticipationForRepository(Progr * @param repositoryURI the participation's repository URL * @return the participation belonging to the provided repositoryURI and repository type or username */ - public ProgrammingExerciseParticipation retrieveParticipationForRepository(String repositoryTypeOrUserName, String repositoryURI) { + public ProgrammingExerciseParticipation retrieveParticipationWithSubmissionsByRepository(String repositoryTypeOrUserName, String repositoryURI) { if (repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString()) || repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { - return solutionParticipationRepository.findByRepositoryUriElseThrow(repositoryURI); + return solutionParticipationRepository.findWithSubmissionsByRepositoryUriElseThrow(repositoryURI); } if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { - return templateParticipationRepository.findByRepositoryUriElseThrow(repositoryURI); + return templateParticipationRepository.findWithSubmissionsByRepositoryUriElseThrow(repositoryURI); } if (repositoryTypeOrUserName.equals(RepositoryType.AUXILIARY.toString())) { throw new EntityNotFoundException("Auxiliary repositories do not have participations."); } - return studentParticipationRepository.findByRepositoryUriElseThrow(repositoryURI); + return studentParticipationRepository.findWithSubmissionsByRepositoryUriElseThrow(repositoryURI); } /** diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java index 9833a9ae040b..2e7627d7e22c 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java @@ -45,6 +45,7 @@ import de.tum.cit.aet.artemis.programming.domain.ProgrammingExerciseParticipation; import de.tum.cit.aet.artemis.programming.domain.Repository; import de.tum.cit.aet.artemis.programming.domain.RepositoryType; +import de.tum.cit.aet.artemis.programming.domain.VcsAccessLog; import de.tum.cit.aet.artemis.programming.domain.VcsRepositoryUri; import de.tum.cit.aet.artemis.programming.dto.FileMove; import de.tum.cit.aet.artemis.programming.service.localvc.VcsAccessLogService; @@ -454,12 +455,10 @@ public void pullChanges(Repository repository) { * @param domainId the id of the domain Object (participation) owning the repository * @throws GitAPIException if the staging/committing process fails. */ - public void commitChanges(Repository repository, User user, Long domainId) throws GitAPIException { + public Optional commitChanges(Repository repository, User user, Long domainId) throws GitAPIException { gitService.stageAllChanges(repository); gitService.commitAndPush(repository, "Changes by Online Editor", true, user); - if (vcsAccessLogService.isPresent()) { - vcsAccessLogService.get().storeCodeEditorAccessLog(repository, user, domainId); - } + return vcsAccessLogService.isPresent() ? vcsAccessLogService.get().createPreliminaryCodeEditorAccessLog(repository, user, domainId) : Optional.empty(); } /** 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 0bfbbbe700e0..08fc53cda745 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 @@ -21,6 +21,8 @@ public class ArtemisGitServletService extends GitServlet { private final LocalVCServletService localVCServletService; + private int c; + /** * Constructor for ArtemisGitServlet. * @@ -53,14 +55,19 @@ public void init() { // Add filters that every request to the JGit Servlet goes through, one for each fetch request, and one for each push request. this.addUploadPackFilter(new LocalVCFetchFilter(localVCServletService)); - this.addReceivePackFilter(new LocalVCPushFilter(localVCServletService)); + + // TODO figure out a way to keep state for an HTTP session + this.addReceivePackFilter(new LocalVCPushFilter(localVCServletService, (v) -> { + this.c = v + 1; + return c; + })); this.setReceivePackFactory((request, repository) -> { ReceivePack receivePack = new ReceivePack(repository); // Add a hook that prevents illegal actions on push (delete branch, rename branch, force push). - receivePack.setPreReceiveHook(new LocalVCPrePushHook(localVCServletService, (User) request.getAttribute("user"))); + receivePack.setPreReceiveHook(new LocalVCPrePushHook(localVCServletService, (User) request.getSession().getAttribute("user"))); // Add a hook that triggers the creation of a new submission after the push went through successfully. - receivePack.setPostReceiveHook(new LocalVCPostPushHook(localVCServletService)); + receivePack.setPostReceiveHook(new LocalVCPostPushHook(localVCServletService, request)); return receivePack; }); @@ -72,4 +79,8 @@ public void init() { return uploadPack; }); } + + void setC(int c) { + this.c = c; + } } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/HttpsConstants.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/HttpsConstants.java new file mode 100644 index 000000000000..6502266dfdf0 --- /dev/null +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/HttpsConstants.java @@ -0,0 +1,12 @@ +package de.tum.cit.aet.artemis.programming.service.localvc; + +public class HttpsConstants { + + public static final String USER_KEY = "user"; + + public static final String PARTICIPATION_KEY = "participation"; + + public static final String VCS_ACCESS_LOG_KEY = "vcs_access_log"; + + public static final String EXERCISE_KEY = "exercise"; +} diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHook.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHook.java index 97e7523991d7..f08d32371243 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHook.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHook.java @@ -8,6 +8,8 @@ import org.eclipse.jgit.transport.PreUploadHook; import org.eclipse.jgit.transport.UploadPack; +import de.tum.cit.aet.artemis.programming.domain.VcsAccessLog; + public class LocalVCFetchPreUploadHook implements PreUploadHook { private final LocalVCServletService localVCServletService; @@ -21,7 +23,11 @@ public LocalVCFetchPreUploadHook(LocalVCServletService localVCServletService, Ht @Override public void onBeginNegotiateRound(UploadPack uploadPack, Collection collection, int clientOffered) { - localVCServletService.updateVCSAccessLogForCloneAndPullHTTPS(request, clientOffered); + var vcsAccessLog = request.getAttribute(HttpsConstants.VCS_ACCESS_LOG_KEY); + String authorizationHeader = request.getHeader(LocalVCServletService.AUTHORIZATION_HEADER); + if (vcsAccessLog instanceof VcsAccessLog log) { + localVCServletService.updateAndStoreVCSAccessLogForCloneAndPullHTTPS(log, authorizationHeader, clientOffered); + } } @Override diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHookSSH.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHookSSH.java index 09f79348c180..7b2a5465cc97 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHookSSH.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHookSSH.java @@ -1,6 +1,5 @@ package de.tum.cit.aet.artemis.programming.service.localvc; -import java.nio.file.Path; import java.util.Collection; import org.apache.sshd.server.session.ServerSession; @@ -14,17 +13,14 @@ public class LocalVCFetchPreUploadHookSSH implements PreUploadHook { private final ServerSession serverSession; - private final Path rootDir; - - public LocalVCFetchPreUploadHookSSH(LocalVCServletService localVCServletService, ServerSession serverSession, Path rootDir) { + public LocalVCFetchPreUploadHookSSH(LocalVCServletService localVCServletService, ServerSession serverSession) { this.localVCServletService = localVCServletService; this.serverSession = serverSession; - this.rootDir = rootDir; } @Override public void onBeginNegotiateRound(UploadPack uploadPack, Collection collection, int clientOffered) { - localVCServletService.updateVCSAccessLogForCloneAndPullSSH(serverSession, rootDir, clientOffered); + localVCServletService.updateAndStoreVCSAccessLogForCloneAndPullSSH(serverSession, clientOffered); } @Override diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPostPushHook.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPostPushHook.java index fb925717a387..0d824e7d5eea 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPostPushHook.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPostPushHook.java @@ -2,7 +2,11 @@ import java.util.Collection; import java.util.Iterator; +import java.util.Optional; +import jakarta.servlet.http.HttpServletRequest; + +import org.apache.sshd.server.session.ServerSession; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.PostReceiveHook; import org.eclipse.jgit.transport.ReceiveCommand; @@ -10,6 +14,10 @@ import de.tum.cit.aet.artemis.core.exception.LocalCIException; import de.tum.cit.aet.artemis.core.exception.VersionControlException; +import de.tum.cit.aet.artemis.programming.domain.ProgrammingExercise; +import de.tum.cit.aet.artemis.programming.domain.ProgrammingExerciseParticipation; +import de.tum.cit.aet.artemis.programming.domain.VcsAccessLog; +import de.tum.cit.aet.artemis.programming.service.localvc.ssh.SshConstants; /** * Contains an onPostReceive method that is called by JGit after a push has been received (i.e. after the pushed files were successfully written to disk). @@ -18,8 +26,24 @@ public class LocalVCPostPushHook implements PostReceiveHook { private final LocalVCServletService localVCServletService; - public LocalVCPostPushHook(LocalVCServletService localVCServletService) { + private final ProgrammingExerciseParticipation participation; + + private final ProgrammingExercise exercise; + + private final VcsAccessLog vcsAccessLog; + + public LocalVCPostPushHook(LocalVCServletService localVCServletService, ServerSession serverSession) { + this.localVCServletService = localVCServletService; + this.participation = serverSession.getAttribute(SshConstants.PARTICIPATION_KEY); + this.exercise = serverSession.getAttribute(SshConstants.REPOSITORY_EXERCISE_KEY); + this.vcsAccessLog = serverSession.getAttribute(SshConstants.VCS_ACCESS_LOG_KEY); + } + + public LocalVCPostPushHook(LocalVCServletService localVCServletService, HttpServletRequest request) { this.localVCServletService = localVCServletService; + this.participation = (ProgrammingExerciseParticipation) request.getSession().getAttribute(HttpsConstants.PARTICIPATION_KEY); + this.exercise = (ProgrammingExercise) request.getSession().getAttribute(HttpsConstants.EXERCISE_KEY); + this.vcsAccessLog = (VcsAccessLog) request.getSession().getAttribute(HttpsConstants.VCS_ACCESS_LOG_KEY); } /** @@ -60,7 +84,7 @@ public void onPostReceive(ReceivePack receivePack, Collection co Repository repository = receivePack.getRepository(); try { - localVCServletService.processNewPush(commitHash, repository); + localVCServletService.processNewPush(commitHash, repository, Optional.ofNullable(exercise), Optional.ofNullable(participation), Optional.ofNullable(vcsAccessLog)); } catch (LocalCIException e) { // Return an error message to the user. diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPushFilter.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPushFilter.java index df082b7362c2..46bf296f465e 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPushFilter.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPushFilter.java @@ -1,6 +1,7 @@ package de.tum.cit.aet.artemis.programming.service.localvc; import java.io.IOException; +import java.util.function.IntUnaryOperator; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; @@ -15,6 +16,7 @@ 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.exception.localvc.LocalVCInternalException; +import de.tum.cit.aet.artemis.programming.domain.VcsAccessLog; import de.tum.cit.aet.artemis.programming.web.repository.RepositoryActionType; /** @@ -26,8 +28,11 @@ public class LocalVCPushFilter extends OncePerRequestFilter { private final LocalVCServletService localVCServletService; - public LocalVCPushFilter(LocalVCServletService localVCServletService) { + private final IntUnaryOperator callbck; + + public LocalVCPushFilter(LocalVCServletService localVCServletService, IntUnaryOperator op) { this.localVCServletService = localVCServletService; + this.callbck = op; } /** @@ -51,11 +56,11 @@ public void doFilterInternal(HttpServletRequest servletRequest, HttpServletRespo // We need to extract the content of the request here as it is garbage collected before it can be used asynchronously String authorizationHeader = servletRequest.getHeader(LocalVCServletService.AUTHORIZATION_HEADER); String method = servletRequest.getMethod(); - LocalVCRepositoryUri localVCRepositoryUri = localVCServletService.parseRepositoryUri(servletRequest); - - this.localVCServletService.updateVCSAccessLogForPushHTTPS(method, authorizationHeader, localVCRepositoryUri); - + var vcsAccessLog = servletRequest.getAttribute(HttpsConstants.VCS_ACCESS_LOG_KEY); + if (vcsAccessLog instanceof VcsAccessLog accessLog) { + this.localVCServletService.updateAndStoreVCSAccessLogForPushHTTPS(method, authorizationHeader, accessLog); + } + this.callbck.applyAsInt(4); filterChain.doFilter(servletRequest, servletResponse); - } } 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 986f3837b7af..47ca59c4e785 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 @@ -16,7 +16,6 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.concurrent.CompletableFuture; import jakarta.servlet.http.HttpServletRequest; @@ -52,6 +51,7 @@ 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.util.TimeLogUtil; +import de.tum.cit.aet.artemis.exercise.domain.participation.Participation; import de.tum.cit.aet.artemis.programming.domain.AuthenticationMechanism; import de.tum.cit.aet.artemis.programming.domain.Commit; import de.tum.cit.aet.artemis.programming.domain.ProgrammingExercise; @@ -60,6 +60,7 @@ import de.tum.cit.aet.artemis.programming.domain.ProgrammingSubmission; import de.tum.cit.aet.artemis.programming.domain.RepositoryType; import de.tum.cit.aet.artemis.programming.domain.SolutionProgrammingExerciseParticipation; +import de.tum.cit.aet.artemis.programming.domain.VcsAccessLog; import de.tum.cit.aet.artemis.programming.repository.ParticipationVCSAccessTokenRepository; import de.tum.cit.aet.artemis.programming.repository.ProgrammingExerciseRepository; import de.tum.cit.aet.artemis.programming.service.AuxiliaryRepositoryService; @@ -253,9 +254,7 @@ public void authenticateAndAuthorizeGitRequest(HttpServletRequest request, Repos var authenticationMechanism = resolveAuthenticationMechanism(authorizationHeader, user); var ipAddress = request.getRemoteAddr(); - authorizeUser(repositoryTypeOrUserName, user, exercise, repositoryAction, authenticationMechanism, ipAddress, localVCRepositoryUri); - - request.setAttribute("user", user); + authorizeUser(repositoryTypeOrUserName, user, exercise, repositoryAction, authenticationMechanism, ipAddress, localVCRepositoryUri, Optional.of(request), Optional.empty()); log.debug("Authorizing user {} for repository {} took {}", user.getLogin(), localVCRepositoryUri, TimeLogUtil.formatDurationFrom(timeNanoStart)); } @@ -421,9 +420,12 @@ private UsernameAndPassword extractUsernameAndPassword(String authorizationHeade * @throws LocalVCForbiddenException If the user is not allowed to access the repository. */ public void authorizeUser(String repositoryTypeOrUserName, User user, ProgrammingExercise exercise, RepositoryActionType repositoryActionType, - AuthenticationMechanism authenticationMechanism, String ipAddress, LocalVCRepositoryUri localVCRepositoryUri) throws LocalVCForbiddenException { + AuthenticationMechanism authenticationMechanism, String ipAddress, LocalVCRepositoryUri localVCRepositoryUri, Optional request, + Optional serverSession) throws LocalVCForbiddenException { - if (repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString()) || auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise)) { + // Only load auxiliary repositories if a user is at least teaching assistant; students are not allowed to access them + if (authorizationCheckService.isAtLeastTeachingAssistantForExercise(exercise, user) && (repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString()) + || auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise))) { // Test and auxiliary repositories are only accessible by instructors and higher. try { repositoryAccessService.checkAccessTestOrAuxRepositoryElseThrow(repositoryActionType == RepositoryActionType.WRITE, exercise, user, repositoryTypeOrUserName); @@ -436,11 +438,13 @@ public void authorizeUser(String repositoryTypeOrUserName, User user, Programmin ProgrammingExerciseParticipation participation; try { - participation = programmingExerciseParticipationService.retrieveParticipationForRepository(exercise, repositoryTypeOrUserName, - localVCRepositoryUri.isPracticeRepository(), true); + // participation = programmingExerciseParticipationService.retrieveParticipationForRepository(exercise, repositoryTypeOrUserName, + // localVCRepositoryUri.isPracticeRepository(), true); // TODO Add this back in when we have figured out what is incorrect in the playwright configuration for (MySQL, Local) - // participation = programmingExerciseParticipationService.retrieveParticipationForRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString()); + // Here we load the participation of the repository, with submissions. It is reused throughout the workflow. + // + participation = programmingExerciseParticipationService.retrieveParticipationWithSubmissionsByRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString()); } catch (EntityNotFoundException e) { throw new LocalVCInternalException( @@ -453,14 +457,7 @@ public void authorizeUser(String repositoryTypeOrUserName, User user, Programmin catch (AccessForbiddenException e) { throw new LocalVCForbiddenException(e); } - - // Asynchronously store an VCS access log entry - CompletableFuture.runAsync(() -> storeAccessLogAsync(user, participation, repositoryActionType, authenticationMechanism, ipAddress, localVCRepositoryUri)) - .exceptionally(ex -> { - log.warn("Failed to asynchronously obtain commit hash or store access log for repository {}. Error: {}", localVCRepositoryUri.getRelativeRepositoryPath(), - ex.getMessage()); - return null; - }); + cacheAttributesInRequestOrSession(user, participation, repositoryActionType, authenticationMechanism, ipAddress, localVCRepositoryUri, request, serverSession); } /** @@ -474,8 +471,9 @@ public void authorizeUser(String repositoryTypeOrUserName, User user, Programmin * @param ipAddress the IP address of the user accessing the repository * @param localVCRepositoryUri the URI of the localVC repository */ - private void storeAccessLogAsync(User user, ProgrammingExerciseParticipation participation, RepositoryActionType repositoryActionType, - AuthenticationMechanism authenticationMechanism, String ipAddress, LocalVCRepositoryUri localVCRepositoryUri) { + private void cacheAttributesInRequestOrSession(User user, ProgrammingExerciseParticipation participation, RepositoryActionType repositoryActionType, + AuthenticationMechanism authenticationMechanism, String ipAddress, LocalVCRepositoryUri localVCRepositoryUri, Optional request, + Optional serverSession) { try { String commitHash; String relativeRepositoryPath = localVCRepositoryUri.getRelativeRepositoryPath().toString(); @@ -483,9 +481,22 @@ private void storeAccessLogAsync(User user, ProgrammingExerciseParticipation par commitHash = getLatestCommitHash(repository); } - RepositoryActionType finalRepositoryActionType = repositoryActionType == RepositoryActionType.READ ? RepositoryActionType.PULL : RepositoryActionType.PUSH; - vcsAccessLogService.ifPresent(service -> service.storeAccessLog(user, participation, finalRepositoryActionType, authenticationMechanism, commitHash, ipAddress)); + var finalRepositoryActionType = repositoryActionType == RepositoryActionType.READ ? RepositoryActionType.PULL : RepositoryActionType.PUSH; + var preliminaryAccessLog = new VcsAccessLog(user, (Participation) participation, user.getName(), user.getEmail(), finalRepositoryActionType, authenticationMechanism, + commitHash, ipAddress); + if (request.isPresent()) { + request.get().getSession().setAttribute(HttpsConstants.USER_KEY, user); + request.get().getSession().setAttribute(HttpsConstants.PARTICIPATION_KEY, participation); + request.get().getSession().setAttribute(HttpsConstants.VCS_ACCESS_LOG_KEY, preliminaryAccessLog); + } + else if (serverSession.isPresent()) { + serverSession.get().setAttribute(SshConstants.VCS_ACCESS_LOG_KEY, preliminaryAccessLog); + serverSession.get().setAttribute(SshConstants.PARTICIPATION_KEY, participation); + } + else { + log.warn("authorizeUser: Neither HTTP request nor server session found"); + } } catch (Exception e) { log.warn("Failed to obtain commit hash or store access log for repository {}. Error: {}", localVCRepositoryUri.getRelativeRepositoryPath().toString(), e.getMessage()); @@ -521,6 +532,19 @@ else if (exception instanceof LocalVCForbiddenException) { * @throws VersionControlException if the commit belongs to the wrong branch (i.e. not the default branch of the participation). */ public void processNewPush(String commitHash, Repository repository) { + processNewPush(commitHash, repository, Optional.empty(), Optional.empty(), Optional.empty()); + } + + /** + * Create a submission, trigger the respective build, and process the results. + * + * @param commitHash the hash of the last commit. + * @param repository the remote repository which was pushed to. //TODO fix docs + * @throws ContinuousIntegrationException if something goes wrong with the CI configuration. + * @throws VersionControlException if the commit belongs to the wrong branch (i.e. not the default branch of the participation). + */ + public void processNewPush(String commitHash, Repository repository, Optional cachedExercise, + Optional cachedParticipation, Optional vcsAccessLog) { long timeNanoStart = System.nanoTime(); Path repositoryFolderPath = repository.getDirectory().toPath(); @@ -529,10 +553,9 @@ public void processNewPush(String commitHash, Repository repository) { String repositoryTypeOrUserName = localVCRepositoryUri.getRepositoryTypeOrUserName(); String projectKey = localVCRepositoryUri.getProjectKey(); - - ProgrammingExercise exercise = getProgrammingExercise(projectKey); - - ProgrammingExerciseParticipation participation = getProgrammingExerciseParticipation(localVCRepositoryUri, repositoryTypeOrUserName, exercise); + ProgrammingExercise exercise = cachedExercise.orElseGet(() -> getProgrammingExercise(projectKey)); + ProgrammingExerciseParticipation participation = cachedParticipation + .orElseGet(() -> getProgrammingExerciseParticipation(localVCRepositoryUri, repositoryTypeOrUserName, exercise)); RepositoryType repositoryType = getRepositoryType(repositoryTypeOrUserName, exercise); @@ -559,7 +582,13 @@ public void processNewPush(String commitHash, Repository repository) { // For push the correct commitHash is only available here, therefore the preliminary null value is overwritten String finalCommitHash = commitHash; - vcsAccessLogService.ifPresent(service -> service.updateCommitHash(participation, finalCommitHash)); + if (vcsAccessLog.isPresent()) { + vcsAccessLog.get().setCommitHash(finalCommitHash); + vcsAccessLogService.ifPresent(service -> service.saveVcsAccesslog(vcsAccessLog.get())); + } + else { + log.error("No vcsAccessLog in processNewPush! It should not be empty at this point"); + } } catch (GitAPIException | IOException e) { // This catch clause does not catch exceptions that happen during runBuildJob() as that method is called asynchronously. @@ -576,8 +605,8 @@ private ProgrammingExerciseParticipation getProgrammingExerciseParticipation(Loc ProgrammingExercise exercise) { ProgrammingExerciseParticipation participation; try { - participation = programmingExerciseParticipationService.retrieveParticipationForRepository(exercise, repositoryTypeOrUserName, - localVCRepositoryUri.isPracticeRepository(), true); + participation = programmingExerciseParticipationService.retrieveParticipationWithSubmissionsByRepository(exercise, repositoryTypeOrUserName, + localVCRepositoryUri.isPracticeRepository()); } catch (EntityNotFoundException e) { throw new VersionControlException("Could not find participation for repository " + repositoryTypeOrUserName + " of exercise " + exercise, e); @@ -737,7 +766,7 @@ private Commit extractCommitInfo(String commitHash, Repository repository) throw private ProgrammingExerciseParticipation retrieveParticipationFromLocalVCRepositoryUri(LocalVCRepositoryUri localVCRepositoryUri) { String repositoryTypeOrUserName = localVCRepositoryUri.getRepositoryTypeOrUserName(); var repositoryURL = localVCRepositoryUri.toString().replace("/git-upload-pack", "").replace("/git-receive-pack", ""); - return programmingExerciseParticipationService.retrieveParticipationForRepository(repositoryTypeOrUserName, repositoryURL); + return programmingExerciseParticipationService.retrieveParticipationWithSubmissionsByRepository(repositoryTypeOrUserName, repositoryURL); } /** @@ -768,24 +797,23 @@ public static String getDefaultBranchOfRepository(Repository repository) { * This method logs the access information based on the incoming HTTP request. It checks if the action * is performed by a build job user and, if not, records the user's repository action (clone or pull). * The action type is determined based on the number of offers (`clientOffered`). + * // TODO docs * - * @param request the {@link HttpServletRequest} containing the HTTP request data, including headers. * @param clientOffered the number of objects offered by the client in the operation, used to determine * if the action is a clone (if 0) or a pull (if greater than 0). */ @Async - public void updateVCSAccessLogForCloneAndPullHTTPS(HttpServletRequest request, int clientOffered) { + public void updateAndStoreVCSAccessLogForCloneAndPullHTTPS(VcsAccessLog vcsAccessLog, String authorizationHeader, int clientOffered) { + try { - String authorizationHeader = request.getHeader(LocalVCServletService.AUTHORIZATION_HEADER); UsernameAndPassword usernameAndPassword = extractUsernameAndPassword(authorizationHeader); String userName = usernameAndPassword.username(); if (userName.equals(BUILD_USER_NAME)) { return; } RepositoryActionType repositoryActionType = getRepositoryActionReadType(clientOffered); - var participation = getExerciseParticipationFromRequest(request); - - vcsAccessLogService.ifPresent(service -> service.updateRepositoryActionType(participation, repositoryActionType)); + vcsAccessLog.setRepositoryActionType(repositoryActionType); + vcsAccessLogService.ifPresent(service -> service.saveVcsAccesslog(vcsAccessLog)); } catch (Exception ignored) { } @@ -799,13 +827,11 @@ public void updateVCSAccessLogForCloneAndPullHTTPS(HttpServletRequest request, i * * @param method the HTTP method of the request (expected to be "POST" for logging to occur) * @param authorizationHeader the authorization header containing the username and password in basic authentication format - * @param localVcUri the {@link LocalVCRepositoryUri} identifying the repository in the local version control system * * This method is asynchronous. - * */ @Async - public void updateVCSAccessLogForPushHTTPS(String method, String authorizationHeader, LocalVCRepositoryUri localVcUri) { + public void updateAndStoreVCSAccessLogForPushHTTPS(String method, String authorizationHeader, VcsAccessLog vcsAccessLog) { if (!method.equals("POST")) { return; } @@ -816,9 +842,8 @@ public void updateVCSAccessLogForPushHTTPS(String method, String authorizationHe return; } RepositoryActionType repositoryActionType = RepositoryActionType.PUSH; - var participation = retrieveParticipationFromLocalVCRepositoryUri(localVcUri); - - vcsAccessLogService.ifPresent(service -> service.updateRepositoryActionType(participation, repositoryActionType)); + vcsAccessLog.setRepositoryActionType(repositoryActionType); + vcsAccessLogService.ifPresent(service -> service.saveVcsAccesslog(vcsAccessLog)); } catch (Exception ignored) { } @@ -832,19 +857,19 @@ public void updateVCSAccessLogForPushHTTPS(String method, String authorizationHe * fetches participation details from the local VC repository URI. * * @param session the {@link ServerSession} representing the SSH session. - * @param rootDir the {@link Path} to the root directory of the repository. * @param clientOffered the number of objects offered by the client in the operation, used to determine * if the action is a clone (if 0) or a pull (if greater than 0). */ @Async - public void updateVCSAccessLogForCloneAndPullSSH(ServerSession session, Path rootDir, int clientOffered) { + public void updateAndStoreVCSAccessLogForCloneAndPullSSH(ServerSession session, int clientOffered) { try { if (session.getAttribute(SshConstants.USER_KEY).getName().equals(BUILD_USER_NAME)) { return; } + var accessLog = session.getAttribute(SshConstants.VCS_ACCESS_LOG_KEY); RepositoryActionType repositoryActionType = getRepositoryActionReadType(clientOffered); - var participation = retrieveParticipationFromLocalVCRepositoryUri(getLocalVCRepositoryUri(rootDir)); - vcsAccessLogService.ifPresent(service -> service.updateRepositoryActionType(participation, repositoryActionType)); + accessLog.setRepositoryActionType(repositoryActionType); + vcsAccessLogService.ifPresent(service -> service.saveVcsAccesslog(accessLog)); } catch (Exception ignored) { } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/SshGitLocationResolverService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/SshGitLocationResolverService.java index d45bfda1c179..ffb2850e127f 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/SshGitLocationResolverService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/SshGitLocationResolverService.java @@ -7,6 +7,7 @@ import java.nio.file.FileSystem; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Optional; import org.apache.sshd.git.GitLocationResolver; import org.apache.sshd.server.session.ServerSession; @@ -71,6 +72,7 @@ public Path resolveRootDirectory(String command, String[] args, ServerSession se // git-upload-pack means fetch (read operation), git-receive-pack means push (write operation) final var repositoryAction = gitCommand.equals("git-upload-pack") ? RepositoryActionType.READ : gitCommand.equals("git-receive-pack") ? RepositoryActionType.WRITE : null; final var user = session.getAttribute(SshConstants.USER_KEY); + session.setAttribute(SshConstants.REPOSITORY_EXERCISE_KEY, exercise); if (session.getAttribute(SshConstants.IS_BUILD_AGENT_KEY) && repositoryAction == RepositoryActionType.READ) { // We already checked for build agent authenticity @@ -78,7 +80,7 @@ public Path resolveRootDirectory(String command, String[] args, ServerSession se else { try { localVCServletService.authorizeUser(repositoryTypeOrUserName, user, exercise, repositoryAction, AuthenticationMechanism.SSH, session.getClientAddress().toString(), - localVCRepositoryUri); + localVCRepositoryUri, Optional.empty(), Optional.of(session)); } catch (LocalVCForbiddenException e) { log.error("User {} does not have access to the repository {}", user.getLogin(), repositoryPath); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java index af1972ce2e1c..69cf2e3e1246 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java @@ -2,6 +2,8 @@ import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_LOCALVC; +import java.util.Optional; + import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.slf4j.Logger; @@ -82,6 +84,10 @@ public void updateRepositoryActionType(ProgrammingExerciseParticipation particip }); } + public void saveVcsAccesslog(VcsAccessLog vcsAccessLog) { + vcsAccessLogRepository.save(vcsAccessLog); + } + /** * Stores the log for a push from the code editor. * @@ -90,17 +96,17 @@ public void updateRepositoryActionType(ProgrammingExerciseParticipation particip * @param participationId The id of the participation belonging to the repository * @throws GitAPIException if an error occurs while retrieving the git log */ - public void storeCodeEditorAccessLog(Repository repo, User user, Long participationId) throws GitAPIException { + public Optional createPreliminaryCodeEditorAccessLog(Repository repo, User user, Long participationId) throws GitAPIException { try (Git git = new Git(repo)) { String lastCommitHash = git.log().setMaxCount(1).call().iterator().next().getName(); var participation = participationRepository.findById(participationId); if (participation.isPresent() && participation.get() instanceof ProgrammingExerciseParticipation programmingParticipation) { log.debug("Storing access operation for user {}", user); - VcsAccessLog accessLogEntry = new VcsAccessLog(user, (Participation) programmingParticipation, user.getName(), user.getEmail(), RepositoryActionType.WRITE, - AuthenticationMechanism.CODE_EDITOR, lastCommitHash, null); - vcsAccessLogRepository.save(accessLogEntry); + return Optional.of(new VcsAccessLog(user, (Participation) programmingParticipation, user.getName(), user.getEmail(), RepositoryActionType.WRITE, + AuthenticationMechanism.CODE_EDITOR, lastCommitHash, null)); } } + return Optional.empty(); } } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/ssh/SshConstants.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/ssh/SshConstants.java index 1ea41e9d2d6e..f7a0f02258ed 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/ssh/SshConstants.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/ssh/SshConstants.java @@ -6,6 +6,9 @@ import org.springframework.context.annotation.Profile; import de.tum.cit.aet.artemis.core.domain.User; +import de.tum.cit.aet.artemis.programming.domain.ProgrammingExercise; +import de.tum.cit.aet.artemis.programming.domain.ProgrammingExerciseParticipation; +import de.tum.cit.aet.artemis.programming.domain.VcsAccessLog; @Profile(PROFILE_LOCALVC) public class SshConstants { @@ -13,4 +16,10 @@ public class SshConstants { public static final AttributeRepository.AttributeKey IS_BUILD_AGENT_KEY = new AttributeRepository.AttributeKey<>(); public static final AttributeRepository.AttributeKey USER_KEY = new AttributeRepository.AttributeKey<>(); + + public static final AttributeRepository.AttributeKey REPOSITORY_EXERCISE_KEY = new AttributeRepository.AttributeKey<>(); + + public static final AttributeRepository.AttributeKey VCS_ACCESS_LOG_KEY = new AttributeRepository.AttributeKey<>(); + + public static final AttributeRepository.AttributeKey PARTICIPATION_KEY = new AttributeRepository.AttributeKey<>(); } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/ssh/SshGitCommand.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/ssh/SshGitCommand.java index 7ec968646217..e078bce7fdbe 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/ssh/SshGitCommand.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/ssh/SshGitCommand.java @@ -85,13 +85,13 @@ public void run() { if (GenericUtils.isNotBlank(protocol)) { uploadPack.setExtraParameters(Collections.singleton(protocol)); } - uploadPack.setPreUploadHook(new LocalVCFetchPreUploadHookSSH(localVCServletService, getServerSession(), rootDir)); + uploadPack.setPreUploadHook(new LocalVCFetchPreUploadHookSSH(localVCServletService, getServerSession())); uploadPack.upload(getInputStream(), getOutputStream(), getErrorStream()); } else if (RemoteConfig.DEFAULT_RECEIVE_PACK.equals(subCommand)) { var receivePack = new ReceivePack(repository); receivePack.setPreReceiveHook(new LocalVCPrePushHook(localVCServletService, user)); - receivePack.setPostReceiveHook(new LocalVCPostPushHook(localVCServletService)); + receivePack.setPostReceiveHook(new LocalVCPostPushHook(localVCServletService, getServerSession())); receivePack.receive(getInputStream(), getOutputStream(), getErrorStream()); } else { diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryResource.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryResource.java index f9ca7350afd1..4a8e1e2810a0 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryResource.java @@ -281,12 +281,12 @@ public ResponseEntity commitChanges(Long domainId) { return executeAndCheckForExceptions(() -> { Repository repository = getRepository(domainId, RepositoryActionType.WRITE, true); - repositoryService.commitChanges(repository, user, domainId); + var vcsAccessLog = repositoryService.commitChanges(repository, user, domainId); // Trigger a build, and process the result. Only implemented for local CI. // For GitLab + Jenkins, webhooks were added when creating the repository, // that notify the CI system when the commit happens and thus trigger the build. if (profileService.isLocalVcsCiActive()) { - localVCServletService.orElseThrow().processNewPush(null, repository); + localVCServletService.orElseThrow().processNewPush(null, repository, Optional.empty(), Optional.empty(), vcsAccessLog); } return new ResponseEntity<>(HttpStatus.OK); }); From cc8d4978c2e6080b8f7493c5ab249cea1efb7ee9 Mon Sep 17 00:00:00 2001 From: entholzer Date: Mon, 18 Nov 2024 18:26:23 +0100 Subject: [PATCH 02/14] fixed access log again --- ...xerciseStudentParticipationRepository.java | 6 + ...ammingExerciseParticipationRepository.java | 6 + ...ammingExerciseParticipationRepository.java | 6 + .../repository/VcsAccessLogRepository.java | 77 ++++++++- ...ogrammingExerciseParticipationService.java | 22 +++ .../localvc/ArtemisGitServletService.java | 15 +- .../localvc/LocalVCFetchPreUploadHook.java | 8 +- .../localvc/LocalVCFetchPreUploadHookSSH.java | 2 + .../service/localvc/LocalVCPostPushHook.java | 11 +- .../service/localvc/LocalVCPushFilter.java | 14 +- .../localvc/LocalVCServletService.java | 162 +++++++++++------- .../SshGitLocationResolverService.java | 6 +- .../service/localvc/VcsAccessLogService.java | 29 ++-- 13 files changed, 239 insertions(+), 125 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java index 6323ff4d68ab..b7fa36e826b4 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java @@ -85,6 +85,12 @@ default ProgrammingExerciseStudentParticipation findWithSubmissionsByRepositoryU return getValueElseThrow(findWithSubmissionsByRepositoryUri(repositoryUri)); } + Optional findByRepositoryUri(String repositoryUri); + + default ProgrammingExerciseStudentParticipation findByRepositoryUriElseThrow(String repositoryUri) { + return getValueElseThrow(findByRepositoryUri(repositoryUri)); + } + @EntityGraph(type = LOAD, attributePaths = { "submissions" }) Optional findWithSubmissionsByExerciseIdAndStudentLogin(long exerciseId, String username); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/SolutionProgrammingExerciseParticipationRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/SolutionProgrammingExerciseParticipationRepository.java index 716c0b6301d9..572834303685 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/SolutionProgrammingExerciseParticipationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/SolutionProgrammingExerciseParticipationRepository.java @@ -50,6 +50,12 @@ default SolutionProgrammingExerciseParticipation findWithSubmissionsByRepository return getValueElseThrow(findWithSubmissionsByRepositoryUri(repositoryUri)); } + Optional findByRepositoryUri(String repositoryUri); + + default SolutionProgrammingExerciseParticipation findByRepositoryUriElseThrow(String repositoryUri) { + return getValueElseThrow(findByRepositoryUri(repositoryUri)); + } + @EntityGraph(type = LOAD, attributePaths = { "results", "submissions", "submissions.results" }) Optional findWithEagerResultsAndSubmissionsByProgrammingExerciseId(long exerciseId); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/TemplateProgrammingExerciseParticipationRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/TemplateProgrammingExerciseParticipationRepository.java index b661c03c00f2..b692ed3e8532 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/TemplateProgrammingExerciseParticipationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/TemplateProgrammingExerciseParticipationRepository.java @@ -55,6 +55,12 @@ default TemplateProgrammingExerciseParticipation findWithSubmissionsByRepository return getValueElseThrow(findWithSubmissionsByRepositoryUri(repositoryUri)); } + Optional findByRepositoryUri(String repositoryUri); + + default TemplateProgrammingExerciseParticipation findByRepositoryUriElseThrow(String repositoryUri) { + return getValueElseThrow(findByRepositoryUri(repositoryUri)); + } + @EntityGraph(type = LOAD, attributePaths = { "results", "results.feedbacks", "results.feedbacks.testCase", "submissions" }) Optional findWithEagerResultsAndFeedbacksAndTestCasesAndSubmissionsByProgrammingExerciseId(long exerciseId); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java index 76722cfd5eea..a82922245659 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java @@ -26,22 +26,81 @@ @Repository public interface VcsAccessLogRepository extends ArtemisJpaRepository { - /** - * Find the access log entry which does not have any commit hash yet - * - * @param participationId The id of the participation the repository belongs to - * @return a log entry belonging to the participationId, which has no commit hash - */ - @Query(""" + // @Transactional // ok because of modifying query + // @Modifying + // @Query(""" + // UPDATE VcsAccessLog vcsAccessLog + // SET vcsAccessLog.repositoryActionType = :repositoryActionType + // WHERE vcsAccessLog.id = ( + // SELECT MAX(log.id) + // FROM VcsAccessLog log + // WHERE log.participation.id = :participationId + // ) + // """) + // void updateRepositoryActionTypeForNewestLog(@Param("participationId") long participationId, @Param("repositoryActionType") RepositoryActionType repositoryActionType); - SELECT vcsAccessLog + // @Transactional // ok because of modifying query + // @Modifying + // @Query(""" + // UPDATE VcsAccessLog vcsAccessLog + // SET vcsAccessLog.repositoryActionType = :repositoryActionType + // WHERE vcsAccessLog.id = ( + // SELECT log.id + // FROM VcsAccessLog log + // JOIN ProgrammingExerciseStudentParticipation participation ON log.participation.id = participation.id + // WHERE participation.repositoryUri = :repositoryUri + // ORDER BY log.id DESC + // LIMIT 1 + // ) + // """) + // void updateRepositoryActionTypeForNewestLog(@Param("repositoryUri") String repositoryUri, @Param("repositoryActionType") RepositoryActionType repositoryActionType); + // + // + // @Transactional // ok because of modifying query + // @Modifying + // @Query(""" + // UPDATE VcsAccessLog vcsAccessLog + // SET vcsAccessLog.commitHash = :commitHash + // WHERE vcsAccessLog.participation.id = :participationId + // ORDER BY vcsAccessLog.id DESC + // LIMIT 1 + // """) + // void updateCommitHashForNewestLog(@Param("participationId") long participationId, @Param("commitHash") String commitHash); + + @Query(""" + SELECT vcsAccessLog FROM VcsAccessLog vcsAccessLog WHERE vcsAccessLog.participation.id = :participationId - ORDER BY vcsAccessLog.timestamp DESC + ORDER BY vcsAccessLog.id DESC LIMIT 1 """) Optional findNewestByParticipationId(@Param("participationId") long participationId); + @Query(""" + SELECT vcsAccessLog + FROM VcsAccessLog vcsAccessLog + LEFT JOIN FETCH ProgrammingExerciseStudentParticipation participation ON vcsAccessLog.participation.id = participation.id + WHERE participation.repositoryUri = :repositoryUri + ORDER BY vcsAccessLog.id DESC + LIMIT 1 + """) + Optional findNewestByRepositoryUri(@Param("repositoryUri") String repositoryUri); + + // @Transactional // ok because of modifying query + // @Modifying + // @Query(""" + // UPDATE VcsAccessLog vcsAccessLog + // SET vcsAccessLog.commitHash = :commitHash + // WHERE vcsAccessLog.id = ( + // SELECT MAX(log.id) + // FROM VcsAccessLog log + // LEFT JOIN FETCH ProgrammingExerciseStudentParticipation participation ON log.participation + // WHERE participation.repositoryUri = :repositoryUri + // ) + // """) + // void updateCommitHashForNewestLog(@Param("repositoryUri") String repositoryUri, @Param("commitHash") String commitHash); + // + /** * Retrieves a list of {@link VcsAccessLog} entities associated with the specified participation ID. * The results are ordered by the log ID in ascending order. diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java index 7a4ec1c8e1a0..016381d8a255 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java @@ -494,6 +494,28 @@ public ProgrammingExerciseParticipation retrieveParticipationWithSubmissionsByRe throw new EntityNotFoundException("Auxiliary repositories do not have participations."); } return studentParticipationRepository.findWithSubmissionsByRepositoryUriElseThrow(repositoryURI); + + } + + /** + * Get the participation for a given repository url and a repository type or user name. This method is used by the local VC system to get the + * participation for logging operations on the repository. + * + * @param repositoryTypeOrUserName the name of the user or the type of the repository + * @param repositoryURI the participation's repository URL + * @return the participation belonging to the provided repositoryURI and repository type or username + */ + public ProgrammingExerciseParticipation retrieveParticipationByRepository(String repositoryTypeOrUserName, String repositoryURI) { + if (repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString()) || repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { + return solutionParticipationRepository.findByRepositoryUriElseThrow(repositoryURI); + } + if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { + return templateParticipationRepository.findByRepositoryUriElseThrow(repositoryURI); + } + if (repositoryTypeOrUserName.equals(RepositoryType.AUXILIARY.toString())) { + throw new EntityNotFoundException("Auxiliary repositories do not have participations."); + } + return studentParticipationRepository.findByRepositoryUriElseThrow(repositoryURI); } /** 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 08fc53cda745..c8124d7711f0 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 @@ -21,8 +21,6 @@ public class ArtemisGitServletService extends GitServlet { private final LocalVCServletService localVCServletService; - private int c; - /** * Constructor for ArtemisGitServlet. * @@ -55,19 +53,15 @@ public void init() { // Add filters that every request to the JGit Servlet goes through, one for each fetch request, and one for each push request. this.addUploadPackFilter(new LocalVCFetchFilter(localVCServletService)); - - // TODO figure out a way to keep state for an HTTP session - this.addReceivePackFilter(new LocalVCPushFilter(localVCServletService, (v) -> { - this.c = v + 1; - return c; - })); + this.addReceivePackFilter(new LocalVCPushFilter(localVCServletService)); this.setReceivePackFactory((request, repository) -> { ReceivePack receivePack = new ReceivePack(repository); // Add a hook that prevents illegal actions on push (delete branch, rename branch, force push). + // user is always null here receivePack.setPreReceiveHook(new LocalVCPrePushHook(localVCServletService, (User) request.getSession().getAttribute("user"))); // Add a hook that triggers the creation of a new submission after the push went through successfully. - receivePack.setPostReceiveHook(new LocalVCPostPushHook(localVCServletService, request)); + receivePack.setPostReceiveHook(new LocalVCPostPushHook(localVCServletService)); return receivePack; }); @@ -80,7 +74,4 @@ public void init() { }); } - void setC(int c) { - this.c = c; - } } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHook.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHook.java index f08d32371243..6b5bdf6c5ca1 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHook.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHook.java @@ -8,8 +8,6 @@ import org.eclipse.jgit.transport.PreUploadHook; import org.eclipse.jgit.transport.UploadPack; -import de.tum.cit.aet.artemis.programming.domain.VcsAccessLog; - public class LocalVCFetchPreUploadHook implements PreUploadHook { private final LocalVCServletService localVCServletService; @@ -23,11 +21,9 @@ public LocalVCFetchPreUploadHook(LocalVCServletService localVCServletService, Ht @Override public void onBeginNegotiateRound(UploadPack uploadPack, Collection collection, int clientOffered) { - var vcsAccessLog = request.getAttribute(HttpsConstants.VCS_ACCESS_LOG_KEY); String authorizationHeader = request.getHeader(LocalVCServletService.AUTHORIZATION_HEADER); - if (vcsAccessLog instanceof VcsAccessLog log) { - localVCServletService.updateAndStoreVCSAccessLogForCloneAndPullHTTPS(log, authorizationHeader, clientOffered); - } + localVCServletService.updateAndStoreVCSAccessLogForCloneAndPullHTTPS(request, authorizationHeader, clientOffered); + } @Override diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHookSSH.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHookSSH.java index 7b2a5465cc97..6e9ace03a769 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHookSSH.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHookSSH.java @@ -25,9 +25,11 @@ public void onBeginNegotiateRound(UploadPack uploadPack, Collection collection, int i, int i1, boolean b) { + int bs = 4; } @Override public void onSendPack(UploadPack uploadPack, Collection collection, Collection collection1) { + int ss = 4; } } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPostPushHook.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPostPushHook.java index 0d824e7d5eea..f901ab89a85d 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPostPushHook.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPostPushHook.java @@ -32,6 +32,8 @@ public class LocalVCPostPushHook implements PostReceiveHook { private final VcsAccessLog vcsAccessLog; + private HttpServletRequest request; + public LocalVCPostPushHook(LocalVCServletService localVCServletService, ServerSession serverSession) { this.localVCServletService = localVCServletService; this.participation = serverSession.getAttribute(SshConstants.PARTICIPATION_KEY); @@ -39,11 +41,12 @@ public LocalVCPostPushHook(LocalVCServletService localVCServletService, ServerSe this.vcsAccessLog = serverSession.getAttribute(SshConstants.VCS_ACCESS_LOG_KEY); } - public LocalVCPostPushHook(LocalVCServletService localVCServletService, HttpServletRequest request) { + public LocalVCPostPushHook(LocalVCServletService localVCServletService) { this.localVCServletService = localVCServletService; - this.participation = (ProgrammingExerciseParticipation) request.getSession().getAttribute(HttpsConstants.PARTICIPATION_KEY); - this.exercise = (ProgrammingExercise) request.getSession().getAttribute(HttpsConstants.EXERCISE_KEY); - this.vcsAccessLog = (VcsAccessLog) request.getSession().getAttribute(HttpsConstants.VCS_ACCESS_LOG_KEY); + // For HTTPs we are unable to store the attributes in the sesseion or request unfortunately + this.participation = null; + this.exercise = null; + this.vcsAccessLog = null; } /** diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPushFilter.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPushFilter.java index 46bf296f465e..c83848f281cf 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPushFilter.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPushFilter.java @@ -1,7 +1,6 @@ package de.tum.cit.aet.artemis.programming.service.localvc; import java.io.IOException; -import java.util.function.IntUnaryOperator; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; @@ -16,7 +15,6 @@ 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.exception.localvc.LocalVCInternalException; -import de.tum.cit.aet.artemis.programming.domain.VcsAccessLog; import de.tum.cit.aet.artemis.programming.web.repository.RepositoryActionType; /** @@ -28,11 +26,8 @@ public class LocalVCPushFilter extends OncePerRequestFilter { private final LocalVCServletService localVCServletService; - private final IntUnaryOperator callbck; - - public LocalVCPushFilter(LocalVCServletService localVCServletService, IntUnaryOperator op) { + public LocalVCPushFilter(LocalVCServletService localVCServletService) { this.localVCServletService = localVCServletService; - this.callbck = op; } /** @@ -56,11 +51,8 @@ public void doFilterInternal(HttpServletRequest servletRequest, HttpServletRespo // We need to extract the content of the request here as it is garbage collected before it can be used asynchronously String authorizationHeader = servletRequest.getHeader(LocalVCServletService.AUTHORIZATION_HEADER); String method = servletRequest.getMethod(); - var vcsAccessLog = servletRequest.getAttribute(HttpsConstants.VCS_ACCESS_LOG_KEY); - if (vcsAccessLog instanceof VcsAccessLog accessLog) { - this.localVCServletService.updateAndStoreVCSAccessLogForPushHTTPS(method, authorizationHeader, accessLog); - } - this.callbck.applyAsInt(4); + // this.localVCServletService.updateAndStoreVCSAccessLogForPushHTTPS(method, servletRequest, authorizationHeader); + filterChain.doFilter(servletRequest, servletResponse); } } 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 47ca59c4e785..182dd9b85139 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 @@ -221,6 +221,11 @@ public void authenticateAndAuthorizeGitRequest(HttpServletRequest request, Repos String authorizationHeader = request.getHeader(LocalVCServletService.AUTHORIZATION_HEADER); + // The first request does not contain an authorizationHeader + if (authorizationHeader == null) { + throw new LocalVCAuthException("No authorization header provided"); + } + // If it is a fetch request, we check if it is the build agent that is fetching the repository. if (repositoryAction == RepositoryActionType.READ) { UsernameAndPassword usernameAndPassword = extractUsernameAndPassword(authorizationHeader); @@ -252,13 +257,33 @@ public void authenticateAndAuthorizeGitRequest(HttpServletRequest request, Repos throw new LocalVCForbiddenException(); } - var authenticationMechanism = resolveAuthenticationMechanism(authorizationHeader, user); - var ipAddress = request.getRemoteAddr(); - authorizeUser(repositoryTypeOrUserName, user, exercise, repositoryAction, authenticationMechanism, ipAddress, localVCRepositoryUri, Optional.of(request), Optional.empty()); + var optionalParticipation = authorizeUser(repositoryTypeOrUserName, user, exercise, repositoryAction, localVCRepositoryUri, false); + storePreliminaryVcsAccessLogForHTTPs(request, localVCRepositoryUri, user, repositoryAction, optionalParticipation); log.debug("Authorizing user {} for repository {} took {}", user.getLogin(), localVCRepositoryUri, TimeLogUtil.formatDurationFrom(timeNanoStart)); } + private void storePreliminaryVcsAccessLogForHTTPs(HttpServletRequest request, LocalVCRepositoryUri localVCRepositoryUri, User user, RepositoryActionType repositoryAction, + Optional optionalParticipation) throws LocalVCAuthException { + if (optionalParticipation.isPresent()) { + ProgrammingExerciseParticipation participation = optionalParticipation.get(); + var ipAddress = request.getRemoteAddr(); + var authenticationMechanism = resolveHTTPSAuthenticationMechanism(request.getHeader(LocalVCServletService.AUTHORIZATION_HEADER), user); + + String commitHash = null; + try { + commitHash = getLatestCommitHash(repositories.get(localVCRepositoryUri.getRelativeRepositoryPath().toString())); + } + catch (GitAPIException e) { + log.warn("Failed to obtain commit hash for repository {}. Error: {}", localVCRepositoryUri.getRelativeRepositoryPath().toString(), e.getMessage()); + } + + String finalCommitHash = commitHash; + RepositoryActionType finalRepositoryAction = repositoryAction == RepositoryActionType.WRITE ? RepositoryActionType.PUSH : RepositoryActionType.CLONE; + vcsAccessLogService.ifPresent(service -> service.storeAccessLog(user, participation, finalRepositoryAction, authenticationMechanism, finalCommitHash, ipAddress)); + } + } + /** * Resolves the user's authentication mechanism for the repository * @@ -267,7 +292,7 @@ public void authenticateAndAuthorizeGitRequest(HttpServletRequest request, Repos * @return the authentication type * @throws LocalVCAuthException if extracting the token or password from the authorizationHeader fails */ - private AuthenticationMechanism resolveAuthenticationMechanism(String authorizationHeader, User user) throws LocalVCAuthException { + private AuthenticationMechanism resolveHTTPSAuthenticationMechanism(String authorizationHeader, User user) throws LocalVCAuthException { UsernameAndPassword usernameAndPassword = extractUsernameAndPassword(authorizationHeader); String password = usernameAndPassword.password(); @@ -304,6 +329,7 @@ private User authenticateUser(String authorizationHeader, ProgrammingExercise ex return user; } + // TODO put this in a method // Note: we first check if the user has used a vcs access token instead of a password if (password.startsWith(TOKEN_PREFIX) && password.length() == VCS_ACCESS_TOKEN_LENGTH) { try { @@ -413,38 +439,24 @@ private UsernameAndPassword extractUsernameAndPassword(String authorizationHeade * @param user The user that wants to access the repository. * @param exercise The exercise the repository belongs to. * @param repositoryActionType The type of the action the user wants to perform. - * @param authenticationMechanism The authentication mechanism used by the user to authenticate to the repository - * @param ipAddress The ip address of the user * @param localVCRepositoryUri The URI of the local repository. - * * @throws LocalVCForbiddenException If the user is not allowed to access the repository. */ - public void authorizeUser(String repositoryTypeOrUserName, User user, ProgrammingExercise exercise, RepositoryActionType repositoryActionType, - AuthenticationMechanism authenticationMechanism, String ipAddress, LocalVCRepositoryUri localVCRepositoryUri, Optional request, - Optional serverSession) throws LocalVCForbiddenException { + public Optional authorizeUser(String repositoryTypeOrUserName, User user, ProgrammingExercise exercise, + RepositoryActionType repositoryActionType, LocalVCRepositoryUri localVCRepositoryUri, boolean usingSSH) throws LocalVCForbiddenException { - // Only load auxiliary repositories if a user is at least teaching assistant; students are not allowed to access them - if (authorizationCheckService.isAtLeastTeachingAssistantForExercise(exercise, user) && (repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString()) - || auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise))) { - // Test and auxiliary repositories are only accessible by instructors and higher. - try { - repositoryAccessService.checkAccessTestOrAuxRepositoryElseThrow(repositoryActionType == RepositoryActionType.WRITE, exercise, user, repositoryTypeOrUserName); - } - catch (AccessForbiddenException e) { - throw new LocalVCForbiddenException(e); - } - return; + if (checkIfRepositoryIsAuxiliaryOrTestRepository(exercise, repositoryTypeOrUserName, repositoryActionType, user)) { + return Optional.empty(); } ProgrammingExerciseParticipation participation; try { - // participation = programmingExerciseParticipationService.retrieveParticipationForRepository(exercise, repositoryTypeOrUserName, - // localVCRepositoryUri.isPracticeRepository(), true); - - // TODO Add this back in when we have figured out what is incorrect in the playwright configuration for (MySQL, Local) - // Here we load the participation of the repository, with submissions. It is reused throughout the workflow. - // - participation = programmingExerciseParticipationService.retrieveParticipationWithSubmissionsByRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString()); + if (usingSSH) { + participation = programmingExerciseParticipationService.retrieveParticipationWithSubmissionsByRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString()); + } + else { + participation = programmingExerciseParticipationService.retrieveParticipationByRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString()); + } } catch (EntityNotFoundException e) { throw new LocalVCInternalException( @@ -457,7 +469,34 @@ public void authorizeUser(String repositoryTypeOrUserName, User user, Programmin catch (AccessForbiddenException e) { throw new LocalVCForbiddenException(e); } - cacheAttributesInRequestOrSession(user, participation, repositoryActionType, authenticationMechanism, ipAddress, localVCRepositoryUri, request, serverSession); + return Optional.of(participation); + } + + /** + * Checks if the provided repository is an auxiliary or test repository. + * Only load auxiliary repositories if a user is at least teaching assistant; students are not allowed to access them + * // TODO docs + * + * @param exercise + * @param repositoryTypeOrUserName + * @param repositoryActionType + * @param user + * @return + * @throws LocalVCForbiddenException + */ + private boolean checkIfRepositoryIsAuxiliaryOrTestRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName, RepositoryActionType repositoryActionType, + User user) throws LocalVCForbiddenException { + if (authorizationCheckService.isAtLeastTeachingAssistantForExercise(exercise, user) && (repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString()) + || auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise))) { + try { + repositoryAccessService.checkAccessTestOrAuxRepositoryElseThrow(repositoryActionType == RepositoryActionType.WRITE, exercise, user, repositoryTypeOrUserName); + } + catch (AccessForbiddenException e) { + throw new LocalVCForbiddenException(e); + } + return true; + } + return false; } /** @@ -465,41 +504,34 @@ public void authorizeUser(String repositoryTypeOrUserName, User user, Programmin * This method runs without blocking the user during repository access checks. * * @param user the user accessing the repository - * @param participation the participation associated with the repository + * @param optionalParticipation the participation associated with the repository * @param repositoryActionType the action performed on the repository (READ or WRITE) * @param authenticationMechanism the mechanism used for authentication (e.g., token, basic auth) * @param ipAddress the IP address of the user accessing the repository * @param localVCRepositoryUri the URI of the localVC repository */ - private void cacheAttributesInRequestOrSession(User user, ProgrammingExerciseParticipation participation, RepositoryActionType repositoryActionType, - AuthenticationMechanism authenticationMechanism, String ipAddress, LocalVCRepositoryUri localVCRepositoryUri, Optional request, - Optional serverSession) { - try { - String commitHash; - String relativeRepositoryPath = localVCRepositoryUri.getRelativeRepositoryPath().toString(); - try (Repository repository = resolveRepository(relativeRepositoryPath)) { - commitHash = getLatestCommitHash(repository); - } + public void cacheAttributesInSshSession(User user, Optional optionalParticipation, RepositoryActionType repositoryActionType, + AuthenticationMechanism authenticationMechanism, String ipAddress, LocalVCRepositoryUri localVCRepositoryUri, ServerSession serverSession) { + if (optionalParticipation.isPresent()) { + ProgrammingExerciseParticipation participation = optionalParticipation.get(); + try { + String commitHash; + String relativeRepositoryPath = localVCRepositoryUri.getRelativeRepositoryPath().toString(); + try (Repository repository = resolveRepository(relativeRepositoryPath)) { + commitHash = getLatestCommitHash(repository); + } - var finalRepositoryActionType = repositoryActionType == RepositoryActionType.READ ? RepositoryActionType.PULL : RepositoryActionType.PUSH; - var preliminaryAccessLog = new VcsAccessLog(user, (Participation) participation, user.getName(), user.getEmail(), finalRepositoryActionType, authenticationMechanism, - commitHash, ipAddress); + var finalRepositoryActionType = repositoryActionType == RepositoryActionType.READ ? RepositoryActionType.PULL : RepositoryActionType.PUSH; + var preliminaryAccessLog = new VcsAccessLog(user, (Participation) participation, user.getName(), user.getEmail(), finalRepositoryActionType, + authenticationMechanism, commitHash, ipAddress); - if (request.isPresent()) { - request.get().getSession().setAttribute(HttpsConstants.USER_KEY, user); - request.get().getSession().setAttribute(HttpsConstants.PARTICIPATION_KEY, participation); - request.get().getSession().setAttribute(HttpsConstants.VCS_ACCESS_LOG_KEY, preliminaryAccessLog); + serverSession.setAttribute(SshConstants.VCS_ACCESS_LOG_KEY, preliminaryAccessLog); + serverSession.setAttribute(SshConstants.PARTICIPATION_KEY, participation); } - else if (serverSession.isPresent()) { - serverSession.get().setAttribute(SshConstants.VCS_ACCESS_LOG_KEY, preliminaryAccessLog); - serverSession.get().setAttribute(SshConstants.PARTICIPATION_KEY, participation); + catch (Exception e) { + log.warn("Failed to obtain commit hash or store access log for repository {}. Error: {}", localVCRepositoryUri.getRelativeRepositoryPath().toString(), + e.getMessage()); } - else { - log.warn("authorizeUser: Neither HTTP request nor server session found"); - } - } - catch (Exception e) { - log.warn("Failed to obtain commit hash or store access log for repository {}. Error: {}", localVCRepositoryUri.getRelativeRepositoryPath().toString(), e.getMessage()); } } @@ -587,7 +619,8 @@ public void processNewPush(String commitHash, Repository repository, Optional service.saveVcsAccesslog(vcsAccessLog.get())); } else { - log.error("No vcsAccessLog in processNewPush! It should not be empty at this point"); + // for HTTPs we cannot keep the vcsAccessLog in the session + vcsAccessLogService.ifPresent(service -> service.updateCommitHash(participation, finalCommitHash)); } } catch (GitAPIException | IOException e) { @@ -802,8 +835,7 @@ public static String getDefaultBranchOfRepository(Repository repository) { * @param clientOffered the number of objects offered by the client in the operation, used to determine * if the action is a clone (if 0) or a pull (if greater than 0). */ - @Async - public void updateAndStoreVCSAccessLogForCloneAndPullHTTPS(VcsAccessLog vcsAccessLog, String authorizationHeader, int clientOffered) { + public void updateAndStoreVCSAccessLogForCloneAndPullHTTPS(HttpServletRequest request, String authorizationHeader, int clientOffered) { try { UsernameAndPassword usernameAndPassword = extractUsernameAndPassword(authorizationHeader); @@ -811,9 +843,10 @@ public void updateAndStoreVCSAccessLogForCloneAndPullHTTPS(VcsAccessLog vcsAcces if (userName.equals(BUILD_USER_NAME)) { return; } + LocalVCRepositoryUri localVCRepositoryUri = parseRepositoryUri(request); RepositoryActionType repositoryActionType = getRepositoryActionReadType(clientOffered); - vcsAccessLog.setRepositoryActionType(repositoryActionType); - vcsAccessLogService.ifPresent(service -> service.saveVcsAccesslog(vcsAccessLog)); + + vcsAccessLogService.ifPresent(service -> service.updateRepositoryActionType(localVCRepositoryUri, repositoryActionType)); } catch (Exception ignored) { } @@ -827,11 +860,11 @@ public void updateAndStoreVCSAccessLogForCloneAndPullHTTPS(VcsAccessLog vcsAcces * * @param method the HTTP method of the request (expected to be "POST" for logging to occur) * @param authorizationHeader the authorization header containing the username and password in basic authentication format - * + *

* This method is asynchronous. */ @Async - public void updateAndStoreVCSAccessLogForPushHTTPS(String method, String authorizationHeader, VcsAccessLog vcsAccessLog) { + public void updateAndStoreVCSAccessLogForPushHTTPS(String method, HttpServletRequest servletRequest, String authorizationHeader) { if (!method.equals("POST")) { return; } @@ -841,9 +874,8 @@ public void updateAndStoreVCSAccessLogForPushHTTPS(String method, String authori if (userName.equals(BUILD_USER_NAME)) { return; } - RepositoryActionType repositoryActionType = RepositoryActionType.PUSH; - vcsAccessLog.setRepositoryActionType(repositoryActionType); - vcsAccessLogService.ifPresent(service -> service.saveVcsAccesslog(vcsAccessLog)); + LocalVCRepositoryUri localVCRepositoryUri = parseRepositoryUri(servletRequest); + vcsAccessLogService.ifPresent(service -> service.updateRepositoryActionType(localVCRepositoryUri, RepositoryActionType.PUSH)); } catch (Exception ignored) { } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/SshGitLocationResolverService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/SshGitLocationResolverService.java index ffb2850e127f..fbe1f6450384 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/SshGitLocationResolverService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/SshGitLocationResolverService.java @@ -7,7 +7,6 @@ import java.nio.file.FileSystem; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Optional; import org.apache.sshd.git.GitLocationResolver; import org.apache.sshd.server.session.ServerSession; @@ -79,8 +78,9 @@ public Path resolveRootDirectory(String command, String[] args, ServerSession se } else { try { - localVCServletService.authorizeUser(repositoryTypeOrUserName, user, exercise, repositoryAction, AuthenticationMechanism.SSH, session.getClientAddress().toString(), - localVCRepositoryUri, Optional.empty(), Optional.of(session)); + var participation = localVCServletService.authorizeUser(repositoryTypeOrUserName, user, exercise, repositoryAction, localVCRepositoryUri, true); + localVCServletService.cacheAttributesInSshSession(user, participation, repositoryAction, AuthenticationMechanism.SSH, session.getClientAddress().toString(), + localVCRepositoryUri, session); } catch (LocalVCForbiddenException e) { log.error("User {} does not have access to the repository {}", user.getLogin(), repositoryPath); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java index 69cf2e3e1246..a8fd279b5dcf 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java @@ -65,25 +65,23 @@ public void storeAccessLog(User user, ProgrammingExerciseParticipation participa */ @Async public void updateCommitHash(ProgrammingExerciseParticipation participation, String commitHash) { - vcsAccessLogRepository.findNewestByParticipationId(participation.getId()).ifPresent(entry -> { - entry.setCommitHash(commitHash); - vcsAccessLogRepository.save(entry); - }); + var vcsAccessLog = vcsAccessLogRepository.findNewestByParticipationId(participation.getId()); + if (vcsAccessLog.isPresent()) { + vcsAccessLog.get().setCommitHash(commitHash); + vcsAccessLogRepository.save(vcsAccessLog.get()); + } } - /** - * Updates the repository action type of the newest log entry. This method is not Async, as it should already be called from an @Async context - * - * @param participation The participation to which the repository belongs to - * @param repositoryActionType The repositoryActionType which should get set for the newest access log entry - */ - public void updateRepositoryActionType(ProgrammingExerciseParticipation participation, RepositoryActionType repositoryActionType) { - vcsAccessLogRepository.findNewestByParticipationId(participation.getId()).ifPresent(entry -> { - entry.setRepositoryActionType(repositoryActionType); - vcsAccessLogRepository.save(entry); - }); + @Async + public void updateRepositoryActionType(LocalVCRepositoryUri localVCRepositoryUri, RepositoryActionType repositoryActionType) { + var vcsAccessLog = vcsAccessLogRepository.findNewestByRepositoryUri(localVCRepositoryUri.toString()); + if (vcsAccessLog.isPresent()) { + vcsAccessLog.get().setRepositoryActionType(repositoryActionType); + vcsAccessLogRepository.save(vcsAccessLog.get()); + } } + @Async public void saveVcsAccesslog(VcsAccessLog vcsAccessLog) { vcsAccessLogRepository.save(vcsAccessLog); } @@ -109,4 +107,5 @@ public Optional createPreliminaryCodeEditorAccessLog(Repository re } return Optional.empty(); } + } From f945d1eb2c0e18542e2c480157a754c59fa8c08f Mon Sep 17 00:00:00 2001 From: entholzer Date: Mon, 18 Nov 2024 19:43:00 +0100 Subject: [PATCH 03/14] more fixes of access log --- .../programming/repository/VcsAccessLogRepository.java | 8 ++++---- .../service/localvc/LocalVCServletService.java | 6 ++++-- .../programming/service/localvc/VcsAccessLogService.java | 3 ++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java index a82922245659..02600b0e6a32 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java @@ -77,11 +77,11 @@ public interface VcsAccessLogRepository extends ArtemisJpaRepository findNewestByParticipationId(@Param("participationId") long participationId); @Query(""" - SELECT vcsAccessLog - FROM VcsAccessLog vcsAccessLog - LEFT JOIN FETCH ProgrammingExerciseStudentParticipation participation ON vcsAccessLog.participation.id = participation.id + SELECT log + FROM VcsAccessLog log + LEFT JOIN TREAT (log.participation AS ProgrammingExerciseStudentParticipation) participation WHERE participation.repositoryUri = :repositoryUri - ORDER BY vcsAccessLog.id DESC + ORDER BY log.id DESC LIMIT 1 """) Optional findNewestByRepositoryUri(@Param("repositoryUri") String repositoryUri); 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 182dd9b85139..371362fb41db 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 @@ -279,7 +279,7 @@ private void storePreliminaryVcsAccessLogForHTTPs(HttpServletRequest request, Lo } String finalCommitHash = commitHash; - RepositoryActionType finalRepositoryAction = repositoryAction == RepositoryActionType.WRITE ? RepositoryActionType.PUSH : RepositoryActionType.CLONE; + RepositoryActionType finalRepositoryAction = repositoryAction == RepositoryActionType.WRITE ? RepositoryActionType.PUSH : RepositoryActionType.PULL; vcsAccessLogService.ifPresent(service -> service.storeAccessLog(user, participation, finalRepositoryAction, authenticationMechanism, finalCommitHash, ipAddress)); } } @@ -836,7 +836,9 @@ public static String getDefaultBranchOfRepository(Repository repository) { * if the action is a clone (if 0) or a pull (if greater than 0). */ public void updateAndStoreVCSAccessLogForCloneAndPullHTTPS(HttpServletRequest request, String authorizationHeader, int clientOffered) { - + if (!request.getMethod().equals("POST")) { + return; + } try { UsernameAndPassword usernameAndPassword = extractUsernameAndPassword(authorizationHeader); String userName = usernameAndPassword.username(); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java index a8fd279b5dcf..1a049ea1a722 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java @@ -74,7 +74,8 @@ public void updateCommitHash(ProgrammingExerciseParticipation participation, Str @Async public void updateRepositoryActionType(LocalVCRepositoryUri localVCRepositoryUri, RepositoryActionType repositoryActionType) { - var vcsAccessLog = vcsAccessLogRepository.findNewestByRepositoryUri(localVCRepositoryUri.toString()); + var repositoryURL = localVCRepositoryUri.toString().replace("/git-upload-pack", "").replace("/git-receive-pack", ""); + var vcsAccessLog = vcsAccessLogRepository.findNewestByRepositoryUri(repositoryURL); if (vcsAccessLog.isPresent()) { vcsAccessLog.get().setRepositoryActionType(repositoryActionType); vcsAccessLogRepository.save(vcsAccessLog.get()); From 483d9fdb8f635e6a3962bdb1820fd0d8be5a81de Mon Sep 17 00:00:00 2001 From: entholzer Date: Mon, 18 Nov 2024 20:28:28 +0100 Subject: [PATCH 04/14] cleanup 1 --- .../repository/VcsAccessLogRepository.java | 68 ++++--------------- ...ogrammingExerciseParticipationService.java | 46 ------------- .../service/localvc/LocalVCPostPushHook.java | 6 +- .../localvc/LocalVCServletService.java | 61 ++++------------- 4 files changed, 25 insertions(+), 156 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java index 02600b0e6a32..6c9bf8778b6b 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java @@ -26,47 +26,12 @@ @Repository public interface VcsAccessLogRepository extends ArtemisJpaRepository { - // @Transactional // ok because of modifying query - // @Modifying - // @Query(""" - // UPDATE VcsAccessLog vcsAccessLog - // SET vcsAccessLog.repositoryActionType = :repositoryActionType - // WHERE vcsAccessLog.id = ( - // SELECT MAX(log.id) - // FROM VcsAccessLog log - // WHERE log.participation.id = :participationId - // ) - // """) - // void updateRepositoryActionTypeForNewestLog(@Param("participationId") long participationId, @Param("repositoryActionType") RepositoryActionType repositoryActionType); - - // @Transactional // ok because of modifying query - // @Modifying - // @Query(""" - // UPDATE VcsAccessLog vcsAccessLog - // SET vcsAccessLog.repositoryActionType = :repositoryActionType - // WHERE vcsAccessLog.id = ( - // SELECT log.id - // FROM VcsAccessLog log - // JOIN ProgrammingExerciseStudentParticipation participation ON log.participation.id = participation.id - // WHERE participation.repositoryUri = :repositoryUri - // ORDER BY log.id DESC - // LIMIT 1 - // ) - // """) - // void updateRepositoryActionTypeForNewestLog(@Param("repositoryUri") String repositoryUri, @Param("repositoryActionType") RepositoryActionType repositoryActionType); - // - // - // @Transactional // ok because of modifying query - // @Modifying - // @Query(""" - // UPDATE VcsAccessLog vcsAccessLog - // SET vcsAccessLog.commitHash = :commitHash - // WHERE vcsAccessLog.participation.id = :participationId - // ORDER BY vcsAccessLog.id DESC - // LIMIT 1 - // """) - // void updateCommitHashForNewestLog(@Param("participationId") long participationId, @Param("commitHash") String commitHash); - + /** + * Retrieves the most recent {@link VcsAccessLog} for a given participation ID. + * + * @param participationId the ID of the participation to filter by. + * @return an {@link Optional} containing the newest {@link VcsAccessLog}, or empty if none exists. + */ @Query(""" SELECT vcsAccessLog FROM VcsAccessLog vcsAccessLog @@ -76,6 +41,12 @@ public interface VcsAccessLogRepository extends ArtemisJpaRepository findNewestByParticipationId(@Param("participationId") long participationId); + /** + * Retrieves the most recent {@link VcsAccessLog} for a specific repository URI of a participation. + * + * @param repositoryUri the URI of the participation to filter by. + * @return an Optional containing the newest {@link VcsAccessLog} of the participation, or empty if none exists. + */ @Query(""" SELECT log FROM VcsAccessLog log @@ -86,21 +57,6 @@ LEFT JOIN TREAT (log.participation AS ProgrammingExerciseStudentParticipation) p """) Optional findNewestByRepositoryUri(@Param("repositoryUri") String repositoryUri); - // @Transactional // ok because of modifying query - // @Modifying - // @Query(""" - // UPDATE VcsAccessLog vcsAccessLog - // SET vcsAccessLog.commitHash = :commitHash - // WHERE vcsAccessLog.id = ( - // SELECT MAX(log.id) - // FROM VcsAccessLog log - // LEFT JOIN FETCH ProgrammingExerciseStudentParticipation participation ON log.participation - // WHERE participation.repositoryUri = :repositoryUri - // ) - // """) - // void updateCommitHashForNewestLog(@Param("repositoryUri") String repositoryUri, @Param("commitHash") String commitHash); - // - /** * Retrieves a list of {@link VcsAccessLog} entities associated with the specified participation ID. * The results are ordered by the log ID in ascending order. diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java index 016381d8a255..8ebadb2a0927 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java @@ -429,52 +429,6 @@ public void resetRepository(VcsRepositoryUri targetURL, VcsRepositoryUri sourceU gitService.commitAndPush(targetRepo, "Reset Exercise", true, null); } - /** - * Get the participation for a given exercise and a repository type or user name. This method is used by the local VC system and by the local CI system to get the - * participation. - * - * @param exercise the exercise for which to get the participation. - * @param repositoryTypeOrUserName the repository type ("exercise", "solution", or "tests") or username (e.g. "artemis_test_user_1") as extracted from the repository URI. - * @param isPracticeRepository whether the repository is a practice repository, i.e. the repository URI contains "-practice-". - * - * @return the participation. - * @throws EntityNotFoundException if the participation could not be found. - */ - public ProgrammingExerciseParticipation retrieveParticipationWithSubmissionsByRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName, - boolean isPracticeRepository) { - - boolean isAuxiliaryRepository = auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise); - - // For pushes to the tests repository, the solution repository is built first, and thus we need the solution participation. - // Can possibly be used by auxiliary repositories - if (repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString()) || repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString()) || isAuxiliaryRepository) { - return solutionParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); - } - - if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { - return templateParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); - } - - if (exercise.isTeamMode()) { - // The repositoryTypeOrUserName is the team short name. - // For teams, there is no practice participation. - return findTeamParticipationByExerciseAndTeamShortNameOrThrow(exercise, repositoryTypeOrUserName); - } - - // If the exercise is an exam exercise and the repository's owner is at least an editor, the repository could be a test run repository, or it could be the instructor's - // assignment repository. - // There is no way to tell from the repository URI, and only one participation will be created, even if both are used. - // This participation has "testRun = true" set if the test run was created first, and "testRun = false" set if the instructor's assignment repository was created first. - // If the exercise is an exam exercise, and the repository's owner is at least an editor, get the participation without regard for the testRun flag. - boolean isExamEditorRepository = exercise.isExamExercise() - && authorizationCheckService.isAtLeastEditorForExercise(exercise, userRepository.getUserByLoginElseThrow(repositoryTypeOrUserName)); - if (isExamEditorRepository) { - return studentParticipationRepository.findWithSubmissionsByExerciseIdAndStudentLoginOrThrow(exercise.getId(), repositoryTypeOrUserName); - } - - return findStudentParticipationByExerciseAndStudentLoginAndTestRunOrThrow(exercise, repositoryTypeOrUserName, isPracticeRepository); - } - /** * Get the participation for a given repository url and a repository type or user name. This method is used by the local VC system to get the * participation for logging operations on the repository. diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPostPushHook.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPostPushHook.java index f901ab89a85d..fba96413df8f 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPostPushHook.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPostPushHook.java @@ -4,8 +4,6 @@ import java.util.Iterator; import java.util.Optional; -import jakarta.servlet.http.HttpServletRequest; - import org.apache.sshd.server.session.ServerSession; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.PostReceiveHook; @@ -32,8 +30,6 @@ public class LocalVCPostPushHook implements PostReceiveHook { private final VcsAccessLog vcsAccessLog; - private HttpServletRequest request; - public LocalVCPostPushHook(LocalVCServletService localVCServletService, ServerSession serverSession) { this.localVCServletService = localVCServletService; this.participation = serverSession.getAttribute(SshConstants.PARTICIPATION_KEY); @@ -43,7 +39,7 @@ public LocalVCPostPushHook(LocalVCServletService localVCServletService, ServerSe public LocalVCPostPushHook(LocalVCServletService localVCServletService) { this.localVCServletService = localVCServletService; - // For HTTPs we are unable to store the attributes in the sesseion or request unfortunately + // For HTTPs we are unable to store the attributes in the session or request unfortunately this.participation = null; this.exercise = null; this.vcsAccessLog = null; 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 371362fb41db..4f845a00a3ee 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 @@ -569,9 +569,13 @@ public void processNewPush(String commitHash, Repository repository) { /** * Create a submission, trigger the respective build, and process the results. + * This method can be called with some values, to avoid loading them again from the database * - * @param commitHash the hash of the last commit. - * @param repository the remote repository which was pushed to. //TODO fix docs + * @param commitHash the hash of the last commit. + * @param repository the remote repository which was pushed to. + * @param cachedExercise the exercise which is potentially already loaded + * @param cachedParticipation the participation which is potentially already loaded + * @param vcsAccessLog the vcsAccessLog which is potentially already loaded * @throws ContinuousIntegrationException if something goes wrong with the CI configuration. * @throws VersionControlException if the commit belongs to the wrong branch (i.e. not the default branch of the participation). */ @@ -586,8 +590,8 @@ public void processNewPush(String commitHash, Repository repository, Optional getProgrammingExercise(projectKey)); - ProgrammingExerciseParticipation participation = cachedParticipation - .orElseGet(() -> getProgrammingExerciseParticipation(localVCRepositoryUri, repositoryTypeOrUserName, exercise)); + + ProgrammingExerciseParticipation participation = cachedParticipation.orElseGet(() -> retrieveParticipationFromLocalVCRepositoryUri(localVCRepositoryUri)); RepositoryType repositoryType = getRepositoryType(repositoryTypeOrUserName, exercise); @@ -634,19 +638,6 @@ public void processNewPush(String commitHash, Repository repository, Optional - * This method logs the access information if the HTTP request is a POST request and the action - * is not performed by a build job user. The repository action type is set as a push action. - * - * @param method the HTTP method of the request (expected to be "POST" for logging to occur) - * @param authorizationHeader the authorization header containing the username and password in basic authentication format - *

- * This method is asynchronous. - */ - @Async - public void updateAndStoreVCSAccessLogForPushHTTPS(String method, HttpServletRequest servletRequest, String authorizationHeader) { - if (!method.equals("POST")) { - return; - } - try { - UsernameAndPassword usernameAndPassword = extractUsernameAndPassword(authorizationHeader); - String userName = usernameAndPassword.username(); - if (userName.equals(BUILD_USER_NAME)) { - return; - } - LocalVCRepositoryUri localVCRepositoryUri = parseRepositoryUri(servletRequest); - vcsAccessLogService.ifPresent(service -> service.updateRepositoryActionType(localVCRepositoryUri, RepositoryActionType.PUSH)); - } - catch (Exception ignored) { - } - } - /** * Updates the VCS access log for clone and pull actions performed over SSH. *

From 5e5705e775249aad1513e89f51916641b34ed171 Mon Sep 17 00:00:00 2001 From: entholzer Date: Mon, 18 Nov 2024 22:31:10 +0100 Subject: [PATCH 05/14] handle aux repos correctly --- ...ogrammingExerciseParticipationService.java | 58 +++++++++++++++++-- .../localvc/LocalVCServletService.java | 56 +++++++++++++----- 2 files changed, 92 insertions(+), 22 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java index 8ebadb2a0927..89a4e0d3292d 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java @@ -54,6 +54,8 @@ public class ProgrammingExerciseParticipationService { private static final Logger log = LoggerFactory.getLogger(ProgrammingExerciseParticipationService.class); + public static final String AUXILIARY_REPOSITORY_NO_PARTICIPATION = "Auxiliary repositories do not have participations."; + private final ProgrammingExerciseStudentParticipationRepository studentParticipationRepository; private final SolutionProgrammingExerciseParticipationRepository solutionParticipationRepository; @@ -220,6 +222,52 @@ public List findStudentParticipationsBy return studentParticipationRepository.findAllByExerciseIdAndStudentLogin(exercise.getId(), username); } + /** + * Get the participation for a given exercise and a repository type or user name. This method is used by the local VC system and by the local CI system to get the + * participation. + * + * @param exercise the exercise for which to get the participation. + * @param repositoryTypeOrUserName the repository type ("exercise", "solution", or "tests") or username (e.g. "artemis_test_user_1") as extracted from the repository URI. + * @param isPracticeRepository whether the repository is a practice repository, i.e. the repository URI contains "-practice-". + * + * @return the participation. + * @throws EntityNotFoundException if the participation could not be found. + */ + public ProgrammingExerciseParticipation dretrieveParticipationWithSubmissionsByRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName, + boolean isPracticeRepository) { + + boolean isAuxiliaryRepository = auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise); + + // For pushes to the tests repository, the solution repository is built first, and thus we need the solution participation. + // Can possibly be used by auxiliary repositories + if (repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString()) || repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString()) || isAuxiliaryRepository) { + return solutionParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); + } + + if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { + return templateParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); + } + + if (exercise.isTeamMode()) { + // The repositoryTypeOrUserName is the team short name. + // For teams, there is no practice participation. + return findTeamParticipationByExerciseAndTeamShortNameOrThrow(exercise, repositoryTypeOrUserName); + } + + // If the exercise is an exam exercise and the repository's owner is at least an editor, the repository could be a test run repository, or it could be the instructor's + // assignment repository. + // There is no way to tell from the repository URI, and only one participation will be created, even if both are used. + // This participation has "testRun = true" set if the test run was created first, and "testRun = false" set if the instructor's assignment repository was created first. + // If the exercise is an exam exercise, and the repository's owner is at least an editor, get the participation without regard for the testRun flag. + boolean isExamEditorRepository = exercise.isExamExercise() + && authorizationCheckService.isAtLeastEditorForExercise(exercise, userRepository.getUserByLoginElseThrow(repositoryTypeOrUserName)); + if (isExamEditorRepository) { + return studentParticipationRepository.findWithSubmissionsByExerciseIdAndStudentLoginOrThrow(exercise.getId(), repositoryTypeOrUserName); + } + + return findStudentParticipationByExerciseAndStudentLoginAndTestRunOrThrow(exercise, repositoryTypeOrUserName, isPracticeRepository); + } + /** * Try to find a programming exercise participation for the given id. * It contains the last submission which might be illegal! @@ -444,13 +492,14 @@ public ProgrammingExerciseParticipation retrieveParticipationWithSubmissionsByRe if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { return templateParticipationRepository.findWithSubmissionsByRepositoryUriElseThrow(repositoryURI); } - if (repositoryTypeOrUserName.equals(RepositoryType.AUXILIARY.toString())) { - throw new EntityNotFoundException("Auxiliary repositories do not have participations."); - } return studentParticipationRepository.findWithSubmissionsByRepositoryUriElseThrow(repositoryURI); } + public ProgrammingExerciseParticipation retrieveSolutionPar(Exercise exercise) { + return solutionParticipationRepository.findByProgrammingExerciseIdElseThrow(exercise.getId()); + } + /** * Get the participation for a given repository url and a repository type or user name. This method is used by the local VC system to get the * participation for logging operations on the repository. @@ -466,9 +515,6 @@ public ProgrammingExerciseParticipation retrieveParticipationByRepository(String if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { return templateParticipationRepository.findByRepositoryUriElseThrow(repositoryURI); } - if (repositoryTypeOrUserName.equals(RepositoryType.AUXILIARY.toString())) { - throw new EntityNotFoundException("Auxiliary repositories do not have participations."); - } return studentParticipationRepository.findByRepositoryUriElseThrow(repositoryURI); } 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 4f845a00a3ee..9128c59757d8 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 @@ -459,6 +459,9 @@ public Optional authorizeUser(String repositor } } catch (EntityNotFoundException e) { + if (isRepositoryAuxiliaryRepository(exercise, repositoryTypeOrUserName)) { + throw new LocalVCForbiddenException(e); + } throw new LocalVCInternalException( "No participation found for repository with repository type or username " + repositoryTypeOrUserName + " in exercise " + exercise.getId(), e); } @@ -472,6 +475,10 @@ public Optional authorizeUser(String repositor return Optional.of(participation); } + private boolean isRepositoryAuxiliaryRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName) { + return auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise); + } + /** * Checks if the provided repository is an auxiliary or test repository. * Only load auxiliary repositories if a user is at least teaching assistant; students are not allowed to access them @@ -590,9 +597,18 @@ public void processNewPush(String commitHash, Repository repository, Optional getProgrammingExercise(projectKey)); - - ProgrammingExerciseParticipation participation = cachedParticipation.orElseGet(() -> retrieveParticipationFromLocalVCRepositoryUri(localVCRepositoryUri)); - + ProgrammingExerciseParticipation participation; + try { + participation = cachedParticipation.orElseGet(() -> retrieveParticipationFromLocalVCRepositoryUri(localVCRepositoryUri)); + } + catch (EntityNotFoundException e) { + if (isRepositoryAuxiliaryRepository(exercise, repositoryTypeOrUserName)) { + participation = retrieveSolutionParticipation(exercise); + } + else { + throw e; + } + } RepositoryType repositoryType = getRepositoryType(repositoryTypeOrUserName, exercise); try { @@ -624,7 +640,8 @@ public void processNewPush(String commitHash, Repository repository, Optional service.updateCommitHash(participation, finalCommitHash)); + var finalParticipation = participation; + vcsAccessLogService.ifPresent(service -> service.updateCommitHash(finalParticipation, finalCommitHash)); } } catch (GitAPIException | IOException e) { @@ -638,6 +655,10 @@ public void processNewPush(String commitHash, Repository repository, Optional service.storeAccessLog(user, participation, RepositoryActionType.CLONE_FAIL, mechanism, "", ipAddress)); } From ce69c4916af58e0dad73311d45ee1a1bbe98484d Mon Sep 17 00:00:00 2001 From: entholzer Date: Mon, 18 Nov 2024 22:48:58 +0100 Subject: [PATCH 06/14] fix retrieval of test repository solution participation --- ...ogrammingExerciseParticipationService.java | 8 +++---- .../localvc/LocalVCServletService.java | 21 ++++++++++++------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java index 89a4e0d3292d..18fbbfd792f2 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java @@ -485,9 +485,9 @@ public void resetRepository(VcsRepositoryUri targetURL, VcsRepositoryUri sourceU * @param repositoryURI the participation's repository URL * @return the participation belonging to the provided repositoryURI and repository type or username */ - public ProgrammingExerciseParticipation retrieveParticipationWithSubmissionsByRepository(String repositoryTypeOrUserName, String repositoryURI) { + public ProgrammingExerciseParticipation retrieveParticipationWithSubmissionsByRepository(String repositoryTypeOrUserName, String repositoryURI, ProgrammingExercise exercise) { if (repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString()) || repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { - return solutionParticipationRepository.findWithSubmissionsByRepositoryUriElseThrow(repositoryURI); + return solutionParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); } if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { return templateParticipationRepository.findWithSubmissionsByRepositoryUriElseThrow(repositoryURI); @@ -508,9 +508,9 @@ public ProgrammingExerciseParticipation retrieveSolutionPar(Exercise exercise) { * @param repositoryURI the participation's repository URL * @return the participation belonging to the provided repositoryURI and repository type or username */ - public ProgrammingExerciseParticipation retrieveParticipationByRepository(String repositoryTypeOrUserName, String repositoryURI) { + public ProgrammingExerciseParticipation retrieveParticipationByRepository(String repositoryTypeOrUserName, String repositoryURI, ProgrammingExercise exercise) { if (repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString()) || repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { - return solutionParticipationRepository.findByRepositoryUriElseThrow(repositoryURI); + return solutionParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); } if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { return templateParticipationRepository.findByRepositoryUriElseThrow(repositoryURI); 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 9128c59757d8..fc4a17c358a2 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 @@ -452,10 +452,11 @@ public Optional authorizeUser(String repositor ProgrammingExerciseParticipation participation; try { if (usingSSH) { - participation = programmingExerciseParticipationService.retrieveParticipationWithSubmissionsByRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString()); + participation = programmingExerciseParticipationService.retrieveParticipationWithSubmissionsByRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString(), + exercise); } else { - participation = programmingExerciseParticipationService.retrieveParticipationByRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString()); + participation = programmingExerciseParticipationService.retrieveParticipationByRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString(), exercise); } } catch (EntityNotFoundException e) { @@ -493,8 +494,12 @@ private boolean isRepositoryAuxiliaryRepository(ProgrammingExercise exercise, St */ private boolean checkIfRepositoryIsAuxiliaryOrTestRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName, RepositoryActionType repositoryActionType, User user) throws LocalVCForbiddenException { - if (authorizationCheckService.isAtLeastTeachingAssistantForExercise(exercise, user) && (repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString()) - || auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise))) { + if (!authorizationCheckService.isAtLeastTeachingAssistantForExercise(exercise, user) && repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { + throw new LocalVCForbiddenException(); + } + + if (authorizationCheckService.isAtLeastTeachingAssistantForExercise(exercise, user) + && (auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise))) { try { repositoryAccessService.checkAccessTestOrAuxRepositoryElseThrow(repositoryActionType == RepositoryActionType.WRITE, exercise, user, repositoryTypeOrUserName); } @@ -599,7 +604,7 @@ public void processNewPush(String commitHash, Repository repository, Optional getProgrammingExercise(projectKey)); ProgrammingExerciseParticipation participation; try { - participation = cachedParticipation.orElseGet(() -> retrieveParticipationFromLocalVCRepositoryUri(localVCRepositoryUri)); + participation = cachedParticipation.orElseGet(() -> retrieveParticipationFromLocalVCRepositoryUri(localVCRepositoryUri, exercise)); } catch (EntityNotFoundException e) { if (isRepositoryAuxiliaryRepository(exercise, repositoryTypeOrUserName)) { @@ -821,10 +826,10 @@ private Commit extractCommitInfo(String commitHash, Repository repository) throw * @param localVCRepositoryUri the {@link LocalVCRepositoryUri} containing details about the repository. * @return the {@link ProgrammingExerciseParticipation} corresponding to the repository URI. */ - private ProgrammingExerciseParticipation retrieveParticipationFromLocalVCRepositoryUri(LocalVCRepositoryUri localVCRepositoryUri) { + private ProgrammingExerciseParticipation retrieveParticipationFromLocalVCRepositoryUri(LocalVCRepositoryUri localVCRepositoryUri, ProgrammingExercise exercise) { String repositoryTypeOrUserName = localVCRepositoryUri.getRepositoryTypeOrUserName(); var repositoryURL = localVCRepositoryUri.toString().replace("/git-upload-pack", "").replace("/git-receive-pack", ""); - return programmingExerciseParticipationService.retrieveParticipationWithSubmissionsByRepository(repositoryTypeOrUserName, repositoryURL); + return programmingExerciseParticipationService.retrieveParticipationWithSubmissionsByRepository(repositoryTypeOrUserName, repositoryURL, exercise); } /** @@ -910,7 +915,7 @@ public void createVCSAccessLogForFailedAuthenticationAttempt(HttpServletRequest User user = userRepository.findOneByLogin(usernameAndPassword.username()).orElseThrow(LocalVCAuthException::new); AuthenticationMechanism mechanism = usernameAndPassword.password().startsWith("vcpat-") ? AuthenticationMechanism.VCS_ACCESS_TOKEN : AuthenticationMechanism.PASSWORD; LocalVCRepositoryUri localVCRepositoryUri = parseRepositoryUri(servletRequest); - var participation = retrieveParticipationFromLocalVCRepositoryUri(localVCRepositoryUri); + var participation = retrieveParticipationFromLocalVCRepositoryUri(localVCRepositoryUri, null); var ipAddress = servletRequest.getRemoteAddr(); vcsAccessLogService.ifPresent(service -> service.storeAccessLog(user, participation, RepositoryActionType.CLONE_FAIL, mechanism, "", ipAddress)); } From c8e3bcc407b400b04a835cd87007ae32cfd28d78 Mon Sep 17 00:00:00 2001 From: entholzer Date: Mon, 18 Nov 2024 22:55:07 +0100 Subject: [PATCH 07/14] cleanup 2 --- ...xerciseStudentParticipationRepository.java | 43 ------- ...ammingExerciseParticipationRepository.java | 13 --- ...ogrammingExerciseParticipationService.java | 109 +----------------- .../localvc/LocalVCFetchPreUploadHookSSH.java | 2 - .../service/localvc/LocalVCPushFilter.java | 5 - 5 files changed, 1 insertion(+), 171 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java index b7fa36e826b4..eb4ad5567440 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java @@ -91,41 +91,9 @@ default ProgrammingExerciseStudentParticipation findByRepositoryUriElseThrow(Str return getValueElseThrow(findByRepositoryUri(repositoryUri)); } - @EntityGraph(type = LOAD, attributePaths = { "submissions" }) - Optional findWithSubmissionsByExerciseIdAndStudentLogin(long exerciseId, String username); - - default ProgrammingExerciseStudentParticipation findWithSubmissionsByExerciseIdAndStudentLoginOrThrow(long exerciseId, String username) { - return getValueElseThrow(findWithSubmissionsByExerciseIdAndStudentLogin(exerciseId, username)); - } - - Optional findByExerciseIdAndStudentLoginAndTestRun(long exerciseId, String username, boolean testRun); - @EntityGraph(type = LOAD, attributePaths = { "team.students" }) Optional findByExerciseIdAndTeamId(long exerciseId, long teamId); - @Query(""" - SELECT DISTINCT participation - FROM ProgrammingExerciseStudentParticipation participation - LEFT JOIN FETCH participation.team team - LEFT JOIN FETCH team.students - WHERE participation.exercise.id = :exerciseId - AND participation.team.shortName = :teamShortName - """) - Optional findWithEagerStudentsByExerciseIdAndTeamShortName(@Param("exerciseId") long exerciseId, - @Param("teamShortName") String teamShortName); - - @Query(""" - SELECT DISTINCT participation - FROM ProgrammingExerciseStudentParticipation participation - LEFT JOIN FETCH participation.submissions - LEFT JOIN FETCH participation.team team - LEFT JOIN FETCH team.students - WHERE participation.exercise.id = :exerciseId - AND participation.team.shortName = :teamShortName - """) - Optional findWithSubmissionsAndEagerStudentsByExerciseIdAndTeamShortName(@Param("exerciseId") long exerciseId, - @Param("teamShortName") String teamShortName); - @Query(""" SELECT DISTINCT participation FROM ProgrammingExerciseStudentParticipation participation @@ -162,17 +130,6 @@ Optional findWithSubmissionsAndEagerStu List findWithSubmissionsByExerciseIdAndParticipationIds(@Param("exerciseId") long exerciseId, @Param("participationIds") Collection participationIds); - @Query(""" - SELECT participation - FROM ProgrammingExerciseStudentParticipation participation - LEFT JOIN FETCH participation.submissions - WHERE participation.exercise.id = :exerciseId - AND participation.student.login = :username - AND participation.testRun = :testRun - """) - Optional findWithSubmissionsByExerciseIdAndStudentLoginAndTestRun(@Param("exerciseId") long exerciseId, - @Param("username") String username, @Param("testRun") boolean testRun); - @Query(""" SELECT participation.repositoryUri FROM ProgrammingExerciseStudentParticipation participation diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/SolutionProgrammingExerciseParticipationRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/SolutionProgrammingExerciseParticipationRepository.java index 572834303685..2949576a13cb 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/SolutionProgrammingExerciseParticipationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/SolutionProgrammingExerciseParticipationRepository.java @@ -43,19 +43,6 @@ public interface SolutionProgrammingExerciseParticipationRepository """) Optional findByBuildPlanIdWithResults(@Param("buildPlanId") String buildPlanId); - @EntityGraph(type = LOAD, attributePaths = { "submissions" }) - Optional findWithSubmissionsByRepositoryUri(String repositoryUri); - - default SolutionProgrammingExerciseParticipation findWithSubmissionsByRepositoryUriElseThrow(String repositoryUri) { - return getValueElseThrow(findWithSubmissionsByRepositoryUri(repositoryUri)); - } - - Optional findByRepositoryUri(String repositoryUri); - - default SolutionProgrammingExerciseParticipation findByRepositoryUriElseThrow(String repositoryUri) { - return getValueElseThrow(findByRepositoryUri(repositoryUri)); - } - @EntityGraph(type = LOAD, attributePaths = { "results", "submissions", "submissions.results" }) Optional findWithEagerResultsAndSubmissionsByProgrammingExerciseId(long exerciseId); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java index 18fbbfd792f2..1e4e818a0cd6 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java @@ -22,8 +22,6 @@ import de.tum.cit.aet.artemis.core.domain.User; import de.tum.cit.aet.artemis.core.exception.EntityNotFoundException; import de.tum.cit.aet.artemis.core.exception.VersionControlException; -import de.tum.cit.aet.artemis.core.repository.UserRepository; -import de.tum.cit.aet.artemis.core.service.AuthorizationCheckService; import de.tum.cit.aet.artemis.exam.domain.StudentExam; import de.tum.cit.aet.artemis.exercise.domain.Exercise; import de.tum.cit.aet.artemis.exercise.domain.InitializationState; @@ -54,8 +52,6 @@ public class ProgrammingExerciseParticipationService { private static final Logger log = LoggerFactory.getLogger(ProgrammingExerciseParticipationService.class); - public static final String AUXILIARY_REPOSITORY_NO_PARTICIPATION = "Auxiliary repositories do not have participations."; - private final ProgrammingExerciseStudentParticipationRepository studentParticipationRepository; private final SolutionProgrammingExerciseParticipationRepository solutionParticipationRepository; @@ -70,16 +66,9 @@ public class ProgrammingExerciseParticipationService { private final GitService gitService; - private final AuthorizationCheckService authorizationCheckService; - - private final UserRepository userRepository; - - private final AuxiliaryRepositoryService auxiliaryRepositoryService; - public ProgrammingExerciseParticipationService(SolutionProgrammingExerciseParticipationRepository solutionParticipationRepository, TemplateProgrammingExerciseParticipationRepository templateParticipationRepository, ProgrammingExerciseStudentParticipationRepository studentParticipationRepository, - ParticipationRepository participationRepository, TeamRepository teamRepository, GitService gitService, Optional versionControlService, - AuthorizationCheckService authorizationCheckService, UserRepository userRepository, AuxiliaryRepositoryService auxiliaryRepositoryService) { + ParticipationRepository participationRepository, TeamRepository teamRepository, GitService gitService, Optional versionControlService) { this.studentParticipationRepository = studentParticipationRepository; this.solutionParticipationRepository = solutionParticipationRepository; this.templateParticipationRepository = templateParticipationRepository; @@ -87,9 +76,6 @@ public ProgrammingExerciseParticipationService(SolutionProgrammingExercisePartic this.teamRepository = teamRepository; this.versionControlService = versionControlService; this.gitService = gitService; - this.authorizationCheckService = authorizationCheckService; - this.userRepository = userRepository; - this.auxiliaryRepositoryService = auxiliaryRepositoryService; } /** @@ -126,29 +112,6 @@ public TemplateProgrammingExerciseParticipation findTemplateParticipationByProgr return templateParticipation.get(); } - /** - * Tries to retrieve a team participation for the given exercise and team short name. - * - * @param exercise the exercise for which to find a participation. - * @param teamShortName of the team to which the participation belongs. - * @return the participation for the given exercise and team. - * @throws EntityNotFoundException if the team participation was not found. - */ - public ProgrammingExerciseStudentParticipation findTeamParticipationByExerciseAndTeamShortNameOrThrow(ProgrammingExercise exercise, String teamShortName) { - - Optional participationOptional; - - // It is important to fetch all students of the team here, because the local VC and local CI system use this participation to check if the authenticated user is part of the - // team. - participationOptional = studentParticipationRepository.findWithSubmissionsAndEagerStudentsByExerciseIdAndTeamShortName(exercise.getId(), teamShortName); - - if (participationOptional.isEmpty()) { - throw new EntityNotFoundException("Participation could not be found by exerciseId " + exercise.getId() + " and team short name " + teamShortName); - } - - return participationOptional.get(); - } - /** * Tries to retrieve a student participation for the given team exercise and user * @@ -161,30 +124,6 @@ public Optional findTeamParticipationBy return studentParticipationRepository.findTeamParticipationByExerciseIdAndStudentId(exercise.getId(), user.getId()); } - /** - * Tries to retrieve a student participation for the given exercise and username and test run flag. - * - * @param exercise the exercise for which to find a participation. - * @param username of the user to which the participation belongs. - * @param isTestRun true if the participation is a test run participation. - * @return the participation for the given exercise and user. - * @throws EntityNotFoundException if there is no participation for the given exercise and user. - */ - @NotNull - public ProgrammingExerciseStudentParticipation findStudentParticipationByExerciseAndStudentLoginAndTestRunOrThrow(ProgrammingExercise exercise, String username, - boolean isTestRun) { - - Optional participationOptional; - - participationOptional = studentParticipationRepository.findWithSubmissionsByExerciseIdAndStudentLoginAndTestRun(exercise.getId(), username, isTestRun); - - if (participationOptional.isEmpty()) { - throw new EntityNotFoundException("Participation could not be found by exerciseId " + exercise.getId() + " and user " + username); - } - - return participationOptional.get(); - } - /** * Tries to retrieve a student participation for the given exercise id and username. * @@ -222,52 +161,6 @@ public List findStudentParticipationsBy return studentParticipationRepository.findAllByExerciseIdAndStudentLogin(exercise.getId(), username); } - /** - * Get the participation for a given exercise and a repository type or user name. This method is used by the local VC system and by the local CI system to get the - * participation. - * - * @param exercise the exercise for which to get the participation. - * @param repositoryTypeOrUserName the repository type ("exercise", "solution", or "tests") or username (e.g. "artemis_test_user_1") as extracted from the repository URI. - * @param isPracticeRepository whether the repository is a practice repository, i.e. the repository URI contains "-practice-". - * - * @return the participation. - * @throws EntityNotFoundException if the participation could not be found. - */ - public ProgrammingExerciseParticipation dretrieveParticipationWithSubmissionsByRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName, - boolean isPracticeRepository) { - - boolean isAuxiliaryRepository = auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise); - - // For pushes to the tests repository, the solution repository is built first, and thus we need the solution participation. - // Can possibly be used by auxiliary repositories - if (repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString()) || repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString()) || isAuxiliaryRepository) { - return solutionParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); - } - - if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { - return templateParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); - } - - if (exercise.isTeamMode()) { - // The repositoryTypeOrUserName is the team short name. - // For teams, there is no practice participation. - return findTeamParticipationByExerciseAndTeamShortNameOrThrow(exercise, repositoryTypeOrUserName); - } - - // If the exercise is an exam exercise and the repository's owner is at least an editor, the repository could be a test run repository, or it could be the instructor's - // assignment repository. - // There is no way to tell from the repository URI, and only one participation will be created, even if both are used. - // This participation has "testRun = true" set if the test run was created first, and "testRun = false" set if the instructor's assignment repository was created first. - // If the exercise is an exam exercise, and the repository's owner is at least an editor, get the participation without regard for the testRun flag. - boolean isExamEditorRepository = exercise.isExamExercise() - && authorizationCheckService.isAtLeastEditorForExercise(exercise, userRepository.getUserByLoginElseThrow(repositoryTypeOrUserName)); - if (isExamEditorRepository) { - return studentParticipationRepository.findWithSubmissionsByExerciseIdAndStudentLoginOrThrow(exercise.getId(), repositoryTypeOrUserName); - } - - return findStudentParticipationByExerciseAndStudentLoginAndTestRunOrThrow(exercise, repositoryTypeOrUserName, isPracticeRepository); - } - /** * Try to find a programming exercise participation for the given id. * It contains the last submission which might be illegal! diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHookSSH.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHookSSH.java index 6e9ace03a769..7b2a5465cc97 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHookSSH.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHookSSH.java @@ -25,11 +25,9 @@ public void onBeginNegotiateRound(UploadPack uploadPack, Collection collection, int i, int i1, boolean b) { - int bs = 4; } @Override public void onSendPack(UploadPack uploadPack, Collection collection, Collection collection1) { - int ss = 4; } } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPushFilter.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPushFilter.java index c83848f281cf..6237412fefa0 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPushFilter.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPushFilter.java @@ -48,11 +48,6 @@ public void doFilterInternal(HttpServletRequest servletRequest, HttpServletRespo return; } - // We need to extract the content of the request here as it is garbage collected before it can be used asynchronously - String authorizationHeader = servletRequest.getHeader(LocalVCServletService.AUTHORIZATION_HEADER); - String method = servletRequest.getMethod(); - // this.localVCServletService.updateAndStoreVCSAccessLogForPushHTTPS(method, servletRequest, authorizationHeader); - filterChain.doFilter(servletRequest, servletResponse); } } From 10f2260371aee7ef6d1af1efa05a968126f3f50d Mon Sep 17 00:00:00 2001 From: entholzer Date: Tue, 19 Nov 2024 15:02:57 +0100 Subject: [PATCH 08/14] cleanup 3 --- ...ogrammingExerciseParticipationService.java | 14 +- .../localvc/LocalVCServletService.java | 137 ++++++++++-------- 2 files changed, 84 insertions(+), 67 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java index 1e4e818a0cd6..e3421ac65a04 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java @@ -378,14 +378,15 @@ public void resetRepository(VcsRepositoryUri targetURL, VcsRepositoryUri sourceU * @param repositoryURI the participation's repository URL * @return the participation belonging to the provided repositoryURI and repository type or username */ - public ProgrammingExerciseParticipation retrieveParticipationWithSubmissionsByRepository(String repositoryTypeOrUserName, String repositoryURI, ProgrammingExercise exercise) { + public ProgrammingExerciseParticipation fetchParticipationWithSubmissionsByRepository(String repositoryTypeOrUserName, String repositoryURI, ProgrammingExercise exercise) { + var repositoryURL = repositoryURI.replace("/git-upload-pack", "").replace("/git-receive-pack", ""); if (repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString()) || repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { return solutionParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); } if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { - return templateParticipationRepository.findWithSubmissionsByRepositoryUriElseThrow(repositoryURI); + return templateParticipationRepository.findWithSubmissionsByRepositoryUriElseThrow(repositoryURL); } - return studentParticipationRepository.findWithSubmissionsByRepositoryUriElseThrow(repositoryURI); + return studentParticipationRepository.findWithSubmissionsByRepositoryUriElseThrow(repositoryURL); } @@ -401,14 +402,15 @@ public ProgrammingExerciseParticipation retrieveSolutionPar(Exercise exercise) { * @param repositoryURI the participation's repository URL * @return the participation belonging to the provided repositoryURI and repository type or username */ - public ProgrammingExerciseParticipation retrieveParticipationByRepository(String repositoryTypeOrUserName, String repositoryURI, ProgrammingExercise exercise) { + public ProgrammingExerciseParticipation fetchParticipationByRepository(String repositoryTypeOrUserName, String repositoryURI, ProgrammingExercise exercise) { + var repositoryURL = repositoryURI.replace("/git-upload-pack", "").replace("/git-receive-pack", ""); if (repositoryTypeOrUserName.equals(RepositoryType.SOLUTION.toString()) || repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { return solutionParticipationRepository.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exercise.getId()); } if (repositoryTypeOrUserName.equals(RepositoryType.TEMPLATE.toString())) { - return templateParticipationRepository.findByRepositoryUriElseThrow(repositoryURI); + return templateParticipationRepository.findByRepositoryUriElseThrow(repositoryURL); } - return studentParticipationRepository.findByRepositoryUriElseThrow(repositoryURI); + return studentParticipationRepository.findByRepositoryUriElseThrow(repositoryURL); } /** 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 fc4a17c358a2..90d6a05706b2 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 @@ -309,29 +309,45 @@ private User authenticateUser(String authorizationHeader, ProgrammingExercise ex throws LocalVCAuthException, AuthenticationException { UsernameAndPassword usernameAndPassword = extractUsernameAndPassword(authorizationHeader); - String username = usernameAndPassword.username(); - String password = usernameAndPassword.password(); + String passwordOrToken = usernameAndPassword.password(); + User user = userRepository.findOneByLogin(username).orElseThrow(LocalVCAuthException::new); try { - SecurityUtils.checkUsernameAndPasswordValidity(username, password); + SecurityUtils.checkUsernameAndPasswordValidity(username, passwordOrToken); } catch (AccessForbiddenException | AuthenticationException e) { - if (!StringUtils.isEmpty(password)) { - log.warn("Failed login attempt for user {} with password {} due to issue: {}", username, password, e.getMessage()); + if (!StringUtils.isEmpty(passwordOrToken)) { + log.warn("Failed login attempt for user {} with password {} due to issue: {}", username, passwordOrToken, e.getMessage()); } throw new LocalVCAuthException(e.getMessage()); } // check user VCS access token - if (Objects.equals(user.getVcsAccessToken(), password) && user.getVcsAccessTokenExpiryDate() != null && user.getVcsAccessTokenExpiryDate().isAfter(ZonedDateTime.now())) { + if (Objects.equals(user.getVcsAccessToken(), passwordOrToken) && user.getVcsAccessTokenExpiryDate() != null + && user.getVcsAccessTokenExpiryDate().isAfter(ZonedDateTime.now())) { return user; } - // TODO put this in a method + // check user participation VCS access token + if (tryAuthenticationWithVCSAccessToken(user, passwordOrToken, exercise, localVCRepositoryUri)) { + return user; + } + + // if the user does not have an access token or used a password, we try to authenticate the user with it + // Try to authenticate the user based on the configured options, this can include sending the data to an external system (e.g. LDAP) or using internal authentication. + UsernamePasswordAuthenticationToken authenticationToken = new UsernamePasswordAuthenticationToken(username, passwordOrToken); + authenticationManager.authenticate(authenticationToken); + + return user; + } + + private boolean tryAuthenticationWithVCSAccessToken(User user, String providedToken, ProgrammingExercise exercise, LocalVCRepositoryUri localVCRepositoryUri) + throws LocalVCAuthException { + // Note: we first check if the user has used a vcs access token instead of a password - if (password.startsWith(TOKEN_PREFIX) && password.length() == VCS_ACCESS_TOKEN_LENGTH) { + if (providedToken.startsWith(TOKEN_PREFIX) && providedToken.length() == VCS_ACCESS_TOKEN_LENGTH) { try { // check participation vcs access token @@ -347,9 +363,9 @@ private User authenticateUser(String authorizationHeader, ProgrammingExercise ex } if (studentParticipation.isPresent()) { var token = participationVCSAccessTokenRepository.findByUserIdAndParticipationId(user.getId(), studentParticipation.get().getId()); - if (token.isPresent() && Objects.equals(token.get().getVcsAccessToken(), password)) { + if (token.isPresent() && Objects.equals(token.get().getVcsAccessToken(), providedToken)) { user.setVcsAccessToken(token.get().getVcsAccessToken()); - return user; + return true; } } } @@ -357,14 +373,7 @@ private User authenticateUser(String authorizationHeader, ProgrammingExercise ex throw new LocalVCAuthException(); } } - - // if the user does not have an access token or has used a password, we try to authenticate the user with it - - // Try to authenticate the user based on the configured options, this can include sending the data to an external system (e.g. LDAP) or using internal authentication. - UsernamePasswordAuthenticationToken authenticationToken = new UsernamePasswordAuthenticationToken(username, password); - authenticationManager.authenticate(authenticationToken); - - return user; + return false; } /** @@ -404,23 +413,19 @@ private ProgrammingExercise getProgrammingExerciseOrThrow(String projectKey) { } } - private String checkAuthorizationHeader(String authorizationHeader) throws LocalVCAuthException { + private UsernameAndPassword extractUsernameAndPassword(String authorizationHeader) throws LocalVCAuthException { if (authorizationHeader == null) { throw new LocalVCAuthException("No authorization header provided"); } - String[] basicAuthCredentialsEncoded = authorizationHeader.split(" "); if (!("Basic".equals(basicAuthCredentialsEncoded[0]))) { - throw new LocalVCAuthException(); + throw new LocalVCAuthException("Non basic authorization header provided"); } // Return decoded basic auth credentials which contain the username and the password. - return new String(Base64.getDecoder().decode(basicAuthCredentialsEncoded[1])); - } + String basicAuthCredentials = new String(Base64.getDecoder().decode(basicAuthCredentialsEncoded[1])); - private UsernameAndPassword extractUsernameAndPassword(String authorizationHeader) throws LocalVCAuthException { - String basicAuthCredentials = checkAuthorizationHeader(authorizationHeader); int separatorIndex = basicAuthCredentials.indexOf(":"); if (separatorIndex == -1) { @@ -449,57 +454,72 @@ public Optional authorizeUser(String repositor return Optional.empty(); } + ProgrammingExerciseParticipation participation = tryToLoadParticipation(usingSSH, repositoryTypeOrUserName, localVCRepositoryUri, exercise); + + checkAccessForRepository(participation, user, exercise, repositoryActionType); + + return Optional.of(participation); + } + + private ProgrammingExerciseParticipation tryToLoadParticipation(boolean usingSSH, String repositoryTypeOrUserName, LocalVCRepositoryUri localVCRepositoryUri, + ProgrammingExercise exercise) throws LocalVCForbiddenException { ProgrammingExerciseParticipation participation; try { if (usingSSH) { - participation = programmingExerciseParticipationService.retrieveParticipationWithSubmissionsByRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString(), + participation = programmingExerciseParticipationService.fetchParticipationWithSubmissionsByRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString(), exercise); } else { - participation = programmingExerciseParticipationService.retrieveParticipationByRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString(), exercise); + participation = programmingExerciseParticipationService.fetchParticipationByRepository(repositoryTypeOrUserName, localVCRepositoryUri.toString(), exercise); } } catch (EntityNotFoundException e) { - if (isRepositoryAuxiliaryRepository(exercise, repositoryTypeOrUserName)) { - throw new LocalVCForbiddenException(e); + // If the repository was not found, this could mean it is an auxiliary repository (which do not have participations) + if (auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise)) { + return programmingExerciseParticipationService.findSolutionParticipationByProgrammingExerciseId(exercise.getId()); } throw new LocalVCInternalException( "No participation found for repository with repository type or username " + repositoryTypeOrUserName + " in exercise " + exercise.getId(), e); } + return participation; + } + private void checkAccessForRepository(ProgrammingExerciseParticipation participation, User user, ProgrammingExercise exercise, RepositoryActionType repositoryActionType) + throws LocalVCForbiddenException { try { repositoryAccessService.checkAccessRepositoryElseThrow(participation, user, exercise, repositoryActionType); } catch (AccessForbiddenException e) { throw new LocalVCForbiddenException(e); } - return Optional.of(participation); - } - - private boolean isRepositoryAuxiliaryRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName) { - return auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise); } /** * Checks if the provided repository is an auxiliary or test repository. * Only load auxiliary repositories if a user is at least teaching assistant; students are not allowed to access them - * // TODO docs * * @param exercise * @param repositoryTypeOrUserName * @param repositoryActionType * @param user - * @return + * @return true if user is TA and * @throws LocalVCForbiddenException */ private boolean checkIfRepositoryIsAuxiliaryOrTestRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName, RepositoryActionType repositoryActionType, User user) throws LocalVCForbiddenException { - if (!authorizationCheckService.isAtLeastTeachingAssistantForExercise(exercise, user) && repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { - throw new LocalVCForbiddenException(); + + // Students should not be able to access Test or Aux repositories. + // To save on db queries we do not check that, until after we tried to fetch the user repository as a user repository + if (!authorizationCheckService.isAtLeastTeachingAssistantForExercise(exercise, user)) { + if (repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { + throw new LocalVCForbiddenException(); + } + return false; } - if (authorizationCheckService.isAtLeastTeachingAssistantForExercise(exercise, user) - && (auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTypeOrUserName, exercise))) { + // 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); } @@ -512,8 +532,7 @@ private boolean checkIfRepositoryIsAuxiliaryOrTestRepository(ProgrammingExercise } /** - * Asynchronously retrieves the latest commit hash from the specified repository and logs the access to the repository. - * This method runs without blocking the user during repository access checks. + * When cloning/pushing with SSH we can keep data loaded inside the SSH session, to avoid unnecessary database queries. * * @param user the user accessing the repository * @param optionalParticipation the participation associated with the repository @@ -602,19 +621,16 @@ public void processNewPush(String commitHash, Repository repository, Optional getProgrammingExercise(projectKey)); + RepositoryType repositoryType = getRepositoryType(repositoryTypeOrUserName, exercise); + + ProgrammingExerciseParticipation participation = cachedParticipation.orElseGet(() -> fetchParticipationFromLocalVCRepositoryUri(localVCRepositoryUri, exercise)); + ProgrammingExerciseParticipation participation; - try { - participation = cachedParticipation.orElseGet(() -> retrieveParticipationFromLocalVCRepositoryUri(localVCRepositoryUri, exercise)); + if (repositoryType.equals(RepositoryType.AUXILIARY) || repositoryType.equals(RepositoryType.TESTS)) { + participation = retrieveSolutionParticipation(exercise); } - catch (EntityNotFoundException e) { - if (isRepositoryAuxiliaryRepository(exercise, repositoryTypeOrUserName)) { - participation = retrieveSolutionParticipation(exercise); - } - else { - throw e; - } + else { } - RepositoryType repositoryType = getRepositoryType(repositoryTypeOrUserName, exercise); try { if (repositoryType.equals(RepositoryType.TESTS)) { @@ -637,14 +653,13 @@ public void processNewPush(String commitHash, Repository repository, Optional service.saveVcsAccesslog(vcsAccessLog.get())); } else { - // for HTTPs we cannot keep the vcsAccessLog in the session var finalParticipation = participation; vcsAccessLogService.ifPresent(service -> service.updateCommitHash(finalParticipation, finalCommitHash)); } @@ -826,10 +841,9 @@ private Commit extractCommitInfo(String commitHash, Repository repository) throw * @param localVCRepositoryUri the {@link LocalVCRepositoryUri} containing details about the repository. * @return the {@link ProgrammingExerciseParticipation} corresponding to the repository URI. */ - private ProgrammingExerciseParticipation retrieveParticipationFromLocalVCRepositoryUri(LocalVCRepositoryUri localVCRepositoryUri, ProgrammingExercise exercise) { - String repositoryTypeOrUserName = localVCRepositoryUri.getRepositoryTypeOrUserName(); - var repositoryURL = localVCRepositoryUri.toString().replace("/git-upload-pack", "").replace("/git-receive-pack", ""); - return programmingExerciseParticipationService.retrieveParticipationWithSubmissionsByRepository(repositoryTypeOrUserName, repositoryURL, exercise); + private ProgrammingExerciseParticipation fetchParticipationFromLocalVCRepositoryUri(LocalVCRepositoryUri localVCRepositoryUri, ProgrammingExercise exercise) { + return programmingExerciseParticipationService.fetchParticipationWithSubmissionsByRepository(localVCRepositoryUri.getRepositoryTypeOrUserName(), + localVCRepositoryUri.toString(), exercise); } /** @@ -904,7 +918,7 @@ public void updateAndStoreVCSAccessLogForCloneAndPullSSH(ServerSession session, * Adds a failed VCS access attempt to the log. *

* This method logs a failed clone attempt, associating it with the user and participation retrieved - * from the incoming HTTP request. It assumes that the failed attempt used password authentication. + * from the incoming HTTP request. * * @param servletRequest the {@link HttpServletRequest} containing the HTTP request data. */ @@ -915,11 +929,12 @@ public void createVCSAccessLogForFailedAuthenticationAttempt(HttpServletRequest User user = userRepository.findOneByLogin(usernameAndPassword.username()).orElseThrow(LocalVCAuthException::new); AuthenticationMechanism mechanism = usernameAndPassword.password().startsWith("vcpat-") ? AuthenticationMechanism.VCS_ACCESS_TOKEN : AuthenticationMechanism.PASSWORD; LocalVCRepositoryUri localVCRepositoryUri = parseRepositoryUri(servletRequest); - var participation = retrieveParticipationFromLocalVCRepositoryUri(localVCRepositoryUri, null); + var participation = fetchParticipationFromLocalVCRepositoryUri(localVCRepositoryUri, null); var ipAddress = servletRequest.getRemoteAddr(); vcsAccessLogService.ifPresent(service -> service.storeAccessLog(user, participation, RepositoryActionType.CLONE_FAIL, mechanism, "", ipAddress)); } - catch (LocalVCAuthException ignored) { + catch (LocalVCAuthException | EntityNotFoundException ignored) { + // Caught when: 1) no user, or 2) no participation was found. In both cases it does not make sense to write a log } } From 7812d47c4f4f6881199fa1c52bd32e3733057636 Mon Sep 17 00:00:00 2001 From: entholzer Date: Tue, 19 Nov 2024 15:23:19 +0100 Subject: [PATCH 09/14] cleanup 4 --- .../localvc/LocalVCServletService.java | 48 +++++++++++-------- 1 file changed, 28 insertions(+), 20 deletions(-) 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 90d6a05706b2..d9ffeff3e0e9 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 @@ -621,15 +621,21 @@ public void processNewPush(String commitHash, Repository repository, Optional getProgrammingExercise(projectKey)); - RepositoryType repositoryType = getRepositoryType(repositoryTypeOrUserName, exercise); - - ProgrammingExerciseParticipation participation = cachedParticipation.orElseGet(() -> fetchParticipationFromLocalVCRepositoryUri(localVCRepositoryUri, exercise)); - ProgrammingExerciseParticipation participation; - if (repositoryType.equals(RepositoryType.AUXILIARY) || repositoryType.equals(RepositoryType.TESTS)) { - participation = retrieveSolutionParticipation(exercise); + RepositoryType repositoryType = getRepositoryTypeWithoutAuxiliary(repositoryTypeOrUserName, exercise); + ; + + try { + participation = cachedParticipation.orElseGet(() -> fetchParticipationFromLocalVCRepositoryUri(localVCRepositoryUri, exercise)); } - else { + catch (EntityNotFoundException e) { + repositoryType = getRepositoryType(repositoryTypeOrUserName, exercise); + if (repositoryType.equals(RepositoryType.AUXILIARY) || repositoryType.equals(RepositoryType.TESTS)) { + participation = retrieveSolutionParticipation(exercise); + } + else { + throw e; + } } try { @@ -766,19 +772,21 @@ else if (auxiliaryRepositoryService.isAuxiliaryRepositoryOfExercise(repositoryTy return RepositoryType.USER; } } - // - // private ProgrammingExerciseParticipation getProgrammingExerciseParticipation(LocalVCRepositoryUri localVCRepositoryUri, String repositoryTypeOrUserName, - // ProgrammingExercise exercise) { - // ProgrammingExerciseParticipation participation; - // try { - // participation = programmingExerciseParticipationService.retrieveParticipationWithSubmissionsByRepository(exercise, repositoryTypeOrUserName, - // localVCRepositoryUri.isPracticeRepository()); - // } - // catch (EntityNotFoundException e) { - // throw new VersionControlException("Could not find participation for repository " + repositoryTypeOrUserName + " of exercise " + exercise, e); - // } - // return participation; - // } + + private RepositoryType getRepositoryTypeWithoutAuxiliary(String repositoryTypeOrUserName, ProgrammingExercise exercise) { + if (repositoryTypeOrUserName.equals("exercise")) { + return RepositoryType.TEMPLATE; + } + else if (repositoryTypeOrUserName.equals("solution")) { + return RepositoryType.SOLUTION; + } + else if (repositoryTypeOrUserName.equals("tests")) { + return RepositoryType.TESTS; + } + else { + return RepositoryType.USER; + } + } /** * TODO: this could be done asynchronously to shorten the duration of the push operation From ed27a1bd7270f817592ed8b35fcd4d8f4b6f6048 Mon Sep 17 00:00:00 2001 From: entholzer Date: Tue, 19 Nov 2024 22:37:05 +0100 Subject: [PATCH 10/14] cleanup 5 --- ...ammingExerciseParticipationRepository.java | 7 -- .../repository/VcsAccessLogRepository.java | 8 +- ...ogrammingExerciseParticipationService.java | 2 +- .../localvc/ArtemisGitServletService.java | 4 +- .../service/localvc/HttpsConstants.java | 12 --- .../localvc/LocalVCFetchPreUploadHook.java | 1 - .../service/localvc/LocalVCPrePushHook.java | 2 +- .../localvc/LocalVCServletService.java | 78 ++++++++++--------- .../service/localvc/VcsAccessLogService.java | 11 +++ ...ngExerciseParticipationTestRepository.java | 24 ++++++ .../util/ProgrammingExerciseUtilService.java | 10 +-- 11 files changed, 90 insertions(+), 69 deletions(-) delete mode 100644 src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/HttpsConstants.java create mode 100644 src/test/java/de/tum/cit/aet/artemis/programming/test_repository/TemplateProgrammingExerciseParticipationTestRepository.java diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/TemplateProgrammingExerciseParticipationRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/TemplateProgrammingExerciseParticipationRepository.java index b692ed3e8532..15624a14e31a 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/TemplateProgrammingExerciseParticipationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/TemplateProgrammingExerciseParticipationRepository.java @@ -41,13 +41,6 @@ public interface TemplateProgrammingExerciseParticipationRepository """) Optional findByBuildPlanIdWithResults(@Param("buildPlanId") String buildPlanId); - @EntityGraph(type = LOAD, attributePaths = { "results", "submissions" }) - Optional findWithEagerResultsAndSubmissionsByProgrammingExerciseId(long exerciseId); - - default TemplateProgrammingExerciseParticipation findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(long exerciseId) { - return getValueElseThrow(findWithEagerResultsAndSubmissionsByProgrammingExerciseId(exerciseId)); - } - @EntityGraph(type = LOAD, attributePaths = { "submissions" }) Optional findWithSubmissionsByRepositoryUri(String repositoryUri); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java index 6c9bf8778b6b..fe83cecfd74b 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java @@ -48,11 +48,11 @@ public interface VcsAccessLogRepository extends ArtemisJpaRepository findNewestByRepositoryUri(@Param("repositoryUri") String repositoryUri); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java index e3421ac65a04..a09602907baa 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java @@ -390,7 +390,7 @@ public ProgrammingExerciseParticipation fetchParticipationWithSubmissionsByRepos } - public ProgrammingExerciseParticipation retrieveSolutionPar(Exercise exercise) { + public ProgrammingExerciseParticipation retrieveSolutionParticipation(Exercise exercise) { return solutionParticipationRepository.findByProgrammingExerciseIdElseThrow(exercise.getId()); } 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 c8124d7711f0..6f31748488f2 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 @@ -58,8 +58,8 @@ public void init() { this.setReceivePackFactory((request, repository) -> { ReceivePack receivePack = new ReceivePack(repository); // Add a hook that prevents illegal actions on push (delete branch, rename branch, force push). - // user is always null here - receivePack.setPreReceiveHook(new LocalVCPrePushHook(localVCServletService, (User) request.getSession().getAttribute("user"))); + // the user inside the request is always null here + receivePack.setPreReceiveHook(new LocalVCPrePushHook(localVCServletService, (User) request.getAttribute("user"))); // Add a hook that triggers the creation of a new submission after the push went through successfully. receivePack.setPostReceiveHook(new LocalVCPostPushHook(localVCServletService)); return receivePack; diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/HttpsConstants.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/HttpsConstants.java deleted file mode 100644 index 6502266dfdf0..000000000000 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/HttpsConstants.java +++ /dev/null @@ -1,12 +0,0 @@ -package de.tum.cit.aet.artemis.programming.service.localvc; - -public class HttpsConstants { - - public static final String USER_KEY = "user"; - - public static final String PARTICIPATION_KEY = "participation"; - - public static final String VCS_ACCESS_LOG_KEY = "vcs_access_log"; - - public static final String EXERCISE_KEY = "exercise"; -} diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHook.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHook.java index 6b5bdf6c5ca1..fc2ad46c5647 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHook.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCFetchPreUploadHook.java @@ -23,7 +23,6 @@ public LocalVCFetchPreUploadHook(LocalVCServletService localVCServletService, Ht public void onBeginNegotiateRound(UploadPack uploadPack, Collection collection, int clientOffered) { String authorizationHeader = request.getHeader(LocalVCServletService.AUTHORIZATION_HEADER); localVCServletService.updateAndStoreVCSAccessLogForCloneAndPullHTTPS(request, authorizationHeader, clientOffered); - } @Override diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPrePushHook.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPrePushHook.java index 9c4eb9a9e128..9a6df5eea2de 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPrePushHook.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCPrePushHook.java @@ -58,7 +58,7 @@ public void onPreReceive(ReceivePack receivePack, Collection com String defaultBranchName; try { - defaultBranchName = LocalVCServletService.getDefaultBranchOfRepository(repository); + defaultBranchName = LocalVCService.getDefaultBranchOfRepository(repository.getDirectory().toPath().toString()); } catch (LocalVCInternalException e) { command.setResult(ReceiveCommand.Result.REJECTED_OTHER_REASON, "An error occurred while checking the branch."); 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 d9ffeff3e0e9..30eeb87da1d1 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 @@ -33,7 +33,6 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Profile; import org.springframework.http.HttpStatus; -import org.springframework.scheduling.annotation.Async; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.AuthenticationException; @@ -221,7 +220,7 @@ public void authenticateAndAuthorizeGitRequest(HttpServletRequest request, Repos String authorizationHeader = request.getHeader(LocalVCServletService.AUTHORIZATION_HEADER); - // The first request does not contain an authorizationHeader + // The first request does not contain an authorizationHeader, the client expects this response if (authorizationHeader == null) { throw new LocalVCAuthException("No authorization header provided"); } @@ -305,6 +304,18 @@ private AuthenticationMechanism resolveHTTPSAuthenticationMechanism(String autho return AuthenticationMechanism.PARTICIPATION_VCS_ACCESS_TOKEN; } + /** + * Authenticates a user based on the provided authorization header for a specific programming exercise/repository. + * Authentication is tried with: 1) user VCS access token, 2) user participation VCS access token 3) password + * + * @param authorizationHeader the authorization header containing authentication credentials + * @param exercise the programming exercise the user is attempting to access + * @param localVCRepositoryUri the URI of the local version control repository the user is attempting to access + * + * @return the authenticated {@link User} if authentication is successful + * @throws LocalVCAuthException if an error occurs during authentication with the local version control system + * @throws AuthenticationException if the authentication credentials are invalid or authentication fails + */ private User authenticateUser(String authorizationHeader, ProgrammingExercise exercise, LocalVCRepositoryUri localVCRepositoryUri) throws LocalVCAuthException, AuthenticationException { @@ -318,7 +329,7 @@ private User authenticateUser(String authorizationHeader, ProgrammingExercise ex SecurityUtils.checkUsernameAndPasswordValidity(username, passwordOrToken); } catch (AccessForbiddenException | AuthenticationException e) { - if (!StringUtils.isEmpty(passwordOrToken)) { + if (StringUtils.isNotEmpty(passwordOrToken)) { log.warn("Failed login attempt for user {} with password {} due to issue: {}", username, passwordOrToken, e.getMessage()); } throw new LocalVCAuthException(e.getMessage()); @@ -331,7 +342,7 @@ private User authenticateUser(String authorizationHeader, ProgrammingExercise ex } // check user participation VCS access token - if (tryAuthenticationWithVCSAccessToken(user, passwordOrToken, exercise, localVCRepositoryUri)) { + if (tryAuthenticationWithParticipationVCSAccessToken(user, passwordOrToken, exercise, localVCRepositoryUri)) { return user; } @@ -343,7 +354,16 @@ private User authenticateUser(String authorizationHeader, ProgrammingExercise ex return user; } - private boolean tryAuthenticationWithVCSAccessToken(User user, String providedToken, ProgrammingExercise exercise, LocalVCRepositoryUri localVCRepositoryUri) + /** + * Attempts to authenticate a user with the provided participation VCS access token + * + * @param user the user attempting authentication + * @param providedToken the participation VCS access token provided by the user + * @param exercise the programming exercise containing the repository the user tries to access + * @param localVCRepositoryUri the URI of the local version control repository the user tries to access + * @return {@code true} if the authentication is successful, {@code false} otherwise + */ + private boolean tryAuthenticationWithParticipationVCSAccessToken(User user, String providedToken, ProgrammingExercise exercise, LocalVCRepositoryUri localVCRepositoryUri) throws LocalVCAuthException { // Note: we first check if the user has used a vcs access token instead of a password @@ -362,9 +382,9 @@ private boolean tryAuthenticationWithVCSAccessToken(User user, String providedTo studentParticipation = participations.stream().filter(participation -> participation.getRepositoryUri().equals(localVCRepositoryUri.toString())).findAny(); } if (studentParticipation.isPresent()) { - var token = participationVCSAccessTokenRepository.findByUserIdAndParticipationId(user.getId(), studentParticipation.get().getId()); - if (token.isPresent() && Objects.equals(token.get().getVcsAccessToken(), providedToken)) { - user.setVcsAccessToken(token.get().getVcsAccessToken()); + var storedToken = participationVCSAccessTokenRepository.findByUserIdAndParticipationId(user.getId(), studentParticipation.get().getId()); + if (storedToken.isPresent() && Objects.equals(storedToken.get().getVcsAccessToken(), providedToken)) { + user.setVcsAccessToken(storedToken.get().getVcsAccessToken()); return true; } } @@ -413,6 +433,13 @@ private ProgrammingExercise getProgrammingExerciseOrThrow(String projectKey) { } } + /** + * Extracts the username and password from a Basic Authorization header. + * + * @param authorizationHeader the authorization header containing Basic credentials + * @return a {@link UsernameAndPassword} object with the extracted username and password + * @throws LocalVCAuthException if the header is missing, invalid, or improperly formatted + */ private UsernameAndPassword extractUsernameAndPassword(String authorizationHeader) throws LocalVCAuthException { if (authorizationHeader == null) { throw new LocalVCAuthException("No authorization header provided"); @@ -462,7 +489,7 @@ public Optional authorizeUser(String repositor } private ProgrammingExerciseParticipation tryToLoadParticipation(boolean usingSSH, String repositoryTypeOrUserName, LocalVCRepositoryUri localVCRepositoryUri, - ProgrammingExercise exercise) throws LocalVCForbiddenException { + ProgrammingExercise exercise) throws LocalVCInternalException { ProgrammingExerciseParticipation participation; try { if (usingSSH) { @@ -626,7 +653,8 @@ public void processNewPush(String commitHash, Repository repository, Optional fetchParticipationFromLocalVCRepositoryUri(localVCRepositoryUri, exercise)); + participation = cachedParticipation.orElseGet(() -> programmingExerciseParticipationService + .fetchParticipationWithSubmissionsByRepository(localVCRepositoryUri.getRepositoryTypeOrUserName(), localVCRepositoryUri.toString(), exercise)); } catch (EntityNotFoundException e) { repositoryType = getRepositoryType(repositoryTypeOrUserName, exercise); @@ -634,7 +662,7 @@ public void processNewPush(String commitHash, Repository repository, Optional @@ -907,7 +913,6 @@ public void updateAndStoreVCSAccessLogForCloneAndPullHTTPS(HttpServletRequest re * @param clientOffered the number of objects offered by the client in the operation, used to determine * if the action is a clone (if 0) or a pull (if greater than 0). */ - @Async public void updateAndStoreVCSAccessLogForCloneAndPullSSH(ServerSession session, int clientOffered) { try { if (session.getAttribute(SshConstants.USER_KEY).getName().equals(BUILD_USER_NAME)) { @@ -937,7 +942,8 @@ public void createVCSAccessLogForFailedAuthenticationAttempt(HttpServletRequest User user = userRepository.findOneByLogin(usernameAndPassword.username()).orElseThrow(LocalVCAuthException::new); AuthenticationMechanism mechanism = usernameAndPassword.password().startsWith("vcpat-") ? AuthenticationMechanism.VCS_ACCESS_TOKEN : AuthenticationMechanism.PASSWORD; LocalVCRepositoryUri localVCRepositoryUri = parseRepositoryUri(servletRequest); - var participation = fetchParticipationFromLocalVCRepositoryUri(localVCRepositoryUri, null); + var participation = programmingExerciseParticipationService.fetchParticipationWithSubmissionsByRepository(localVCRepositoryUri.getRepositoryTypeOrUserName(), + localVCRepositoryUri.toString(), null); var ipAddress = servletRequest.getRemoteAddr(); vcsAccessLogService.ifPresent(service -> service.storeAccessLog(user, participation, RepositoryActionType.CLONE_FAIL, mechanism, "", ipAddress)); } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java index 1a049ea1a722..1700c7be4d41 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java @@ -72,6 +72,12 @@ public void updateCommitHash(ProgrammingExerciseParticipation participation, Str } } + /** + * Updates the commit hash of the newest log entry + * + * @param localVCRepositoryUri The localVCRepositoryUri of the participation to which vcsAccessLog belongs to + * @param repositoryActionType The repository action type to which the vcsAccessLog should get updated to + */ @Async public void updateRepositoryActionType(LocalVCRepositoryUri localVCRepositoryUri, RepositoryActionType repositoryActionType) { var repositoryURL = localVCRepositoryUri.toString().replace("/git-upload-pack", "").replace("/git-receive-pack", ""); @@ -82,6 +88,11 @@ public void updateRepositoryActionType(LocalVCRepositoryUri localVCRepositoryUri } } + /** + * Saves an vcsAccessLog + * + * @param vcsAccessLog The vcsAccessLog to save + */ @Async public void saveVcsAccesslog(VcsAccessLog vcsAccessLog) { vcsAccessLogRepository.save(vcsAccessLog); diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/test_repository/TemplateProgrammingExerciseParticipationTestRepository.java b/src/test/java/de/tum/cit/aet/artemis/programming/test_repository/TemplateProgrammingExerciseParticipationTestRepository.java new file mode 100644 index 000000000000..4eb0d9f8c22b --- /dev/null +++ b/src/test/java/de/tum/cit/aet/artemis/programming/test_repository/TemplateProgrammingExerciseParticipationTestRepository.java @@ -0,0 +1,24 @@ +package de.tum.cit.aet.artemis.programming.test_repository; + +import static org.springframework.data.jpa.repository.EntityGraph.EntityGraphType.LOAD; + +import java.util.Optional; + +import org.springframework.context.annotation.Primary; +import org.springframework.data.jpa.repository.EntityGraph; +import org.springframework.stereotype.Repository; + +import de.tum.cit.aet.artemis.programming.domain.TemplateProgrammingExerciseParticipation; +import de.tum.cit.aet.artemis.programming.repository.TemplateProgrammingExerciseParticipationRepository; + +@Repository +@Primary +public interface TemplateProgrammingExerciseParticipationTestRepository extends TemplateProgrammingExerciseParticipationRepository { + + @EntityGraph(type = LOAD, attributePaths = { "results", "submissions" }) + Optional findWithEagerResultsAndSubmissionsByProgrammingExerciseId(long exerciseId); + + default TemplateProgrammingExerciseParticipation findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(long exerciseId) { + return getValueElseThrow(findWithEagerResultsAndSubmissionsByProgrammingExerciseId(exerciseId)); + } +} diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseUtilService.java b/src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseUtilService.java index e27ce95ac8ca..2a7bd69dd25e 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseUtilService.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseUtilService.java @@ -70,7 +70,6 @@ import de.tum.cit.aet.artemis.programming.repository.SolutionProgrammingExerciseParticipationRepository; import de.tum.cit.aet.artemis.programming.repository.StaticCodeAnalysisCategoryRepository; import de.tum.cit.aet.artemis.programming.repository.SubmissionPolicyRepository; -import de.tum.cit.aet.artemis.programming.repository.TemplateProgrammingExerciseParticipationRepository; import de.tum.cit.aet.artemis.programming.repository.hestia.CodeHintRepository; import de.tum.cit.aet.artemis.programming.repository.hestia.ExerciseHintRepository; import de.tum.cit.aet.artemis.programming.repository.hestia.ProgrammingExerciseSolutionEntryRepository; @@ -79,6 +78,7 @@ import de.tum.cit.aet.artemis.programming.test_repository.ProgrammingExerciseTestCaseTestRepository; import de.tum.cit.aet.artemis.programming.test_repository.ProgrammingExerciseTestRepository; import de.tum.cit.aet.artemis.programming.test_repository.ProgrammingSubmissionTestRepository; +import de.tum.cit.aet.artemis.programming.test_repository.TemplateProgrammingExerciseParticipationTestRepository; /** * Service responsible for initializing the database with specific testdata related to programming exercises for use in integration tests. @@ -97,7 +97,7 @@ public class ProgrammingExerciseUtilService { protected String artemisVersionControlUrl; @Autowired - private TemplateProgrammingExerciseParticipationRepository templateProgrammingExerciseParticipationRepo; + private TemplateProgrammingExerciseParticipationTestRepository templateProgrammingExerciseParticipationTestRepo; @Autowired private ProgrammingExerciseTestRepository programmingExerciseRepository; @@ -206,7 +206,7 @@ public ProgrammingExercise addTemplateParticipationForProgrammingExercise(Progra participation.setBuildPlanId(exercise.generateBuildPlanId(BuildPlanType.TEMPLATE)); participation.setRepositoryUri(String.format("%s/git/%s/%s.git", artemisVersionControlUrl, exercise.getProjectKey(), repoName)); participation.setInitializationState(InitializationState.INITIALIZED); - templateProgrammingExerciseParticipationRepo.save(participation); + templateProgrammingExerciseParticipationTestRepo.save(participation); exercise.setTemplateParticipation(participation); return programmingExerciseRepository.save(exercise); } @@ -774,7 +774,7 @@ public Result addProgrammingSubmissionWithResult(ProgrammingExercise exercise, P * @return the newly created result */ public Result addTemplateSubmissionWithResult(long exerciseId) { - var templateParticipation = templateProgrammingExerciseParticipationRepo.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exerciseId); + var templateParticipation = templateProgrammingExerciseParticipationTestRepo.findWithEagerResultsAndSubmissionsByProgrammingExerciseIdElseThrow(exerciseId); ProgrammingSubmission submission = new ProgrammingSubmission(); submission = submissionRepository.save(submission); Result result = resultRepo.save(new Result().participation(templateParticipation)); @@ -785,7 +785,7 @@ public Result addTemplateSubmissionWithResult(long exerciseId) { result.setSubmission(submission); result = resultRepo.save(result); templateParticipation.addResult(result); - templateProgrammingExerciseParticipationRepo.save(templateParticipation); + templateProgrammingExerciseParticipationTestRepo.save(templateParticipation); return result; } From 48f3255de4ca68a9f533244cbec68180569b804a Mon Sep 17 00:00:00 2001 From: entholzer Date: Tue, 19 Nov 2024 22:57:55 +0100 Subject: [PATCH 11/14] fix tests --- .../programming/hestia/util/HestiaUtilTestService.java | 4 ++-- .../base/AbstractSpringIntegrationLocalCILocalVCTest.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/hestia/util/HestiaUtilTestService.java b/src/test/java/de/tum/cit/aet/artemis/programming/hestia/util/HestiaUtilTestService.java index 92e857ca6eea..f4bae09afa51 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/hestia/util/HestiaUtilTestService.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/hestia/util/HestiaUtilTestService.java @@ -26,11 +26,11 @@ import de.tum.cit.aet.artemis.programming.domain.Repository; import de.tum.cit.aet.artemis.programming.repository.ProgrammingExerciseBuildConfigRepository; import de.tum.cit.aet.artemis.programming.repository.SolutionProgrammingExerciseParticipationRepository; -import de.tum.cit.aet.artemis.programming.repository.TemplateProgrammingExerciseParticipationRepository; import de.tum.cit.aet.artemis.programming.service.GitService; import de.tum.cit.aet.artemis.programming.test_repository.ProgrammingExerciseStudentParticipationTestRepository; import de.tum.cit.aet.artemis.programming.test_repository.ProgrammingExerciseTestRepository; import de.tum.cit.aet.artemis.programming.test_repository.ProgrammingSubmissionTestRepository; +import de.tum.cit.aet.artemis.programming.test_repository.TemplateProgrammingExerciseParticipationTestRepository; import de.tum.cit.aet.artemis.programming.util.GitUtilService; import de.tum.cit.aet.artemis.programming.util.LocalRepository; import de.tum.cit.aet.artemis.programming.util.ProgrammingExerciseUtilService; @@ -57,7 +57,7 @@ public class HestiaUtilTestService { private ProgrammingSubmissionTestRepository programmingSubmissionRepository; @Autowired - private TemplateProgrammingExerciseParticipationRepository templateProgrammingExerciseParticipationRepository; + private TemplateProgrammingExerciseParticipationTestRepository templateProgrammingExerciseParticipationRepository; @Autowired private SolutionProgrammingExerciseParticipationRepository solutionProgrammingExerciseParticipationRepository; diff --git a/src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java b/src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java index 27c299ffa2ac..ecaddcbfd2e5 100644 --- a/src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java @@ -49,13 +49,13 @@ import de.tum.cit.aet.artemis.programming.icl.TestBuildAgentConfiguration; import de.tum.cit.aet.artemis.programming.repository.ProgrammingExerciseBuildConfigRepository; import de.tum.cit.aet.artemis.programming.repository.SolutionProgrammingExerciseParticipationRepository; -import de.tum.cit.aet.artemis.programming.repository.TemplateProgrammingExerciseParticipationRepository; import de.tum.cit.aet.artemis.programming.service.ProgrammingMessagingService; import de.tum.cit.aet.artemis.programming.service.localci.LocalCIService; import de.tum.cit.aet.artemis.programming.service.localvc.LocalVCService; import de.tum.cit.aet.artemis.programming.test_repository.BuildJobTestRepository; import de.tum.cit.aet.artemis.programming.test_repository.ProgrammingExerciseStudentParticipationTestRepository; import de.tum.cit.aet.artemis.programming.test_repository.ProgrammingExerciseTestRepository; +import de.tum.cit.aet.artemis.programming.test_repository.TemplateProgrammingExerciseParticipationTestRepository; // Must start up an actual web server such that the tests can communicate with the ArtemisGitServlet using JGit. // Otherwise, only MockMvc requests could be used. The port this runs on is defined at server.port (see @TestPropertySource). @@ -101,7 +101,7 @@ public abstract class AbstractSpringIntegrationLocalCILocalVCTest extends Abstra protected ProgrammingExerciseBuildConfigRepository programmingExerciseBuildConfigRepository; @Autowired - protected TemplateProgrammingExerciseParticipationRepository templateProgrammingExerciseParticipationRepository; + protected TemplateProgrammingExerciseParticipationTestRepository templateProgrammingExerciseParticipationRepository; @Autowired protected SolutionProgrammingExerciseParticipationRepository solutionProgrammingExerciseParticipationRepository; From 5bdfea26fca3250e707007161d3226087a02e11e Mon Sep 17 00:00:00 2001 From: entholzer Date: Tue, 19 Nov 2024 23:31:07 +0100 Subject: [PATCH 12/14] fix server style --- .../ProgrammingExerciseParticipationService.java | 2 ++ .../programming/service/RepositoryService.java | 1 + .../service/localvc/LocalVCServletService.java | 11 +++++++---- .../service/localvc/VcsAccessLogService.java | 3 ++- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java index a09602907baa..c6fc7c1c5c3f 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java @@ -376,6 +376,7 @@ public void resetRepository(VcsRepositoryUri targetURL, VcsRepositoryUri sourceU * * @param repositoryTypeOrUserName the name of the user or the type of the repository * @param repositoryURI the participation's repository URL + * @param exercise the exercise the participation belongs to * @return the participation belonging to the provided repositoryURI and repository type or username */ public ProgrammingExerciseParticipation fetchParticipationWithSubmissionsByRepository(String repositoryTypeOrUserName, String repositoryURI, ProgrammingExercise exercise) { @@ -400,6 +401,7 @@ public ProgrammingExerciseParticipation retrieveSolutionParticipation(Exercise e * * @param repositoryTypeOrUserName the name of the user or the type of the repository * @param repositoryURI the participation's repository URL + * @param exercise the exercise the participation belongs to * @return the participation belonging to the provided repositoryURI and repository type or username */ public ProgrammingExerciseParticipation fetchParticipationByRepository(String repositoryTypeOrUserName, String repositoryURI, ProgrammingExercise exercise) { diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java index 2e7627d7e22c..9c09f451a390 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java @@ -453,6 +453,7 @@ public void pullChanges(Repository repository) { * @param repository for which to execute the commit. * @param user the user who has committed the changes in the online editor * @param domainId the id of the domain Object (participation) owning the repository + * @return an Optional of a preliminary VcsAccessLog * @throws GitAPIException if the staging/committing process fails. */ public Optional commitChanges(Repository repository, User user, Long domainId) throws GitAPIException { 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 30eeb87da1d1..235d841cf045 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 @@ -472,6 +472,8 @@ private UsernameAndPassword extractUsernameAndPassword(String authorizationHeade * @param exercise The exercise the repository belongs to. * @param repositoryActionType The type of the action the user wants to perform. * @param localVCRepositoryUri The URI of the local repository. + * @param usingSSH The flag specifying whether the method is called from the SSH or HTTPs context + * @return the ProgrammingParticipation Optional, containing the participation fetched during authorization * @throws LocalVCForbiddenException If the user is not allowed to access the repository. */ public Optional authorizeUser(String repositoryTypeOrUserName, User user, ProgrammingExercise exercise, @@ -525,10 +527,10 @@ private void checkAccessForRepository(ProgrammingExerciseParticipation participa * Checks if the provided repository is an auxiliary or test repository. * Only load auxiliary repositories if a user is at least teaching assistant; students are not allowed to access them * - * @param exercise - * @param repositoryTypeOrUserName - * @param repositoryActionType - * @param user + * @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 user is TA and * @throws LocalVCForbiddenException */ @@ -567,6 +569,7 @@ private boolean checkIfRepositoryIsAuxiliaryOrTestRepository(ProgrammingExercise * @param authenticationMechanism the mechanism used for authentication (e.g., token, basic auth) * @param ipAddress the IP address of the user accessing the repository * @param localVCRepositoryUri the URI of the localVC repository + * @param serverSession the SSH serverSession, where the data gets stored */ public void cacheAttributesInSshSession(User user, Optional optionalParticipation, RepositoryActionType repositoryActionType, AuthenticationMechanism authenticationMechanism, String ipAddress, LocalVCRepositoryUri localVCRepositoryUri, ServerSession serverSession) { diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java index 1700c7be4d41..47530cbac1fb 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java @@ -99,11 +99,12 @@ public void saveVcsAccesslog(VcsAccessLog vcsAccessLog) { } /** - * Stores the log for a push from the code editor. + * Creates a preliminary access log for a push from the code editor, and returns it * * @param repo The repository to which the push is executed * @param user The user submitting the change * @param participationId The id of the participation belonging to the repository + * @return an Optional containing the preliminary VcsAccessLog, if one was created * @throws GitAPIException if an error occurs while retrieving the git log */ public Optional createPreliminaryCodeEditorAccessLog(Repository repo, User user, Long participationId) throws GitAPIException { From 40be7f9c472140181d6d7b5e0b4594cede8d7904 Mon Sep 17 00:00:00 2001 From: entholzer Date: Sun, 22 Dec 2024 10:51:37 +0100 Subject: [PATCH 13/14] add suggestions --- .../service/RepositoryService.java | 16 ++++++-- .../localvc/LocalVCServletService.java | 38 +++++++++++++------ .../service/localvc/VcsAccessLogService.java | 2 +- .../web/repository/RepositoryResource.java | 4 +- 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java index 9c09f451a390..1ca095cac600 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java @@ -452,13 +452,23 @@ public void pullChanges(Repository repository) { * * @param repository for which to execute the commit. * @param user the user who has committed the changes in the online editor - * @param domainId the id of the domain Object (participation) owning the repository - * @return an Optional of a preliminary VcsAccessLog * @throws GitAPIException if the staging/committing process fails. */ - public Optional commitChanges(Repository repository, User user, Long domainId) throws GitAPIException { + public void commitChanges(Repository repository, User user) throws GitAPIException { gitService.stageAllChanges(repository); gitService.commitAndPush(repository, "Changes by Online Editor", true, user); + } + + /** + * Saves a preliminary access log for a push from the code editor, and returns it + * + * @param repository for which to execute the commit. + * @param user the user who has committed the changes in the online editor + * @param domainId the id that serves as an abstract identifier for retrieving the repository. + * @return an Optional of a preliminary VcsAccessLog + * @throws GitAPIException if accessing the repository fails + */ + public Optional savePreliminaryCodeEditorAccessLog(Repository repository, User user, Long domainId) throws GitAPIException { return vcsAccessLogService.isPresent() ? vcsAccessLogService.get().createPreliminaryCodeEditorAccessLog(repository, user, domainId) : Optional.empty(); } 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 235d841cf045..bb33dfc715cf 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 @@ -257,12 +257,22 @@ public void authenticateAndAuthorizeGitRequest(HttpServletRequest request, Repos } var optionalParticipation = authorizeUser(repositoryTypeOrUserName, user, exercise, repositoryAction, localVCRepositoryUri, false); - storePreliminaryVcsAccessLogForHTTPs(request, localVCRepositoryUri, user, repositoryAction, optionalParticipation); + savePreliminaryVcsAccessLogForHTTPs(request, localVCRepositoryUri, user, repositoryAction, optionalParticipation); log.debug("Authorizing user {} for repository {} took {}", user.getLogin(), localVCRepositoryUri, TimeLogUtil.formatDurationFrom(timeNanoStart)); } - private void storePreliminaryVcsAccessLogForHTTPs(HttpServletRequest request, LocalVCRepositoryUri localVCRepositoryUri, User user, RepositoryActionType repositoryAction, + /** + * Determines whether a given request to access a local VC repository (either via fetch of push) is authenticated and authorized. + * + * @param request The object containing all information about the incoming request. + * @param localVCRepositoryUri The uri of the requested repository + * @param user The user + * @param repositoryAction Indicates whether the method should authenticate a fetch or a push request. For a push request, additional checks are conducted. + * @param optionalParticipation The participation for which the access log should be stored. If an empty Optional is provided, the method does nothing + * @throws LocalVCAuthException If the user authentication fails or the user is not authorized to access a certain repository. + */ + private void savePreliminaryVcsAccessLogForHTTPs(HttpServletRequest request, LocalVCRepositoryUri localVCRepositoryUri, User user, RepositoryActionType repositoryAction, Optional optionalParticipation) throws LocalVCAuthException { if (optionalParticipation.isPresent()) { ProgrammingExerciseParticipation participation = optionalParticipation.get(); @@ -279,7 +289,7 @@ private void storePreliminaryVcsAccessLogForHTTPs(HttpServletRequest request, Lo String finalCommitHash = commitHash; RepositoryActionType finalRepositoryAction = repositoryAction == RepositoryActionType.WRITE ? RepositoryActionType.PUSH : RepositoryActionType.PULL; - vcsAccessLogService.ifPresent(service -> service.storeAccessLog(user, participation, finalRepositoryAction, authenticationMechanism, finalCommitHash, ipAddress)); + vcsAccessLogService.ifPresent(service -> service.saveAccessLog(user, participation, finalRepositoryAction, authenticationMechanism, finalCommitHash, ipAddress)); } } @@ -371,7 +381,6 @@ private boolean tryAuthenticationWithParticipationVCSAccessToken(User user, Stri try { // check participation vcs access token - // var part = programmingExerciseParticipationService.findTeamParticipationByExerciseAndTeamShortNameOrThrow() List participations; Optional studentParticipation; if (exercise.isTeamMode()) { @@ -525,24 +534,30 @@ private void checkAccessForRepository(ProgrammingExerciseParticipation participa /** * Checks if the provided repository is an auxiliary or test repository. - * Only load auxiliary repositories if a user is at least teaching assistant; students are not allowed to access them + * 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. * * @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 user is TA and - * @throws LocalVCForbiddenException + * @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 */ private boolean checkIfRepositoryIsAuxiliaryOrTestRepository(ProgrammingExercise exercise, String repositoryTypeOrUserName, RepositoryActionType repositoryActionType, User user) throws LocalVCForbiddenException { - // Students should not be able to access Test or Aux repositories. - // To save on db queries we do not check that, until after we tried to fetch the user repository as a user repository + // 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. if (!authorizationCheckService.isAtLeastTeachingAssistantForExercise(exercise, user)) { if (repositoryTypeOrUserName.equals(RepositoryType.TESTS.toString())) { throw new LocalVCForbiddenException(); } + // The user is a student, and the repository is not a test repository return false; } @@ -555,8 +570,10 @@ private boolean checkIfRepositoryIsAuxiliaryOrTestRepository(ProgrammingExercise catch (AccessForbiddenException e) { throw new LocalVCForbiddenException(e); } + // 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; } @@ -653,7 +670,6 @@ public void processNewPush(String commitHash, Repository repository, Optional getProgrammingExercise(projectKey)); ProgrammingExerciseParticipation participation; RepositoryType repositoryType = getRepositoryTypeWithoutAuxiliary(repositoryTypeOrUserName, exercise); - ; try { participation = cachedParticipation.orElseGet(() -> programmingExerciseParticipationService @@ -948,7 +964,7 @@ public void createVCSAccessLogForFailedAuthenticationAttempt(HttpServletRequest var participation = programmingExerciseParticipationService.fetchParticipationWithSubmissionsByRepository(localVCRepositoryUri.getRepositoryTypeOrUserName(), localVCRepositoryUri.toString(), null); var ipAddress = servletRequest.getRemoteAddr(); - vcsAccessLogService.ifPresent(service -> service.storeAccessLog(user, participation, RepositoryActionType.CLONE_FAIL, mechanism, "", ipAddress)); + vcsAccessLogService.ifPresent(service -> service.saveAccessLog(user, participation, RepositoryActionType.CLONE_FAIL, mechanism, "", ipAddress)); } catch (LocalVCAuthException | EntityNotFoundException ignored) { // Caught when: 1) no user, or 2) no participation was found. In both cases it does not make sense to write a log diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java index 47530cbac1fb..583ceeebadaa 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/VcsAccessLogService.java @@ -48,7 +48,7 @@ public class VcsAccessLogService { * @param ipAddress The ip address of the user accessing the repository */ @Async - public void storeAccessLog(User user, ProgrammingExerciseParticipation participation, RepositoryActionType actionType, AuthenticationMechanism authenticationMechanism, + public void saveAccessLog(User user, ProgrammingExerciseParticipation participation, RepositoryActionType actionType, AuthenticationMechanism authenticationMechanism, String commitHash, String ipAddress) { log.debug("Storing access operation for user {}", user); diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryResource.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryResource.java index 4a8e1e2810a0..abe9b9ec40e2 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/web/repository/RepositoryResource.java @@ -281,7 +281,9 @@ public ResponseEntity commitChanges(Long domainId) { return executeAndCheckForExceptions(() -> { Repository repository = getRepository(domainId, RepositoryActionType.WRITE, true); - var vcsAccessLog = repositoryService.commitChanges(repository, user, domainId); + repositoryService.commitChanges(repository, user); + var vcsAccessLog = repositoryService.savePreliminaryCodeEditorAccessLog(repository, user, domainId); + // Trigger a build, and process the result. Only implemented for local CI. // For GitLab + Jenkins, webhooks were added when creating the repository, // that notify the CI system when the commit happens and thus trigger the build. From bf2a651fba2844acfc0a8ae6f9fe3fa7c084ce76 Mon Sep 17 00:00:00 2001 From: entholzer Date: Fri, 3 Jan 2025 15:24:06 +0100 Subject: [PATCH 14/14] Improve intendation --- .../repository/VcsAccessLogRepository.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java index fe83cecfd74b..acd9c46fe47f 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java @@ -33,11 +33,11 @@ public interface VcsAccessLogRepository extends ArtemisJpaRepository findNewestByParticipationId(@Param("participationId") long participationId); @@ -48,12 +48,12 @@ public interface VcsAccessLogRepository extends ArtemisJpaRepository findNewestByRepositoryUri(@Param("repositoryUri") String repositoryUri);