Merge pull request #109 from virtualidentityag/develop

merge tenant id fix
virtualidentityag · Sep 18, 2023 · 3537a2b · 3537a2b
2 parents a40aa4f + e75e970
commit 3537a2b
Show file tree

Hide file tree

Showing 12 changed files with 175 additions and 184 deletions.
diff --git a/google_checks_light.xml b/google_checks_light.xml
@@ -37,14 +37,6 @@
         <property name="eachLine" value="true"/>
     </module>
 
-	<!-- 
-    <module name="LineLength">
-        <property name="fileExtensions" value="java"/>
-        <property name="max" value="100"/>
-        <property name="ignorePattern" value="^package.*|^import.*|a href|href|http://|https://|ftp://"/>
-    </module>
-     -->
-
     <module name="TreeWalker">
         <module name="OuterTypeFilename"/>
         <module name="IllegalTokenText">
@@ -130,7 +122,6 @@
         <module name="MissingSwitchDefault"/>
         <module name="FallThrough"/>
         <module name="UpperEll"/>
-        <!-- <module name="ModifierOrder"/>  -->
         <module name="EmptyLineSeparator">
             <property name="tokens"
              value="PACKAGE_DEF, IMPORT, STATIC_IMPORT, CLASS_DEF, INTERFACE_DEF, ENUM_DEF,
@@ -164,25 +155,11 @@
             <property name="tokens" value="METHOD_REF"/>
             <property name="option" value="nl"/>
         </module>
-        <!-- 
-        <module name="PackageName">
-            <property name="format" value="^[a-z]+(\.[a-z][a-z0-9]*)*$"/>
-            <message key="name.invalidPattern"
-             value="Package name ''{0}'' must match pattern ''{1}''."/>
-        </module>
-         -->
         <module name="TypeName">
             <property name="tokens" value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF, ANNOTATION_DEF"/>
             <message key="name.invalidPattern"
              value="Type name ''{0}'' must match pattern ''{1}''."/>
         </module>
-        <!-- 
-        <module name="MemberName">
-            <property name="format" value="^[a-z][a-z0-9][a-zA-Z0-9]*$"/>
-            <message key="name.invalidPattern"
-             value="Member name ''{0}'' must match pattern ''{1}''."/>
-        </module>
-         -->
         <module name="ParameterName">
             <property name="format" value="^[a-z]([a-z0-9][a-zA-Z0-9]*)?$"/>
             <message key="name.invalidPattern"
@@ -237,25 +214,8 @@
             <property name="lineWrappingIndentation" value="4"/>
             <property name="arrayInitIndent" value="2"/>
         </module>
-        <!-- 
-        <module name="AbbreviationAsWordInName">
-            <property name="ignoreFinal" value="false"/>
-            <property name="allowedAbbreviationLength" value="1"/>
-            <property name="tokens"
-             value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF, ANNOTATION_DEF, ANNOTATION_FIELD_DEF,
-                    PARAMETER_DEF, VARIABLE_DEF, METHOD_DEF"/>
-        </module>
-         -->
         <module name="OverloadMethodsDeclarationOrder"/>
         <module name="VariableDeclarationUsageDistance"/>
-        <!-- 
-        <module name="CustomImportOrder">
-            <property name="sortImportsInGroupAlphabetically" value="true"/>
-            <property name="separateLineBetweenGroups" value="true"/>
-            <property name="customImportOrderRules" value="STATIC###THIRD_PARTY_PACKAGE"/>
-            <property name="tokens" value="IMPORT, STATIC_IMPORT, PACKAGE_DEF"/>
-        </module>
-         -->
         <module name="MethodParamPad">
             <property name="tokens"
              value="CTOR_DEF, LITERAL_NEW, METHOD_CALL, METHOD_DEF,
@@ -306,7 +266,6 @@
         </module>
         -->
         <module name="JavadocMethod">
-            <property name="scope" value="public"/>
             <property name="allowMissingParamTags" value="true"/>
             <property name="allowMissingReturnTag" value="true"/>
             <property name="allowedAnnotations" value="Override, Test"/>

diff --git a/pom.xml b/pom.xml
@@ -546,14 +546,13 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-checkstyle-plugin</artifactId>
-        <version>3.1.1</version>
+        <version>3.3.0</version>
         <executions>
           <execution>
             <id>validate</id>
             <phase>validate</phase>
             <configuration>
               <configLocation>google_checks_light.xml</configLocation>
-              <encoding>UTF-8</encoding>
               <failsOnError>true</failsOnError>
               <consoleOutput>true</consoleOutput>
               <failOnViolation>true</failOnViolation>

diff --git a/src/main/java/de/caritas/cob/agencyservice/api/admin/service/AgencyAdminService.java b/src/main/java/de/caritas/cob/agencyservice/api/admin/service/AgencyAdminService.java
@@ -26,7 +26,6 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.stream.Collectors;
-
 import lombok.NonNull;
 import lombok.RequiredArgsConstructor;
 import org.springframework.beans.factory.annotation.Autowired;

diff --git a/src/main/java/de/caritas/cob/agencyservice/api/tenant/AccessTokenTenantResolver.java b/src/main/java/de/caritas/cob/agencyservice/api/tenant/AccessTokenTenantResolver.java
@@ -6,10 +6,10 @@
 import java.util.Optional;
 import jakarta.servlet.http.HttpServletRequest;
 import lombok.AllArgsConstructor;
-import lombok.NonNull;
 import lombok.extern.slf4j.Slf4j;
-import org.keycloak.KeycloakSecurityContext;
-import org.keycloak.adapters.springsecurity.token.KeycloakAuthenticationToken;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.oauth2.jwt.Jwt;
 import org.springframework.stereotype.Component;
 
 
@@ -22,31 +22,39 @@ public class AccessTokenTenantResolver implements TenantResolver {
 
   @Override
   public Optional<Long> resolve(HttpServletRequest request) {
-    return resolveTenantIdFromTokenClaims(request);
+    return resolveTenantIdFromTokenClaims();
   }
 
-  private Optional<Long> resolveTenantIdFromTokenClaims(HttpServletRequest request) {
-    Map<String, Object> claimMap = getClaimMap(request);
+  private Optional<Long> resolveTenantIdFromTokenClaims() {
+    Map<String, Object> claimMap = getClaimMap();
     log.debug("Found tenantId in claim : " + claimMap.toString());
     return getUserTenantIdAttribute(claimMap);
   }
 
   private Optional<Long> getUserTenantIdAttribute(Map<String, Object> claimMap) {
     if (claimMap.containsKey(TENANT_ID)) {
-      Integer tenantId = (Integer) claimMap.get(TENANT_ID);
-      return Optional.of(Long.valueOf(tenantId));
-    } else {
-      return Optional.empty();
+      Object tenantIdObject = claimMap.get(TENANT_ID);
+      if (tenantIdObject instanceof Long tenantId) {
+        return Optional.of(tenantId);
+      }
+      if (tenantIdObject instanceof Integer tenantId) {
+        return Optional.of(Long.valueOf(tenantId));
+      }
     }
+    return Optional.empty();
   }
 
-  private Map<String, Object> getClaimMap(HttpServletRequest request) {
-    KeycloakSecurityContext keycloakSecContext =
-        ((KeycloakAuthenticationToken) request.getUserPrincipal()).getAccount()
-            .getKeycloakSecurityContext();
-    return keycloakSecContext.getToken().getOtherClaims();
+  private Map<String, Object> getClaimMap() {
+    Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+    if (authentication != null) {
+      var jwt = (Jwt) authentication.getPrincipal();
+      return jwt.getClaims();
+    } else {
+      return Map.of();
+    }
   }
 
+
   @Override
   public boolean canResolve(HttpServletRequest request) {
     return resolve(request).isPresent();

diff --git a/src/main/java/de/caritas/cob/agencyservice/api/tenant/TechnicalUserTenantResolver.java b/src/main/java/de/caritas/cob/agencyservice/api/tenant/TechnicalUserTenantResolver.java
@@ -1,29 +1,49 @@
 package de.caritas.cob.agencyservice.api.tenant;
 
+import com.google.common.collect.Lists;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import jakarta.servlet.http.HttpServletRequest;
-import org.keycloak.adapters.springsecurity.token.KeycloakAuthenticationToken;
-import org.keycloak.representations.AccessToken;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.oauth2.jwt.Jwt;
 import org.springframework.stereotype.Component;
 
 @Component
 public class TechnicalUserTenantResolver implements TenantResolver {
 
   @Override
   public Optional<Long> resolve(HttpServletRequest request) {
-    return isTechnicalUserRole(request) ? Optional.of(0L) : Optional.empty();
+    return isTechnicalUserRole() ? Optional.of(0L) : Optional.empty();
   }
 
-  private boolean isTechnicalUserRole(HttpServletRequest request) {
-    AccessToken token = ((KeycloakAuthenticationToken) request.getUserPrincipal()).getAccount()
-        .getKeycloakSecurityContext().getToken();
-    return hasRoles(token) && token.getRealmAccess().getRoles().contains("technical");
+  private boolean isTechnicalUserRole() {
+
+    Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+    if (authentication != null) {
+      Jwt jwt = (Jwt) authentication.getPrincipal();
+      return getRealmRoles(jwt).contains("technical");
+    }
+    return false;
   }
 
-  private boolean hasRoles(AccessToken accessToken) {
-    return accessToken.getRealmAccess() != null && accessToken.getRealmAccess().getRoles() != null;
+  private Collection<String> getRealmRoles(Jwt jwt) {
+
+    if (jwt != null) {
+      var claims = jwt.getClaims();
+      if (claims.containsKey("realm_access")) {
+        Map<String, Object> realmAccess = (Map<String, Object>) claims.get("realm_access");
+        if (realmAccess.containsKey("roles")) {
+          return (List<String>) realmAccess.get("roles");
+        }
+      }
+    }
+    return Lists.newArrayList();
   }
 
+
   @Override
   public boolean canResolve(HttpServletRequest request) {
     return resolve(request).isPresent();

diff --git a/src/main/java/de/caritas/cob/agencyservice/api/tenant/TenantResolverService.java b/src/main/java/de/caritas/cob/agencyservice/api/tenant/TenantResolverService.java
@@ -12,6 +12,8 @@
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
 import org.springframework.security.access.AccessDeniedException;
+import org.springframework.security.core.context.SecurityContext;
+import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.stereotype.Component;
 
 @Slf4j
@@ -39,7 +41,8 @@ public class TenantResolverService {
   private boolean multitenancyWithSingleDomain;
 
   private List<TenantResolver> nonAuthenticatedTenantResolvers() {
-    return newArrayList(multitenancyWithSingleDomainTenantResolver, customHeaderTenantResolver, subdomainTenantResolver);
+    return newArrayList(multitenancyWithSingleDomainTenantResolver, customHeaderTenantResolver,
+        subdomainTenantResolver);
   }
 
   private List<TenantResolver> tenantIdCrossValidationResolvers() {
@@ -51,7 +54,7 @@ private ArrayList<TenantResolver> authenticatedTenantResolvers() {
   }
 
   public Long resolve(HttpServletRequest request) {
-    if (userIsAuthenticated(request)) {
+    if (userIsAuthenticated()) {
       return resolveForAuthenticatedUser(request);
     } else {
       return resolveForNonAuthenticatedUser(request);
@@ -88,7 +91,7 @@ private Long resolveForNonAuthenticatedUser(HttpServletRequest request) {
     return tenantId.get();
   }
 
-  private  void validateResolvedTenantMatch(Optional<Long> tenantId,
+  private void validateResolvedTenantMatch(Optional<Long> tenantId,
       Optional<Long> tenantIdFromHeaderOrSubdomain) {
     if (tenantId.isPresent() && tenantIdFromHeaderOrSubdomain.isPresent()) {
       if (!tenantId.get().equals(tenantIdFromHeaderOrSubdomain.get())) {
@@ -109,7 +112,12 @@ private Optional<Long> getFirstResolvedTenant(HttpServletRequest request,
     return Optional.empty();
   }
 
-  private boolean userIsAuthenticated(HttpServletRequest request) {
-    return request.getUserPrincipal() != null;
+  private boolean userIsAuthenticated() {
+    /* after upgrade to oauth2ResourceServer security configuration (spring 6.x upgrade)
+       for authenticated users request.getUserPrincipal() might be still null at the time of HttpTenantFilter is executed
+       but BearerTokenAuthenticationFilter has already set the principal in the SecurityContext */
+    SecurityContext context = SecurityContextHolder.getContext();
+    return context.getAuthentication() != null
+        && context.getAuthentication().isAuthenticated();
   }
 }
diff --git a/src/main/java/de/caritas/cob/agencyservice/config/SecurityConfig.java b/src/main/java/de/caritas/cob/agencyservice/config/SecurityConfig.java
@@ -19,6 +19,7 @@
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.config.http.SessionCreationPolicy;
+import org.springframework.security.oauth2.server.resource.web.authentication.BearerTokenAuthenticationFilter;
 import org.springframework.security.web.SecurityFilterChain;
 import org.springframework.security.web.csrf.CsrfFilter;
 import org.springframework.web.servlet.config.annotation.PathMatchConfigurer;
@@ -74,7 +75,7 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
 
     if (multitenancy) {
       httpSecurity = httpSecurity
-          .addFilterAfter(httpTenantFilter, CsrfFilter.class);
+          .addFilterAfter(httpTenantFilter, BearerTokenAuthenticationFilter.class);
     }
 
     httpSecurity.sessionManagement().sessionCreationPolicy(SessionCreationPolicy.STATELESS)

diff --git a/src/main/java/de/caritas/cob/agencyservice/config/security/AuthorisationService.java b/src/main/java/de/caritas/cob/agencyservice/config/security/AuthorisationService.java
@@ -21,26 +21,6 @@ public class AuthorisationService {
   private final RoleAuthorizationAuthorityMapper roleAuthorizationAuthorityMapper =
       new RoleAuthorizationAuthorityMapper();
 
-  public boolean hasAuthority(String authorityName) {
-    return getAuthentication().getAuthorities().stream()
-        .anyMatch(role -> authorityName.equals(role.getAuthority()));
-  }
-
-  public boolean hasRole(String roleName) {
-    var roles = extractRealmRoles(getPrincipal());
-    return roles != null && roles.contains(roleName);
-  }
-
-  public Optional<Long> findTenantIdInAccessToken() {
-    Jwt principal = getPrincipal();
-    Long tenantId = (Long) principal.getClaims().get("tenantId");
-
-    if (tenantId == null) {
-      throw new AccessDeniedException("tenantId attribute not found in the access token");
-    }
-    return Optional.of(tenantId);
-  }
-
   public Object getUsername() {
     return getPrincipal().getClaims().get("username");
   }

diff --git a/src/test/java/de/caritas/cob/agencyservice/api/controller/AuthenticationMockBuilder.java b/src/test/java/de/caritas/cob/agencyservice/api/controller/AuthenticationMockBuilder.java