Skip to content
Snippets Groups Projects
Commit bd98a34b authored by OZGCloud's avatar OZGCloud
Browse files

Merge pull request 'OZG-3056: validate user id' (#48) from OZG-3056_validate_user_id into master

parents 62157aa8 23b3a861
No related branches found
No related tags found
No related merge requests found
Showing with 161 additions and 13 deletions
...@@ -62,7 +62,10 @@ class UserRepository implements PanacheMongoRepository<User> { ...@@ -62,7 +62,10 @@ class UserRepository implements PanacheMongoRepository<User> {
} }
public Optional<User> findById(String id) { public Optional<User> findById(String id) {
return findByIdOptional(new ObjectId(id)); return Optional.ofNullable(id)
.filter(ObjectId::isValid)
.map(ObjectId::new)
.flatMap(this::findByIdOptional);
} }
public Optional<User> findByEmail(String email) { public Optional<User> findByEmail(String email) {
......
...@@ -23,11 +23,13 @@ ...@@ -23,11 +23,13 @@
*/ */
package de.itvsh.kop.user.settings; package de.itvsh.kop.user.settings;
import java.util.Optional;
import javax.enterprise.context.ApplicationScoped; import javax.enterprise.context.ApplicationScoped;
import de.itvsh.kop.common.logging.KopLogging;
import org.bson.types.ObjectId; import org.bson.types.ObjectId;
import de.itvsh.kop.common.logging.KopLogging;
import de.itvsh.kop.user.User; import de.itvsh.kop.user.User;
import io.quarkus.mongodb.panache.PanacheMongoRepository; import io.quarkus.mongodb.panache.PanacheMongoRepository;
...@@ -35,10 +37,14 @@ import io.quarkus.mongodb.panache.PanacheMongoRepository; ...@@ -35,10 +37,14 @@ import io.quarkus.mongodb.panache.PanacheMongoRepository;
@KopLogging @KopLogging
class UserSettingsRepository implements PanacheMongoRepository<User> { class UserSettingsRepository implements PanacheMongoRepository<User> {
public UserSettings updateByUserId(String userId, UserSettings userSettings) { public Optional<UserSettings> updateByUserId(String userId, UserSettings userSettings) {
var savedUser = findById(new ObjectId(userId)); return Optional.ofNullable(userId)
savedUser.setUserSettings(userSettings); .filter(ObjectId::isValid)
update(savedUser); .map(ObjectId::new)
return findById(savedUser.getId()).getUserSettings(); .flatMap(id -> findByIdOptional(id).map(user -> {
user.setUserSettings(userSettings);
update(user);
return findById(user.getId()).getUserSettings();
}));
} }
} }
\ No newline at end of file
...@@ -85,9 +85,10 @@ public class UserSettingsResource { ...@@ -85,9 +85,10 @@ public class UserSettingsResource {
if (Objects.isNull(userSettings)) { if (Objects.isNull(userSettings)) {
throw new FunctionalException(() -> "Request Body missing."); throw new FunctionalException(() -> "Request Body missing.");
} }
var updatedSettings = userSettingsService.updateByUserId(userSettings, userId);
return resourceAssembler.toResource(updatedSettings, userId, userManagerUrl); return userSettingsService.updateByUserId(userSettings, userId)
.map(updatedSettings -> resourceAssembler.toResource(updatedSettings, userId, userManagerUrl))
.orElseThrow(() -> new FunctionalException(() -> "Invalid user id"));
} }
void checkUserAccess(String userId) { void checkUserAccess(String userId) {
......
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
package de.itvsh.kop.user.settings; package de.itvsh.kop.user.settings;
import java.util.Objects; import java.util.Objects;
import java.util.Optional;
import javax.enterprise.context.ApplicationScoped; import javax.enterprise.context.ApplicationScoped;
import javax.inject.Inject; import javax.inject.Inject;
...@@ -39,7 +40,7 @@ class UserSettingsService { ...@@ -39,7 +40,7 @@ class UserSettingsService {
@Inject @Inject
UserSettingsRepository repository; UserSettingsRepository repository;
public UserSettings updateByUserId(UserSettings userSettings, String userId) { public Optional<UserSettings> updateByUserId(UserSettings userSettings, String userId) {
return repository.updateByUserId(userId, userSettings); return repository.updateByUserId(userId, userSettings);
} }
......
...@@ -297,6 +297,26 @@ class UserRepositoryITCase { ...@@ -297,6 +297,26 @@ class UserRepositoryITCase {
assertThat(user).isNotNull(); assertThat(user).isNotNull();
assertThat(user.get()).usingRecursiveComparison().isEqualTo(UserTestFactory.create()); assertThat(user.get()).usingRecursiveComparison().isEqualTo(UserTestFactory.create());
} }
@Test
void shouldNotFindIfIdNull() {
assertThat(repository.findById((String) null)).isEmpty();
}
@Test
void shouldNotFindIfEmptyId() {
assertThat(repository.findById("")).isEmpty();
}
@Test
void shouldNotFindIfBlankId() {
assertThat(repository.findById(" ")).isEmpty();
}
@Test
void shouldNotFindIfInvalidId() {
assertThat(repository.findById("wrong_id")).isEmpty();
}
} }
@DisplayName("Find by email") @DisplayName("Find by email")
......
package de.itvsh.kop.user.settings;
import static org.assertj.core.api.Assertions.*;
import javax.inject.Inject;
import org.bson.types.ObjectId;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import de.itvsh.kop.user.UserTestFactory;
import de.itvsh.kop.user.common.MongoDbTestProfile;
import io.quarkus.test.junit.QuarkusTest;
import io.quarkus.test.junit.TestProfile;
@QuarkusTest
@TestProfile(MongoDbTestProfile.class)
class UserSettingsRepositoryITCase {
@Inject
UserSettingsRepository repository;
@Nested
class TestUpdateByUserId {
@BeforeEach
void beforeEach() {
repository.deleteAll();
}
@Test
void shouldUpdateUserSettings() {
var user = UserTestFactory.create();
repository.persist(user);
var settings = repository.updateByUserId(
user.getId().toString(),
UserSettings.builder().notificationsSendFor(NotificationsSendFor.ALL).build());
assertThat(settings)
.isPresent()
.map(UserSettings::getNotificationsSendFor)
.hasValue(NotificationsSendFor.ALL);
}
@Test
void shouldReturnEmptyIfUserNotExist() {
var user = UserTestFactory.create();
repository.persist(user);
var settings = repository.updateByUserId(
new ObjectId().toString(),
UserSettings.builder().notificationsSendFor(NotificationsSendFor.ALL).build());
assertThat(settings).isEmpty();
}
@Test
void shouldReturnEmptyIfInvalidUserId() {
var user = UserTestFactory.create();
repository.persist(user);
var settings = repository.updateByUserId(
"wrong_id",
UserSettings.builder().notificationsSendFor(NotificationsSendFor.ALL).build());
assertThat(settings).isEmpty();
}
@Test
void shouldReturnEmptyIfUserIdNull() {
var user = UserTestFactory.create();
repository.persist(user);
var settings = repository.updateByUserId(
null,
UserSettings.builder().notificationsSendFor(NotificationsSendFor.ALL).build());
assertThat(settings).isEmpty();
}
}
}
\ No newline at end of file
...@@ -27,6 +27,7 @@ import static org.assertj.core.api.Assertions.*; ...@@ -27,6 +27,7 @@ import static org.assertj.core.api.Assertions.*;
import static org.mockito.ArgumentMatchers.*; import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*; import static org.mockito.Mockito.*;
import java.util.Optional;
import java.util.UUID; import java.util.UUID;
import javax.ws.rs.core.UriInfo; import javax.ws.rs.core.UriInfo;
...@@ -49,6 +50,7 @@ import de.itvsh.kop.user.UserService; ...@@ -49,6 +50,7 @@ import de.itvsh.kop.user.UserService;
import de.itvsh.kop.user.UserTestFactory; import de.itvsh.kop.user.UserTestFactory;
import de.itvsh.kop.user.common.errorhandling.AccessForbiddenException; import de.itvsh.kop.user.common.errorhandling.AccessForbiddenException;
import de.itvsh.kop.user.common.errorhandling.FunctionalException; import de.itvsh.kop.user.common.errorhandling.FunctionalException;
import io.quarkus.hal.HalEntityWrapper;
class UserSettingsResourceTest { class UserSettingsResourceTest {
...@@ -133,6 +135,30 @@ class UserSettingsResourceTest { ...@@ -133,6 +135,30 @@ class UserSettingsResourceTest {
} }
} }
@Nested
class TestUserIdNotExist {
@BeforeEach
void mockAccess() {
doNothing().when(resource).checkUserAccess(anyString());
}
@Test
void shouldThrowFunctionalExceptionIfInvalidUserId() {
var user = UserSettingsTestFactory.create();
assertThatExceptionOfType(FunctionalException.class)
.isThrownBy(() -> resource.updateUserSettings("wrong_id", user))
.withMessageStartingWith("Functional error: Invalid user id");
}
@Test
void shouldThrowFunctionalExceptionIfEmptyUserId() {
var user = UserSettingsTestFactory.create();
assertThatExceptionOfType(FunctionalException.class)
.isThrownBy(() -> resource.updateUserSettings("", user))
.withMessageStartingWith("Functional error: Invalid user id");
}
}
@DisplayName("with filled body") @DisplayName("with filled body")
@Nested @Nested
class TestFilledBody { class TestFilledBody {
...@@ -140,13 +166,14 @@ class UserSettingsResourceTest { ...@@ -140,13 +166,14 @@ class UserSettingsResourceTest {
@BeforeEach @BeforeEach
void mockAccess() { void mockAccess() {
doNothing().when(resource).checkUserAccess(anyString()); doNothing().when(resource).checkUserAccess(anyString());
when(userSettingsService.updateByUserId(any(UserSettings.class), anyString())).thenReturn(updatedUserSettings); when(userSettingsService.updateByUserId(any(UserSettings.class), anyString())).thenReturn(Optional.of(updatedUserSettings));
mockUserManagerUrl(); mockUserManagerUrl();
} }
@Test @Test
void shouldCheckAccess() { void shouldCheckAccess() {
when(resourceAssembler.toResource(any(), anyString(), anyString())).thenReturn(new HalEntityWrapper(null));
resource.updateUserSettings(USER_ID, userSettings); resource.updateUserSettings(USER_ID, userSettings);
verify(resource).checkUserAccess(USER_ID); verify(resource).checkUserAccess(USER_ID);
...@@ -154,13 +181,16 @@ class UserSettingsResourceTest { ...@@ -154,13 +181,16 @@ class UserSettingsResourceTest {
@Test @Test
void shouldCallUserSettingsService() { void shouldCallUserSettingsService() {
when(resourceAssembler.toResource(any(), anyString(), anyString())).thenReturn(new HalEntityWrapper(null));
resource.updateUserSettings(USER_ID, userSettings); resource.updateUserSettings(USER_ID, userSettings);
verify(userSettingsService).updateByUserId(userSettings, USER_ID); verify(userSettingsService).updateByUserId(userSettings, USER_ID);
} }
@Test @Test
void shouldCallResourceAssembler() { void
shouldCallResourceAssembler() {
when(resourceAssembler.toResource(any(), anyString(), anyString())).thenReturn(new HalEntityWrapper(null));
resource.updateUserSettings(USER_ID, userSettings); resource.updateUserSettings(USER_ID, userSettings);
verify(resourceAssembler).toResource(updatedUserSettings, USER_ID, USER_MANAGER_URL); verify(resourceAssembler).toResource(updatedUserSettings, USER_ID, USER_MANAGER_URL);
......
...@@ -68,7 +68,10 @@ class UserSettingsServiceITCase { ...@@ -68,7 +68,10 @@ class UserSettingsServiceITCase {
var savedUserSettings = service.updateByUserId(userSettingsToSave, UserTestFactory.ID.toHexString()); var savedUserSettings = service.updateByUserId(userSettingsToSave, UserTestFactory.ID.toHexString());
assertThat(savedUserSettings.getNotificationsSendFor()).isEqualTo(NotificationsSendFor.ALL); assertThat(savedUserSettings)
.isPresent()
.map(UserSettings::getNotificationsSendFor)
.hasValue(NotificationsSendFor.ALL);
} }
} }
} }
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment