Skip to content

Commit

Permalink
Remove duplicates in New-saml-0530 (#3117)
Browse files Browse the repository at this point in the history 
* renovate: : update dependency webrick to v1.9.0

* Refactor and fix duplicate

found by sonar in https://sonarcloud.io/component_measures?metric=new_duplicated_lines_density&selected=cloudfoundry-identity-parent%3Aserver%2Fsrc%2Fmain%2Fjava%2Forg%2Fcloudfoundry%2Fidentity%2Fuaa%2Fauthentication%2FPasscodeAuthenticationFilter.java&view=list&pullRequest=2908&id=cloudfoundry-identity-parent

* Only show failed tests

make it easier to find the failed tests in output

Signed-off-by: Duane May <duane.may@broadcom.com>

* reduce duplicates

* rebase

* reduce duplicates

* Refactor and fix duplicate (#3112)

found by sonar in https://sonarcloud.io/component_measures?metric=new_duplicated_lines_density&selected=cloudfoundry-identity-parent%3Aserver%2Fsrc%2Fmain%2Fjava%2Forg%2Fcloudfoundry%2Fidentity%2Fuaa%2Fauthentication%2FPasscodeAuthenticationFilter.java&view=list&pullRequest=2908&id=cloudfoundry-identity-parent

* cleanup

* refactor saml bearer usage

* Migrate to Caffeine Caching (#3114)

* Migrate to Caffeine Caching

Guava Cache recommends moving to Caffeine
Mostly a drop-in replacement
Although the refreshAfterWrite works a little different

* more test coverage

* again more test coverage

* sonar

* sonar

---------

Co-authored-by: strehle <markus.strehle@sap.com>

* fix rebase

* fix rebase

---------

Signed-off-by: Duane May <duane.may@broadcom.com>
Co-authored-by: Duane May <duane.may@broadcom.com>
Co-authored-by: Duane May <duanemay@users.noreply.github.com>
  • Loading branch information
@strehle @duanemay
3 people authored Nov 8, 2024
1 parent cc59526 commit 57be436
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 417 deletions.
29 changes: 2 additions & 27 deletions ...src/main/java/org/cloudfoundry/identity/uaa/authentication/AuthzAuthenticationFilter.java
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package org.cloudfoundry.identity.uaa.authentication;

import com.fasterxml.jackson.core.type.TypeReference;
import org.cloudfoundry.identity.uaa.login.AccountSavingAuthenticationSuccessHandler;
import org.cloudfoundry.identity.uaa.oauth.provider.error.OAuth2AuthenticationEntryPoint;
import org.cloudfoundry.identity.uaa.util.JsonUtils;
import org.cloudfoundry.identity.uaa.util.SessionUtils;
import org.cloudfoundry.identity.uaa.util.UaaHttpRequestUtils;
import org.cloudfoundry.identity.uaa.util.UaaStringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -29,7 +28,6 @@
import java.io.IOException;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -118,7 +116,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
HttpServletRequest req = (HttpServletRequest) request;
HttpServletResponse res = (HttpServletResponse) response;

Map<String, String> loginInfo = getCredentials(req);
Map<String, String> loginInfo = UaaHttpRequestUtils.getCredentials(req, parameterNames);

boolean buggyVmcAcceptHeader = false;

Expand Down Expand Up @@ -184,29 +182,6 @@ public String getHeader(String name) {
chain.doFilter(request, response);
}

private Map<String, String> getCredentials(HttpServletRequest request) {
Map<String, String> credentials = new HashMap<>();

for (String paramName : parameterNames) {
String value = request.getParameter(paramName);
if (value != null) {
if (value.startsWith("{")) {
try {
Map<String, String> jsonCredentials = JsonUtils.readValue(value,
new TypeReference<>() {
});
credentials.putAll(jsonCredentials);
} catch (JsonUtils.JsonUtilException e) {
logger.warn("Unknown format of value for request param: " + paramName + ". Ignoring.");
}
} else {
credentials.put(paramName, value);
}
}
}

return credentials;
}

@Override
public void init(FilterConfig filterConfig) {
Expand Down
30 changes: 2 additions & 28 deletions .../main/java/org/cloudfoundry/identity/uaa/authentication/PasscodeAuthenticationFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

package org.cloudfoundry.identity.uaa.authentication;

import com.fasterxml.jackson.core.type.TypeReference;
import org.cloudfoundry.identity.uaa.codestore.ExpiringCode;
import org.cloudfoundry.identity.uaa.codestore.ExpiringCodeStore;
import org.cloudfoundry.identity.uaa.constants.OriginKeys;
Expand All @@ -23,6 +22,7 @@
import org.cloudfoundry.identity.uaa.user.UaaUser;
import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
import org.cloudfoundry.identity.uaa.util.JsonUtils;
import org.cloudfoundry.identity.uaa.util.UaaHttpRequestUtils;
import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -59,8 +59,6 @@
*/
public class PasscodeAuthenticationFilter extends BackwardsCompatibleTokenEndpointAuthenticationFilter {

private final Logger logger = LoggerFactory.getLogger(getClass());

private List<String> parameterNames = List.of();

public PasscodeAuthenticationFilter(UaaUserDatabase uaaUserDatabase, AuthenticationManager authenticationManager, OAuth2RequestFactory oAuth2RequestFactory, ExpiringCodeStore expiringCodeStore) {
Expand Down Expand Up @@ -237,7 +235,7 @@ public Authentication authenticate(Authentication authentication) throws Authent
protected Authentication extractCredentials(HttpServletRequest request) {
String grantType = request.getParameter("grant_type");
if (grantType != null && grantType.equals(GRANT_TYPE_PASSWORD)) {
Map<String, String> credentials = getCredentials(request);
Map<String, String> credentials = UaaHttpRequestUtils.getCredentials(request, parameterNames);
String passcode = credentials.get("passcode");
if (passcode != null) {
return new ExpiringCodeAuthentication(request, passcode);
Expand All @@ -248,30 +246,6 @@ protected Authentication extractCredentials(HttpServletRequest request) {
return null;
}

private Map<String, String> getCredentials(HttpServletRequest request) {
Map<String, String> credentials = new HashMap<>();

for (String paramName : parameterNames) {
String value = request.getParameter(paramName);
if (value != null) {
if (value.startsWith("{")) {
try {
Map<String, String> jsonCredentials = JsonUtils.readValue(value,
new TypeReference<>() {
});
credentials.putAll(jsonCredentials);
} catch (JsonUtils.JsonUtilException e) {
logger.warn("Unknown format of value for request param: {}. Ignoring.", paramName);
}
} else {
credentials.put(paramName, value);
}
}
}

return credentials;
}

public void setParameterNames(List<String> parameterNames) {
this.parameterNames = parameterNames;
}
Expand Down
40 changes: 25 additions & 15 deletions ...ain/java/org/cloudfoundry/identity/uaa/provider/saml/OpenSaml4AuthenticationProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ private static Saml2ResponseValidatorResult validateInResponseTo(AbstractSaml2Au
public static Converter<AssertionToken, Saml2ResponseValidatorResult> createDefaultAssertionValidator() {

return createDefaultAssertionValidatorWithParameters(
params -> params.put(SAML2AssertionValidationParameters.CLOCK_SKEW, Duration.ofMinutes(5)));
params -> params.put(SAML2AssertionValidationParameters.CLOCK_SKEW, Duration.ofMinutes(5)), false);
}

/**
Expand All @@ -286,10 +286,10 @@ public static Converter<AssertionToken, Saml2ResponseValidatorResult> createDefa
* @since 5.8
*/
public static Converter<AssertionToken, Saml2ResponseValidatorResult> createDefaultAssertionValidatorWithParameters(
Consumer<Map<String, Object>> validationContextParameters) {
Consumer<Map<String, Object>> validationContextParameters, boolean saml2bearer) {
return createAssertionValidator(Saml2ErrorCodes.INVALID_ASSERTION,
assertionToken -> SAML20AssertionValidators.attributeValidator,
assertionToken -> createValidationContext(assertionToken, validationContextParameters));
assertionToken -> createValidationContext(assertionToken, validationContextParameters, saml2bearer));
}

/**
Expand Down Expand Up @@ -444,7 +444,7 @@ private static String getStatusCode(Response response) {
return response.getStatus().getStatusCode().getValue();
}

private Converter<AssertionToken, Saml2ResponseValidatorResult> createDefaultAssertionSignatureValidator() {
public static Converter<AssertionToken, Saml2ResponseValidatorResult> createDefaultAssertionSignatureValidator() {
return createAssertionValidator(Saml2ErrorCodes.INVALID_SIGNATURE, assertionToken -> {
RelyingPartyRegistration registration = assertionToken.getToken().getRelyingPartyRegistration();
SignatureTrustEngine engine = OpenSamlVerificationUtils.trustEngine(registration);
Expand All @@ -453,7 +453,7 @@ private Converter<AssertionToken, Saml2ResponseValidatorResult> createDefaultAss
Collections.singletonMap(SAML2AssertionValidationParameters.SIGNATURE_REQUIRED, false)));
}

private Consumer<AssertionToken> createDefaultAssertionElementsDecrypter() {
public static Consumer<AssertionToken> createDefaultAssertionElementsDecrypter() {
return assertionToken -> {
Assertion assertion = assertionToken.getAssertion();
RelyingPartyRegistration registration = assertionToken.getToken().getRelyingPartyRegistration();
Expand All @@ -465,7 +465,7 @@ private Consumer<AssertionToken> createDefaultAssertionElementsDecrypter() {
};
}

private boolean hasName(Assertion assertion) {
public static boolean hasName(Assertion assertion) {
if (assertion == null) {
return false;
}
Expand All @@ -478,7 +478,7 @@ private boolean hasName(Assertion assertion) {
return assertion.getSubject().getNameID().getValue() != null;
}

private static Map<String, List<Object>> getAssertionAttributes(Assertion assertion) {
public static Map<String, List<Object>> getAssertionAttributes(Assertion assertion) {
MultiValueMap<String, Object> attributeMap = new LinkedMultiValueMap<>();
for (AttributeStatement attributeStatement : assertion.getAttributeStatements()) {
for (Attribute attribute : attributeStatement.getAttributes()) {
Expand All @@ -495,15 +495,15 @@ private static Map<String, List<Object>> getAssertionAttributes(Assertion assert
return new LinkedHashMap<>(attributeMap); // gh-11785
}

private static List<String> getSessionIndexes(Assertion assertion) {
public static List<String> getSessionIndexes(Assertion assertion) {
List<String> sessionIndexes = new ArrayList<>();
for (AuthnStatement statement : assertion.getAuthnStatements()) {
sessionIndexes.add(statement.getSessionIndex());
}
return sessionIndexes;
}

private static Object getXmlObjectValue(XMLObject xmlObject) {
public static Object getXmlObjectValue(XMLObject xmlObject) {
if (xmlObject instanceof XSAny xsAny) {
return xsAny.getTextContent();
}
Expand All @@ -526,7 +526,7 @@ private static Object getXmlObjectValue(XMLObject xmlObject) {
return xmlObject;
}

private static Saml2AuthenticationException createAuthenticationException(String code, String message,
public static Saml2AuthenticationException createAuthenticationException(String code, String message,
Exception cause) {
return new Saml2AuthenticationException(new Saml2Error(code, message), cause);
}
Expand All @@ -546,25 +546,33 @@ private static Converter<AssertionToken, Saml2ResponseValidatorResult> createAss
}
} catch (Exception ex) {
String message = String.format("Invalid assertion [%s] for SAML response [%s]: %s", assertion.getID(),
((Response) assertion.getParent()).getID(), ex.getMessage());
assertion.getParent() != null ? ((Response) assertion.getParent()).getID() : assertion.getID(),
ex.getMessage());
return Saml2ResponseValidatorResult.failure(new Saml2Error(errorCode, message));
}
String message = String.format("Invalid assertion [%s] for SAML response [%s]: %s", assertion.getID(),
((Response) assertion.getParent()).getID(), context.getValidationFailureMessage());
assertion.getParent() != null ? ((Response) assertion.getParent()).getID() : assertion.getID(),
context.getValidationFailureMessage());
return Saml2ResponseValidatorResult.failure(new Saml2Error(errorCode, message));
};
}

private static ValidationContext createValidationContext(AssertionToken assertionToken,
Consumer<Map<String, Object>> paramsConsumer) {
Consumer<Map<String, Object>> paramsConsumer,
boolean saml2Bearer) {
Saml2AuthenticationToken token = assertionToken.token;
RelyingPartyRegistration relyingPartyRegistration = token.getRelyingPartyRegistration();
String audience = relyingPartyRegistration.getEntityId();
String recipient = relyingPartyRegistration.getAssertionConsumerServiceLocation();
String recipient;
if (saml2Bearer) {
recipient = relyingPartyRegistration.getAssertionConsumerServiceLocation().replace("/saml/SSO/alias/", "/oauth/token/alias/");
} else {
recipient = relyingPartyRegistration.getAssertionConsumerServiceLocation();
}
String assertingPartyEntityId = relyingPartyRegistration.getAssertingPartyDetails().getEntityId();
Map<String, Object> params = new HashMap<>();
Assertion assertion = assertionToken.getAssertion();
if (assertionContainsInResponseTo(assertion)) {
if (!saml2Bearer && assertionContainsInResponseTo(assertion)) {
String requestId = getAuthnRequestId(token.getAuthenticationRequest());
params.put(SAML2AssertionValidationParameters.SC_VALID_IN_RESPONSE_TO, requestId);
}
Expand Down Expand Up @@ -736,5 +744,7 @@ public static class AssertionToken {
this.token = token;
this.assertion = assertion;
}

public Assertion getAssertion() { return this.assertion; }
}
}
Loading

0 comments on commit 57be436

Please sign in to comment.