From f58c9f0fac89fbdd97dac3291d73e6698b95557d Mon Sep 17 00:00:00 2001 From: Neil Derraugh Date: Thu, 4 Jun 2020 20:09:56 -0400 Subject: 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 Change-Id: I7328fc291df62ad9a4789a5640fc3cb46418bdd5 Signed-off-by: sebdet --- .../java/org/onap/sdc/security/PortalClient.java | 46 ++++++----- .../security/filters/RestrictionAccessFilter.java | 95 ++++++++++++---------- .../security/filters/SessionValidationFilter.java | 2 +- 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 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 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 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 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 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); -- cgit 1.2.3-korg