Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ plugins {
}

group = 'com.flexcodelabs'
version = '0.0.75'
version = '0.0.76'
description = 'Flextuma App'

java {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ private Pagination<T> doFindAllPaginated(Pageable pageable, List<String> filter,
}

@SuppressWarnings("unchecked")
private Specification<T> buildTenantSpec() {
protected Specification<T> buildTenantSpec() {
Optional<com.flexcodelabs.flextuma.core.entities.auth.User> currentUser = currentUserResolver == null
? Optional.empty()
: Optional.ofNullable(currentUserResolver.getCurrentUser()).orElse(Optional.empty());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

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;

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
Expand Down Expand Up @@ -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<PersonalAccessToken> 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);
}

}
Original file line number Diff line number Diff line change
@@ -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<SecurityContextHolder> 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());
}
}