cxf-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cohei...@apache.org
Subject cxf-fediz git commit: Findbugs work on the Fediz services
Date Wed, 23 Nov 2016 16:57:24 GMT
Repository: cxf-fediz
Updated Branches:
  refs/heads/1.2.x-fixes f1159f3a7 -> 94d12ade2


Findbugs work on the Fediz services


Project: http://git-wip-us.apache.org/repos/asf/cxf-fediz/repo
Commit: http://git-wip-us.apache.org/repos/asf/cxf-fediz/commit/94d12ade
Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/94d12ade
Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/94d12ade

Branch: refs/heads/1.2.x-fixes
Commit: 94d12ade2cc8c0325350519f7e0302734a9e5f62
Parents: f1159f3
Author: Colm O hEigeartaigh <coheigea@apache.org>
Authored: Wed Nov 23 13:31:31 2016 +0000
Committer: Colm O hEigeartaigh <coheigea@apache.org>
Committed: Wed Nov 23 16:57:16 2016 +0000

----------------------------------------------------------------------
 .../fediz/service/idp/FederationEntryPoint.java |  3 +-
 .../service/idp/STSAuthenticationProvider.java  |  2 +-
 .../idp/STSKrbAuthenticationProvider.java       |  5 ++
 .../cxf/fediz/service/idp/STSUserDetails.java   | 24 +++++++++
 .../service/idp/beans/CommonsURLValidator.java  | 52 ++++++++++++++++++++
 .../service/idp/beans/STSClientAction.java      |  2 +-
 .../kerberos/KerberosServiceRequestToken.java   | 17 +++++--
 .../idp/kerberos/PassThroughKerberosClient.java | 13 ++++-
 .../ApplicationProtocolControllerImpl.java      |  2 +-
 .../fediz/service/idp/rest/IdpServiceImpl.java  | 25 ++++++----
 .../WEB-INF/federation-signin-request.xml       |  2 +-
 .../WEB-INF/federation-validate-request.xml     |  6 +++
 .../fediz/service/sts/FileClaimsHandler.java    |  2 +-
 .../sts/realms/RealmFileClaimsHandler.java      |  2 +-
 14 files changed, 136 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/FederationEntryPoint.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/FederationEntryPoint.java
b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/FederationEntryPoint.java
index 1a39ef2..b73dfd8 100644
--- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/FederationEntryPoint.java
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/FederationEntryPoint.java
@@ -96,7 +96,8 @@ public class FederationEntryPoint implements AuthenticationEntryPoint,
         if (loginUri == null) {
             LOG.warn("wauth value '" + wauth + "' not supported");
             response.sendError(
-                    HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "wauth value '" + wauth
+ "' not supported");
+                    HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "The wauth value that was
supplied is not supported");
+            return;
         }
         redirectUrl = new StringBuilder(extractFullContextPath(servletRequest))
             .append(loginUri).append("?").append(servletRequest.getQueryString()).toString();

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSAuthenticationProvider.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSAuthenticationProvider.java
b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSAuthenticationProvider.java
index ab84b11..c53604d 100644
--- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSAuthenticationProvider.java
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSAuthenticationProvider.java
@@ -92,7 +92,7 @@ public abstract class STSAuthenticationProvider implements AuthenticationProvide
             
             List<Claim> claims = parseClaimsInAssertion(assertion.getSaml2());
             for (Claim c : claims) {
-                if (roleURI.equals(c.getClaimType())) {
+                if (c.getClaimType() != null && roleURI.equals(c.getClaimType().toString()))
{
                     Object oValue = c.getValue();
                     if ((oValue instanceof List<?>) && !((List<?>)oValue).isEmpty())
{
                         List<?> values = (List<?>)oValue;

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSKrbAuthenticationProvider.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSKrbAuthenticationProvider.java
b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSKrbAuthenticationProvider.java
index 9a5dae8..62f4817 100644
--- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSKrbAuthenticationProvider.java
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSKrbAuthenticationProvider.java
@@ -132,6 +132,11 @@ public class STSKrbAuthenticationProvider extends STSAuthenticationProvider
{
                     new SAMLTokenPrincipalImpl(new SamlAssertionWrapper(token.getToken()));
             }
             
+            if (kerberosPrincipal == null) {
+                LOG.info("Failed to authenticate user '" + kerberosRequestToken.getName());
+                return null;
+            }
+            
             List<GrantedAuthority> authorities = createAuthorities(token);
             
             KerberosServiceRequestToken ksrt = 

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSUserDetails.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSUserDetails.java
b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSUserDetails.java
index bc084d7..080bcb4 100644
--- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSUserDetails.java
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/STSUserDetails.java
@@ -46,4 +46,28 @@ public class STSUserDetails extends User {
         return this.token;
     }
 
+    @Override
+    public boolean equals(Object object) {
+        if (!(object instanceof STSUserDetails)) {
+            return false;
+        }
+        
+        if (token != null && !token.equals(((STSUserDetails)object).token)) {
+            return false;
+        } else  if (token == null && ((STSUserDetails)object).token != null) {
+            return false;
+        }
+        
+        return super.equals(object);
+    }
+    
+    @Override
+    public int hashCode() {
+        int hashCode = 17;
+        if (token != null) {
+            hashCode *= 31 * token.hashCode();
+        }
+        
+        return hashCode * super.hashCode();
+    }
 }

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/CommonsURLValidator.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/CommonsURLValidator.java
b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/CommonsURLValidator.java
new file mode 100644
index 0000000..25780d2
--- /dev/null
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/CommonsURLValidator.java
@@ -0,0 +1,52 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cxf.fediz.service.idp.beans;
+
+import org.apache.commons.validator.routines.UrlValidator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.stereotype.Component;
+import org.springframework.webflow.execution.RequestContext;
+
+/**
+ * Validate a URL using Commons Validator
+ */
+@Component
+public class CommonsURLValidator {
+
+    private static final Logger LOG = LoggerFactory.getLogger(CommonsURLValidator.class);
+
+    public boolean isValid(RequestContext context, String endpointAddress)
+        throws Exception {
+        if (endpointAddress == null) {
+            return true;
+        }
+        
+        // The endpointAddress address must be a valid URL + start with http(s)
+        // Validate it first using commons-validator
+        UrlValidator urlValidator = new UrlValidator(new String[] {"http", "https"}, UrlValidator.ALLOW_LOCAL_URLS);
+        if (!urlValidator.isValid(endpointAddress)) {
+            LOG.warn("The given endpointAddress parameter {} is not a valid URL", endpointAddress);
+            return false;
+        }
+        
+        return true;
+    }
+    
+}

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/STSClientAction.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/STSClientAction.java
b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/STSClientAction.java
index fc47c23..ca87991 100644
--- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/STSClientAction.java
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/STSClientAction.java
@@ -411,7 +411,7 @@ public class STSClientAction {
         writer.writeAttribute("Dialect",
                 HTTP_SCHEMAS_XMLSOAP_ORG_WS_2005_05_IDENTITY);
 
-        if (realmClaims != null && realmClaims.size() > 0) {
+        if (realmClaims.size() > 0) {
             for (RequestClaim item : realmClaims) {
                 LOG.debug("  {}", item.getClaimType().toString());
                 writer.writeStartElement("ic", "ClaimType",

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/kerberos/KerberosServiceRequestToken.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/kerberos/KerberosServiceRequestToken.java
b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/kerberos/KerberosServiceRequestToken.java
index 40308e4..2aba9cf 100644
--- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/kerberos/KerberosServiceRequestToken.java
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/kerberos/KerberosServiceRequestToken.java
@@ -67,7 +67,11 @@ public class KerberosServiceRequestToken extends AbstractAuthenticationToken
{
                                        Collection<? extends GrantedAuthority> authorities,

                                        byte[] token) {
         super(authorities);
-        this.token = token;
+        if (token != null) {
+            this.token = Arrays.copyOf(token, token.length);
+        } else {
+            this.token = null;
+        }
         this.principal = principal;
         super.setAuthenticated(true);
     }
@@ -81,7 +85,11 @@ public class KerberosServiceRequestToken extends AbstractAuthenticationToken
{
      */
     public KerberosServiceRequestToken(byte[] token) {
         super(null);
-        this.token = token;
+        if (token != null) {
+            this.token = Arrays.copyOf(token, token.length);
+        } else {
+            this.token = null;
+        }
         this.principal = null;
     }
     
@@ -134,6 +142,9 @@ public class KerberosServiceRequestToken extends AbstractAuthenticationToken
{
     /** Returns the Kerberos token
      */
     public byte[] getToken() {
-        return this.token;
+        if (token != null) {
+            return Arrays.copyOf(token, token.length);
+        }
+        return null;
     }
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/kerberos/PassThroughKerberosClient.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/kerberos/PassThroughKerberosClient.java
b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/kerberos/PassThroughKerberosClient.java
index bfebe4b..3b2ba9f 100644
--- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/kerberos/PassThroughKerberosClient.java
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/kerberos/PassThroughKerberosClient.java
@@ -19,6 +19,8 @@
 
 package org.apache.cxf.fediz.service.idp.kerberos;
 
+import java.util.Arrays;
+
 import org.apache.cxf.fediz.core.util.DOMUtils;
 import org.apache.cxf.ws.security.kerberos.KerberosClient;
 import org.apache.cxf.ws.security.tokenstore.SecurityToken;
@@ -61,11 +63,18 @@ public class PassThroughKerberosClient extends KerberosClient {
     }
 
     public byte[] getToken() {
-        return token;
+        if (token != null) {
+            return Arrays.copyOf(token, token.length);
+        }
+        return null;
     }
 
     public void setToken(byte[] token) {
-        this.token = token;
+        if (token != null) {
+            this.token = Arrays.copyOf(token, token.length);
+        } else {
+            this.token = null;
+        }
     }
 
 }

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/protocols/ApplicationProtocolControllerImpl.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/protocols/ApplicationProtocolControllerImpl.java
b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/protocols/ApplicationProtocolControllerImpl.java
index 4f55141..6e4822a 100644
--- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/protocols/ApplicationProtocolControllerImpl.java
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/protocols/ApplicationProtocolControllerImpl.java
@@ -40,7 +40,7 @@ public class ApplicationProtocolControllerImpl implements ProtocolController<App
     @Override
     public ApplicationProtocolHandler getProtocolHandler(String protocol) {
         for (ApplicationProtocolHandler protocolHandler : protocolHandlers) {
-            if (protocolHandler.equals(protocol)) {
+            if (protocolHandler.getProtocol() != null && protocolHandler.getProtocol().equals(protocol))
{
                 return protocolHandler;
             }
         }

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/rest/IdpServiceImpl.java
----------------------------------------------------------------------
diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/rest/IdpServiceImpl.java
b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/rest/IdpServiceImpl.java
index 36f859d..d4b5c40 100644
--- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/rest/IdpServiceImpl.java
+++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/rest/IdpServiceImpl.java
@@ -129,9 +129,11 @@ public class IdpServiceImpl implements IdpService {
     @Override
     public Response addApplicationToIdp(UriInfo ui, String realm, Application application)
{
         Idp idp = idpDAO.getIdp(realm, Arrays.asList("all"));
-        if (idp.getApplications().contains(application.getRealm())) {
-            LOG.warn("Application '" + application.getRealm() + "' already added");
-            throw new WebApplicationException(Status.CONFLICT);
+        for (Application idpApplication : idp.getApplications()) {
+            if (idpApplication.getRealm() != null && idpApplication.getRealm().equals(application.getRealm()))
{
+                LOG.warn("Application '" + application.getRealm() + "' already added");
+                throw new WebApplicationException(Status.CONFLICT);
+            }
         }
         Application application2 = applicationDAO.getApplication(application.getRealm(),
null);
         idpDAO.addApplicationToIdp(idp, application2);
@@ -165,9 +167,11 @@ public class IdpServiceImpl implements IdpService {
     @Override
     public Response addTrustedIdpToIdp(UriInfo ui, String realm, TrustedIdp trustedIdp) {
         Idp idp = idpDAO.getIdp(realm, Arrays.asList("all"));
-        if (idp.getTrustedIdps().contains(trustedIdp.getRealm())) {
-            LOG.warn("Trusted IDP '" + trustedIdp.getRealm() + "' already added");
-            throw new WebApplicationException(Status.CONFLICT);
+        for (TrustedIdp idpTrustedIdp : idp.getTrustedIdps()) {
+            if (idpTrustedIdp.getRealm() != null && idpTrustedIdp.getRealm().equals(trustedIdp.getRealm()))
{
+                LOG.warn("Trusted IDP '" + trustedIdp.getRealm() + "' already added");
+                throw new WebApplicationException(Status.CONFLICT);
+            }
         }
         TrustedIdp trustedIpd2 = trustedIdpDAO.getTrustedIDP(trustedIdp.getRealm());
         
@@ -199,9 +203,12 @@ public class IdpServiceImpl implements IdpService {
     @Override
     public Response addClaimToIdp(UriInfo ui, String realm, Claim claim) {
         Idp idp = idpDAO.getIdp(realm, Arrays.asList("all"));
-        if (idp.getClaimTypesOffered().contains(claim.getClaimType().toString())) {
-            LOG.warn("Claim '" + claim.getClaimType() + "' already added");
-            throw new WebApplicationException(Status.CONFLICT);
+        for (Claim idpClaim : idp.getClaimTypesOffered()) {
+            if (idpClaim.getClaimType() != null 
+                && idpClaim.getClaimType().toString().equals(claim.getClaimType().toString()))
{
+                LOG.warn("Claim '" + claim.getClaimType() + "' already added");
+                throw new WebApplicationException(Status.CONFLICT);
+            }
         }
         Claim claim2 = claimDAO.getClaim(claim.getClaimType().toString());
         idpDAO.addClaimToIdp(idp, claim2);

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/idp/src/main/webapp/WEB-INF/federation-signin-request.xml
----------------------------------------------------------------------
diff --git a/services/idp/src/main/webapp/WEB-INF/federation-signin-request.xml b/services/idp/src/main/webapp/WEB-INF/federation-signin-request.xml
index 08e1d2b..1d0b39e 100644
--- a/services/idp/src/main/webapp/WEB-INF/federation-signin-request.xml
+++ b/services/idp/src/main/webapp/WEB-INF/federation-signin-request.xml
@@ -98,7 +98,7 @@
         </transition>
         <transition on-exception="java.lang.Throwable" to="viewBadRequest" />
     </action-state>
-
+    
     <decision-state id="checkWauthTypeSupported">
         <on-entry>
             <!-- Here, home realm is guaranteed to be THIS realm -->

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/idp/src/main/webapp/WEB-INF/federation-validate-request.xml
----------------------------------------------------------------------
diff --git a/services/idp/src/main/webapp/WEB-INF/federation-validate-request.xml b/services/idp/src/main/webapp/WEB-INF/federation-validate-request.xml
index 114bb5d..733d5d3 100644
--- a/services/idp/src/main/webapp/WEB-INF/federation-validate-request.xml
+++ b/services/idp/src/main/webapp/WEB-INF/federation-validate-request.xml
@@ -61,6 +61,12 @@
             test="requestParameters.SAMLResponse == null or requestParameters.SAMLResponse.length()
== 0"
             then="viewBadRequest" else="signinResponse" />
     </decision-state>
+    
+    <action-state id="validateWReplyForSignout">
+        <evaluate expression="commonsURLValidator.isValid(flowRequestContext, flowScope.wreply)"/>
+        <transition on="yes" to="selectSignOutProcess" />
+        <transition on="no" to="viewBadRequest" />
+    </action-state>
 	
     <decision-state id="selectSignOutProcess">
         <on-entry>

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/sts/src/main/java/org/apache/cxf/fediz/service/sts/FileClaimsHandler.java
----------------------------------------------------------------------
diff --git a/services/sts/src/main/java/org/apache/cxf/fediz/service/sts/FileClaimsHandler.java
b/services/sts/src/main/java/org/apache/cxf/fediz/service/sts/FileClaimsHandler.java
index e6ca110..bfe0b97 100644
--- a/services/sts/src/main/java/org/apache/cxf/fediz/service/sts/FileClaimsHandler.java
+++ b/services/sts/src/main/java/org/apache/cxf/fediz/service/sts/FileClaimsHandler.java
@@ -76,7 +76,7 @@ public class FileClaimsHandler implements ClaimsHandler {
             return new ProcessedClaimCollection();
         }
 
-        if (claims != null && claims.size() > 0) {
+        if (claims.size() > 0) {
             ProcessedClaimCollection claimCollection = new ProcessedClaimCollection();
             for (Claim requestClaim : claims) { 
                 String claimValue = claimMap.get(requestClaim.getClaimType().toString());

http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/94d12ade/services/sts/src/main/java/org/apache/cxf/fediz/service/sts/realms/RealmFileClaimsHandler.java
----------------------------------------------------------------------
diff --git a/services/sts/src/main/java/org/apache/cxf/fediz/service/sts/realms/RealmFileClaimsHandler.java
b/services/sts/src/main/java/org/apache/cxf/fediz/service/sts/realms/RealmFileClaimsHandler.java
index 1088811..fefc343 100644
--- a/services/sts/src/main/java/org/apache/cxf/fediz/service/sts/realms/RealmFileClaimsHandler.java
+++ b/services/sts/src/main/java/org/apache/cxf/fediz/service/sts/realms/RealmFileClaimsHandler.java
@@ -97,7 +97,7 @@ public class RealmFileClaimsHandler implements ClaimsHandler {
         }
         LOG.fine("Claims found for principal '" + parameters.getPrincipal().getName() + "'");
 
-        if (claims != null && claims.size() > 0) {
+        if (claims.size() > 0) {
             ProcessedClaimCollection claimCollection = new ProcessedClaimCollection();
             for (Claim requestClaim : claims) { 
                 String claimValue = claimMap.get(requestClaim.getClaimType().toString());


Mime
View raw message