Call "Optional#isPresent()" before accessing the value in PortalClient

- Fixed unchecked instances of Optional
- Added a comment describing my understanding of the intent of
the areUserRolesChanged method

Issue-ID: SDC-3101
Signed-off-by: Neil Derraugh <neil.derraugh@yoppworks.com>
Change-Id: I7328fc291df62ad9a4789a5640fc3cb46418bdd5
Signed-off-by: sebdet <sebastien.determe@intl.att.com>

3 files changed, 80 insertions, 63 deletions

diff --git a/security-util-lib/src/main/java/org/onap/sdc/security/PortalClient.java b/security-util-lib/src/main/java/org/onap/sdc/security/PortalClient.java
index bba8da9..1a7775a 100644
--- a/security-util-lib/src/main/java/org/onap/sdc/security/PortalClient.java
+++ b/security-util-lib/src/main/java/org/onap/sdc/security/PortalClient.java
@@ -20,28 +20,31 @@
 
 package org.onap.sdc.security;
 
-import org.json.simple.JSONObject;
-import org.json.simple.JSONValue;
+import static org.onap.portalsdk.core.onboarding.util.CipherUtil.decryptPKC;
+import static org.onap.sdc.security.utils.SecurityLogsUtils.PORTAL_TARGET_ENTITY;
+import static org.onap.sdc.security.utils.SecurityLogsUtils.fullOptionalData;
+
 import com.google.common.annotations.VisibleForTesting;
+import java.io.IOException;
+import java.security.InvalidParameterException;
+import java.util.Base64;
+import java.util.Optional;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.util.EntityUtils;
+import org.json.simple.JSONObject;
+import org.json.simple.JSONValue;
 import org.onap.portalsdk.core.onboarding.exception.PortalAPIException;
 import org.onap.portalsdk.core.onboarding.util.PortalApiProperties;
+import org.onap.portalsdk.core.restful.domain.EcompRole;
 import org.onap.portalsdk.core.restful.domain.EcompUser;
 import org.onap.sdc.security.logging.elements.LogFieldsMdcHandler;
 import org.onap.sdc.security.logging.enums.EcompLoggerErrorCode;
 import org.onap.sdc.security.logging.wrappers.Logger;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
-import java.io.IOException;
-import java.security.InvalidParameterException;
-import java.util.Base64;
-import static org.onap.portalsdk.core.onboarding.util.CipherUtil.decryptPKC;
-import static org.onap.sdc.security.utils.SecurityLogsUtils.PORTAL_TARGET_ENTITY;
-import static org.onap.sdc.security.utils.SecurityLogsUtils.fullOptionalData;
 
 
 @Component/*("portalUtils")*/
@@ -78,30 +81,29 @@ public class PortalClient {
         this.httpClient = httpClient;
     }
 
-    public String fetchUserRolesFromPortal(String userId) {
-        String fetchedUserRoleFromPortal;
+    public Optional<String> fetchUserRolesFromPortal(String userId) {
         try {
             EcompUser ecompUser = extractObjectFromResponseJson(getResponseFromPortal(userId));
             log.debug("GET USER ROLES response for user {}: {}", userId, ecompUser);
             checkIfSingleRoleProvided(ecompUser);
-            fetchedUserRoleFromPortal = ecompUser.getRoles().stream().findFirst().get().getName();
+            Optional<EcompRole> firstRole = ecompUser.getRoles().stream().findFirst();
+            if (firstRole.isPresent()) {
+                return Optional.of(firstRole.get().getName());
+            }
         } catch (IOException | PortalAPIException e) {
-            log.error(EcompLoggerErrorCode.BUSINESS_PROCESS_ERROR, LogFieldsMdcHandler.getInstance().getServiceName(),
-                    fullOptionalData(PORTAL_TARGET_ENTITY, "/fetchRolesFromPortal"),"GET USER ROLES from portal failed: {}", e.getMessage());
             log.debug("Fetching user roles from Portal failed", e);
             throw new RestrictionAccessFilterException(e);
         }
-        return fetchedUserRoleFromPortal;
+        return Optional.empty();
     }
 
 
-
     public static void checkIfSingleRoleProvided(EcompUser user) throws PortalAPIException {
-        if(user.getRoles() == null) {
+        if (user.getRoles() == null) {
             log.debug(RECEIVED_NULL_ROLES, user);
             //BeEcompErrorManager.getInstance().logInvalidInputError(CHECK_ROLES, RECEIVED_NULL_ROLES, BeEcompErrorManager.ErrorSeverity.ERROR);
             throw new PortalAPIException(RECEIVED_NULL_ROLES + user);
-        }else if(user.getRoles().size() > 1) {
+        } else if (user.getRoles().size() > 1) {
             log.debug(RECEIVED_MULTIPLE_ROLES, user);
             //BeEcompErrorManager.getInstance().logInvalidInputError(CHECK_ROLES, RECEIVED_MULTIPLE_ROLES2, BeEcompErrorManager.ErrorSeverity.ERROR);
             throw new PortalAPIException(RECEIVED_MULTIPLE_ROLES2 + user);
@@ -127,9 +129,9 @@ public class PortalClient {
 
         HttpGet httpGet = new HttpGet(url);
         String encodedBasicAuthCred = Base64.getEncoder()
-                .encodeToString((portalConfiguration.getDecryptedPortalUser() + ":" +
-                        portalConfiguration.getDecryptedPortalPassword())
-                        .getBytes());
+            .encodeToString((portalConfiguration.getDecryptedPortalUser() + ":" +
+                portalConfiguration.getDecryptedPortalPassword())
+                .getBytes());
         httpGet.setHeader(UEB_KEY, portalConfiguration.getUebKey());
         httpGet.setHeader(AUTHORIZATION, "Basic " + encodedBasicAuthCred);
         httpGet.setHeader(CONTENT_TYPE_HEADER, "application/json");
@@ -137,6 +139,7 @@ public class PortalClient {
     }
 
     private static class PortalConfiguration {
+
         private static final String PROPERTY_NOT_SET = "%s property value is not set in portal.properties file";
         private String decryptedPortalUser;
         private String decryptedPortalPassword;
@@ -153,7 +156,8 @@ public class PortalClient {
 
         }
 
-        private PortalConfiguration(IPortalConfiguration portalConfiguration) throws org.onap.portalsdk.core.onboarding.exception.CipherUtilException {
+        private PortalConfiguration(IPortalConfiguration portalConfiguration)
+            throws org.onap.portalsdk.core.onboarding.exception.CipherUtilException {
             this.decryptedPortalUser = decryptPKC(portalConfiguration.getPortalUser());
             this.decryptedPortalPassword = decryptPKC(portalConfiguration.getPortalPass());
             this.decryptedPortalAppName = decryptPKC(portalConfiguration.getPortalAppName());
diff --git a/security-util-lib/src/main/java/org/onap/sdc/security/filters/RestrictionAccessFilter.java b/security-util-lib/src/main/java/org/onap/sdc/security/filters/RestrictionAccessFilter.java
index 1b53882..812537d 100644
--- a/security-util-lib/src/main/java/org/onap/sdc/security/filters/RestrictionAccessFilter.java
+++ b/security-util-lib/src/main/java/org/onap/sdc/security/filters/RestrictionAccessFilter.java
@@ -20,32 +20,40 @@
 
 package org.onap.sdc.security.filters;
 
-import com.google.common.annotations.VisibleForTesting;
-import org.apache.commons.collections.CollectionUtils;
-import org.apache.commons.lang3.ArrayUtils;
-import org.apache.commons.lang3.StringUtils;
-import org.onap.sdc.security.*;
-import org.onap.sdc.security.logging.wrappers.*;
-import org.onap.sdc.security.logging.elements.*;
-import org.onap.sdc.security.logging.enums.*;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.stereotype.Component;
+import static org.onap.sdc.security.utils.SecurityLogsUtils.PORTAL_TARGET_ENTITY;
+import static org.onap.sdc.security.utils.SecurityLogsUtils.fullOptionalData;
 
-import javax.servlet.FilterChain;
-import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
-import javax.servlet.http.Cookie;
-import javax.servlet.http.HttpServletResponse;
+import com.google.common.annotations.VisibleForTesting;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
-
-import static org.onap.sdc.security.utils.SecurityLogsUtils.PORTAL_TARGET_ENTITY;
-import static org.onap.sdc.security.utils.SecurityLogsUtils.fullOptionalData;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletResponse;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.onap.sdc.security.AuthenticationCookie;
+import org.onap.sdc.security.CipherUtil;
+import org.onap.sdc.security.CipherUtilException;
+import org.onap.sdc.security.ISessionValidationFilterConfiguration;
+import org.onap.sdc.security.IUsersThreadLocalHolder;
+import org.onap.sdc.security.PortalClient;
+import org.onap.sdc.security.RedirectException;
+import org.onap.sdc.security.RepresentationUtils;
+import org.onap.sdc.security.RestrictionAccessFilterException;
+import org.onap.sdc.security.logging.elements.LogFieldsMdcHandler;
+import org.onap.sdc.security.logging.enums.EcompLoggerErrorCode;
+import org.onap.sdc.security.logging.wrappers.Logger;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
 
 @Component("restrictionAccessFilter")
 public class RestrictionAccessFilter extends SessionValidationFilter {
@@ -63,7 +71,7 @@ public class RestrictionAccessFilter extends SessionValidationFilter {
 
     @Autowired
     public RestrictionAccessFilter(ISessionValidationFilterConfiguration configuration,
-                                   IUsersThreadLocalHolder threadLocalUtils,PortalClient portalClient) {
+        IUsersThreadLocalHolder threadLocalUtils, PortalClient portalClient) {
         this.filterConfiguration = configuration;
         this.threadLocalUtils = threadLocalUtils;
         this.portalClient = portalClient;
@@ -72,8 +80,8 @@ public class RestrictionAccessFilter extends SessionValidationFilter {
 
     @Override
     public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain)
-            throws IOException, ServletException {
-        super.doFilter(servletRequest, servletResponse,filterChain);
+        throws IOException, ServletException {
+        super.doFilter(servletRequest, servletResponse, filterChain);
     }
 
     @Override
@@ -85,23 +93,23 @@ public class RestrictionAccessFilter extends SessionValidationFilter {
             authenticationCookie = getAuthenticationCookie(cookie);
             if (!CollectionUtils.isEmpty(authenticationCookie.getRoles())) {
                 log.debug("Cookie already contains a role in its set of roles authenticationCookie Roles: {}",
-                        authenticationCookie.getRoles());
+                    authenticationCookie.getRoles());
                 threadLocalUtils.setUserContext(authenticationCookie);
                 return cookie;
             }
-            String fetchedRole = portalClient.fetchUserRolesFromPortal(authenticationCookie.getUserID());
+            Optional<String> fetchedRole = portalClient.fetchUserRolesFromPortal(authenticationCookie.getUserID());
             log.debug("addRoleToCookie: Finished fetching user role from Portal. Adding it to the cookie");
-            updatedRolesSet.add(fetchedRole);
+            if (fetchedRole.isPresent()) {
+                updatedRolesSet.add(fetchedRole.get());
+            }
             authenticationCookie.setRoles(updatedRolesSet);
             String changedCookieJson = RepresentationUtils.toRepresentation(authenticationCookie);
             log.debug("addRoleToCookie: Changed cookie Json: {}", changedCookieJson);
             cookie.setValue(CipherUtil.encryptPKC(changedCookieJson, getFilterConfiguration().getSecurityKey()));
             threadLocalUtils.setUserContext(authenticationCookie);
         } catch (IOException e) {
-            log.error(EcompLoggerErrorCode.BUSINESS_PROCESS_ERROR, LogFieldsMdcHandler.getInstance().getServiceName(), fullOptionalData(PORTAL_TARGET_ENTITY, "/fetchRolesFromPortal"),"Exception: {}", e.getMessage());
             throw new RestrictionAccessFilterException(e);
         } catch (CipherUtilException e) {
-            log.error(EcompLoggerErrorCode.BUSINESS_PROCESS_ERROR, LogFieldsMdcHandler.getInstance().getServiceName(), fullOptionalData(PORTAL_TARGET_ENTITY, "/fetchRolesFromPortal"),"Exception: {}", e.getMessage());
             throw new RedirectException(e);
         }
         return cookie;
@@ -116,14 +124,14 @@ public class RestrictionAccessFilter extends SessionValidationFilter {
 
     @Override
     public void authorizeUserOnSessionExpiration(AuthenticationCookie authenticationCookie, Cookie[] cookies)
-            throws RedirectException {
-            log.debug("Portal fetch user role is enabled");
-            if (!isAuthenticatedUserSimilarToCspUser(authenticationCookie, cookies) ||
-                    areUserRolesChanged(authenticationCookie)) {
-                String msg = String.format(SESSION_IS_EXPIRED_MSG, authenticationCookie.getUserID());
-                log.debug(msg);
-                throw new RedirectException(msg);
-            }
+        throws RedirectException {
+        log.debug("Portal fetch user role is enabled");
+        if (!isAuthenticatedUserSimilarToCspUser(authenticationCookie, cookies) ||
+            areUserRolesChanged(authenticationCookie)) {
+            String msg = String.format(SESSION_IS_EXPIRED_MSG, authenticationCookie.getUserID());
+            log.debug(msg);
+            throw new RedirectException(msg);
+        }
     }
 
     @Override
@@ -134,7 +142,7 @@ public class RestrictionAccessFilter extends SessionValidationFilter {
         httpServletResponse.setContentType("application/json");
         httpServletResponse.setCharacterEncoding("UTF-8");
         httpServletResponse.getWriter().write(RepresentationUtils.toRepresentation
-                ("Your session has expired. Please close the SDC tab and re-enter the SDC application."));
+            ("Your session has expired. Please close the SDC tab and re-enter the SDC application."));
     }
 
     private boolean areUserRolesChanged(AuthenticationCookie authenticationCookie) {
@@ -146,12 +154,17 @@ public class RestrictionAccessFilter extends SessionValidationFilter {
         // TODO: For future reference, when multi roles exist replace to something like:
         // TODO: authenticationCookie.getRoles().stream().forEach((role) -> areRolesEqual = areRolesEqual(user.getRole(), role));
         log.debug("Fetching roles from portal for user {}", authenticationCookie.getUserID());
-        String portalRole = portalClient.fetchUserRolesFromPortal(authenticationCookie.getUserID());
+        Optional<String> portalRole = portalClient.fetchUserRolesFromPortal(authenticationCookie.getUserID());
         log.debug("{} user role on portal is {}, in the cookie is {}", portalRole, cookieRole);
-        return StringUtils.isEmpty(cookieRole) || !cookieRole.equalsIgnoreCase(portalRole);
+        boolean isCookieRoleEmpty = StringUtils.isEmpty(cookieRole);
+        //If the cookieRole is empty or if portalRole is different than cookieRole or if cookieRole is not empty and
+        //portalRole is not empty then return true otherwise false.
+        return isCookieRoleEmpty ||
+            (portalRole.isPresent() ? !cookieRole.equalsIgnoreCase(portalRole.get()) : !isCookieRoleEmpty);
     }
 
-    private boolean isAuthenticatedUserSimilarToCspUser(AuthenticationCookie cookie, Cookie[] cookies) throws RedirectException {
+    private boolean isAuthenticatedUserSimilarToCspUser(AuthenticationCookie cookie, Cookie[] cookies)
+        throws RedirectException {
         String cspUserId = getCookieValue(cookies, CSP_USER_ID);
         if (cspUserId != null && cspUserId.equals(cookie.getUserID())) {
             log.debug("Auth and CSP user IDs are same: {}", cookie.getUserID());
@@ -164,8 +177,8 @@ public class RestrictionAccessFilter extends SessionValidationFilter {
     public String getCookieValue(Cookie[] cookies, String name) throws RedirectException {
         if (ArrayUtils.isNotEmpty(cookies)) {
             List<Cookie> foundCookies = Arrays.stream(cookies)
-                    .filter(c -> c.getName().endsWith(name))
-                    .collect(Collectors.toList());
+                .filter(c -> c.getName().endsWith(name))
+                .collect(Collectors.toList());
             if (foundCookies.size() > 0) {
                 log.debug("getCookieValue: Found {} cookies with name: {}", foundCookies.size(), name);
                 return foundCookies.get(0).getValue();
diff --git a/security-util-lib/src/main/java/org/onap/sdc/security/filters/SessionValidationFilter.java b/security-util-lib/src/main/java/org/onap/sdc/security/filters/SessionValidationFilter.java
index 87780ae..e11223e 100644
--- a/security-util-lib/src/main/java/org/onap/sdc/security/filters/SessionValidationFilter.java
+++ b/security-util-lib/src/main/java/org/onap/sdc/security/filters/SessionValidationFilter.java
@@ -92,7 +92,7 @@ public abstract class SessionValidationFilter implements Filter {
                 filterChain.doFilter(servletRequest, httpResponse);
             }
         } catch (RedirectException e) {
-            log.error(EcompLoggerErrorCode.BUSINESS_PROCESS_ERROR, LogFieldsMdcHandler.getInstance().getServiceName(), new ErrorLogOptionalData(),"Exception is thrown while authenticating cookie: {}", e.getMessage());
+            log.error(EcompLoggerErrorCode.BUSINESS_PROCESS_ERROR, LogFieldsMdcHandler.getInstance().getServiceName(), new ErrorLogOptionalData(),"Exception is thrown while authenticating cookie: {}", e);
             handleRedirectException(httpResponse);
         }
         long durationSec = TimeUnit.NANOSECONDS.toSeconds(System.nanoTime() - starTime);