Make user not accessible from UserDetails

This commit is contained in:
2023-05-14 00:42:53 +02:00
parent 8383fa5055
commit e615d58262
6 changed files with 37 additions and 40 deletions

View File

@@ -1,12 +1,16 @@
package net.kapcake.bankingservice.security;
import lombok.AllArgsConstructor;
import net.kapcake.bankingservice.model.domain.User;
import org.springframework.security.core.CredentialsContainer;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.userdetails.UserDetails;
import java.util.Collection;
public record UserDetailsImpl(User user) implements UserDetails {
@AllArgsConstructor
public class UserDetailsImpl implements UserDetails, CredentialsContainer {
private User user;
@Override
public Collection<? extends GrantedAuthority> getAuthorities() {
@@ -42,4 +46,9 @@ public record UserDetailsImpl(User user) implements UserDetails {
public boolean isEnabled() {
return true;
}
@Override
public void eraseCredentials() {
user.setPassword(null);
}
}

View File

@@ -6,7 +6,6 @@ import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.security.core.userdetails.UsernameNotFoundException;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
@Service
public class UserDetailsServiceImpl implements UserDetailsService {
@@ -17,7 +16,6 @@ public class UserDetailsServiceImpl implements UserDetailsService {
}
@Override
@Transactional
public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
User user = userRepository.findByUsername(username)
.orElseThrow(() -> new UsernameNotFoundException("User (" + username + ") does not exist"));

View File

@@ -45,7 +45,8 @@ public class PaymentService {
public PaymentDTO createPayment(UserDetailsImpl authenticatedUser, PaymentDTO paymentDTO) {
Payment payment = paymentConverter.mapToEntity(paymentDTO);
paymentValidator.validate(authenticatedUser.user(), payment);
List<BankAccount> userBankAccounts = bankAccountRepository.findAllByUsers_username(authenticatedUser.getUsername());
paymentValidator.validate(userBankAccounts, payment);
BankAccount giverAccount = payment.getGiverAccount();
Optional<BankAccount> beneficiaryAccount = bankAccountRepository.findByAccountNumber(payment.getBeneficiaryAccountNumber());
@@ -79,7 +80,7 @@ public class PaymentService {
}
public List<PaymentDTO> getPaymentsForUser(UserDetailsImpl authenticatedUser, PaymentFilter paymentFilter) {
List<BankAccount> userBankAccounts = authenticatedUser.user().getBankAccounts();
List<BankAccount> userBankAccounts = bankAccountRepository.findAllByUsers_username(authenticatedUser.getUsername());
List<Payment> userPayments = paymentRepository.findAllByGiverAccountIn(userBankAccounts);
List<Payment> filteredPayments = getFilteredPayments(paymentFilter, userPayments);
@@ -111,7 +112,7 @@ public class PaymentService {
}
public void deletePayment(UserDetailsImpl authenticatedUser, Long id) {
List<BankAccount> userBankAccounts = authenticatedUser.user().getBankAccounts();
List<BankAccount> userBankAccounts = bankAccountRepository.findAllByUsers_username(authenticatedUser.getUsername());
Payment payment = paymentRepository.findById(id).orElseThrow(() -> new ResourceNotFoundException("A payment with the given id does not exist"));
if (userBankAccounts.stream().noneMatch(account -> account.getId().equals(payment.getGiverAccount().getId()))) {
throw new AccessDeniedException("The payment with id [" + id + "] is not owned by the authenticated user");

View File

@@ -1,11 +1,7 @@
package net.kapcake.bankingservice.validation;
import net.kapcake.bankingservice.exceptions.ValidationException;
import net.kapcake.bankingservice.model.domain.BalanceType;
import net.kapcake.bankingservice.model.domain.IbanValidationResponse;
import net.kapcake.bankingservice.model.domain.Balance;
import net.kapcake.bankingservice.model.domain.Payment;
import net.kapcake.bankingservice.model.domain.User;
import net.kapcake.bankingservice.model.domain.*;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
import org.springframework.web.client.RestClientException;
@@ -29,14 +25,14 @@ public class PaymentValidator {
}
public void validate(User user, Payment payment) throws ValidationException {
validateGiverAccount(user, payment);
public void validate(List<BankAccount> userBankAccounts, Payment payment) throws ValidationException {
validateGiverAccount(userBankAccounts, payment);
validateBeneficiaryAccount(payment);
validateAccountBalance(payment);
}
private static void validateGiverAccount(User user, Payment payment) {
boolean userOwnsGiverAccount = user.getBankAccounts().stream().anyMatch(bankAccount -> bankAccount.getId().equals(payment.getGiverAccount().getId()));
private static void validateGiverAccount(List<BankAccount> userBankAccounts, Payment payment) {
boolean userOwnsGiverAccount = userBankAccounts.stream().anyMatch(bankAccount -> bankAccount.getId().equals(payment.getGiverAccount().getId()));
if (!userOwnsGiverAccount) {
throw new ValidationException("Giver account not owned by authenticated user.");
}

View File

@@ -90,7 +90,6 @@ class PaymentServiceTest {
@Test
void getPaymentsForUser() {
List<BankAccount> bankAccounts = List.of(bankAccount1);
user1.setBankAccounts(bankAccounts);
UserDetailsImpl authenticatedUser = new UserDetailsImpl(user1);
Payment payment1 = new Payment().setId(1L).setCreationDate(new GregorianCalendar(2023, Calendar.MAY, 11, 12, 0).getTime()).setAmount(BigDecimal.valueOf(50)).setCurrency(Currency.EUR).setBeneficiaryAccountNumber(ACCOUNT_NUMBER_2).setGiverAccount(bankAccount1);
PaymentDTO paymentDTO1 = new PaymentDTO().setId(1L).setCreationDate(new GregorianCalendar(2023, Calendar.MAY, 11, 12, 0).getTime()).setAmount(BigDecimal.valueOf(50)).setCurrency(Currency.EUR).setBeneficiaryAccountNumber(ACCOUNT_NUMBER_2).setGiverAccount(1L);
@@ -99,6 +98,7 @@ class PaymentServiceTest {
Payment payment3 = new Payment().setId(3L).setCreationDate(new GregorianCalendar(2023, Calendar.MAY, 11, 12, 2).getTime()).setAmount(BigDecimal.valueOf(50)).setCurrency(Currency.EUR).setBeneficiaryAccountNumber(ACCOUNT_NUMBER_3).setGiverAccount(bankAccount1);
PaymentDTO paymentDTO3 = new PaymentDTO().setId(3L).setCreationDate(new GregorianCalendar(2023, Calendar.MAY, 11, 12, 2).getTime()).setAmount(BigDecimal.valueOf(50)).setCurrency(Currency.EUR).setBeneficiaryAccountNumber(ACCOUNT_NUMBER_3).setGiverAccount(1L);
when(bankAccountRepository.findAllByUsers_username(TEST_USER_1)).thenReturn(bankAccounts);
when(paymentRepository.findAllByGiverAccountIn(bankAccounts)).thenReturn(Arrays.asList(payment1, payment2, payment3));
when(paymentConverter.mapToDTO(payment1)).thenReturn(paymentDTO1);
when(paymentConverter.mapToDTO(payment2)).thenReturn(paymentDTO2);
@@ -180,14 +180,14 @@ class PaymentServiceTest {
@Test
void deletePayment() {
List<BankAccount> bankAccounts1 = List.of(bankAccount1);
user1.setBankAccounts(bankAccounts1);
List<BankAccount> bankAccounts2 = List.of(bankAccount2);
user2.setBankAccounts(bankAccounts2);
UserDetailsImpl authenticatedUser1 = new UserDetailsImpl(user1);
UserDetailsImpl authenticatedUser2 = new UserDetailsImpl(user2);
Payment payment1 = new Payment().setId(1L).setAmount(BigDecimal.valueOf(50)).setCurrency(Currency.EUR).setBeneficiaryAccountNumber(ACCOUNT_NUMBER_2).setGiverAccount(bankAccount1);
Payment payment2 = new Payment().setId(2L).setAmount(BigDecimal.valueOf(50)).setCurrency(Currency.EUR).setBeneficiaryAccountNumber(ACCOUNT_NUMBER_2).setGiverAccount(bankAccount1).setStatus(PaymentStatus.EXECUTED);
when(bankAccountRepository.findAllByUsers_username(TEST_USER_1)).thenReturn(bankAccounts1);
when(bankAccountRepository.findAllByUsers_username(TEST_USER_2)).thenReturn(bankAccounts2);
when(paymentRepository.findById(1L)).thenReturn(Optional.of(payment1));
when(paymentRepository.findById(2L)).thenReturn(Optional.of(payment2));
when(paymentRepository.findById(3L)).thenReturn(Optional.empty());

View File

@@ -4,13 +4,7 @@ import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import net.kapcake.bankingservice.config.BankingServiceConfig;
import net.kapcake.bankingservice.exceptions.ValidationException;
import net.kapcake.bankingservice.model.domain.BalanceType;
import net.kapcake.bankingservice.model.domain.Currency;
import net.kapcake.bankingservice.model.domain.IbanValidationResponse;
import net.kapcake.bankingservice.model.domain.Balance;
import net.kapcake.bankingservice.model.domain.BankAccount;
import net.kapcake.bankingservice.model.domain.Payment;
import net.kapcake.bankingservice.model.domain.User;
import net.kapcake.bankingservice.model.domain.*;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -49,8 +43,8 @@ class PaymentValidatorTest {
private final ObjectMapper objectMapper;
private BankAccount bankAccount;
private Payment payment;
private User user;
private Balance balance;
private List<BankAccount> userBankAccounts;
@Autowired
public PaymentValidatorTest(RestTemplate restTemplate, MockRestServiceServer server) {
@@ -61,15 +55,16 @@ class PaymentValidatorTest {
@BeforeEach
void setUp() throws JsonProcessingException {
user = new User().setId(1L).setUsername(USERNAME);
User user = new User().setId(1L).setUsername(USERNAME);
balance = new Balance().setId(1L).setType(BalanceType.AVAILABLE);
bankAccount = new BankAccount().setId(1L).setBalances(Collections.singletonList(balance));
payment = new Payment();
// Set up valid payment
userBankAccounts = Collections.singletonList(bankAccount);
bankAccount.setAccountNumber(GIVER_ACCOUNT_NUMBER).setUsers(Collections.singletonList(user));
balance.setCurrency(Currency.EUR).setAmount(BigDecimal.valueOf(5.5));
user.setBankAccounts(Collections.singletonList(bankAccount));
user.setBankAccounts(userBankAccounts);
payment.setGiverAccount(bankAccount).setCurrency(Currency.EUR).setBeneficiaryAccountNumber(VALID_BENEFICIARY).setAmount(BigDecimal.valueOf(4));
server
@@ -81,14 +76,12 @@ class PaymentValidatorTest {
@Test
void validate_correctPayment() {
paymentValidator.validate(user, payment);
paymentValidator.validate(userBankAccounts, payment);
}
@Test
void validate_invalidGiverAccount() {
user.setBankAccounts(Collections.emptyList());
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(user, payment));
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(Collections.emptyList(), payment));
Assertions.assertEquals("Giver account not owned by authenticated user.", validationException.getMessage());
}
@@ -101,7 +94,7 @@ class PaymentValidatorTest {
.andExpect(method(HttpMethod.GET))
.andRespond(withServiceUnavailable());
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(user, payment));
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(userBankAccounts, payment));
Assertions.assertEquals("Beneficiary account could not be validated.", validationException.getMessage());
server.reset();
@@ -110,7 +103,7 @@ class PaymentValidatorTest {
.andExpect(method(HttpMethod.GET))
.andRespond(withSuccess());
validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(user, payment));
validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(userBankAccounts, payment));
Assertions.assertEquals("Beneficiary account could not be validated.", validationException.getMessage());
}
@@ -123,7 +116,7 @@ class PaymentValidatorTest {
.andExpect(method(HttpMethod.GET))
.andRespond(withSuccess(objectMapper.writeValueAsString(new IbanValidationResponse(false, Collections.singletonList("Invalid"), VALID_BENEFICIARY)), MediaType.APPLICATION_JSON));
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(user, payment));
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(userBankAccounts, payment));
Assertions.assertEquals("Beneficiary account not valid: [Invalid]", validationException.getMessage());
}
@@ -131,7 +124,7 @@ class PaymentValidatorTest {
void validate_invalidBeneficiaryAccount_giverAndBeneficiarySame() {
bankAccount.setAccountNumber(VALID_BENEFICIARY);
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(user, payment));
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(userBankAccounts, payment));
Assertions.assertEquals("Beneficiary and giver account are the same.", validationException.getMessage());
}
@@ -144,7 +137,7 @@ class PaymentValidatorTest {
.andExpect(method(HttpMethod.GET))
.andRespond(withSuccess(objectMapper.writeValueAsString(new IbanValidationResponse(true, Collections.emptyList(), FORBIDDEN_ACCOUNT_1)), MediaType.APPLICATION_JSON));
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(user, payment));
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(userBankAccounts, payment));
Assertions.assertEquals("Beneficiary account is forbidden.", validationException.getMessage());
}
@@ -152,7 +145,7 @@ class PaymentValidatorTest {
void validate_invalidAccountBalance_notFound() {
balance.setType(BalanceType.END_OF_DAY);
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(user, payment));
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(userBankAccounts, payment));
Assertions.assertEquals("Unable to retrieve available account balance.", validationException.getMessage());
}
@@ -160,7 +153,7 @@ class PaymentValidatorTest {
void validate_invalidAccountBalance_differentCurrencies() {
balance.setCurrency(Currency.USD);
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(user, payment));
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(userBankAccounts, payment));
Assertions.assertEquals("Payment and account currency must be the same.", validationException.getMessage());
}
@@ -168,7 +161,7 @@ class PaymentValidatorTest {
void validate_invalidAccountBalance_insufficientBalance() {
balance.setAmount(BigDecimal.valueOf(2));
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(user, payment));
ValidationException validationException = Assertions.assertThrows(ValidationException.class, () -> paymentValidator.validate(userBankAccounts, payment));
Assertions.assertEquals("Available account balance not sufficient.", validationException.getMessage());
}
}