diff --git a/build.gradle b/build.gradle index 693499e..816efed 100644 --- a/build.gradle +++ b/build.gradle @@ -8,7 +8,7 @@ plugins { } group = 'com.flexcodelabs' -version = '0.0.75' +version = '0.0.76' description = 'Flextuma App' java { diff --git a/src/main/java/com/flexcodelabs/flextuma/core/entities/auth/PersonalAccessToken.java b/src/main/java/com/flexcodelabs/flextuma/core/entities/auth/PersonalAccessToken.java index 928151c..e857193 100644 --- a/src/main/java/com/flexcodelabs/flextuma/core/entities/auth/PersonalAccessToken.java +++ b/src/main/java/com/flexcodelabs/flextuma/core/entities/auth/PersonalAccessToken.java @@ -1,5 +1,6 @@ package com.flexcodelabs.flextuma.core.entities.auth; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; import com.flexcodelabs.flextuma.core.entities.base.BaseEntity; @@ -36,6 +37,7 @@ public class PersonalAccessToken extends BaseEntity { private String name; @Column(nullable = false, unique = true) + @JsonIgnore private String token; @ManyToOne(fetch = FetchType.LAZY) diff --git a/src/main/java/com/flexcodelabs/flextuma/core/services/BaseService.java b/src/main/java/com/flexcodelabs/flextuma/core/services/BaseService.java index aa5e5da..797f8b5 100644 --- a/src/main/java/com/flexcodelabs/flextuma/core/services/BaseService.java +++ b/src/main/java/com/flexcodelabs/flextuma/core/services/BaseService.java @@ -183,7 +183,7 @@ private Pagination doFindAllPaginated(Pageable pageable, List filter, } @SuppressWarnings("unchecked") - private Specification buildTenantSpec() { + protected Specification buildTenantSpec() { Optional currentUser = currentUserResolver == null ? Optional.empty() : Optional.ofNullable(currentUserResolver.getCurrentUser()).orElse(Optional.empty()); diff --git a/src/main/java/com/flexcodelabs/flextuma/modules/auth/services/PersonalAccessTokenService.java b/src/main/java/com/flexcodelabs/flextuma/modules/auth/services/PersonalAccessTokenService.java index 8f85f0f..19334de 100644 --- a/src/main/java/com/flexcodelabs/flextuma/modules/auth/services/PersonalAccessTokenService.java +++ b/src/main/java/com/flexcodelabs/flextuma/modules/auth/services/PersonalAccessTokenService.java @@ -2,6 +2,7 @@ import java.util.UUID; +import org.springframework.data.jpa.domain.Specification; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.JpaSpecificationExecutor; import org.springframework.stereotype.Service; @@ -9,6 +10,7 @@ import com.flexcodelabs.flextuma.core.entities.auth.PersonalAccessToken; import com.flexcodelabs.flextuma.core.repositories.PersonalAccessTokenRepository; import com.flexcodelabs.flextuma.core.repositories.UserRepository; +import com.flexcodelabs.flextuma.core.security.SecurityUtils; import com.flexcodelabs.flextuma.core.services.BaseService; @Service @@ -75,12 +77,49 @@ protected String getTableName() { @Override protected void onPreSave(PersonalAccessToken entity) { - if (entity.getUser() == null) { - String currentUsername = com.flexcodelabs.flextuma.core.security.SecurityUtils.getCurrentUsername(); - if (currentUsername != null) { - userRepository.findByUsername(currentUsername).ifPresent(entity::setUser); - } + // Tokens are strictly personal: always force ownership to the caller and + // ignore any client-supplied user/token/active, regardless of what the + // request body contains -- otherwise a caller could mint a token hashed + // from a secret they already know, pointed at someone else's account. + entity.setUser(null); + entity.setToken(null); + entity.setActive(null); + entity.setRawToken(null); + + String currentUsername = SecurityUtils.getCurrentUsername(); + if (currentUsername != null) { + userRepository.findByUsername(currentUsername).ifPresent(entity::setUser); } } + @Override + protected PersonalAccessToken onPreUpdate(PersonalAccessToken newEntity, PersonalAccessToken oldEntity) { + // Only name/expiresAt are editable via PUT -- clearing the rest here makes + // BaseService#update's getNullPropertyNames() skip them, so a client can't + // reassign ownership, reactivate a revoked token, or rewrite its hash. + newEntity.setUser(null); + newEntity.setToken(null); + newEntity.setActive(null); + newEntity.setScopes(null); + newEntity.setAllowedConnectorIds(null); + newEntity.setAllowSystemConnectors(null); + newEntity.setRawToken(null); + return super.onPreUpdate(newEntity, oldEntity); + } + + @Override + protected Specification buildTenantSpec() { + // Unlike org-shared resources (contacts, tags, ...), a personal access + // token must never be visible to anyone but its owner -- so this can't + // reuse TenantAwareSpecification's org-wide sharing. + if (SecurityUtils.getCurrentUserAuthorities().contains("SUPER_ADMIN")) { + return (root, query, cb) -> cb.conjunction(); + } + String currentUsername = SecurityUtils.getCurrentUsername(); + if (currentUsername == null) { + return (root, query, cb) -> cb.disjunction(); + } + return (root, query, cb) -> cb.equal(root.get("user").get("username"), currentUsername); + } + } diff --git a/src/test/java/com/flexcodelabs/flextuma/modules/auth/services/PersonalAccessTokenServiceTest.java b/src/test/java/com/flexcodelabs/flextuma/modules/auth/services/PersonalAccessTokenServiceTest.java new file mode 100644 index 0000000..7fb4ac0 --- /dev/null +++ b/src/test/java/com/flexcodelabs/flextuma/modules/auth/services/PersonalAccessTokenServiceTest.java @@ -0,0 +1,146 @@ +package com.flexcodelabs.flextuma.modules.auth.services; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.when; + +import java.time.LocalDateTime; +import java.util.Collections; +import java.util.Optional; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; + +import com.flexcodelabs.flextuma.core.entities.auth.PersonalAccessToken; +import com.flexcodelabs.flextuma.core.entities.auth.User; +import com.flexcodelabs.flextuma.core.repositories.PersonalAccessTokenRepository; +import com.flexcodelabs.flextuma.core.repositories.UserRepository; + +/** + * A personal access token grants whoever holds it the full privileges of the + * account it's attached to, so these guard against a caller using the generic + * create/update payload to attach a token to someone else's account, revive a + * revoked one, or plant a hash they already know the plaintext for. + */ +@ExtendWith(MockitoExtension.class) +class PersonalAccessTokenServiceTest { + + @Mock + private PersonalAccessTokenRepository repository; + + @Mock + private UserRepository userRepository; + + @Mock + private SecurityContext securityContext; + + @Mock + private Authentication authentication; + + private MockedStatic securityContextHolderMock; + + private PersonalAccessTokenService service; + + @BeforeEach + void setUp() { + service = new PersonalAccessTokenService(repository, userRepository); + + securityContextHolderMock = Mockito.mockStatic(SecurityContextHolder.class); + securityContextHolderMock.when(SecurityContextHolder::getContext).thenReturn(securityContext); + lenient().when(securityContext.getAuthentication()).thenReturn(authentication); + lenient().when(authentication.isAuthenticated()).thenReturn(true); + } + + @AfterEach + void tearDown() { + securityContextHolderMock.close(); + } + + private void authenticateAs(String username) { + when(authentication.getName()).thenReturn(username); + } + + @Test + void onPreSave_forcesOwnerToCaller_ignoringClientSuppliedUser() { + authenticateAs("alice"); + User alice = new User(); + alice.setUsername("alice"); + when(userRepository.findByUsername("alice")).thenReturn(Optional.of(alice)); + + User bob = new User(); + bob.setUsername("bob"); + PersonalAccessToken entity = new PersonalAccessToken(); + entity.setUser(bob); // attacker-supplied owner in the request body + + service.onPreSave(entity); + + assertEquals("alice", entity.getUser().getUsername()); + } + + @Test + void onPreSave_discardsClientSuppliedTokenAndActive_soAFreshSecretIsGenerated() { + authenticateAs("alice"); + User alice = new User(); + alice.setUsername("alice"); + when(userRepository.findByUsername("alice")).thenReturn(Optional.of(alice)); + + String attackerKnownHash = "deadbeef"; // a hash the attacker already knows the plaintext for + PersonalAccessToken entity = new PersonalAccessToken(); + entity.setToken(attackerKnownHash); + entity.setActive(true); + entity.setRawToken("whatever-the-client-sent"); + + service.onPreSave(entity); + assertNull(entity.getToken()); + assertNull(entity.getActive()); + assertNull(entity.getRawToken()); + + entity.generateToken(); // the entity's own @PrePersist hook + assertNotEquals(attackerKnownHash, entity.getToken()); + assertTrue(entity.getRawToken().startsWith("ft_")); + assertEquals(Boolean.TRUE, entity.getActive()); + } + + @Test + void onPreUpdate_onlyLetsNameAndExpiryChange() { + User attacker = new User(); + attacker.setUsername("mallory"); + + PersonalAccessToken oldEntity = new PersonalAccessToken(); + oldEntity.setName("CI pipeline"); + + PersonalAccessToken incoming = new PersonalAccessToken(); + incoming.setName("Renamed by owner"); + incoming.setExpiresAt(LocalDateTime.now().plusDays(30)); + incoming.setUser(attacker); + incoming.setToken("attacker-chosen-hash"); + incoming.setActive(true); + incoming.setScopes(Collections.singleton("MESSAGES_SEND")); + incoming.setAllowSystemConnectors(true); + incoming.setRawToken("leaked-back-to-attacker"); + + service.onPreUpdate(incoming, oldEntity); + + assertEquals("Renamed by owner", incoming.getName()); + assertNotEquals(null, incoming.getExpiresAt()); + assertNull(incoming.getUser()); + assertNull(incoming.getToken()); + assertNull(incoming.getActive()); + assertNull(incoming.getScopes()); + assertNull(incoming.getAllowedConnectorIds()); + assertNull(incoming.getAllowSystemConnectors()); + assertNull(incoming.getRawToken()); + } +}