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.