From 3db1fecec222ff82c103d97dc57c2e41c02acb75 Mon Sep 17 00:00:00 2001 From: Ola Brustad Date: Fri, 9 Aug 2024 15:04:51 +0200 Subject: [PATCH] Cleaned up code and TODOs --- src/main/java/no/fintlabs/AzureClient.java | 179 +----------------- .../kafka/ResourceGroupConsumerService.java | 1 - 2 files changed, 9 insertions(+), 171 deletions(-) diff --git a/src/main/java/no/fintlabs/AzureClient.java b/src/main/java/no/fintlabs/AzureClient.java index 18494ed..96c54c0 100644 --- a/src/main/java/no/fintlabs/AzureClient.java +++ b/src/main/java/no/fintlabs/AzureClient.java @@ -15,14 +15,12 @@ import lombok.extern.log4j.Log4j2; import no.fintlabs.azure.*; import no.fintlabs.kafka.ResourceGroup; -import no.fintlabs.cache.FintCache; import no.fintlabs.kafka.ResourceGroupMembership; import okhttp3.Request; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; import java.util.*; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; @Component @@ -30,118 +28,14 @@ @RequiredArgsConstructor public class AzureClient { protected final Config config; - protected final ConfigGroup configGroup; protected final ConfigUser configUser; protected final GraphServiceClient graphService; private final AzureUserProducerService azureUserProducerService; private final AzureUserExternalProducerService azureUserExternalProducerService; private final AzureGroupProducerService azureGroupProducerService; - private long startTime; - private final AzureGroupMembershipProducerService azureGroupMembershipProducerService; - private final FintCache resourceGroupCache; - private final FintCache azureGroupCache; -/* -// private void pageThroughAzureGroup(AzureGroup azureGroup, CompletableFuture inPageFuture) { -// inPageFuture.thenAccept(inPage -> { -// int members = 0; -// log.debug("Fetching Azure Groups"); -// DirectoryObjectCollectionWithReferencesPage page = inPage; -// do { -// for (DirectoryObject member : page.getCurrentPage()) { -// // New member detected -// members++; -// azureGroupMembershipProducerService.publishAddedMembership(new AzureGroupMembership(azureGroup.getId(), member)); -// azureGroup.getMembers().add(member.id); -// } -// if (page.getNextPage() == null) { -// break; -// } else { -// log.debug("Processing membership page"); -// page = page.getNextPage().buildRequest().get(); -// } -// } while (page != null); -// -// log.debug("{} memberships detected in groupName {} with groupId {}", members, azureGroup.getDisplayName(), azureGroup.getId()); -// } ); -// } -*/ -/* -// private void pageThroughGroups(CompletableFuture inPageFuture) { -// inPageFuture.thenAccept(inPage -> { -// int groups = 0; -// GroupCollectionPage page = inPage; -// do { -// for (Group group : page.getCurrentPage()) { -// -// if (group.displayName != null && group.displayName.endsWith(configGroup.getSuffix())) { -// groups++; -// AzureGroup newGroup; -// try { -// newGroup = new AzureGroup(group, configGroup); -// } catch (NumberFormatException e) { -// log.warn("Problems converting resourceID to LONG! {}. Skipping creation of group", e); -// continue; -// } -// -// // Put object into cache -// try { -// pageThroughAzureGroup( -// newGroup, -// graphService.groups(group.id).members() -// .buildRequest() -// .select("id") -// .getAsync() -// ); -// } catch (ClientException e) { -// log.error("Error fetching page", e); -// } -// azureGroupProducerService.publish(newGroup); -// } -// } -// if (page.getNextPage() == null) { -// break; -// } else { -// log.debug("Processing group page"); -// page = page.getNextPage().buildRequest().get(); -// } -// } while (page != null); -// -// log.info("{} Group objects detected in Microsoft Entra", groups); -// -// } ); -// -// } -/* -/* -// private List pageThroughGetGroups(GroupCollectionPage inPage) { -// int groups = 0; -// GroupCollectionPage page = inPage; -// List retGroupList = new ArrayList(); -// do { -// for (Group group : page.getCurrentPage()) { -// -// AzureGroup newGroup; -// try { -// newGroup = new AzureGroup(group, configGroup); -// } catch (NumberFormatException e) { -// log.warn("Problems converting resourceID to LONG! {}. Skipping creation of group", e); -// continue; -// } -// retGroupList.add(newGroup); -// } -// if (page.getNextPage() == null) { -// break; -// } else { -// log.debug("Processing group page"); -// page = page.getNextPage().buildRequest().get(); -// } -// } while (page != null); -// log.debug("{} Group objects detected in Microsoft Entra", groups); -// return retGroupList; -// } -*/ + private void pageThroughUsers(UserCollectionPage inPage) { //inPageFuture.thenAccept(inPage -> { int users = 0; @@ -194,35 +88,12 @@ private void pullAllUsers() { log.info("*** <<< Finished pulling users from Microsoft Entra in {} minutes and {} seconds >>> *** ", minutes, seconds); } - catch (ClientException ex) {} + catch (ClientException ex) { + log.error("pullAllUsers failed with message: {}", ex.getMessage().toString()); + } } -/* -// private void pullAllExtUsers() { -// log.debug("--- Starting to pull users with external flag from Azure --- "); -// this.pageThrough( -// graphService.users() -// .buildRequest() -// .select(String.join(",", configUser.AllAttributes())) -// .filter("usertype eq 'member'") -// //.top(10) -// .get() -// ); -// log.debug("--- finished pulling resources from Azure. ---"); -// -// } -// -// public List getAllGroups() { -// return this.pageThroughGetGroups( -// graphService.groups() -// .buildRequest() -// .select(String.format("id,displayName,description,members,%s", configGroup.getFintkontrollidattribute())) -// //.filter(String.format("startsWith(displayName,'%s')",configGroup.getPrefix())) -// .expand(String.format("members($select=%s)", String.join(",", configUser.AllAttributes()))) -// .get() -// ); -// } -*/ + @Scheduled( initialDelayString = "${fint.kontroll.azure-ad-gateway.group-scheduler.pull.initial-delay-ms}", fixedDelayString = "${fint.kontroll.azure-ad-gateway.group-scheduler.pull.delta-delay-ms}" @@ -265,7 +136,7 @@ private CompletableFuture pageThroughGroups(CompletableFuture>> ***"); -// long startTime = System.currentTimeMillis(); -// try { -// this.pageThroughGroups( -// graphService.groups() -// .buildRequest() -// // TODO: Attributes should not be hard-coded [FKS-210] -// .select(String.format("id,displayName,description,members,%s", configGroup.getFintkontrollidattribute())) -// // TODO: Improve MS Graph filter ? [FKS-687] -// //.filter(String.format("displayName ne null",configGroup.getResourceGroupIDattribute())) -// //.filter(String.format("startsWith(displayName,'%s')",configGroup.getPrefix())) -// // No need for expand as members are handled by pageThroughAzureGroup -// //.expand(String.format("members($select=%s)", String.join(",", configUser.AllAttributes()))) -// .getAsync() -// ); -// } catch (ClientException e) { -// log.error("Failed when trying to get groups. ", e); -// } -// long endTime = System.currentTimeMillis(); -// long elapsedTimeInSeconds = (endTime - startTime) / 1000; -// long minutes = elapsedTimeInSeconds / 60; -// long seconds = elapsedTimeInSeconds % 60; -// -// log.info("*** <<< Done fetching all groups from Microsoft Entra in {} minutes and {} seconds >>> ***", minutes, seconds); -// } -*/ public boolean doesGroupExist(String resourceGroupId) { - // TODO: Should this be implemented as a simpler call to MS Graph? [FKS-200] - // Form the selection criteria for the MS Graph request // TODO: Attributes should not be hard-coded [FKS-210] String selectionCriteria = String.format("id,displayName,description,%s", configGroup.getFintkontrollidattribute()); @@ -394,7 +235,6 @@ public void addGroupToAzure(ResourceGroup resourceGroup) { handleGraphApiError(ex); return null; }); - //log.info("Added Group to Azure: {}", resourceGroup.getResourceName()); } public void deleteGroup(String resourceGroupId) { @@ -476,7 +316,7 @@ public void addGroupMembership(ResourceGroupMembership resourceGroupMembership, return; } if(e.getError().error.code.contains("Request_ResourceNotFound")){ - log.warn("AzureGroupRef is not correct: ", resourceGroupMembership.getAzureUserRef(), resourceGroupMembership.getAzureGroupRef()); + log.warn("AzureGroupRef is not correct on user ObjectId {} and group ObjectId {}", resourceGroupMembership.getAzureUserRef(), resourceGroupMembership.getAzureGroupRef()); return; } @@ -490,7 +330,7 @@ public void addGroupMembership(ResourceGroupMembership resourceGroupMembership, else { // Handle other HTTP errors - log.error("HTTP Error while updating group {}: " + e.getError().error.message + " \r", resourceGroupMembership.getAzureGroupRef()); + log.error("HTTP Error while updating group {}: {} \r", resourceGroupMembership.getAzureGroupRef(), e.getError().error.message); } } } @@ -538,8 +378,7 @@ public void deleteGroupMembership(ResourceGroupMembership resourceGroupMembershi private void handleGraphApiError(Throwable ex) { if (ex instanceof CompletionException) { Throwable cause = ex.getCause(); - if (cause instanceof GraphServiceException) { - GraphServiceException gse = (GraphServiceException) cause; + if (cause instanceof GraphServiceException gse) { int statusCode = gse.getResponseCode(); switch (statusCode) { // case 204: diff --git a/src/main/java/no/fintlabs/kafka/ResourceGroupConsumerService.java b/src/main/java/no/fintlabs/kafka/ResourceGroupConsumerService.java index e9f6469..ded7032 100644 --- a/src/main/java/no/fintlabs/kafka/ResourceGroupConsumerService.java +++ b/src/main/java/no/fintlabs/kafka/ResourceGroupConsumerService.java @@ -72,7 +72,6 @@ void updateAzure(String kafkaKey, Optional resourceGroupOptional) String randomUUID = UUID.randomUUID().toString(); log.debug("Starting updateAzure function {}.", randomUUID); ResourceGroup resourceGroup; - // TODO: Split doesGroupExist to POST or PUT. Relates to [FKS-200] and [FKS-202] if (resourceGroupOptional.isPresent()) { resourceGroup = resourceGroupOptional.get(); if (resourceGroup.getResourceName() != null && !azureClient.doesGroupExist(resourceGroup.getId())) {