Introduce pluggable tenant resolver (#2151)

Signed-off-by: Avgustin Marinov <[email protected]>
eclipse-hawkbit · Dec 18, 2024 · ed93d3f · ed93d3f
1 parent 1c211c8
commit ed93d3f
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 44 deletions.
diff --git a/...e/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityAutoConfiguration.java b/...e/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityAutoConfiguration.java
@@ -16,9 +16,10 @@
 
 import org.eclipse.hawkbit.ContextAware;
 import org.eclipse.hawkbit.im.authentication.SpRole;
+import org.eclipse.hawkbit.tenancy.TenantAware.DefaultTenantResolver;
+import org.eclipse.hawkbit.tenancy.TenantAware.TenantResolver;
 import org.eclipse.hawkbit.tenancy.TenantAwareUserProperties;
 import org.eclipse.hawkbit.tenancy.TenantAwareUserProperties.User;
-import org.eclipse.hawkbit.security.DdiSecurityProperties;
 import org.eclipse.hawkbit.security.HawkbitSecurityProperties;
 import org.eclipse.hawkbit.security.InMemoryUserAuthoritiesResolver;
 import org.eclipse.hawkbit.security.MdcHandler;
@@ -56,6 +57,12 @@
 @EnableConfigurationProperties({ SecurityProperties.class, HawkbitSecurityProperties.class, TenantAwareUserProperties.class })
 public class SecurityAutoConfiguration {
 
+    @Bean
+    @ConditionalOnMissingBean
+    public TenantResolver tenantResolver() {
+        return new DefaultTenantResolver();
+    }
+
     /**
      * Creates a {@link ContextAware} (hence {@link TenantAware}) bean based on the given {@link UserAuthoritiesResolver} and
      * {@link SecurityContextSerializer}.
@@ -68,8 +75,9 @@ public class SecurityAutoConfiguration {
     @ConditionalOnMissingBean
     public ContextAware contextAware(
             final UserAuthoritiesResolver authoritiesResolver,
-            @Autowired(required = false) final SecurityContextSerializer securityContextSerializer) {
-        return new SecurityContextTenantAware(authoritiesResolver, securityContextSerializer);
+            @Autowired(required = false) final SecurityContextSerializer securityContextSerializer,
+            @Autowired(required = false) final TenantResolver tenantResolver) {
+        return new SecurityContextTenantAware(authoritiesResolver, securityContextSerializer, tenantResolver);
     }
 
     /**

diff --git a/hawkbit-core/src/main/java/org/eclipse/hawkbit/tenancy/TenantAware.java b/hawkbit-core/src/main/java/org/eclipse/hawkbit/tenancy/TenantAware.java
@@ -9,6 +9,9 @@
  */
 package org.eclipse.hawkbit.tenancy;
 
+import org.springframework.security.core.context.SecurityContext;
+import org.springframework.security.core.context.SecurityContextHolder;
+
 /**
  * Interface for components that are aware of the application's current tenant.
  */
@@ -66,4 +69,29 @@ interface TenantRunner<T> {
          */
         T run();
     }
+
+    /**
+     * Resolves the tenant from the current context.
+     */
+    interface TenantResolver {
+
+        String resolveTenant();
+    }
+
+    class DefaultTenantResolver implements TenantResolver {
+
+        @Override
+        public String resolveTenant() {
+            final SecurityContext context = SecurityContextHolder.getContext();
+            if (context.getAuthentication() != null) {
+                final Object principal = context.getAuthentication().getPrincipal();
+                if (context.getAuthentication().getDetails() instanceof TenantAwareAuthenticationDetails tenantAwareAuthenticationDetails) {
+                    return tenantAwareAuthenticationDetails.getTenant();
+                } else if (principal instanceof TenantAwareUser tenantAwareUser) {
+                    return tenantAwareUser.getTenant();
+                }
+            }
+            return null;
+        }
+    }
 }
diff --git a/...tory-jpa-hibernate/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaConfiguration.java b/...tory-jpa-hibernate/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaConfiguration.java
@@ -49,9 +49,9 @@ public class JpaConfiguration extends JpaBaseConfiguration {
     protected JpaConfiguration(
             final DataSource dataSource, final JpaProperties properties,
             final ObjectProvider<JtaTransactionManager> jtaTransactionManagerProvider,
-            final TenantAware tenantAware) {
+            final TenantAware.TenantResolver tenantResolver) {
         super(dataSource, properties, jtaTransactionManagerProvider);
-        tenantIdentifier = new TenantIdentifier(tenantAware);
+        tenantIdentifier = new TenantIdentifier(tenantResolver);
     }
 
     @Bean

diff --git a/...tory-jpa-hibernate/src/main/java/org/eclipse/hawkbit/repository/jpa/TenantIdentifier.java b/...tory-jpa-hibernate/src/main/java/org/eclipse/hawkbit/repository/jpa/TenantIdentifier.java
@@ -21,16 +21,16 @@
  */
 class TenantIdentifier implements CurrentTenantIdentifierResolver<String> {
 
-    private final TenantAware tenantAware;
+    private final TenantAware.TenantResolver tenantResolver;
 
-    TenantIdentifier(final TenantAware tenantAware) {
-        this.tenantAware = tenantAware;
+    TenantIdentifier(final TenantAware.TenantResolver tenantResolver) {
+        this.tenantResolver = tenantResolver;
     }
 
     @Override
     public String resolveCurrentTenantIdentifier() {
         // on bootstrapping hibernate requests tenant and want to be non-null
-        return Optional.ofNullable(tenantAware.getCurrentTenant()).map(String::toUpperCase).orElse("");
+        return Optional.ofNullable(tenantResolver.resolveTenant()).map(String::toUpperCase).orElse("");
     }
 
     @Override

diff --git a/hawkbit-repository/hawkbit-repository-jpa/pom.xml b/hawkbit-repository/hawkbit-repository-jpa/pom.xml
@@ -31,8 +31,9 @@
             <id>eclipselink</id>
             <activation>
                 <property>
-                    <!-- default, if not set - eclipse link -->
-                    <name>!jpa.vendor</name>
+                    <!-- default, if not set (or not hibernate) - eclipse link -->
+                    <name>jpa.vendor</name>
+                    <value>!hibernate</value>
                 </property>
             </activation>
 

diff --git a/...-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/TestConfiguration.java b/...-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/TestConfiguration.java
@@ -43,6 +43,8 @@
 import org.eclipse.hawkbit.security.SpringSecurityAuditorAware;
 import org.eclipse.hawkbit.security.SystemSecurityContext;
 import org.eclipse.hawkbit.tenancy.TenantAware;
+import org.eclipse.hawkbit.tenancy.TenantAware.DefaultTenantResolver;
+import org.eclipse.hawkbit.tenancy.TenantAware.TenantResolver;
 import org.eclipse.hawkbit.tenancy.UserAuthoritiesResolver;
 import org.eclipse.hawkbit.tenancy.configuration.ControllerPollProperties;
 import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties;
@@ -165,9 +167,16 @@ SecurityContextSerializer securityContextSerializer() {
     }
 
     @Bean
-    ContextAware contextAware(final UserAuthoritiesResolver authoritiesResolver, final SecurityContextSerializer securityContextSerializer) {
+    TenantResolver tenantResolver() {
+        return new DefaultTenantResolver();
+    }
+
+    @Bean
+    ContextAware contextAware(
+            final UserAuthoritiesResolver authoritiesResolver, final SecurityContextSerializer securityContextSerializer,
+            final TenantResolver tenantResolver) {
         // allow spying the security context
-        return org.mockito.Mockito.spy(new SecurityContextTenantAware(authoritiesResolver, securityContextSerializer));
+        return org.mockito.Mockito.spy(new SecurityContextTenantAware(authoritiesResolver, securityContextSerializer, tenantResolver));
     }
 
     @Bean

diff --git a/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/RestConfiguration.java b/hawkbit-rest-core/src/main/java/org/eclipse/hawkbit/rest/RestConfiguration.java
@@ -161,8 +161,8 @@ public ResponseEntity<ExceptionInfo> handleSpServerRtExceptions(final HttpServle
             logRequest(request, ex);
             final ExceptionInfo response = createExceptionInfo(ex);
             final HttpStatus responseStatus;
-            if (ex instanceof AbstractServerRtException) {
-                responseStatus = getStatusOrDefault(((AbstractServerRtException) ex).getError());
+            if (ex instanceof AbstractServerRtException abstractServerRtException) {
+                responseStatus = getStatusOrDefault(abstractServerRtException.getError());
             } else {
                 responseStatus = DEFAULT_RESPONSE_STATUS;
             }
@@ -278,16 +278,20 @@ private static HttpStatus getStatusOrDefault(final SpServerError error) {
         }
 
         private void logRequest(final HttpServletRequest request, final Exception ex) {
-            log.debug("Handling exception {} of request {}", ex.getClass().getName(), request.getRequestURL());
+            if (log.isTraceEnabled()) {
+                log.debug("Handling exception {} of request {}", ex.getClass().getName(), request.getRequestURL(), ex);
+            } else {
+                log.debug("Handling exception {} of request {}", ex.getClass().getName(), request.getRequestURL());
+            }
         }
 
         private ExceptionInfo createExceptionInfo(final Exception ex) {
             final ExceptionInfo response = new ExceptionInfo();
             response.setMessage(ex.getMessage());
             response.setExceptionClass(ex.getClass().getName());
-            if (ex instanceof AbstractServerRtException) {
-                response.setErrorCode(((AbstractServerRtException) ex).getError().getKey());
-                response.setInfo(((AbstractServerRtException) ex).getInfo());
+            if (ex instanceof AbstractServerRtException abstractServerRtException) {
+                response.setErrorCode(abstractServerRtException.getError().getKey());
+                response.setInfo(abstractServerRtException.getInfo());
             }
             return response;
         }
@@ -305,10 +309,7 @@ static class ExcludePathAwareShallowETagFilter extends ShallowEtagHeaderFilter {
         private final String[] excludeAntPaths;
         private final AntPathMatcher antMatcher = new AntPathMatcher();
 
-        /**
-         * @param excludeAntPaths
-         */
-        public ExcludePathAwareShallowETagFilter(final String... excludeAntPaths) {
+         public ExcludePathAwareShallowETagFilter(final String... excludeAntPaths) {
             this.excludeAntPaths = excludeAntPaths;
         }
 

diff --git a/...-security-core/src/main/java/org/eclipse/hawkbit/security/SecurityContextTenantAware.java b/...-security-core/src/main/java/org/eclipse/hawkbit/security/SecurityContextTenantAware.java
@@ -43,20 +43,19 @@ public class SecurityContextTenantAware implements ContextAware {
     public static final String SYSTEM_USER = "system";
 
     private static final Collection<? extends GrantedAuthority> SYSTEM_AUTHORITIES =
-            Collections.singletonList(new SimpleGrantedAuthority(SpringEvalExpressions.SYSTEM_ROLE));
+            List.of(new SimpleGrantedAuthority(SpringEvalExpressions.SYSTEM_ROLE));
 
     private final UserAuthoritiesResolver authoritiesResolver;
     private final SecurityContextSerializer securityContextSerializer;
+    private final TenantResolver tenantResolver;
 
     /**
      * Creates the {@link SecurityContextTenantAware} based on the given {@link UserAuthoritiesResolver}.
      *
-     * @param authoritiesResolver Resolver to retrieve the authorities for a given user. Must
-     *         not be <code>null</code>..
+     * @param authoritiesResolver Resolver to retrieve the authorities for a given user. Must not be <code>null</code>..
      */
     public SecurityContextTenantAware(final UserAuthoritiesResolver authoritiesResolver) {
-        this.authoritiesResolver = authoritiesResolver;
-        this.securityContextSerializer = SecurityContextSerializer.NOP;
+        this(authoritiesResolver, null, null);
     }
 
     /**
@@ -65,36 +64,42 @@ public SecurityContextTenantAware(final UserAuthoritiesResolver authoritiesResol
      * @param authoritiesResolver Resolver to retrieve the authorities for a given user. Must not be <code>null</code>.
      * @param securityContextSerializer Serializer that is used to serialize / deserialize {@link SecurityContext}s.
      */
-    public SecurityContextTenantAware(final UserAuthoritiesResolver authoritiesResolver,
+    public SecurityContextTenantAware(
+            final UserAuthoritiesResolver authoritiesResolver,
             @Nullable final SecurityContextSerializer securityContextSerializer) {
+        this(authoritiesResolver, securityContextSerializer, null);
+    }
+
+    /**
+     * Creates the {@link SecurityContextTenantAware} based on the given {@link UserAuthoritiesResolver}.
+     *
+     * @param authoritiesResolver Resolver to retrieve the authorities for a given user. Must not be <code>null</code>.
+     * @param securityContextSerializer Serializer that is used to serialize / deserialize {@link SecurityContext}s.
+     */
+    public SecurityContextTenantAware(
+            final UserAuthoritiesResolver authoritiesResolver,
+            @Nullable final SecurityContextSerializer securityContextSerializer,
+            @Nullable final TenantResolver tenantResolver) {
         this.authoritiesResolver = authoritiesResolver;
         this.securityContextSerializer = securityContextSerializer == null ? SecurityContextSerializer.NOP : securityContextSerializer;
+        this.tenantResolver = tenantResolver == null ? new DefaultTenantResolver() : tenantResolver;
     }
 
     @Override
     public String getCurrentTenant() {
-        final SecurityContext context = SecurityContextHolder.getContext();
-        if (context.getAuthentication() != null) {
-            final Object principal = context.getAuthentication().getPrincipal();
-            if (context.getAuthentication().getDetails() instanceof TenantAwareAuthenticationDetails) {
-                return ((TenantAwareAuthenticationDetails) context.getAuthentication().getDetails()).getTenant();
-            } else if (principal instanceof TenantAwareUser) {
-                return ((TenantAwareUser) principal).getTenant();
-            }
-        }
-        return null;
+        return tenantResolver.resolveTenant();
     }
 
     @Override
     public String getCurrentUsername() {
         final SecurityContext context = SecurityContextHolder.getContext();
         if (context.getAuthentication() != null) {
             final Object principal = context.getAuthentication().getPrincipal();
-            if (principal instanceof OidcUser) {
-                return ((OidcUser) principal).getPreferredUsername();
+            if (principal instanceof OidcUser oidcUser) {
+                return oidcUser.getPreferredUsername();
             }
-            if (principal instanceof User) {
-                return ((User) principal).getUsername();
+            if (principal instanceof User user) {
+                return user.getUsername();
             }
         }
         return null;
@@ -243,4 +248,4 @@ public void setAuthenticated(final boolean isAuthenticated) {
             delegate.setAuthenticated(isAuthenticated);
         }
     }
-}
+}