Merge "Fixes sonar issues in CMService"

1 files changed, 24 insertions, 47 deletions

diff --git a/auth/auth-certman/src/main/java/org/onap/aaf/auth/cm/service/CMService.java b/auth/auth-certman/src/main/java/org/onap/aaf/auth/cm/service/CMService.java
index dee788e4..376ae1b1 100644
--- a/auth/auth-certman/src/main/java/org/onap/aaf/auth/cm/service/CMService.java
+++ b/auth/auth-certman/src/main/java/org/onap/aaf/auth/cm/service/CMService.java
@@ -74,15 +74,16 @@ public class CMService {
 	public static final String REQUEST = "request";
 	public static final String RENEW = "renew";
 	public static final String DROP = "drop";
-//	public static final String SANS = "san";
 	public static final String IPS = "ips";
 	public static final String DOMAIN = "domain";
+
+	private static final String CERTMAN = ".certman";
+	private static final String ACCESS = ".access";
 	
 	private static final String[] NO_NOTES = new String[0];
 	private final CertDAO certDAO;
 	private final CredDAO credDAO;
 	private final ArtiDAO artiDAO;
-//	private DAO<AuthzTrans, ?>[] daos;
 	private AAF_CM certman;
 
 //	@SuppressWarnings("unchecked")
@@ -94,11 +95,7 @@ public class CMService {
 		certDAO = new CertDAO(trans, hd, cid);
 		credDAO = new CredDAO(trans, hd, cid);
 		artiDAO = new ArtiDAO(trans, hd, cid);
-		
-//		daos =(DAO<AuthzTrans, ?>[]) new DAO<?,?>[] {
-//				hd,cid,certDAO,credDAO,artiDAO
-//		};
-//
+
 		this.certman = certman;
 	}
 	
@@ -119,7 +116,7 @@ public class CMService {
 			
 
 			// Disallow non-AAF CA without special permission
-			if(!ca.getName().equals("aaf") && !trans.fish( new AAFPermission(mechNS+".certman", ca.getName(), REQUEST))) {
+			if(!"aaf".equals(ca.getName()) && !trans.fish( new AAFPermission(mechNS+CERTMAN, ca.getName(), REQUEST))) {
 				return Result.err(Status.ERR_Denied, "'%s' does not have permission to request Certificates from Certificate Authority '%s'", 
 						trans.user(),ca.getName());
 			}
@@ -135,7 +132,7 @@ public class CMService {
 				
 				InetAddress primary = null;
 				// Organize incoming information to get to appropriate Artifact
-				if(fqdns.size()>=1) {
+				if(!fqdns.isEmpty()) {
 					// Accept domain wild cards, but turn into real machines
 					// Need *domain.com:real.machine.domain.com:san.machine.domain.com:...
 					if(fqdns.get(0).startsWith("*")) { // Domain set
@@ -146,16 +143,16 @@ public class CMService {
 						//TODO check for Permission in Add Artifact?
 						String domain = fqdns.get(0).substring(1);
 						fqdns.remove(0);
-						if(fqdns.size()>=1) {
-							InetAddress ia = InetAddress.getByName(fqdns.get(0));
-							if(ia==null) {
-								return Result.err(Result.ERR_Denied, "Request not made from matching IP matching domain");
-							} else if(ia.getHostName().endsWith(domain)) {
-								primary = ia;
-							}
-						} else {
-							return Result.err(Result.ERR_Denied, "Requests using domain require machine declaration");
-						}
+            if(fqdns.isEmpty()) {
+              return Result.err(Result.ERR_Denied, "Requests using domain require machine declaration");
+            }
+
+            InetAddress ia = InetAddress.getByName(fqdns.get(0));
+            if(ia==null) {
+              return Result.err(Result.ERR_Denied, "Request not made from matching IP matching domain");
+            } else if(ia.getHostName().endsWith(domain)) {
+              primary = ia;
+            }
 					
 	 				} else {
 						for(String cn : req.value.fqdns) {
@@ -180,7 +177,6 @@ public class CMService {
 				
 				if(primary==null) {
 					return Result.err(Result.ERR_Denied, "Request not made from matching IP (%s)",trans.ip());
-//					return Result.err(Result.ERR_BadData,"Calling Machine does not match DNS lookup for %s",req.value.fqdns.get(0));
 				}
 				
 				ArtiDAO.Data add = null;
@@ -247,25 +243,10 @@ public class CMService {
 				}
 		
 				// Policy 7: Caller must be the MechID or have specifically delegated permissions
-				if(!(trans.user().equals(req.value.mechid) || trans.fish(new AAFPermission(mechNS + ".certman", ca.getName() , "request")))) {
+        if(!(trans.user().equals(req.value.mechid) || trans.fish(new AAFPermission(mechNS + CERTMAN, ca.getName() , REQUEST)))) {
 					return Result.err(Status.ERR_Denied, "%s must have access to modify x509 certs in NS %s",trans.user(),mechNS);
 				}
 				
-				// Policy 8: SANs only allowed by Exception... need permission
-				// 7/25/2017 - SAN Permission no longer required. CSO
-//				if(fqdns.size()>1 && !certman.aafLurPerm.fish(
-//						new Principal() {
-//							@Override
-//							public String getName() {
-//								return req.value.mechid;
-//							}
-//						},
-//						new AAFPermission(ca.getPermType(), ca.getName(), SANS))) {
-//					if(notes==null) {notes = new ArrayList<>();}
-//					notes.add("Warning: Subject Alternative Names only allowed by Permission: Get CSO Exception.");
-//					return Result.err(Status.ERR_Denied, "%s must have a CSO Exception to work with SAN",trans.user());
-//				}
-				
 				// Make sure Primary is the first in fqdns
 				if(fqdns.size()>1) {
 					for(int i=0;i<fqdns.size();++i) {
@@ -295,9 +276,6 @@ public class CMService {
 					return Result.err(Result.ERR_ActionNotCompleted,"x509 Certificate not signed by CA");
 				}
 				trans.info().printf("X509 Subject: %s", x509ac.getX509().getSubjectDN());
-//				for(String s: x509ac.getTrustChain()) {
-//					trans.warn().printf("Trust Cert: \n%s", s);
-//				}
 				
 				X509Certificate x509 = x509ac.getX509();
 				CertDAO.Data cdd = new CertDAO.Data();
@@ -349,7 +327,7 @@ public class CMService {
 		String ns = Question.domain2ns(mechID);
 		try {
 			if( trans.user().equals(mechID)
-					|| trans.fish(new AAFPermission(ns + ".access", "*", "read"))
+          || trans.fish(new AAFPermission(ns + ACCESS, "*", "read"))
 					|| (trans.org().validate(trans,Organization.Policy.OWNS_MECHID,null,mechID))==null) {
 				return certDAO.readID(trans, mechID);
 			} else {
@@ -496,9 +474,9 @@ public class CMService {
 		}
 		add = data.value.get(0);
 		if( trans.user().equals(add.mechid)
-			|| trans.fish(new AAFPermission(add.ns + ".access", "*", "read"))
-			|| trans.fish(new AAFPermission(add.ns+".certman",add.ca,"read"))
-			|| trans.fish(new AAFPermission(add.ns+".certman",add.ca,"request"))
+      || trans.fish(new AAFPermission(add.ns + ACCESS, "*", "read"))
+      || trans.fish(new AAFPermission(add.ns+CERTMAN,add.ca,"read"))
+      || trans.fish(new AAFPermission(add.ns+CERTMAN,add.ca,"request"))
 			|| (trans.org().validate(trans,Organization.Policy.OWNS_MECHID,null,add.mechid))==null) {
 			return data;
 		} else {
@@ -516,7 +494,7 @@ public class CMService {
 		String ns = FQI.reverseDomain(mechid);
 		
 		String reason;
-		if(trans.fish(new AAFPermission(ns + ".access", "*", "read"))
+    if(trans.fish(new AAFPermission(ns + ACCESS, "*", "read"))
 			|| (reason=trans.org().validate(trans,Organization.Policy.OWNS_MECHID,null,mechid))==null) {
 			return artiDAO.readByMechID(trans, mechid);
 		} else {
@@ -547,8 +525,7 @@ public class CMService {
 		
 		// TODO do some checks?
 
-		Result<List<ArtiDAO.Data>> rv = artiDAO.readByNs(trans, ns);
-		return rv;
+    return artiDAO.readByNs(trans, ns);
 	}
 
 
@@ -646,7 +623,7 @@ public class CMService {
 		
 		String ns = FQI.reverseDomain(add.mechid);
 
-		if(trans.fish(new AAFPermission(ns + ".access", "*", "write"))
+    if(trans.fish(new AAFPermission(ns + ACCESS, "*", "write"))
 				|| trans.user().equals(sponsor)) {
 			return artiDAO.delete(trans, add, false);
 		}