Update

cthit · Jul 30, 2024 · bbef21f · bbef21f
1 parent 3794075
commit bbef21f
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 76 deletions.
diff --git a/app/src/main/java/it/chalmers/gamma/adapter/primary/web/ConsentController.java b/app/src/main/java/it/chalmers/gamma/adapter/primary/web/ConsentController.java
@@ -86,17 +86,6 @@ public ModelAndView getOAuth2Consent(
           "There is a mismatch between registered client scopes, and the scopes specified for this authorization.");
     }
 
-    if (!this.clientFacade.hasAccessToClient(clientId)) {
-      if (htmxRequest) {
-        mv.setViewName("pages/no-access-to-client");
-      } else {
-        mv.setViewName("index");
-        mv.addObject("page", "pages/no-access-to-client");
-      }
-
-      return mv;
-    }
-
     if (htmxRequest) {
       mv.setViewName("pages/authorize");
     } else {

diff --git a/app/src/main/java/it/chalmers/gamma/adapter/primary/web/GammaErrorController.java b/app/src/main/java/it/chalmers/gamma/adapter/primary/web/GammaErrorController.java
@@ -1,5 +1,6 @@
 package it.chalmers.gamma.adapter.primary.web;
 
+import it.chalmers.gamma.app.oauth2.GammaAuthorizationService;
 import jakarta.servlet.RequestDispatcher;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
@@ -22,11 +23,14 @@ public ModelAndView handleRuntimeException(
 
     int statusCode = statusCodeString == null ? 500 : Integer.parseInt(statusCodeString.toString());
 
-    String page =
-        switch (HttpStatus.valueOf(statusCode)) {
-          case NOT_FOUND -> "pages/404";
-          default -> "pages/error";
-        };
+    Exception exception = (Exception) request.getAttribute(RequestDispatcher.ERROR_EXCEPTION);
+    String page = "pages/error";
+
+    if (HttpStatus.valueOf(statusCode) == HttpStatus.NOT_FOUND) {
+      page = "pages/404";
+    } else if (exception instanceof GammaAuthorizationService.UserNotAllowedRuntimeException) {
+      page = "pages/no-access-to-client";
+    }
 
     ModelAndView mv = new ModelAndView();
     if (htmxRequest) {

diff --git a/app/src/main/java/it/chalmers/gamma/adapter/primary/web/GroupsController.java b/app/src/main/java/it/chalmers/gamma/adapter/primary/web/GroupsController.java
@@ -411,14 +411,8 @@ public ModelAndView updateGroup(
 
     if (!bindingResult.hasErrors() && this.groupFacade.groupWithNameAlreadyExists(id, form.name)) {
       bindingResult.addError(
-              new FieldError(
-                      "form",
-                      "name",
-                      form.name,
-                      true,
-                      null,
-                      null,
-                      "Group with name already exists"));
+          new FieldError(
+              "form", "name", form.name, true, null, null, "Group with name already exists"));
     }
 
     if (bindingResult.hasErrors()) {

diff --git a/app/src/main/java/it/chalmers/gamma/app/client/ClientFacade.java b/app/src/main/java/it/chalmers/gamma/app/client/ClientFacade.java
@@ -14,14 +14,11 @@
 import it.chalmers.gamma.app.client.domain.restriction.ClientRestrictionId;
 import it.chalmers.gamma.app.common.PrettyName;
 import it.chalmers.gamma.app.common.Text;
-import it.chalmers.gamma.app.group.domain.GroupRepository;
 import it.chalmers.gamma.app.supergroup.SuperGroupFacade;
 import it.chalmers.gamma.app.supergroup.domain.SuperGroupId;
 import it.chalmers.gamma.app.supergroup.domain.SuperGroupRepository;
 import it.chalmers.gamma.app.user.UserFacade;
-import it.chalmers.gamma.app.user.domain.GammaUser;
 import it.chalmers.gamma.app.user.domain.UserId;
-import it.chalmers.gamma.app.user.domain.UserMembership;
 import it.chalmers.gamma.app.user.domain.UserRepository;
 import it.chalmers.gamma.security.authentication.AuthenticationExtractor;
 import it.chalmers.gamma.security.authentication.UserAuthentication;
@@ -40,21 +37,18 @@ public class ClientFacade extends Facade {
   private final SuperGroupRepository superGroupRepository;
   private final UserRepository userRepository;
   private final PasswordEncoder passwordEncoder;
-  private final GroupRepository groupRepository;
 
   public ClientFacade(
-          AccessGuard accessGuard,
-          ClientRepository clientRepository,
-          SuperGroupRepository superGroupRepository,
-          UserRepository userRepository,
-          PasswordEncoder passwordEncoder,
-          GroupRepository groupRepository) {
+      AccessGuard accessGuard,
+      ClientRepository clientRepository,
+      SuperGroupRepository superGroupRepository,
+      UserRepository userRepository,
+      PasswordEncoder passwordEncoder) {
     super(accessGuard);
     this.clientRepository = clientRepository;
     this.superGroupRepository = superGroupRepository;
     this.userRepository = userRepository;
     this.passwordEncoder = passwordEncoder;
-    this.groupRepository = groupRepository;
   }
 
   @Transactional
@@ -210,33 +204,6 @@ public Optional<UserFacade.UserDTO> getClientOwner(String clientId) {
     return Optional.empty();
   }
 
-  public boolean hasAccessToClient(String clientId) {
-    accessGuard.require(isSignedIn());
-
-    if (AuthenticationExtractor.getAuthentication()
-            instanceof UserAuthentication userAuthentication) {
-      GammaUser user = userAuthentication.gammaUser();
-      Client client = this.clientRepository.get(new ClientId(clientId)).orElseThrow();
-
-      if (client.restrictions().isEmpty()) {
-        return true;
-      }
-
-      List<UserMembership> memberships = this.groupRepository.getAllByUser(user.id());
-      List<SuperGroupId> userSuperGroups =
-              memberships.stream()
-                      .map(UserMembership::group)
-                      .map(group -> group.superGroup().id())
-                      .distinct()
-                      .toList();
-
-      return client.restrictions().get().superGroups().stream()
-              .anyMatch(superGroup -> userSuperGroups.contains(superGroup.id()));
-    }
-
-    return false;
-  }
-
   public record NewClientRestrictions(List<UUID> superGroups) {}
 
   public record NewClient(

diff --git a/app/src/main/java/it/chalmers/gamma/app/oauth2/GammaAuthorizationService.java b/app/src/main/java/it/chalmers/gamma/app/oauth2/GammaAuthorizationService.java
@@ -13,13 +13,9 @@
 import java.util.List;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.userdetails.User;
-import org.springframework.security.oauth2.core.OAuth2AccessToken;
-import org.springframework.security.oauth2.core.oidc.OidcIdToken;
 import org.springframework.security.oauth2.server.authorization.OAuth2Authorization;
-import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationCode;
 import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationService;
 import org.springframework.security.oauth2.server.authorization.OAuth2TokenType;
 import org.springframework.stereotype.Component;
@@ -47,16 +43,16 @@ public void save(OAuth2Authorization authorization) {
         authorization.getAttribute("java.security.Principal");
 
     if (authenticationToken != null && authenticationToken.getPrincipal() instanceof User user) {
-        Client client =
-                this.clientRepository
-                        .get(ClientUid.valueOf(authorization.getRegisteredClientId()))
-                        .orElseThrow();
+      Client client =
+          this.clientRepository
+              .get(ClientUid.valueOf(authorization.getRegisteredClientId()))
+              .orElseThrow();
 
-        // If the client has no restrictions, then any user can sign in.
-        if (client.restrictions().isPresent()
-                && !userPassesRestriction(client.restrictions().get(), user)) {
-            throw new AccessDeniedException("User does not have the necessary authority");
-        }
+      // If the client has no restrictions, then any user can sign in.
+      if (client.restrictions().isPresent()
+          && !userPassesRestriction(client.restrictions().get(), user)) {
+        throw new UserNotAllowedRuntimeException();
+      }
     }
 
     gammaAuthorizationRepository.save(authorization);
@@ -94,5 +90,5 @@ public OAuth2Authorization findByToken(String token, OAuth2TokenType tokenType)
         .orElseThrow();
   }
 
-  public static class UserIsNotAuthorizedException extends RuntimeException {}
+  public static class UserNotAllowedRuntimeException extends RuntimeException {}
 }