Add conditional email access feature changes #294

Merged
tcdlpds merged 2 commits from rajitha/device-mgt-core:conditional-access into master 10 months ago
rajitha commented 1 year ago
Collaborator

Purpose

## Purpose * Include Conditional access back-end changes. * ticket => http://roadmap.entgra.net/issues/10194
rajitha changed title from WIP: Add conditional email access feature changes to Add conditional email access feature changes 12 months ago
rajitha force-pushed conditional-access from 901186fb5d to 6010c8143d 11 months ago
navodzoysa requested changes 11 months ago
public static void init(CEADatasourceConfiguration ceaDatasourceConfiguration) {
dataSource = resolveDatasource(ceaDatasourceConfiguration);
if (dataSource == null) throw new NullPointerException("Datasource is null");
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
}
private static DataSource resolveDatasource(CEADatasourceConfiguration ceaDatasourceConfiguration) {
if (ceaDatasourceConfiguration == null)
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
if (ceaDatasourceConfiguration == null)
throw new NullPointerException("Null is retrieved for Datasource configuration");
JNDILookupDefinition jndiLookupDefinition = ceaDatasourceConfiguration.getJndiLookupDefinition();
if (jndiLookupDefinition == null)
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
String datasourceName = jndiLookupDefinition.getJndiName();
List<JNDILookupDefinition.JNDIProperty>
jndiProperties = ceaDatasourceConfiguration.getJndiLookupDefinition().getJndiProperties();
if (jndiProperties == null || jndiProperties.isEmpty()) return lookupDatasource(datasourceName);
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
}
public static CEAPolicyDAO getCEAPolicyDAO() {
if (productName == null) throw new IllegalStateException("Database is not initialized properly");
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
import org.wso2.carbon.context.PrivilegedCarbonContext;
import java.nio.charset.StandardCharsets;
import java.sql.*;
Collaborator

Remove wildcard imports

Remove wildcard imports
rajitha marked this conversation as resolved
}
private CEAPolicyDTO toCEAPolicyDTO(CEAPolicy ceaPolicy) {
if (ceaPolicy == null) throw new NullPointerException("CEAPolicy can't be null");
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
ceaPolicyDTO.setCreatedTimestamp(new Timestamp(ceaPolicy.getCreated().getTime()));
ceaPolicyDTO.setUpdatedTimestamp(new Timestamp(ceaPolicy.getLastUpdated().getTime()));
ceaPolicyDTO.setTenantId(ceaPolicy.getTenantId());
if (ceaPolicy.getLastSynced() != null)
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
}
private CEAPolicy toCEAPolicy(CEAPolicyDTO ceaPolicyDTO) {
if (ceaPolicyDTO == null) throw new NullPointerException("CEAPolicyDTO can't be null");
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
ceaPolicy.setLastUpdated(new Date(ceaPolicyDTO.getUpdatedTimestamp().getTime()));
ceaPolicy.setSynced(ceaPolicyDTO.isSynced());
ceaPolicy.setTenantId(ceaPolicyDTO.getTenantId());
if (ceaPolicyDTO.getLastSyncedTimestamp() != null)
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
private EnforcementServiceManager enforcementServiceManager;
private TaskService taskService;
private CEAPolicyMonitoringTaskManager ceaPolicyMonitoringTaskManager;
CEAManagementDataHolder() {
Collaborator

Add new line after instance variables

Add new line after instance variables
rajitha marked this conversation as resolved
getInstance().getCeaPolicyMonitoringTaskManager();
CEAConfigManager ceaConfigManager = CEAConfigManager.getInstance();
CEAConfiguration ceaConfiguration = ceaConfigManager.getCeaConfiguration();
if (ceaPolicyMonitoringTaskManager == null)
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
CEAPolicyAlreadyExistsException {
try {
CEAPolicyManagementDAOFactory.openConnection();
if (ceaPolicyDAO.retrieveCEAPolicy() != null) {
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
try {
CEAPolicyManagementDAOFactory.openConnection();
CEAPolicy existingCeaPolicy = ceaPolicyDAO.retrieveCEAPolicy();
if (existingCeaPolicy == null) throw new CEAPolicyNotFoundException("CEA policy not found");
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
try {
CEAPolicyMonitoringTaskManager ceaPolicyMonitoringTaskManager = CEAManagementDataHolder.
getInstance().getCeaPolicyMonitoringTaskManager();
if (ceaPolicyMonitoringTaskManager == null)
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
throw new IllegalStateException("CEA policy monitoring task manager not initialized properly");
CEAPolicyManagementDAOFactory.openConnection();
CEAPolicy existingCeaPolicy = ceaPolicyDAO.retrieveCEAPolicy();
if (existingCeaPolicy == null) throw new CEAPolicyNotFoundException("CEA policy not found");
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
@Override
public void startTask(long monitoringFrequency) throws CEAPolicyMonitoringTaskManagerException {
if (monitoringFrequency <= 0) throw new CEAPolicyMonitoringTaskManagerException("Invalid monitoring frequency");
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
if (monitoringFrequency <= 0) throw new CEAPolicyMonitoringTaskManagerException("Invalid monitoring frequency");
TaskService taskService = CEAManagementDataHolder.getInstance().getTaskService();
if (taskService == null) throw new IllegalStateException("Task service is not initialized");
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import io.entgra.device.mgt.core.cea.mgt.common.bean.*;
Collaborator

Remove wildcard imports

Remove wildcard imports
rajitha marked this conversation as resolved
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.*;
Collaborator

Remove wildcard imports

Remove wildcard imports
rajitha marked this conversation as resolved
Set<EmailOutlookAccessPolicy> emailOutlookAccessPolicies = ceaPolicy.getAccessPolicy().getEmailOutlookAccessPolicy();
if (emailOutlookAccessPolicies.contains(EmailOutlookAccessPolicy.NOT_CONFIGURED)) {
if (log.isDebugEnabled()) {
log.info("CEA email outlook policy not configured, but the support is available in " +
Collaborator

Change to debug log

Change to debug log
rajitha marked this conversation as resolved
if (ceaPolicy.getAccessPolicy().getPOPIMAPAccessPolicy().
equalsName(EmailOutlookAccessPolicy.NOT_CONFIGURED.name())) {
if (log.isDebugEnabled()) {
log.info("CEA POP/IMAP policy not configured, but support is available in " +
Collaborator

Change to debug log

Change to debug log
rajitha marked this conversation as resolved
if (ceaPolicy.getAccessPolicy().getWebOutlookAccessPolicy().
equalsName(WebOutlookAccessPolicy.NOT_CONFIGURED.name())) {
if (log.isDebugEnabled()) {
log.info("CEA Outlook web access policy not configured, but support is available in " +
Collaborator

Change to debug log

Change to debug log
rajitha marked this conversation as resolved
throw new CEAEnforcementException(msg);
}
if (powershellResponse.getResponseBody() == null) return Collections.emptyList();
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
package io.entgra.device.mgt.core.cea.mgt.enforce.Impl.gateway;
import com.microsoft.aad.msal4j.*;
Collaborator

Remove wildcard imports

Remove wildcard imports
rajitha marked this conversation as resolved
import org.apache.commons.logging.LogFactory;
import java.net.MalformedURLException;
import java.util.*;
Collaborator

Remove wildcard imports

Remove wildcard imports
rajitha marked this conversation as resolved
import org.wso2.carbon.user.api.UserStoreManager;
import org.wso2.carbon.user.core.service.RealmService;
import java.util.*;
Collaborator

Remove wildcard imports

Remove wildcard imports
rajitha marked this conversation as resolved
throw new IllegalStateException(msg);
}
List<Device> devices = deviceManagementProviderService.getEnrolledDevicesSince(since);
if (devices == null) return new ArrayList<>();
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
throw new IllegalStateException(msg);
}
List<Device> devices = deviceManagementProviderService.getEnrolledDevicesPriorTo(priorTo);
if (devices == null) return new ArrayList<>();
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
import io.entgra.device.mgt.core.cea.mgt.enforce.exception.PowershellExecutionException;
import io.entgra.device.mgt.core.cea.mgt.enforce.util.shell.Powershell;
import java.io.*;
Collaborator

Remove wildcard imports

Remove wildcard imports
rajitha marked this conversation as resolved
import io.entgra.device.mgt.core.device.mgt.api.jaxrs.beans.CEAPolicyWrapper;
import io.entgra.device.mgt.core.device.mgt.api.jaxrs.beans.ErrorResponse;
import io.entgra.device.mgt.core.device.mgt.api.jaxrs.util.Constants;
import io.swagger.annotations.*;
Collaborator

Remove wildcard imports

Remove wildcard imports
rajitha marked this conversation as resolved
import io.entgra.device.mgt.core.device.mgt.api.jaxrs.util.Constants;
import io.swagger.annotations.*;
import javax.ws.rs.*;
Collaborator

Remove wildcard imports

Remove wildcard imports
rajitha marked this conversation as resolved
package io.entgra.device.mgt.core.device.mgt.api.jaxrs.service.impl.admin;
import io.entgra.device.mgt.core.cea.mgt.common.bean.*;
Collaborator

Remove wildcard imports

Remove wildcard imports
rajitha marked this conversation as resolved
package io.entgra.device.mgt.core.device.mgt.api.jaxrs.service.impl.admin;
import io.entgra.device.mgt.core.cea.mgt.common.bean.*;
import io.entgra.device.mgt.core.cea.mgt.common.bean.enums.*;
Collaborator

Remove wildcard imports

Remove wildcard imports
rajitha marked this conversation as resolved
import org.apache.commons.logging.LogFactory;
import org.apache.http.HttpStatus;
import javax.ws.rs.*;
Collaborator

Remove wildcard imports

Remove wildcard imports
rajitha marked this conversation as resolved
private List<ActiveSyncServerUIConfiguration> constructActiveSyncServerConfigurations
(List<ServerUIConfiguration> serverUIConfigurations) {
List<ActiveSyncServerUIConfiguration> activeSyncServerUIConfigurations = new ArrayList<>();
if (serverUIConfigurations == null) return activeSyncServerUIConfigurations;
Collaborator

Enclose if condition with curly braces

Enclose if condition with curly braces
rajitha marked this conversation as resolved
pahansith requested changes 11 months ago
import java.util.Date;
import java.util.List;
public interface CEAPolicyDAO {

Add doc comments to the class and method definitions

Add doc comments to the class and method definitions
rajitha marked this conversation as resolved
}
public static void beginTransaction() throws CEAPolicyManagementDAOException {
Connection connection = currentConnection.get();

What is the reason for obtaining connection here? Need to add the null check

What is the reason for obtaining connection here? Need to add the null check
rajitha marked this conversation as resolved
preparedStatement.setInt(1, tenantId);
try (ResultSet resultSet = preparedStatement.executeQuery()) {
while (resultSet.next()) {
CEAPolicyDTO ceaPolicyDTO = new CEAPolicyDTO();

Move the object reference declaration outside the loop

Move the object reference declaration outside the loop
rajitha marked this conversation as resolved
try (PreparedStatement preparedStatement = connection.prepareStatement(query)) {
try (ResultSet resultSet = preparedStatement.executeQuery()) {
while (resultSet.next()) {
CEAPolicyDTO ceaPolicyDTO = new CEAPolicyDTO();

Move the object reference declaration outside the loop

Move the object reference declaration outside the loop
rajitha marked this conversation as resolved
if (ceaPolicy == null) throw new NullPointerException("CEAPolicy can't be null");
CEAPolicyDTO ceaPolicyDTO = new CEAPolicyDTO();
CEAPolicyContent ceaPolicyContent = new CEAPolicyContent();
Gson gson = new Gson();

What if the Gson object declared as a static field of the class instead of instantiating it inside the method?

What if the Gson object declared as a static field of the class instead of instantiating it inside the method?
rajitha marked this conversation as resolved
private CEAPolicy toCEAPolicy(CEAPolicyDTO ceaPolicyDTO) {
if (ceaPolicyDTO == null) throw new NullPointerException("CEAPolicyDTO can't be null");
CEAPolicy ceaPolicy = new CEAPolicy();
Gson gson = new Gson();

What if the Gson object declared as a static field of the class instead of instantiating it inside the method?

What if the Gson object declared as a static field of the class instead of instantiating it inside the method?
rajitha marked this conversation as resolved
}
}
protected void setDataSourceService(DataSourceService dataSourceService) {

Why there's no method body for this? If this DataSourceService is not required to initiate the CEAManagementServiceComponent, better to remove the scr reference.

Why there's no method body for this? If this DataSourceService is not required to initiate the CEAManagementServiceComponent, better to remove the scr reference.
rajitha marked this conversation as resolved
ceaPolicyDAO = CEAPolicyManagementDAOFactory.getCEAPolicyDAO();
}
public static CEAManagerImpl getInstance() {

Why there's public constructor to instantiate the class and using this static method to return the instance?

Why there's public constructor to instantiate the class and using this static method to return the instance?
rajitha marked this conversation as resolved
getInstance().getCeaPolicyMonitoringTaskManager();
CEAConfigManager ceaConfigManager = CEAConfigManager.getInstance();
CEAConfiguration ceaConfiguration = ceaConfigManager.getCeaConfiguration();
if (ceaPolicyMonitoringTaskManager == null)

Move this null check to line 71 to avoid unnecessary resource allocation

Move this null check to line 71 to avoid unnecessary resource allocation
rajitha marked this conversation as resolved
CEAPolicyMonitoringTaskManager ceaPolicyMonitoringTaskManager = CEAManagementDataHolder.
getInstance().getCeaPolicyMonitoringTaskManager();
if (ceaPolicyMonitoringTaskManager == null)
throw new IllegalStateException("CEA policy monitoring task manager not initialized properly");

This error should be properly handled with a proper message since if the IllegalStateException is thrown, the deletion of the CEAPolicy will not happen.

This error should be properly handled with a proper message since if the IllegalStateException is thrown, the deletion of the CEAPolicy will not happen.
rajitha marked this conversation as resolved
}
@Override
public void enforce() throws CEAPolicyOperationException {

Better to add doc comments for the future references

Better to add doc comments for the future references
rajitha marked this conversation as resolved
powershell = Powershell.getPowershell();
}
public static ExchangeOnlineCEAEnforcementServiceImpl getInstance() throws UnsupportedOsException {

Method should be synchronized or need to do the null check again inside the synchronized block to prevent creation of two instances of the ExchangeOnlineCEAEnforcementServiceImpl object.

Method should be synchronized or need to do the null check again inside the synchronized block to prevent creation of two instances of the ExchangeOnlineCEAEnforcementServiceImpl object.
rajitha marked this conversation as resolved
List<MailboxProfile> mailboxProfiles = generateMailboxPolicies(validActiveSyncDevices,
notValidActiveSyncDevices);
for (MailboxProfile mailboxProfile : mailboxProfiles) {
PowershellCommand powershellCommand = getCommand(Parser.COMMAND_SetCASMailbox.COMMAND, activeSyncServer);

Declare the object reference out of the loop

Declare the object reference out of the loop
rajitha marked this conversation as resolved
List<ActiveSyncDevice> activeSyncBlockedDevices) {
List<MailboxProfile> mailboxProfiles = new ArrayList<>();
for (ActiveSyncDevice activeSyncDevice : activeSyncAllowedDevices) {
MailboxProfile mailboxProfile = new MailboxProfile();

Declare the object reference out of the loop

Declare the object reference out of the loop
rajitha marked this conversation as resolved
}
for (ActiveSyncDevice activeSyncDevice : activeSyncBlockedDevices) {
MailboxProfile mailboxProfile = new MailboxProfile();

Declare the object reference out of the loop

Declare the object reference out of the loop
rajitha marked this conversation as resolved
private List<ActiveSyncDevice> constructActiveSyncDeviceList(PowershellResponse powershellResponse)
throws CEAEnforcementException {
if (powershellResponse == null) {
String msg = "Powershell response can't be null";

Not required to use additional variable for the message

Not required to use additional variable for the message
rajitha marked this conversation as resolved
}
if (!powershellResponse.isSuccess()) {
String msg = "Powershell request failed while getting active sync devices";

Not required to use additional variable for the message

Not required to use additional variable for the message
rajitha marked this conversation as resolved
if (powershellResponse.getResponseBody() == null) return Collections.emptyList();
if (!powershellResponse.getResponseBody().isJsonArray()) {
String msg = "Unexpected result retrieve when getting active sync devices";

Not required to use additional variable for the message

Not required to use additional variable for the message
rajitha marked this conversation as resolved
for (JsonElement element : elements) {
try {
JsonObject deviceJsonObject = element.getAsJsonObject();
ActiveSyncDevice activeSyncDevice = new ActiveSyncDevice();

Declare the both object references out of the loop

Declare the both object references out of the loop
rajitha marked this conversation as resolved
public interface Powershell {
String OS = System.getProperty("os.name").toLowerCase();
boolean IS_UNIX = (OS.indexOf("nix") >= 0 || OS.indexOf("nux") >= 0 || OS.indexOf("aix") > 0);

Since these IS_UNIX and IS_WINDOWS variables are not used in any subclasses, can't we declare them inside the getPowershell method?

Since these IS_UNIX and IS_WINDOWS variables are not used in any subclasses, can't we declare them inside the getPowershell method?
Poster
Collaborator

Yes we can, but through out the application run time OS remain same, so we can improve the performance by putting OS checking in static fields. Bcz static will ensure to run the OS checking once.

Yes we can, but through out the application run time OS remain same, so we can improve the performance by putting OS checking in static fields. Bcz static will ensure to run the OS checking once.
rajitha marked this conversation as resolved
}
public static CEAManagementService getCEAManagementService() {
PrivilegedCarbonContext ctx = PrivilegedCarbonContext.getThreadLocalCarbonContext();

Use a data holder to store the CEAManagementService and return it when invoking getCEAManagementService to prevent resolving the service each time via the OSGi context.

Use a data holder to store the CEAManagementService and return it when invoking getCEAManagementService to prevent resolving the service each time via the OSGi context.
rajitha marked this conversation as resolved
rajitha force-pushed conditional-access from 5f7d3712cc to 95d3080a7a 11 months ago
rajitha force-pushed conditional-access from 95d3080a7a to 8eb6d6365d 10 months ago
rajitha force-pushed conditional-access from 3b7cbc88cc to 064d953288 10 months ago
tcdlpds requested changes 10 months ago
/*
* Copyright (c) 2018 - 2023, Entgra (Pvt) Ltd. (http://www.entgra.io) All Rights Reserved.
Owner

Correct the licence year.

Correct the licence year.
rajitha marked this conversation as resolved
int tenantId = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantId();
ceaPolicy.setTenantId(tenantId);
CEAPolicyDTO ceaPolicyDTO = toCEAPolicyDTO(ceaPolicy);
String query = "INSERT INTO DM_CEA_POLICIES (POLICY_CONTENT, CREATED_TIMESTAMP, UPDATED_TIMESTAMP, TENANT_ID) " +
Owner

Try enhancing the readability as follows while defining the query format.

    String query = "INSERT INTO DM_CEA_POLICIES "
             + "(POLICY_CONTENT, "
             + "CREATED_TIMESTAMP, "
             + "UPDATED_TIMESTAMP, "
             + "TENANT_ID) VALUES (?, ?, ?, ?)";

You will see the importance of following this format when you defining a complex query, Hence practice this from the begining.

Try enhancing the readability as follows while defining the query format. String query = "INSERT INTO DM_CEA_POLICIES " + "(POLICY_CONTENT, " + "CREATED_TIMESTAMP, " + "UPDATED_TIMESTAMP, " + "TENANT_ID) VALUES (?, ?, ?, ?)"; You will see the importance of following this format when you defining a complex query, Hence practice this from the begining.
rajitha marked this conversation as resolved
tcdlpds requested changes 10 months ago
private CEAPolicy toCEAPolicy(CEAPolicyDTO ceaPolicyDTO) {
if (ceaPolicyDTO == null) {
throw new NullPointerException("CEAPolicyDTO can't be null");
Owner

Since NullPointerExceptions in Java are typically thrown as programming errors rather than recoverable exceptions, it is widely seen as bad practice to throw them. Any attempt by an application to use a null object reference results in the NullPointerException being thrown. This typically implies that the object is not properly initialized since the code is attempting to access a null field or call a function on an object that is not initialized.

Checking for null values and managing them appropriately is generally a preferable way to design your code to prevent null pointer exceptions. To do this, you may need to throw a more focused exception that explains the mistake and its possible solutions, use the null object pattern, or return a default value.

Since NullPointerExceptions in Java are typically thrown as programming errors rather than recoverable exceptions, it is widely seen as bad practice to throw them. Any attempt by an application to use a null object reference results in the NullPointerException being thrown. This typically implies that the object is not properly initialized since the code is attempting to access a null field or call a function on an object that is not initialized. Checking for null values and managing them appropriately is generally a preferable way to design your code to prevent null pointer exceptions. To do this, you may need to throw a more focused exception that explains the mistake and its possible solutions, use the null object pattern, or return a default value.
rajitha marked this conversation as resolved
tcdlpds requested changes 10 months ago
"hence aborting CEA policy deleting procedure";
throw new IllegalStateException(msg);
}
CEAPolicyManagementDAOFactory.openConnection();
Owner

This should be the first statement of the try block. Because if error throws in upper code block then finally block will be executed without opening a connection. Hence Handle upper part of the code in separate try block and the connection opening statement should be the first statement of this try block.

This should be the first statement of the try block. Because if error throws in upper code block then finally block will be executed without opening a connection. Hence Handle upper part of the code in separate try block and the connection opening statement should be the first statement of this try block.
rajitha marked this conversation as resolved
}
} catch (CEAManagementException e) {
String msg = "Error occurred while executing dynamic partitioned task for the CEA policy monitoring";
Owner

It is not required to define msg variable here.

It is not required to define msg variable here.
rajitha marked this conversation as resolved
String msg = "Error occurred while executing dynamic partitioned task for the CEA policy monitoring";
log.error(msg, e);
} catch (CEAConfigManagerException e) {
String msg = "Error occurred while retrieving CEA configuration";
Owner

It is not required to define msg variable here.

It is not required to define msg variable here.
rajitha marked this conversation as resolved
ceaPolicyOperation.enforce();
status = true;
} catch (EnforcementServiceManagerException | CEAPolicyOperationException e) {
String msg = "Error occurred while enforcing the CEA access policy for the tenant id" + ceaPolicy.getTenantId();
Owner

It is not required to define msg variable here.

It is not required to define msg variable here.
rajitha marked this conversation as resolved
try {
ceaManager.updateSyncStatus(status, syncedStartTime);
} catch (CEAManagementException e) {
String msg = "Error occurred while recording sync status";
Owner

It is not required to define msg variable here.

It is not required to define msg variable here.
rajitha marked this conversation as resolved
rajitha force-pushed conditional-access from a3f60cf3c0 to 2ad8f54a46 10 months ago
tcdlpds requested changes 10 months ago
}
@Enforce
public void enforceConditionalAccessPolicy(CEAPolicy ceaPolicy) throws CEAEnforcementException {
Owner

It seems in this method have repeating logic that can be moved to private method and improve the readability of the code.

It seems in this method have repeating logic that can be moved to private method and improve the readability of the code.
rajitha marked this conversation as resolved
}
}
private PowershellCommand getCommand(String command, ActiveSyncServer activeSyncServer)
Owner

Add Java Doc comment

Add Java Doc comment
rajitha marked this conversation as resolved
return commandBuilder.build();
}
private PowershellCommand toAllMailboxesCommand(PowershellCommand command,
Owner

Add Java Doc comment

Add Java Doc comment
rajitha marked this conversation as resolved
return getEXOMailbox;
}
private PowershellRequest getPowershellRequest(PowershellCommand command) {
Owner

Add Java Doc comment

Add Java Doc comment
rajitha marked this conversation as resolved
return powershellRequest;
}
private List<MailboxProfile> generateMailboxPolicies(List<ActiveSyncDevice> activeSyncAllowedDevices,
Owner

Add Java Doc comment

Add Java Doc comment
rajitha marked this conversation as resolved
return mailboxProfiles;
}
private List<ActiveSyncDevice> constructActiveSyncDeviceList(PowershellResponse powershellResponse)
Owner

Add Java Doc comment

Add Java Doc comment
rajitha marked this conversation as resolved
return activeSyncDevices;
}
private List<ActiveSyncDevice> getConnectedActiveSyncDevicesAfter(Date after, ActiveSyncServer activeSyncServer)
Owner

Add Java Doc comment

Add Java Doc comment
rajitha marked this conversation as resolved
return constructActiveSyncDeviceList(powershellResponse);
}
private List<ActiveSyncDevice> getConnectedActiveSyncDevicesBefore(Date before, ActiveSyncServer activeSyncServer)
Owner

Add Java Doc comment

Add Java Doc comment
rajitha marked this conversation as resolved
return constructActiveSyncDeviceList(powershellResponse);
}
private List<ActiveSyncDevice> getAllConnectedActiveSyncDevices(ActiveSyncServer activeSyncServer)
Owner

Add Java Doc comment

Add Java Doc comment
rajitha marked this conversation as resolved
}
}
private IConfidentialClientApplication getOrCreateConfidentialClientApplication(String clientId, String secret, String authority)
Owner

Add Java Doc comment

Add Java Doc comment
rajitha marked this conversation as resolved
public class DeviceMgtUtil {
private static final Log log = LogFactory.getLog(DeviceMgtUtil.class);
public static List<ActiveSyncDevice> getEnrolledActiveSyncDevicesSince(Date since)
Owner

You can add another parameter and have a single method to handle 'getEnrolledActiveSyncDevicesSince' and 'getEnrolledActiveSyncDevicesPriorTo' methods. With that approach we can reduce the repeating code lines.

You can add another parameter and have a single method to handle 'getEnrolledActiveSyncDevicesSince' and 'getEnrolledActiveSyncDevicesPriorTo' methods. With that approach we can reduce the repeating code lines.
rajitha marked this conversation as resolved
return DeviceMgtUtil.constructActiveSyncDeviceList(devices);
}
private static DeviceManagementProviderService getDeviceManagementProviderService() {
Owner

It is not necessary to have a private method for this single line.

It is not necessary to have a private method for this single line.
rajitha marked this conversation as resolved
return realmService.getTenantUserRealm(tenantId).getUserStoreManager();
}
private static String getIdentity(String owner, UserStoreManager userStoreManager)
Owner

Is it required to have this method only for this line?

Is it required to have this method only for this line?
rajitha marked this conversation as resolved
return userStoreManager.getUserClaimValue(owner, Constants.EMAIL_CLAIM_URI, null);
}
private static List<ActiveSyncDevice> constructActiveSyncDeviceList(List<Device> devices)
Owner

Add Java Doc comment.

Add Java Doc comment.
rajitha marked this conversation as resolved
return activeSyncDevices;
}
public static ActiveSyncDevice mapToActiveSyncDevice(Device device, UserStoreManager userStoreManager)
Owner

Add Java Doc comment.

Add Java Doc comment.
rajitha marked this conversation as resolved
return constructResponse(exitCode, getStringContent(standardOutputStringWriter),
getStringContent(errorStringWriter));
} catch (IOException e) {
String msg = "IOException occurred while executing powershell command : "
Owner

Log the msg and error.

Log the msg and error.
rajitha marked this conversation as resolved
+ powershellRequest.getCommand();
throw new PowershellExecutionException(msg, e);
} catch (InterruptedException e) {
String msg = "Thread got interrupted while executing powershell command : "
Owner

Log the msg and error.

Log the msg and error.
rajitha marked this conversation as resolved
) CEAPolicyWrapper ceaPolicyWrapper);
@GET
@Path("/syncNow")
Owner

Since this is API context path it is not needed to have camelcase definition. Instead of it, you can have 'sync-now'. Further we uses nouns in the context paths. Do the adjustment for the context path.

Since this is API context path it is not needed to have camelcase definition. Instead of it, you can have 'sync-now'. Further we uses nouns in the context paths. Do the adjustment for the context path.
rajitha marked this conversation as resolved
/*
* Copyright (c) 2018 - 2023, Entgra (Pvt) Ltd. (http://www.entgra.io) All Rights Reserved.
Owner

Change the Year

Change the Year
rajitha marked this conversation as resolved
}
@GET
@Path("/syncNow")
Owner

Need to modify this place as well. Further do the required modification in the UI level as well.

Need to modify this place as well. Further do the required modification in the UI level as well.
rajitha marked this conversation as resolved
}
}
private CEAPolicy constructCEAPolicy(CEAPolicyWrapper ceaPolicyWrapper) {
Owner

Add Java Doc comment.

Add Java Doc comment.
rajitha marked this conversation as resolved
return ceaPolicy;
}
private List<ActiveSyncServerUIConfiguration> constructActiveSyncServerConfigurations
Owner

Add Java Doc comment.

Add Java Doc comment.
rajitha marked this conversation as resolved
if (serverUIConfigurations == null) {
return activeSyncServerUIConfigurations;
}
for (ServerUIConfiguration serverUIConfiguration : serverUIConfigurations) {
Owner

Put following part into outside of the loop.

ActiveSyncServerUIConfiguration activeSyncServerUIConfiguration

Put following part into outside of the loop. ActiveSyncServerUIConfiguration activeSyncServerUIConfiguration
rajitha marked this conversation as resolved
validateCEAGracePeriod(ceaPolicyWrapper.getGracePeriodEntries());
}
public static void validateActiveSyncServer(ActiveSyncServer activeSyncServer) {
Owner

Add Java Doc comment

Add Java Doc comment
rajitha marked this conversation as resolved
}
}
public static void validateCEAAccessPolicy(AccessPolicyWrapper accessPolicyWrapper) {
Owner

Add Java Doc comment

Add Java Doc comment
rajitha marked this conversation as resolved
}
}
public static void validateCEAGracePeriod(GracePeriodWrapper gracePeriodWrapper) {
Owner

Add Java Doc comment

Add Java Doc comment
rajitha marked this conversation as resolved
rajitha added 1 commit 10 months ago
tcdlpds merged commit 9b4c5e6adb into master 10 months ago

Reviewers

navodzoysa requested changes 11 months ago
pahansith requested changes 11 months ago
tcdlpds requested changes 10 months ago
The pull request has been merged as 9b4c5e6adb.
Sign in to join this conversation.
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: community/device-mgt-core#294
Loading…
There is no content yet.