ls1intum · krusche · Jul 19, 2026
@@ -3,6 +3,7 @@
 import de.tum.cit.aet.helios.environment.Environment;
 import de.tum.cit.aet.helios.environment.EnvironmentService;
 import de.tum.cit.aet.helios.environment.EnvironmentService.ReviewerResolution;
+import de.tum.cit.aet.helios.github.GitHubReviewException;
 import de.tum.cit.aet.helios.github.GitHubService;
 import de.tum.cit.aet.helios.heliosdeployment.HeliosDeployment;
 import de.tum.cit.aet.helios.heliosdeployment.HeliosDeploymentRepository;
@@ -146,8 +147,10 @@ private DeploymentApprovalRequest review(
           currentLogin,
           heliosDeploymentId,
           e.getMessage());
-      throw new ResponseStatusException(
-          HttpStatus.BAD_GATEWAY, "GitHub rejected the request: " + e.getMessage(), e);
+      // Keep the client-facing status at 502: this is an upstream GitHub failure, not the Helios
+      // request being unauthenticated. Returning 401 here would make the client treat the user's
+      // Helios session as expired and bounce them to login.
+      throw new ResponseStatusException(HttpStatus.BAD_GATEWAY, gitHubFailureReason(e), e);
     }
 
     // GitHub accepted — finalise this reviewer's row and consume siblings.
@@ -213,6 +216,19 @@ private static boolean isTerminallyResolved(DeploymentApprovalRequest r) {
         || r.getState() == DeploymentApprovalRequest.State.DECLINED;
   }
 
+  /**
+   * Turns a GitHub review failure into a reviewer-facing message. An {@code HTTP 401} means the
+   * impersonated user's GitHub token (brokered via Keycloak) has expired — the reviewer can only
+   * recover by re-authenticating — so we say so explicitly instead of leaking the raw status.
+   */
+  private static String gitHubFailureReason(IOException e) {
+    if (e instanceof GitHubReviewException gh && gh.getHttpStatus() == 401) {
+      return "GitHub rejected the request because your GitHub authorization has expired. "
+          + "Please sign out of Helios and sign in again, then retry.";
+    }
+    return "GitHub rejected the request: " + e.getMessage();
+  }
+
   private static String buildAuditComment(String login, boolean approve, String userComment) {
     String base =
         (approve ? "Approved by @" : "Declined by @") + login + " via Helios (in-app)";

@@ -14,6 +14,7 @@
 import lombok.extern.log4j.Log4j2;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
+import org.springframework.http.HttpStatusCode;
 import org.springframework.http.ResponseEntity;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.authorization.AuthorizationDeniedException;
@@ -22,6 +23,7 @@
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.annotation.RestControllerAdvice;
+import org.springframework.web.server.ResponseStatusException;
 
 @Log4j2
 @RestControllerAdvice
@@ -138,6 +140,38 @@ public ResponseEntity<ApiError> handleTestFailureAnalysisRateLimitException(
         .body(error);
   }
 
+  // -- EXPLICIT STATUS : ResponseStatusException -------
+  // Endpoints throw ResponseStatusException to signal a specific status + reason (e.g. a 502 when
+  // GitHub rejects an approval). Without this handler the generic Exception handler below would
+  // catch it first (advice handlers run before Spring's ResponseStatusExceptionResolver) and
+  // flatten every one of them to a 500 "internal server error", discarding both the status and
+  // the message the caller intended for the user.
+  @ExceptionHandler(ResponseStatusException.class)
+  public ResponseEntity<ApiError> handleResponseStatusException(
+      ResponseStatusException ex, HttpServletRequest request) {
+
+    HttpStatusCode status = ex.getStatusCode();
+    HttpStatus resolved = HttpStatus.resolve(status.value());
+    ApiError error = new ApiError();
+    error.setStatus(status.value());
+    error.setError(resolved != null ? resolved.getReasonPhrase() : "Error");
+    error.setMessage(ex.getReason() != null ? ex.getReason() : "Request failed");
+    error.setPath(request.getRequestURI());
+    error.setTimestamp(Instant.now());
+
+    // These are deliberate, handled outcomes (auth, conflicts, upstream rejections), so keep them
+    // out of Sentry; log server-side faults (5xx) at WARN for visibility, client faults at DEBUG.
+    if (status.is5xxServerError()) {
+      log.warn("Request to {} failed with {}: {}", request.getRequestURI(), status.value(),
+          error.getMessage());
+    } else {
+      log.debug("Request to {} rejected with {}: {}", request.getRequestURI(), status.value(),
+          error.getMessage());
+    }
+
+    return new ResponseEntity<>(error, status);
+  }
+
   // -- 500 INTERNAL SERVER ERROR (FALLBACK) -------------
   @ExceptionHandler({Exception.class, IOException.class})
   public ResponseEntity<ApiError> handleGeneralException(Exception ex, HttpServletRequest request) {

@@ -0,0 +1,27 @@
+package de.tum.cit.aet.helios.github;
+
+import java.io.IOException;
+
+/**
+ * Raised when GitHub rejects a pending-deployment review (approve/reject) call. Carries the
+ * upstream HTTP status so callers can react to it — most importantly {@code 401}, which means the
+ * impersonated user's GitHub token (brokered through Keycloak) has expired and the reviewer needs
+ * to re-authenticate.
+ *
+ * <p>Extends {@link IOException} so existing {@code throws IOException} signatures and
+ * {@code catch (IOException ...)} handlers on the review path keep working unchanged.
+ */
+public class GitHubReviewException extends IOException {
+
+  private final int httpStatus;
+
+  public GitHubReviewException(int httpStatus, String message) {
+    super(message);
+    this.httpStatus = httpStatus;
+  }
+
+  /** The HTTP status GitHub returned (e.g. 401 for an expired/invalid token). */
+  public int getHttpStatus() {
+    return httpStatus;
+  }
+}
@@ -578,7 +578,8 @@ private void reviewPendingDeployment(
             runId,
             response.code(),
             errorBody);
-        throw new IOException(
+        throw new GitHubReviewException(
+            response.code(),
             "GitHub pending-deployment " + state + " failed: HTTP " + response.code());
       }
       log.info("Successfully set deployment state '{}' for run ID {}", state, runId);

@@ -0,0 +1,67 @@
+package de.tum.cit.aet.helios.deployment.approval;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.when;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+
+import de.tum.cit.aet.helios.auth.AuthService;
+import de.tum.cit.aet.helios.error.GlobalExceptionHandler;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc;
+import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest;
+import org.springframework.context.annotation.Import;
+import org.springframework.http.HttpStatus;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.bean.override.mockito.MockitoBean;
+import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.web.server.ResponseStatusException;
+
+/**
+ * Verifies that {@link GlobalExceptionHandler} lets a deliberate {@link ResponseStatusException}
+ * keep its status and reason on the way to the client, instead of the generic {@code Exception}
+ * handler flattening it to a 500 "internal server error". This is what makes an approval that
+ * GitHub rejects (e.g. an expired token → 502) show the reviewer a real, actionable message.
+ */
+@AutoConfigureMockMvc(addFilters = false)
+@Import(GlobalExceptionHandler.class)
+@ContextConfiguration(classes = DeploymentApprovalController.class)
+@WebMvcTest(DeploymentApprovalController.class)
+class DeploymentApprovalControllerErrorHandlingTest {
+
+  private static final String EXPIRED_AUTH_REASON =
+      "GitHub rejected the request because your GitHub authorization has expired. "
+          + "Please sign out of Helios and sign in again, then retry.";
+
+  @Autowired private MockMvc mockMvc;
+
+  @MockitoBean private DeploymentReviewActionService reviewActionService;
+  @MockitoBean private DeploymentApprovalRequestRepository approvalRequestRepository;
+  @MockitoBean private AuthService authService;
+
+  @Test
+  void badGatewayFromServiceReachesClientWithStatusAndReason() throws Exception {
+    when(reviewActionService.approveAsCurrentUser(anyLong(), any()))
+        .thenThrow(new ResponseStatusException(HttpStatus.BAD_GATEWAY, EXPIRED_AUTH_REASON));
+
+    mockMvc
+        .perform(post("/api/deployments/{deploymentId}/approve", 9436L))
+        .andExpect(status().isBadGateway())
+        .andExpect(jsonPath("$.status").value(502))
+        .andExpect(jsonPath("$.message").value(EXPIRED_AUTH_REASON));
+  }
+
+  @Test
+  void unexpectedExceptionStillFallsBackToInternalServerError() throws Exception {
+    when(reviewActionService.approveAsCurrentUser(anyLong(), any()))
+        .thenThrow(new RuntimeException("boom"));
+
+    mockMvc
+        .perform(post("/api/deployments/{deploymentId}/approve", 9436L))
+        .andExpect(status().isInternalServerError())
+        .andExpect(jsonPath("$.message").value("Error: An internal server error occurred"));
+  }
+}
@@ -16,6 +16,7 @@
 import de.tum.cit.aet.helios.environment.Environment;
 import de.tum.cit.aet.helios.environment.EnvironmentService;
 import de.tum.cit.aet.helios.environment.EnvironmentService.ReviewerResolution;
+import de.tum.cit.aet.helios.github.GitHubReviewException;
 import de.tum.cit.aet.helios.github.GitHubService;
 import de.tum.cit.aet.helios.gitrepo.GitRepository;
 import de.tum.cit.aet.helios.heliosdeployment.HeliosDeployment;
@@ -171,6 +172,33 @@ void marksFailedAtGitHubAndReturnsBadGatewayWhenGitHubCallThrows() throws IOExce
         DeploymentApprovalRequest.State.FAILED_AT_GITHUB, rowCaptor.getValue().getState());
   }
 
+  @Test
+  void returnsActionableExpiredAuthMessageWhenGitHubRejectsWith401() throws IOException {
+    Fixture f = new Fixture();
+    f.reviewersAre("alice", "bob");
+    doThrow(new GitHubReviewException(401, "GitHub pending-deployment approved failed: HTTP 401"))
+        .when(f.gitHubService)
+        .approveDeploymentOnBehalfOfUser(
+            anyString(), anyLong(), any(), anyString(), anyString());
+
+    ResponseStatusException e =
+        assertThrows(
+            ResponseStatusException.class,
+            () -> f.service().approveAsCurrentUser(DEPLOYMENT_ID, f.userWithLogin(REVIEWER)));
+
+    // Still 502 (upstream failure, not a Helios auth failure) but with a reviewer-actionable
+    // reason rather than the raw "HTTP 401".
+    assertEquals(HttpStatus.BAD_GATEWAY, e.getStatusCode());
+    assertEquals(true, e.getReason().contains("expired"));
+    assertEquals(true, e.getReason().contains("sign in again"));
+
+    ArgumentCaptor<DeploymentApprovalRequest> rowCaptor =
+        ArgumentCaptor.forClass(DeploymentApprovalRequest.class);
+    verify(f.approvalRequestRepository).save(rowCaptor.capture());
+    assertEquals(
+        DeploymentApprovalRequest.State.FAILED_AT_GITHUB, rowCaptor.getValue().getState());
+  }
+
   @Test
   void consumesSiblingPendingRowsAfterSuccessfulApproval() throws IOException {
     Fixture f = new Fixture();