Improved AccessContext (#3029)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>

hawkbit-repository/hawkbit-repository-jpa-eclipselink/src/main/java/org/eclipse/hawkbit/repository/jpa/model/AbstractJpaBaseEntity.java

@@ -26,16 +26,14 @@ import lombok.AccessLevel;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
 import lombok.Setter;
+import org.eclipse.hawkbit.auth.SpRole;
 import org.eclipse.hawkbit.repository.jpa.executor.AfterTransactionCommitExecutor;
 import org.eclipse.hawkbit.repository.model.BaseEntity;
-import org.eclipse.hawkbit.tenancy.TenantAwareAuthenticationDetails;
 import org.springframework.data.annotation.CreatedBy;
 import org.springframework.data.annotation.CreatedDate;
 import org.springframework.data.annotation.LastModifiedBy;
 import org.springframework.data.annotation.LastModifiedDate;
 import org.springframework.data.jpa.domain.support.AuditingEntityListener;
-import org.springframework.security.core.Authentication;
-import org.springframework.security.core.context.SecurityContextHolder;
 
 /**
  * Base hawkBit entity class containing the common attributes for EclipseLink.
@@ -95,7 +93,7 @@ public abstract class AbstractJpaBaseEntity implements BaseEntity {
 
     @LastModifiedBy
     public void setLastModifiedBy(final String lastModifiedBy) {
-        if (this.lastModifiedBy != null && isController()) {
+        if (this.lastModifiedBy != null && SpRole.isController()) {
             // initialized and controller = doesn't update
             return;
         }
@@ -110,7 +108,7 @@ public abstract class AbstractJpaBaseEntity implements BaseEntity {
 
     @LastModifiedDate
     public void setLastModifiedAt(final long lastModifiedAt) {
-        if (this.lastModifiedAt != 0 && isController()) {
+        if (this.lastModifiedAt != 0 && SpRole.isController()) {
             // initialized and controller = doesn't update
             return;
         }
@@ -197,11 +195,4 @@ public abstract class AbstractJpaBaseEntity implements BaseEntity {
         // fire events onl AFTER transaction commit
         AfterTransactionCommitExecutor.afterCommit(runnable);
     }
-
-    protected boolean isController() {
-        final Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
-        return authentication != null
-                && authentication.getDetails() instanceof TenantAwareAuthenticationDetails tenantAwareDetails
-                && tenantAwareDetails.controller();
-    }
 }

hawkbit-repository/hawkbit-repository-jpa-eclipselink/src/main/java/org/eclipse/hawkbit/repository/jpa/model/AbstractJpaTenantAwareBaseEntity.java

@@ -86,9 +86,7 @@ public abstract class AbstractJpaTenantAwareBaseEntity extends AbstractJpaBaseEn
         final String currentTenant = AccessContext.tenant();
         if (currentTenant == null) {
             throw new TenantNotExistException(
-                    String.format(
-                            "AccessContext %s does not exists, cannot create entity %s with id %d",
-                            AccessContext.tenant(), getClass(), getId()));
+                    String.format("Tenant not found in the context, cannot create entity %s with id %d", getClass(), getId()));
         }
         setTenant(currentTenant.toUpperCase());
     }

hawkbit-repository/hawkbit-repository-jpa-hibernate/src/main/java/org/eclipse/hawkbit/repository/jpa/model/AbstractJpaBaseEntity.java

@@ -9,8 +9,6 @@
  */
 package org.eclipse.hawkbit.repository.jpa.model;
 
-import java.io.Serial;
-
 import jakarta.persistence.Access;
 import jakarta.persistence.AccessType;
 import jakarta.persistence.Column;
@@ -28,16 +26,14 @@ import lombok.AccessLevel;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
 import lombok.Setter;
+import org.eclipse.hawkbit.auth.SpRole;
 import org.eclipse.hawkbit.repository.jpa.executor.AfterTransactionCommitExecutor;
 import org.eclipse.hawkbit.repository.model.BaseEntity;
-import org.eclipse.hawkbit.tenancy.TenantAwareAuthenticationDetails;
 import org.springframework.data.annotation.CreatedBy;
 import org.springframework.data.annotation.CreatedDate;
 import org.springframework.data.annotation.LastModifiedBy;
 import org.springframework.data.annotation.LastModifiedDate;
 import org.springframework.data.jpa.domain.support.AuditingEntityListener;
-import org.springframework.security.core.Authentication;
-import org.springframework.security.core.context.SecurityContextHolder;
 
 /**
  * Base hawkBit entity class containing the common attributes for Hibernate.
@@ -51,9 +47,6 @@ public abstract class AbstractJpaBaseEntity implements BaseEntity {
 
     protected static final int USERNAME_FIELD_LENGTH = 64;
 
-    @Serial
-    private static final long serialVersionUID = 1L;
-
     @Setter // should be used just for test purposes
     @Getter
     @Id
@@ -98,7 +91,7 @@ public abstract class AbstractJpaBaseEntity implements BaseEntity {
 
     @LastModifiedBy
     public void setLastModifiedBy(final String lastModifiedBy) {
-        if (this.lastModifiedBy != null && isController()) {
+        if (this.lastModifiedBy != null && SpRole.isController()) {
             // initialized and controller = doesn't update
             return;
         }
@@ -114,7 +107,7 @@ public abstract class AbstractJpaBaseEntity implements BaseEntity {
 
     @LastModifiedDate
     public void setLastModifiedAt(final long lastModifiedAt) {
-        if (this.lastModifiedAt != 0 && isController()) {
+        if (this.lastModifiedAt != 0 && SpRole.isController()) {
             // initialized and controller = doesn't update
             return;
         }
@@ -202,11 +195,4 @@ public abstract class AbstractJpaBaseEntity implements BaseEntity {
         // fire events onl AFTER transaction commit
         AfterTransactionCommitExecutor.afterCommit(runnable);
     }
-
-    protected boolean isController() {
-        final Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
-        return authentication != null
-                && authentication.getDetails() instanceof TenantAwareAuthenticationDetails tenantAwareDetails
-                && tenantAwareDetails.controller();
-    }
 }

hawkbit-repository/hawkbit-repository-jpa-hibernate/src/main/java/org/eclipse/hawkbit/repository/jpa/model/AbstractJpaTenantAwareBaseEntity.java

@@ -9,7 +9,6 @@
  */
 package org.eclipse.hawkbit.repository.jpa.model;
 
-import java.io.Serial;
 import java.util.Objects;
 
 import jakarta.persistence.Column;
@@ -36,9 +35,6 @@ import org.hibernate.annotations.TenantId;
 @MappedSuperclass
 public abstract class AbstractJpaTenantAwareBaseEntity extends AbstractJpaBaseEntity implements TenantAwareBaseEntity {
 
-    @Serial
-    private static final long serialVersionUID = 1L;
-
     @Column(name = "tenant", nullable = false, insertable = true, updatable = false, length = 40)
     @Size(min = 1, max = 40)
     @NotNull
@@ -86,9 +82,7 @@ public abstract class AbstractJpaTenantAwareBaseEntity extends AbstractJpaBaseEn
         final String currentTenant = AccessContext.tenant();
         if (currentTenant == null) {
             throw new TenantNotExistException(
-                    String.format(
-                            "AccessContext %s does not exists, cannot create entity %s with id %d",
-                            AccessContext.tenant(), getClass(), getId()));
+                    String.format("Tenant not found in the context, cannot create entity %s with id %d", getClass(), getId()));
         }
         setTenant(currentTenant.toUpperCase());
     }

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/ql/rsql/RsqlToSqlTest.java

@@ -33,7 +33,7 @@ import org.springframework.test.context.ContextConfiguration;
         "hawkbit.rsql.caseInsensitiveDB=true",
         "spring.main.allow-bean-definition-overriding=true",
         "spring.main.banner-mode=off",
-        "logging.level.root=ERROR" })
+        "logging.level.root=WARN" })
 @ContextConfiguration(classes = { JpaRepositoryConfiguration.class, TestConfiguration.class })
 @Disabled("For manual run only, while playing around with RSQL to SQL")
 @SuppressWarnings("java:S2699") // java:S2699 - manual test, don't actually does assertions

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/SecurityContextCtxTest.java

@@ -21,11 +21,11 @@ import java.util.function.Supplier;
 import lombok.SneakyThrows;
 import org.eclipse.hawkbit.auth.SpPermission;
 import org.eclipse.hawkbit.context.AccessContext;
+import org.eclipse.hawkbit.context.Principal;
 import org.eclipse.hawkbit.repository.AutoAssignHandler;
 import org.eclipse.hawkbit.repository.jpa.AbstractJpaIntegrationTest;
 import org.eclipse.hawkbit.repository.model.Rollout;
 import org.eclipse.hawkbit.repository.model.TargetFilterQuery;
-import org.eclipse.hawkbit.tenancy.TenantAwareAuthenticationDetails;
 import org.junit.jupiter.api.Test;
 import org.mockito.MockedStatic;
 import org.mockito.Mockito;
@@ -129,9 +129,7 @@ class SecurityContextCtxTest extends AbstractJpaIntegrationTest {
     private static SecurityContext createUserContext(final int testId) {
         final SecurityContext userContext = SecurityContextHolder.createEmptyContext();
         final UsernamePasswordAuthenticationToken userPassAuthentication = new UsernamePasswordAuthenticationToken(
-                "user", null, AUTHORITIES.stream().map(SimpleGrantedAuthority::new).toList());
-        final TenantAwareAuthenticationDetails details = new TenantAwareAuthenticationDetails("my_tenant_" + testId, false);
-        userPassAuthentication.setDetails(details);
+                new Principal("my_tenant_" + testId, "user"), null, AUTHORITIES.stream().map(SimpleGrantedAuthority::new).toList());
         userContext.setAuthentication(userPassAuthentication);
 
         assertThat(userContext).isNotNull();

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/ControllerManagementTest.java

@@ -36,7 +36,6 @@ import jakarta.validation.ConstraintViolationException;
 
 import org.assertj.core.api.Assertions;
 import org.eclipse.hawkbit.auth.SpPermission;
-import org.eclipse.hawkbit.ql.jpa.SpecificationBuilder;
 import org.eclipse.hawkbit.repository.RepositoryProperties;
 import org.eclipse.hawkbit.repository.TargetTypeManagement;
 import org.eclipse.hawkbit.repository.UpdateMode;
@@ -108,7 +107,7 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
         final int allowedAttributes = quotaManagement.getMaxAttributeEntriesPerTarget();
         testdataFactory.createTarget(controllerId);
 
-        final WithUser withController = SecurityContextSwitch.withController("controller", CONTROLLER_ROLE);
+        final WithUser withController = SecurityContextSwitch.withController("controller");
         assertThatExceptionOfType(AssignmentQuotaExceededException.class)
                 .isThrownBy(() -> runAs(withController, () -> writeAttributes(controllerId, allowedAttributes + 1, "key", "value")))
                 .withMessageContaining("" + allowedAttributes);
@@ -184,7 +183,7 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
         final Long actionId = createTargetAndAssignDs();
 
         SecurityContextSwitch
-                .getAs(SecurityContextSwitch.withController("controller", CONTROLLER_ROLE), () -> {
+                .getAs(SecurityContextSwitch.withController("controller"), () -> {
                     // Fails as one entry is already in there from the assignment
                     assertThatExceptionOfType(AssignmentQuotaExceededException.class)
                             .isThrownBy(() -> writeStatus(actionId, allowStatusEntries))
@@ -243,7 +242,8 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
         assertThat(actionId1).isNotNull();
         final ActionStatusCreateBuilder status = ActionStatusCreate.builder().actionId(actionId1).status(Status.WARNING);
         for (int i = 0; i < maxStatusEntries; i++) {
-            controllerManagement.addInformationalActionStatus(status.messages(List.of("Msg " + i)).timestamp(System.currentTimeMillis()).build());
+            controllerManagement.addInformationalActionStatus(
+                    status.messages(List.of("Msg " + i)).timestamp(System.currentTimeMillis()).build());
         }
         final ActionStatusCreate actionStatusCreate = status.build();
         assertThatExceptionOfType(AssignmentQuotaExceededException.class)
@@ -255,7 +255,8 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
         assertThat(actionId2).isNotEqualTo(actionId1);
         final ActionStatusCreateBuilder statusWarning = ActionStatusCreate.builder().actionId(actionId2).status(Status.WARNING);
         for (int i = 0; i < maxStatusEntries; i++) {
-            controllerManagement.addUpdateActionStatus(statusWarning.messages(List.of("Msg " + i)).timestamp(System.currentTimeMillis()).build());
+            controllerManagement.addUpdateActionStatus(
+                    statusWarning.messages(List.of("Msg " + i)).timestamp(System.currentTimeMillis()).build());
         }
         final ActionStatusCreate actionStatusCreateQE = statusWarning.build();
         assertThatExceptionOfType(AssignmentQuotaExceededException.class)
@@ -663,7 +664,7 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
                 new ByteArrayInputStream(random), null, artifactSize, null,
                 findFirstModuleByType(ds, osType).orElseThrow().getId(), "file1", false));
         final Artifact artifact2 = artifactManagement.create(new ArtifactUpload(
-                new ByteArrayInputStream(random), null,  artifactSize, null,
+                new ByteArrayInputStream(random), null, artifactSize, null,
                 findFirstModuleByType(ds2, osType).orElseThrow().getId(), "file1", false));
         assertThat(artifact.getSha1Hash()).isEqualTo(artifact2.getSha1Hash());
 
@@ -867,7 +868,7 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
                     .as("Expected an ConcurrencyFailureException to be thrown!")
                     .isThrownBy(() -> controllerManagement.findOrRegisterTargetIfItDoesNotExist("AA", LOCALHOST));
 
-            verify(mockTargetRepository, times(10 /* default retry max */+ 1)).findOne(any(Specification.class));
+            verify(mockTargetRepository, times(10 /* default retry max */ + 1)).findOne(any(Specification.class));
         } finally {
             // revert
             ((JpaControllerManagement) controllerManagement).setTargetRepository(targetRepository);
@@ -1161,7 +1162,7 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
         final String controllerId = "test123";
         final Target target = testdataFactory.createTarget(controllerId);
 
-        SecurityContextSwitch.getAs(SecurityContextSwitch.withController("controller", CONTROLLER_ROLE, SpPermission.READ_TARGET), () -> {
+        SecurityContextSwitch.getAs(SecurityContextSwitch.withController("controller"), () -> {
             addAttributeAndVerify(controllerId);
             addSecondAttributeAndVerify(controllerId);
             updateAttributeAndVerify(controllerId);
@@ -1769,7 +1770,8 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
         testData.put("test1", "testdata1");
         controllerManagement.updateControllerAttributes(controllerId, testData, null);
 
-        assertThat(targetManagement.getControllerAttributes(controllerId)).as("Controller Attributes are wrong").isEqualTo(testData);
+        SecurityContextSwitch.getAs(SecurityContextSwitch.withUser("bumlux", SpPermission.READ_TARGET), () ->
+                assertThat(targetManagement.getControllerAttributes(controllerId)).as("Controller Attributes are wrong").isEqualTo(testData));
     }
 
     private void addSecondAttributeAndVerify(final String controllerId) {
@@ -1778,8 +1780,8 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
         controllerManagement.updateControllerAttributes(controllerId, testData, null);
 
         testData.put("test1", "testdata1");
-        assertThat(targetManagement.getControllerAttributes(controllerId)).as("Controller Attributes are wrong")
-                .isEqualTo(testData);
+        SecurityContextSwitch.getAs(SecurityContextSwitch.withUser("bumlux", SpPermission.READ_TARGET), () ->
+                assertThat(targetManagement.getControllerAttributes(controllerId)).as("Controller Attributes are wrong").isEqualTo(testData));
     }
 
     private void updateAttributeAndVerify(final String controllerId) {
@@ -1789,8 +1791,8 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
         controllerManagement.updateControllerAttributes(controllerId, testData, null);
 
         testData.put("test2", "testdata20");
-        assertThat(targetManagement.getControllerAttributes(controllerId)).as("Controller Attributes are wrong")
-                .isEqualTo(testData);
+        SecurityContextSwitch.getAs(SecurityContextSwitch.withUser("bumlux", SpPermission.READ_TARGET), () ->
+                assertThat(targetManagement.getControllerAttributes(controllerId)).as("Controller Attributes are wrong").isEqualTo(testData));
     }
 
     private void updateTargetAttributesWithUpdateModeRemove(final String controllerId) {

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/ManagementSecurityTest.java

@@ -384,7 +384,7 @@ class ManagementSecurityTest extends AbstractJpaIntegrationTest {
                 } catch (final NoSuchMethodException | IllegalAccessException | InvocationTargetException e1) {
                     log.debug("{} is not a builder. Throws could not instantiate", clazz.getName());
                 }
-                log.error("Could not instantiate {}", clazz.getName(), e);
+                log.debug("Could not instantiate {} (but if inside instance, could be fine)", clazz.getName(), e);
                 throw e;
             }
         }
@@ -401,12 +401,12 @@ class ManagementSecurityTest extends AbstractJpaIntegrationTest {
 
     @SneakyThrows
     protected void assertPermissionsCheck(final Method managementInterfaceMethod, final Object managementObject, final String... permissions) {
+        final Object[] params = new Object[managementInterfaceMethod.getParameterCount()];
+        for (int i = 0; i < params.length; i++) {
+            params[i] = instance(managementInterfaceMethod.getParameterTypes()[i]);
+        }
         final Callable<?> callable = () -> {
             try {
-                final Object[] params = new Object[managementInterfaceMethod.getParameterCount()];
-                for (int i = 0; i < params.length; i++) {
-                    params[i] = instance(managementInterfaceMethod.getParameterTypes()[i]);
-                }
                 return managementInterfaceMethod.invoke(managementObject, params);
             } catch (final InvocationTargetException e) {
                 if (e.getCause() instanceof RuntimeException re) {

hawkbit-repository/hawkbit-repository-jpa/src/test/resources/jpa-test.properties

@@ -8,6 +8,8 @@
 # SPDX-License-Identifier: EPL-2.0
 #
 
+logging.level.root=WARN
+# could be used to suppress some logging if the root level is set to INFO or DEBUG, e.g. when debugging a test case
 NOISE_SUPPRESS_LEVEL=WARN
 
 ### General logging configuration - START

hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/TestConfiguration.java

@@ -23,6 +23,7 @@ import org.eclipse.hawkbit.artifact.fs.FileArtifactStorage;
 import org.eclipse.hawkbit.artifact.urlresolver.PropertyBasedArtifactUrlResolver;
 import org.eclipse.hawkbit.artifact.urlresolver.PropertyBasedArtifactUrlResolverProperties;
 import org.eclipse.hawkbit.auth.Hierarchy;
+import org.eclipse.hawkbit.auth.SpRole;
 import org.eclipse.hawkbit.context.AccessContext;
 import org.eclipse.hawkbit.repository.RepositoryConfiguration;
 import org.eclipse.hawkbit.repository.RolloutApprovalStrategy;
@@ -138,7 +139,7 @@ public class TestConfiguration implements AsyncConfigurer {
 
     @Bean
     AuditorAware<String> auditorAware() {
-        return () -> Optional.ofNullable(AccessContext.actor());
+        return () -> Optional.ofNullable(SpRole.isController() ? "CONTROLLER_PLUG_AND_PLAY" : AccessContext.actor());
     }
 
     @Bean

hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java

@@ -122,6 +122,7 @@ public abstract class AbstractIntegrationTest {
     protected static final Pageable PAGE = PageRequest.of(0, 500, Sort.by(Direction.ASC, "id"));
     protected static final Pageable UNPAGED = Pageable.unpaged();
 
+    protected static final String CONTROLLER_PLUG_AND_PLAY = "CONTROLLER_PLUG_AND_PLAY";
     protected static final URI LOCALHOST = URI.create("http://127.0.0.1");
     protected static final Random RND = TestdataFactory.RND;
 

hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/SecurityContextSwitch.java

@@ -23,9 +23,8 @@ import java.util.function.Supplier;
 import lombok.AccessLevel;
 import lombok.NoArgsConstructor;
 import org.eclipse.hawkbit.auth.SpRole;
+import org.eclipse.hawkbit.context.Principal;
 import org.eclipse.hawkbit.repository.SystemManagement;
-import org.eclipse.hawkbit.tenancy.TenantAwareAuthenticationDetails;
-import org.eclipse.hawkbit.tenancy.TenantAwareUser;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.Authentication;
@@ -92,8 +91,8 @@ public class SecurityContextSwitch {
         });
     }
 
-    public static WithUser withController(final String principal, final String... authorities) {
-        return withTenantAndUser(DEFAULT_TENANT, principal, authorities, true, true);
+    public static WithUser withController(final String principal) { // authorized controller
+        return withTenantAndUser(DEFAULT_TENANT, principal, new String[] { CONTROLLER_ROLE }, true, true);
     }
 
     public static WithUser withUser(final String principal, final String... authorities) {
@@ -146,12 +145,8 @@ public class SecurityContextSwitch {
 
         @Override
         public Authentication getAuthentication() {
-            final TestingAuthenticationToken testingAuthenticationToken = new TestingAuthenticationToken(
-                    new TenantAwareUser(annotation.principal(), "***", null, annotation.tenant()),
-                    annotation.credentials(), annotation.authorities());
-            testingAuthenticationToken.setDetails(
-                    new TenantAwareAuthenticationDetails(annotation.tenant(), annotation.controller()));
-            return testingAuthenticationToken;
+            return new TestingAuthenticationToken(
+                    new Principal(annotation.tenant(), annotation.principal()), annotation.credentials(), annotation.authorities());
         }
 
         @Override

hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/WithUser.java

@@ -28,6 +28,13 @@ import org.springframework.security.test.context.support.WithSecurityContextFact
 @Inherited
 public @interface WithUser {
 
+    /**
+     * Gets the test actor.
+     *
+     * @return test actor
+     */
+    String principal() default "TestActor";
+
     /**
      * Gets the test tenant id.
      *
@@ -35,13 +42,6 @@ public @interface WithUser {
      */
     String tenant() default "DEFAULT";
 
-    /**
-     * Gets the test principal.
-     *
-     * @return test principal
-     */
-    String principal() default "TestPrincipal";
-
     /**
      * Gets the test credentials.
      *

hawkbit-repository/hawkbit-repository-test/src/main/resources/hawkbit-test-defaults.properties

@@ -10,7 +10,7 @@
 
 # Test utility properties for easier fault investigation - START
 ## Logging - START
-logging.level.root=ERROR
+logging.level.root=WARN
 logging.level.org.eclipse.hawkbit.repository.test.matcher.EventVerifier=ERROR
 logging.level.org.eclipse.persistence=ERROR
 spring.jpa.properties.eclipselink.logging.level=FINE