From 00cc27b0fa1346435c7685300286f1adc98b69fb Mon Sep 17 00:00:00 2001 From: efiacor Date: Wed, 17 Jul 2019 12:44:17 +0000 Subject: Authz unit test and code cleanup Change-Id: I5c6a099ca4e6479df37505aa6bc645d8a26a42e1 Issue-ID: DMAAP-1226 Signed-off-by: efiacor --- .../dmaap/datarouter/authz/impl/AuthRespImpl.java | 26 ++++--- .../authz/impl/AuthRespSupplementImpl.java | 8 +- .../dmaap/datarouter/authz/impl/AuthzResource.java | 11 ++- .../datarouter/authz/impl/ProvAuthorizer.java | 88 +++++++++------------- 4 files changed, 59 insertions(+), 74 deletions(-) (limited to 'datarouter-prov/src/main/java/org') diff --git a/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/AuthRespImpl.java b/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/AuthRespImpl.java index f3278332..c7d71996 100644 --- a/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/AuthRespImpl.java +++ b/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/AuthRespImpl.java @@ -44,21 +44,23 @@ public class AuthRespImpl implements AuthorizationResponse { /** Constructor. This version will not be used in Data Router R1 since we will not have advice and obligations. * * @param authorized flag indicating whether the response carried a permit response (true) - * or something else (false). + * or something else (false). * @param advice list of advice elements returned in the response. * @param obligations list of obligation elements returned in the response. */ - public AuthRespImpl(boolean authorized, List advice, List obligations) { + private AuthRespImpl(boolean authorized, List advice, + List obligations) { this.authorized = authorized; - this.advice = (advice == null ? null : new ArrayList (advice)); - this.obligations = (obligations == null ? null : new ArrayList (obligations)); + this.advice = (advice == null ? null : new ArrayList<>(advice)); + this.obligations = (obligations == null ? null : new ArrayList<>(obligations)); } /** Constructor. Simple version for authorization responses that have no advice and no obligations. * - * @param authorized flag indicating whether the response carried a permit (true) or something else (false). + * @param authorized flag indicating whether the response carried a permit (true) + * or something else (false). */ - public AuthRespImpl(boolean authorized) { + AuthRespImpl(boolean authorized) { this(authorized, null, null); } @@ -69,25 +71,25 @@ public class AuthRespImpl implements AuthorizationResponse { */ @Override public boolean isAuthorized() { - return authorized; + return authorized; } /** * Returns any advice elements that were included in the authorization response. * - * @return A list of objects implementing the AuthorizationResponseSupplement interface, with each object representing an - * advice element from the authorization response. + * @return A list of objects implementing the AuthorizationResponseSupplement interface, + * with each object representing an advice element from the authorization response. */ @Override public List getAdvice() { - return advice; + return advice; } /** * Returns any obligation elements that were included in the authorization response. * - * @return A list of objects implementing the AuthorizationResponseSupplement interface, with each object representing an - * obligation element from the authorization response. + * @return A list of objects implementing the AuthorizationResponseSupplement interface, + * with each object representing an obligation element from the authorization response. */ @Override public List getObligations() { diff --git a/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/AuthRespSupplementImpl.java b/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/AuthRespSupplementImpl.java index d995270e..b61c00e5 100644 --- a/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/AuthRespSupplementImpl.java +++ b/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/AuthRespSupplementImpl.java @@ -36,17 +36,17 @@ import org.onap.dmaap.datarouter.authz.AuthorizationResponseSupplement; */ public class AuthRespSupplementImpl implements AuthorizationResponseSupplement { - private String id = null; - private Map attributes = null; + private String id; + private Map attributes; /** Constructor, available within the package. * * @param id The identifier for the advice or obligation element * @param attributes The attributes (name-value pairs) for the advice or obligation element. */ - AuthRespSupplementImpl (String id, Map attributes) { + AuthRespSupplementImpl(String id, Map attributes) { this.id = id; - this.attributes = new HashMap(attributes); + this.attributes = new HashMap<>(attributes); } /** Return the identifier for the supplementary information element. diff --git a/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/AuthzResource.java b/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/AuthzResource.java index 0357fa74..c248468f 100644 --- a/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/AuthzResource.java +++ b/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/AuthzResource.java @@ -30,7 +30,6 @@ import java.util.regex.Pattern; /** Internal representation of an authorization resource (the entity to which access is being requested). Consists * of a type and an identifier. The constructor takes the request URI from an HTTP request and checks it against * patterns for the the different resource types. In DR R1, there are four resource types: - *
    *
  • the feeds collection resource, the target of POST requests to create a new feed and GET requests to list * the existing feeds. This is the root resource for the DR provisioning system, and it has no explicit id. *
    • @@ -53,10 +52,10 @@ public class AuthzResource { private String id = ""; /* Construct an AuthzResource by matching a request URI against the various patterns */ - public AuthzResource(String rURI) { - if (rURI != null) { + AuthzResource(String requestUri) { + if (requestUri != null) { for (ResourceType t : ResourceType.values()) { - Matcher m = t.getPattern().matcher(rURI); + Matcher m = t.getPattern().matcher(requestUri); if (m.find(0)) { this.type = t; if (m.group("id") != null) { @@ -83,13 +82,13 @@ public class AuthzResource { */ public enum ResourceType { FEEDS_COLLECTION("((://[^/]+/)|(^/))(?)$"), - SUBS_COLLECTION ("((://[^/]+/)|(^/{0,1}))subscribe/(?[^/]+)$"), + SUBS_COLLECTION("((://[^/]+/)|(^/{0,1}))subscribe/(?[^/]+)$"), FEED("((://[^/]+/)|(^/{0,1}))feed/(?[^/]+)$"), SUB("((://[^/]+/)|(^/{0,1}))subs/(?[^/]+)$"); private Pattern uriPattern; - private ResourceType(String patternString) { + ResourceType(String patternString) { this.uriPattern = Pattern.compile(patternString); } diff --git a/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/ProvAuthorizer.java b/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/ProvAuthorizer.java index 745e339d..595b626c 100644 --- a/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/ProvAuthorizer.java +++ b/datarouter-prov/src/main/java/org/onap/dmaap/datarouter/authz/impl/ProvAuthorizer.java @@ -23,17 +23,15 @@ package org.onap.dmaap.datarouter.authz.impl; -import java.util.Map; - -import javax.servlet.http.HttpServletRequest; - import com.att.eelf.configuration.EELFLogger; import com.att.eelf.configuration.EELFManager; +import java.util.Map; +import javax.servlet.http.HttpServletRequest; import org.onap.dmaap.datarouter.authz.AuthorizationResponse; import org.onap.dmaap.datarouter.authz.Authorizer; import org.onap.dmaap.datarouter.authz.impl.AuthzResource.ResourceType; -/** Authorizer for the provisioning API for Data Router R1 +/** Authorizer for the provisioning API for Data Router R1. * * @author J. F. Lucas * @@ -45,6 +43,7 @@ public class ProvAuthorizer implements Authorizer { private static final String SUBJECT_HEADER = "X-DMAAP-DR-ON-BEHALF-OF"; // HTTP header carrying requester identity private static final String SUBJECT_HEADER_GROUP = "X-DMAAP-DR-ON-BEHALF-OF-GROUP"; // HTTP header carrying requester identity by group Rally : US708115 + /** Constructor. For the moment, do nothing special. Make it a singleton? * */ @@ -63,7 +62,7 @@ public class ProvAuthorizer implements Authorizer { */ @Override public AuthorizationResponse decide(HttpServletRequest request) { - return this.decide(request, null); + return this.decide(request, null); } /** @@ -79,80 +78,66 @@ public class ProvAuthorizer implements Authorizer { @Override public AuthorizationResponse decide(HttpServletRequest request, Map additionalAttrs) { - log.trace ("Entering decide()"); - + log.trace("Entering decide()"); boolean decision = false; - // Extract interesting parts of the HTTP request String method = request.getMethod(); AuthzResource resource = new AuthzResource(request.getRequestURI()); - String subject = (request.getHeader(SUBJECT_HEADER)); // identity of the requester - String subjectgroup = (request.getHeader(SUBJECT_HEADER_GROUP)); // identity of the requester by group Rally : US708115 - - log.trace("Method: " + method + " -- Type: " + resource.getType() + " -- Id: " + resource.getId() + - " -- Subject: " + subject); + String subject = (request.getHeader(SUBJECT_HEADER)); + String subjectgroup = (request.getHeader(SUBJECT_HEADER_GROUP)); + log.trace("Method: " + method + " -- Type: " + resource.getType() + " -- Id: " + resource.getId() + + " -- Subject: " + subject); // Choose authorization method based on the resource type ResourceType resourceType = resource.getType(); if (resourceType != null) { - switch (resourceType) { - - case FEEDS_COLLECTION: - decision = allowFeedsCollectionAccess(resource, method, subject, subjectgroup); - break; - - case SUBS_COLLECTION: - decision = allowSubsCollectionAccess(resource, method, subject, subjectgroup); - break; - - case FEED: - decision = allowFeedAccess(resource, method, subject, subjectgroup); - break; - - case SUB: - decision = allowSubAccess(resource, method, subject, subjectgroup); - break; - - default: - decision = false; - break; + case FEEDS_COLLECTION: + decision = allowFeedsCollectionAccess(method); + break; + case SUBS_COLLECTION: + decision = allowSubsCollectionAccess(method); + break; + case FEED: + decision = allowFeedAccess(resource, method, subject, subjectgroup); + break; + case SUB: + decision = allowSubAccess(resource, method, subject, subjectgroup); + break; + default: + decision = false; + break; } } - log.debug("Exit decide(): " + method + "|" + resourceType + "|" + resource.getId() + "|" + subject + " ==> " + decision); + log.debug("Exit decide(): " + method + "|" + resourceType + "|" + resource.getId() + "|" + + subject + " ==> " + decision); return new AuthRespImpl(decision); } - private boolean allowFeedsCollectionAccess(AuthzResource resource, String method, String subject, String subjectgroup) { - + private boolean allowFeedsCollectionAccess(String method) { // Allow GET or POST unconditionally return method != null && ("GET".equalsIgnoreCase(method) || "POST".equalsIgnoreCase(method)); } - private boolean allowSubsCollectionAccess(AuthzResource resource, String method, String subject, String subjectgroup) { - + private boolean allowSubsCollectionAccess(String method) { // Allow GET or POST unconditionally return method != null && ("GET".equalsIgnoreCase(method) || "POST".equalsIgnoreCase(method)); } - private boolean allowFeedAccess(AuthzResource resource, String method, String subject, String subjectgroup) { + private boolean allowFeedAccess(AuthzResource resource, String method, String subject, String subjectgroup) { boolean decision = false; - // Allow GET, PUT, or DELETE if requester (subject) is the owner (publisher) of the feed - if ( method != null && ("GET".equalsIgnoreCase(method) || "PUT".equalsIgnoreCase(method) || - "DELETE".equalsIgnoreCase(method))) { + if ( method != null && ("GET".equalsIgnoreCase(method) || "PUT".equalsIgnoreCase(method) || "DELETE".equalsIgnoreCase(method))) { String owner = provData.getFeedOwner(resource.getId()); decision = (owner != null) && owner.equals(subject); - //Verifying by group Rally : US708115 - if(subjectgroup != null) { - String feedowner = provData.getGroupByFeedGroupId(subject, resource.getId()); - decision = (feedowner != null) && feedowner.equals(subjectgroup); + if (subjectgroup != null) { + String feedOwner = provData.getGroupByFeedGroupId(subject, resource.getId()); + decision = (feedOwner != null) && feedOwner.equals(subjectgroup); } } - return decision; } @@ -160,14 +145,13 @@ public class ProvAuthorizer implements Authorizer { boolean decision = false; // Allow GET, PUT, or DELETE if requester (subject) is the owner of the subscription (subscriber) - if (method != null && ("GET".equalsIgnoreCase(method) || "PUT".equalsIgnoreCase(method) || - "DELETE".equalsIgnoreCase(method) || "POST".equalsIgnoreCase(method))) { + if (method != null && ("GET".equalsIgnoreCase(method) || "PUT".equalsIgnoreCase(method) || "DELETE".equalsIgnoreCase(method) || "POST".equalsIgnoreCase(method))) { String owner = provData.getSubscriptionOwner(resource.getId()); decision = (owner != null) && owner.equals(subject); //Verifying by group Rally : US708115 - if(subjectgroup != null) { + if (subjectgroup != null) { String feedowner = provData.getGroupBySubGroupId(subject, resource.getId()); decision = (feedowner != null) && feedowner.equals(subjectgroup); } -- cgit 1.2.3-korg