From 5d93af8209b0101165edd406c23c73883b11d8c8 Mon Sep 17 00:00:00 2001 From: Witold Brzozowski Date: Wed, 30 Oct 2024 15:00:16 +0100 Subject: [PATCH 1/6] Throw error if user leader is missing when accepting requests --- .../request/LeaderNotFoundException.java | 19 +++++++++++++++++++ .../request/normal/NormalRequestService.java | 8 ++++---- .../ActiveDirectoryUserLeaderProvider.java | 6 ++++++ ...3_5_0_7__drop_email_unique_constraints.sql | 2 ++ ...3_5_0_7__drop_email_unique_constraints.sql | 2 ++ .../src/helpers/errors/ErrorCodeMappings.js | 1 + 6 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 src/main/java/info/fingo/urlopia/request/LeaderNotFoundException.java create mode 100644 src/main/resources/scripts/U3_5_0_7__drop_email_unique_constraints.sql create mode 100644 src/main/resources/scripts/V3_5_0_7__drop_email_unique_constraints.sql diff --git a/src/main/java/info/fingo/urlopia/request/LeaderNotFoundException.java b/src/main/java/info/fingo/urlopia/request/LeaderNotFoundException.java new file mode 100644 index 00000000..11f1df5a --- /dev/null +++ b/src/main/java/info/fingo/urlopia/request/LeaderNotFoundException.java @@ -0,0 +1,19 @@ +package info.fingo.urlopia.request; + +import info.fingo.urlopia.api.v2.BaseCustomException; +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(code = HttpStatus.PRECONDITION_FAILED, reason = "LEADER_NOT_FOUND") +public class LeaderNotFoundException extends BaseCustomException { + private static final String ERROR_MSG = "Leader has not been found"; + + public LeaderNotFoundException() { + super(ERROR_MSG); + } + + @Override + public HttpStatus getHttpStatus() { + return HttpStatus.PRECONDITION_FAILED; + } +} diff --git a/src/main/java/info/fingo/urlopia/request/normal/NormalRequestService.java b/src/main/java/info/fingo/urlopia/request/normal/NormalRequestService.java index 6884fb2e..4ca59b84 100644 --- a/src/main/java/info/fingo/urlopia/request/normal/NormalRequestService.java +++ b/src/main/java/info/fingo/urlopia/request/normal/NormalRequestService.java @@ -68,12 +68,12 @@ public Request create(Long userId, BaseRequestInput requestInput) { log.info("New normal request with id: %d has been created".formatted(request.getId())); var leader = userService.getAcceptanceLeaderForUser(user); - if (leader != null) { - this.acceptanceService.create(request, leader); - } else { - this.accept(request); + if (leader == null) { + throw new LeaderNotFoundException(); } + this.acceptanceService.create(request, leader); + return requestRepository .findById(request.getId()) .orElseThrow(NoSuchElementException::new); diff --git a/src/main/java/info/fingo/urlopia/user/ActiveDirectoryUserLeaderProvider.java b/src/main/java/info/fingo/urlopia/user/ActiveDirectoryUserLeaderProvider.java index 5e0ab753..c82228b1 100644 --- a/src/main/java/info/fingo/urlopia/user/ActiveDirectoryUserLeaderProvider.java +++ b/src/main/java/info/fingo/urlopia/user/ActiveDirectoryUserLeaderProvider.java @@ -41,10 +41,13 @@ private User getUserLeaderUnsafe(User user) throws NamingException { var userDN = userSearch.stream().findFirst().map(NameClassPair::getNameInNamespace).orElse(""); var organizationalUnits = extractOrganizationalUnitsDNs(userDN); + LOGGER.info("Starting search of the '{}' manager", userDN); + // Step 2: Find first existing and valid OU manager. for (var ouDn : organizationalUnits) { var controls = new SearchControls(); controls.setSearchScope(SearchControls.SUBTREE_SCOPE); + LOGGER.info("Looking for a manager in the '{}' group", ouDn); var ouSearch = activeDirectory.newSearch().distinguishedName(ouDn).search(controls); for (var result : ouSearch) { var attributes = result.getAttributes(); @@ -53,7 +56,10 @@ private User getUserLeaderUnsafe(User user) throws NamingException { if (!managedByDN.isBlank() && !managedByDN.equals(userDN)) { var manager = getManagerDetails(managedByDN); if (manager.isPresent()) { + LOGGER.info("Manager of '{}' OU found: '{}'", ouDn, managedByDN); return manager.get(); + } else { + LOGGER.warn("Manager of '{}' OU has not been found", ouDn); } } } diff --git a/src/main/resources/scripts/U3_5_0_7__drop_email_unique_constraints.sql b/src/main/resources/scripts/U3_5_0_7__drop_email_unique_constraints.sql new file mode 100644 index 00000000..4edd422f --- /dev/null +++ b/src/main/resources/scripts/U3_5_0_7__drop_email_unique_constraints.sql @@ -0,0 +1,2 @@ +CREATE UNIQUE INDEX users_mail_index ON users (mail); +CREATE UNIQUE INDEX users_mail_key ON users (mail); \ No newline at end of file diff --git a/src/main/resources/scripts/V3_5_0_7__drop_email_unique_constraints.sql b/src/main/resources/scripts/V3_5_0_7__drop_email_unique_constraints.sql new file mode 100644 index 00000000..75acde8b --- /dev/null +++ b/src/main/resources/scripts/V3_5_0_7__drop_email_unique_constraints.sql @@ -0,0 +1,2 @@ +ALTER TABLE users DROP CONSTRAINT IF EXISTS users_mail_key; +DROP INDEX IF EXISTS users_mail_key; \ No newline at end of file diff --git a/view.react/src/helpers/errors/ErrorCodeMappings.js b/view.react/src/helpers/errors/ErrorCodeMappings.js index a04f28fe..593c562d 100644 --- a/view.react/src/helpers/errors/ErrorCodeMappings.js +++ b/view.react/src/helpers/errors/ErrorCodeMappings.js @@ -17,6 +17,7 @@ const loginErrorCodeMappings = { const requestsErrorCodeMappings = { "END_DATE_IS_BEFORE_START_DATE": "Data końcowa nie powinna być przed datą początkową", "NOT_ENOUGH_DAYS": "Nie posiadasz wystarczającej liczby dni urlopowych", + "LEADER_NOT_FOUND": "Nie została znaleziona osoba wymagana do akceptacji wniosku", "REQUEST_OVERLAPPING": "Wybrany termin pokrywa się z terminem jednej z wcześniej utworzonych okoliczności", } From f452d4382174a9df50b126eb3076bca3ecee2726 Mon Sep 17 00:00:00 2001 From: Witold Brzozowski Date: Wed, 30 Oct 2024 15:11:08 +0100 Subject: [PATCH 2/6] Fix --- .../api/v2/request/normal/NormalRequestServiceSpec.groovy | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/groovy/info/fingo/urlopia/api/v2/request/normal/NormalRequestServiceSpec.groovy b/src/test/groovy/info/fingo/urlopia/api/v2/request/normal/NormalRequestServiceSpec.groovy index d1177ac2..8ac72a43 100644 --- a/src/test/groovy/info/fingo/urlopia/api/v2/request/normal/NormalRequestServiceSpec.groovy +++ b/src/test/groovy/info/fingo/urlopia/api/v2/request/normal/NormalRequestServiceSpec.groovy @@ -172,6 +172,7 @@ class NormalRequestServiceSpec extends Specification{ getTeams() >> Set.of(team) } userRepository.findById(userId) >> Optional.of(user) + userService.getAcceptanceLeaderForUser(user) >> team.getLeader() and: "request mock with repo with endDate before startDate" def request = Mock(Request){ From 2284181f9490ab6db9e7e6e71bd921e83f15c4f4 Mon Sep 17 00:00:00 2001 From: Witold Brzozowski Date: Thu, 31 Oct 2024 11:40:52 +0100 Subject: [PATCH 3/6] Change migration script number --- ...onstraints.sql => U3_5_0_8__drop_email_unique_constraints.sql} | 0 ...onstraints.sql => V3_5_0_8__drop_email_unique_constraints.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename src/main/resources/scripts/{U3_5_0_7__drop_email_unique_constraints.sql => U3_5_0_8__drop_email_unique_constraints.sql} (100%) rename src/main/resources/scripts/{V3_5_0_7__drop_email_unique_constraints.sql => V3_5_0_8__drop_email_unique_constraints.sql} (100%) diff --git a/src/main/resources/scripts/U3_5_0_7__drop_email_unique_constraints.sql b/src/main/resources/scripts/U3_5_0_8__drop_email_unique_constraints.sql similarity index 100% rename from src/main/resources/scripts/U3_5_0_7__drop_email_unique_constraints.sql rename to src/main/resources/scripts/U3_5_0_8__drop_email_unique_constraints.sql diff --git a/src/main/resources/scripts/V3_5_0_7__drop_email_unique_constraints.sql b/src/main/resources/scripts/V3_5_0_8__drop_email_unique_constraints.sql similarity index 100% rename from src/main/resources/scripts/V3_5_0_7__drop_email_unique_constraints.sql rename to src/main/resources/scripts/V3_5_0_8__drop_email_unique_constraints.sql From f2fc41ea924af3639830cceafd4556baa226ccd1 Mon Sep 17 00:00:00 2001 From: Witold Brzozowski Date: Thu, 31 Oct 2024 12:16:09 +0100 Subject: [PATCH 4/6] Update --- ...onstraints.sql => U3_5_0_9__drop_email_unique_constraints.sql} | 0 ...onstraints.sql => V3_5_0_9__drop_email_unique_constraints.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename src/main/resources/scripts/{U3_5_0_8__drop_email_unique_constraints.sql => U3_5_0_9__drop_email_unique_constraints.sql} (100%) rename src/main/resources/scripts/{V3_5_0_8__drop_email_unique_constraints.sql => V3_5_0_9__drop_email_unique_constraints.sql} (100%) diff --git a/src/main/resources/scripts/U3_5_0_8__drop_email_unique_constraints.sql b/src/main/resources/scripts/U3_5_0_9__drop_email_unique_constraints.sql similarity index 100% rename from src/main/resources/scripts/U3_5_0_8__drop_email_unique_constraints.sql rename to src/main/resources/scripts/U3_5_0_9__drop_email_unique_constraints.sql diff --git a/src/main/resources/scripts/V3_5_0_8__drop_email_unique_constraints.sql b/src/main/resources/scripts/V3_5_0_9__drop_email_unique_constraints.sql similarity index 100% rename from src/main/resources/scripts/V3_5_0_8__drop_email_unique_constraints.sql rename to src/main/resources/scripts/V3_5_0_9__drop_email_unique_constraints.sql From 875223d388af42cc52639512c7befaa590b3e102 Mon Sep 17 00:00:00 2001 From: boavenn Date: Mon, 4 Nov 2024 12:16:42 +0100 Subject: [PATCH 5/6] Add more logs --- .../ActiveDirectoryUserLeaderProvider.java | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/main/java/info/fingo/urlopia/user/ActiveDirectoryUserLeaderProvider.java b/src/main/java/info/fingo/urlopia/user/ActiveDirectoryUserLeaderProvider.java index c82228b1..159682b6 100644 --- a/src/main/java/info/fingo/urlopia/user/ActiveDirectoryUserLeaderProvider.java +++ b/src/main/java/info/fingo/urlopia/user/ActiveDirectoryUserLeaderProvider.java @@ -45,15 +45,24 @@ private User getUserLeaderUnsafe(User user) throws NamingException { // Step 2: Find first existing and valid OU manager. for (var ouDn : organizationalUnits) { + LOGGER.info("Looking for a manager in the '{}' group", ouDn); var controls = new SearchControls(); controls.setSearchScope(SearchControls.SUBTREE_SCOPE); - LOGGER.info("Looking for a manager in the '{}' group", ouDn); + var ouSearch = activeDirectory.newSearch().distinguishedName(ouDn).search(controls); + if (ouSearch.isEmpty()) { + LOGGER.warn("No search results for '{}' OU", ouDn); + } + for (var result : ouSearch) { - var attributes = result.getAttributes(); - var managedBy = attributes.get(Attribute.MANAGED_BY.getKey()); - var managedByDN = managedBy != null ? (String) managedBy.get() : ""; - if (!managedByDN.isBlank() && !managedByDN.equals(userDN)) { + var managedBy = result.getAttributes().get(Attribute.MANAGED_BY.getKey()); + if (managedBy == null) { + LOGGER.warn("Missing managedBy attribute for '{}' OU", ouDn); + continue; + } + + var managedByDN = (String) managedBy.get(); + if (!managedByDN.equals(userDN)) { var manager = getManagerDetails(managedByDN); if (manager.isPresent()) { LOGGER.info("Manager of '{}' OU found: '{}'", ouDn, managedByDN); From 4f134c878b4fe30074887aa5a59b523e4683a970 Mon Sep 17 00:00:00 2001 From: boavenn Date: Mon, 4 Nov 2024 14:27:49 +0100 Subject: [PATCH 6/6] Escape special AD characters --- .../config/ad/ActiveDirectorySearcher.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/info/fingo/urlopia/config/ad/ActiveDirectorySearcher.java b/src/main/java/info/fingo/urlopia/config/ad/ActiveDirectorySearcher.java index 3b6d4bac..4c51517b 100644 --- a/src/main/java/info/fingo/urlopia/config/ad/ActiveDirectorySearcher.java +++ b/src/main/java/info/fingo/urlopia/config/ad/ActiveDirectorySearcher.java @@ -43,37 +43,37 @@ public ActiveDirectorySearcher objectClasses(List ob } public ActiveDirectorySearcher memberOf(String group) { - var value = String.format("(memberOf=%s)", group); + var value = String.format("(memberOf=%s)", escapeSpecialCharacters(group)); filter.append(value); return this; } public ActiveDirectorySearcher principalName(String principalName) { - var value = String.format("(userPrincipalName=%s)", principalName); + var value = String.format("(userPrincipalName=%s)", escapeSpecialCharacters(principalName)); filter.append(value); return this; } public ActiveDirectorySearcher mail(String mail) { - var value = String.format("(mail=%s)", mail); + var value = String.format("(mail=%s)", escapeSpecialCharacters(mail)); filter.append(value); return this; } public ActiveDirectorySearcher name(String name) { - var value = String.format("(name=%s)", name); + var value = String.format("(name=%s)", escapeSpecialCharacters(name)); filter.append(value); return this; } public ActiveDirectorySearcher distinguishedName(String distinguishedName) { - var value = String.format("(distinguishedName=%s)", distinguishedName); + var value = String.format("(distinguishedName=%s)", escapeSpecialCharacters(distinguishedName)); filter.append(value); return this; } public ActiveDirectorySearcher excludeDistinguishedName(String distinguishedName) { - var value = String.format("(!(distinguishedName=%s))", distinguishedName); + var value = String.format("(!(distinguishedName=%s))", escapeSpecialCharacters(distinguishedName)); filter.append(value); return this; } @@ -89,6 +89,11 @@ public ActiveDirectorySearcher isDisabled(){ return this; } + // Based on https://learn.microsoft.com/en-us/archive/technet-wiki/5392.active-directory-ldap-syntax-filters#Special_Characters + private String escapeSpecialCharacters(String query) { + return query.replaceAll("\\\\", "\\\\5C"); + } + public List search() { var controls = new SearchControls(); controls.setSearchScope(SearchControls.SUBTREE_SCOPE);