From ed27a1bd7270f817592ed8b35fcd4d8f4b6f6048 Mon Sep 17 00:00:00 2001 From: entholzer Date: Tue, 19 Nov 2024 22:37:05 +0100 Subject: [PATCH] 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; }