From a4ff2f1670d5e763742604006b5672fb5d2d063f Mon Sep 17 00:00:00 2001
From: Hamza Remmal <hamzaremmalpriv@gmail.com>
Date: Thu, 10 Aug 2023 14:59:57 +0100
Subject: [PATCH] Change the integrity check to use the @PreAuthorize instead
 of writing the full logic

---
 .../api/v1/controller/FeedbackController.java | 19 ++++------------
 .../v1/controller/SubmissionController.java   | 22 ++++---------------
 .../epfl/autograde/config/SecurityConfig.java |  2 ++
 3 files changed, 10 insertions(+), 33 deletions(-)

diff --git a/autograde-service/src/main/java/ch/epfl/autograde/api/v1/controller/FeedbackController.java b/autograde-service/src/main/java/ch/epfl/autograde/api/v1/controller/FeedbackController.java
index fcfee1ef..b45042ed 100644
--- a/autograde-service/src/main/java/ch/epfl/autograde/api/v1/controller/FeedbackController.java
+++ b/autograde-service/src/main/java/ch/epfl/autograde/api/v1/controller/FeedbackController.java
@@ -7,6 +7,7 @@ import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
+import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.web.bind.annotation.*;
 
 /**
@@ -18,22 +19,17 @@ import org.springframework.web.bind.annotation.*;
 @Slf4j
 @RestController
 @RequestMapping("/api/v1/feedback")
-public final class FeedbackController {
-
-    /** Service to generate and check the integrity of the requests */
-    private final IntegrityService integrity;
+public class FeedbackController {
 
     /** Service to communicate with Moodle */
     private final MoodleWebService moodle;
 
     /**
      * @param moodle service to use to communicate with Moodle.
-     * @param integrity service to use for the integrity checks.
      */
     @Autowired
-    public FeedbackController(MoodleWebService moodle, IntegrityService integrity) {
+    public FeedbackController(MoodleWebService moodle) {
         this.moodle = moodle;
-        this.integrity = integrity;
     }
 
     /**
@@ -46,19 +42,12 @@ public final class FeedbackController {
      * @apiNote This end point should only be called by the autograde-service itself
      */
     @PostMapping("/upload")
+    @PreAuthorize("@integrityService.check(#signature, #id)")
     public ResponseEntity<?> submitGrade(@RequestParam int id,
                                          @RequestParam String signature,
                                          @RequestBody UploadFeedbackDTO feedback)
     {
         log.info("Received an 'upload feedback' request for submission with id {}", id);
-        // HR : Check the integrity of the request
-        if (!integrity.check(signature, id)) {
-            log.error("Integrity check failed for 'upload feedback' request for submission {}", id);
-            return ResponseEntity
-                    .status(HttpStatus.UNAUTHORIZED)
-                    .body("The provided signature is not correct !");
-        }
-        log.info("Integrity check for 'upload feedback' request for submission {} was successful", id);
         // HR : Upload the grade and feedback to Moodle
         try {
             var res = moodle.upload_feedback(id, feedback.getGrade(), feedback.getFeedback().toJson());
diff --git a/autograde-service/src/main/java/ch/epfl/autograde/api/v1/controller/SubmissionController.java b/autograde-service/src/main/java/ch/epfl/autograde/api/v1/controller/SubmissionController.java
index c6aeba74..a9e27eaa 100644
--- a/autograde-service/src/main/java/ch/epfl/autograde/api/v1/controller/SubmissionController.java
+++ b/autograde-service/src/main/java/ch/epfl/autograde/api/v1/controller/SubmissionController.java
@@ -1,13 +1,12 @@
 package ch.epfl.autograde.api.v1.controller;
 
-import ch.epfl.autograde.api.v1.service.IntegrityService;
 import ch.epfl.autograde.api.v1.service.MoodleWebService;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.core.io.InputStreamResource;
 import org.springframework.http.HttpHeaders;
-import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
+import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
@@ -25,22 +24,17 @@ import static org.springframework.http.MediaType.APPLICATION_OCTET_STREAM_VALUE;
 @Slf4j
 @RestController
 @RequestMapping("/api/v1/submission")
-public final class SubmissionController {
+public class SubmissionController {
 
     /** Service to communicate with Moodle */
     private final MoodleWebService moodle;
 
-    /** Service to generate and check the integrity of the requests */
-    private final IntegrityService integrity;
-
     /**
      * @param moodle service to use to communicate with Moodle.
-     * @param integrity service to use for the integrity checks.
      */
     @Autowired
-    public SubmissionController(MoodleWebService moodle, IntegrityService integrity) {
+    public SubmissionController(MoodleWebService moodle) {
         this.moodle = moodle;
-        this.integrity = integrity;
     }
 
     /**
@@ -55,17 +49,9 @@ public final class SubmissionController {
      * @apiNote This end point should only be called by the autograde-service itself
      */
     @GetMapping("/download")
+    @PreAuthorize("@integrityService.check(#signature, #id)")
     public ResponseEntity<?> download(@RequestParam int id, @RequestParam String signature) {
         log.info("Received a 'download submission' request for submission {}", id);
-
-        // HR : Check the integrity of the signature
-        if (!integrity.check(signature, id)){
-            log.error("Integrity check failed for 'download submission' request for submission {}", id);
-            return ResponseEntity
-                    .status(HttpStatus.UNAUTHORIZED)
-                    .body("The provided signature is not correct !");
-        }
-        log.info("Integrity check for 'download submission' request for submission {} was successful", id);
         // HR : Serve the actual file, the integrity of the signature was checked
         try (var file = moodle.download_submission(id)) {
             // HR : Prepare the headers
diff --git a/autograde-service/src/main/java/ch/epfl/autograde/config/SecurityConfig.java b/autograde-service/src/main/java/ch/epfl/autograde/config/SecurityConfig.java
index 290cf2e6..d373f814 100644
--- a/autograde-service/src/main/java/ch/epfl/autograde/config/SecurityConfig.java
+++ b/autograde-service/src/main/java/ch/epfl/autograde/config/SecurityConfig.java
@@ -4,6 +4,7 @@ import ch.epfl.autograde.auth.filter.ApiKeyAuthenticationFilter;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity;
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer;
@@ -18,6 +19,7 @@ import org.springframework.security.web.authentication.www.BasicAuthenticationFi
  */
 @Configuration
 @EnableWebSecurity
+@EnableMethodSecurity
 public class SecurityConfig {
 
     /** autograde custom API-KEY authentication filter */
-- 
GitLab