Skip to content

Apply Java naming conventions and behavior-preserving idiom cleanup #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marevol merged 1 commit into from
Jul 5, 2026
Merged

Apply Java naming conventions and behavior-preserving idiom cleanup #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 17 additions & 15 deletions core/src/main/java/org/codelibs/saml2/core/authn/AuthnRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public AuthnRequest(final Saml2Settings settings, final AuthnRequestParams param

final StringSubstitutor substitutor = generateSubstitutor(params, settings);
authnRequestString = postProcessXml(substitutor.replace(getAuthnRequestTemplate()), params, settings);
LOGGER.debug("AuthNRequest --> " + authnRequestString);
LOGGER.debug("AuthNRequest --> {}", authnRequestString);
}

/**
Expand Down Expand Up @@ -218,27 +218,27 @@ private StringSubstitutor generateSubstitutor(final AuthnRequestParams params, f
String subjectStr = "";
final String nameIdValueReq = params.getNameIdValueReq();
if (nameIdValueReq != null && !nameIdValueReq.isEmpty()) {
final String nameIDFormat = settings.getSpNameIDFormat();
final String nameIdFormat = settings.getSpNameIDFormat();
subjectStr = "<saml:Subject>";
subjectStr += "<saml:NameID Format=\"" + Util.toXml(nameIDFormat) + "\">" + Util.toXml(nameIdValueReq) + "</saml:NameID>";
subjectStr += "<saml:NameID Format=\"" + Util.toXml(nameIdFormat) + "\">" + Util.toXml(nameIdValueReq) + "</saml:NameID>";
subjectStr += "<saml:SubjectConfirmation Method=\"urn:oasis:names:tc:SAML:2.0:cm:bearer\"></saml:SubjectConfirmation>";
subjectStr += "</saml:Subject>";
}
valueMap.put("subjectStr", subjectStr);

String nameIDPolicyStr = "";
String nameIdPolicyStr = "";
if (params.isSetNameIdPolicy()) {
String nameIDPolicyFormat = settings.getSpNameIDFormat();
String nameIdPolicyFormat = settings.getSpNameIDFormat();
if (settings.getWantNameIdEncrypted()) {
nameIDPolicyFormat = Constants.NAMEID_ENCRYPTED;
nameIdPolicyFormat = Constants.NAMEID_ENCRYPTED;
}
String allowCreateStr = "";
if (params.isAllowCreate()) {
allowCreateStr = " AllowCreate=\"true\"";
}
nameIDPolicyStr = "<samlp:NameIDPolicy Format=\"" + Util.toXml(nameIDPolicyFormat) + "\"" + allowCreateStr + " />";
nameIdPolicyStr = "<samlp:NameIDPolicy Format=\"" + Util.toXml(nameIdPolicyFormat) + "\"" + allowCreateStr + " />";
}
valueMap.put("nameIDPolicyStr", nameIDPolicyStr);
valueMap.put("nameIDPolicyStr", nameIdPolicyStr);

String providerStr = "";
final Organization organization = settings.getOrganization();
Expand All @@ -252,7 +252,7 @@ private StringSubstitutor generateSubstitutor(final AuthnRequestParams params, f

final String issueInstantString = Util.formatDateTime(issueInstant.getTimeInMillis());
valueMap.put("issueInstant", issueInstantString);
valueMap.put("id", Util.toXml(String.valueOf(id)));
valueMap.put("id", Util.toXml(id));
valueMap.put("assertionConsumerServiceURL", Util.toXml(String.valueOf(settings.getSpAssertionConsumerServiceUrl())));
valueMap.put("protocolBinding", Util.toXml(settings.getSpAssertionConsumerServiceBinding()));
valueMap.put("spEntityid", Util.toXml(settings.getSpEntityId()));
Expand All @@ -261,12 +261,14 @@ private StringSubstitutor generateSubstitutor(final AuthnRequestParams params, f
final List<String> requestedAuthnContexts = settings.getRequestedAuthnContext();
if (requestedAuthnContexts != null && !requestedAuthnContexts.isEmpty()) {
final String requestedAuthnContextCmp = settings.getRequestedAuthnContextComparison();
requestedAuthnContextStr = "<samlp:RequestedAuthnContext Comparison=\"" + Util.toXml(requestedAuthnContextCmp) + "\">";
final StringBuilder requestedAuthnContextBuilder =
new StringBuilder("<samlp:RequestedAuthnContext Comparison=\"" + Util.toXml(requestedAuthnContextCmp) + "\">");
for (final String requestedAuthnContext : requestedAuthnContexts) {
requestedAuthnContextStr +=
"<saml:AuthnContextClassRef>" + Util.toXml(requestedAuthnContext) + "</saml:AuthnContextClassRef>";
requestedAuthnContextBuilder.append("<saml:AuthnContextClassRef>").append(Util.toXml(requestedAuthnContext))
.append("</saml:AuthnContextClassRef>");
}
requestedAuthnContextStr += "</samlp:RequestedAuthnContext>";
requestedAuthnContextBuilder.append("</samlp:RequestedAuthnContext>");
requestedAuthnContextStr = requestedAuthnContextBuilder.toString();
}

valueMap.put("requestedAuthnContextStr", requestedAuthnContextStr);
Expand All @@ -277,13 +279,13 @@ private StringSubstitutor generateSubstitutor(final AuthnRequestParams params, f
/**
* @return the AuthnRequest's template
*/
private static StringBuilder getAuthnRequestTemplate() {
private static String getAuthnRequestTemplate() {
final StringBuilder template = new StringBuilder();
template.append(
"<samlp:AuthnRequest xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\" ID=\"${id}\" Version=\"2.0\" IssueInstant=\"${issueInstant}\"${providerStr}${forceAuthnStr}${isPassiveStr}${destinationStr} ProtocolBinding=\"${protocolBinding}\" AssertionConsumerServiceURL=\"${assertionConsumerServiceURL}\">");
template.append("<saml:Issuer>${spEntityid}</saml:Issuer>");
template.append("${subjectStr}${nameIDPolicyStr}${requestedAuthnContextStr}</samlp:AuthnRequest>");
return template;
return template.toString();
}

/**
Expand Down
105 changes: 49 additions & 56 deletions core/src/main/java/org/codelibs/saml2/core/authn/SamlResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
Expand Down Expand Up @@ -350,7 +351,7 @@ public boolean isValid(final String requestId) {
if (expiresAt == null) {
final List<Instant> notOnOrAfters = getAssertionNotOnOrAfter();
expiresAt = notOnOrAfters.isEmpty() ? Instant.now().plusSeconds(settings.getClockDrift() + 300)
: java.util.Collections.min(notOnOrAfters);
: Collections.min(notOnOrAfters);
}
if (replayCache.registerAndCheck(assertionId, expiresAt)) {
throw new ValidationException("The Assertion was already processed (replay detected): " + assertionId,
Expand Down Expand Up @@ -389,24 +390,24 @@ private void validateSubjectConfirmation(final String responseInResponseTo) {

final NodeList subjectConfirmationDataNodes = scn.getChildNodes();
for (int c = 0; c < subjectConfirmationDataNodes.getLength(); c++) {
if (subjectConfirmationDataNodes.item(c).getLocalName() != null
&& "SubjectConfirmationData".equals(subjectConfirmationDataNodes.item(c).getLocalName())) {
final Node subjectConfirmationDataNode = subjectConfirmationDataNodes.item(c);
if ("SubjectConfirmationData".equals(subjectConfirmationDataNode.getLocalName())) {

final Node recipient = subjectConfirmationDataNodes.item(c).getAttributes().getNamedItem("Recipient");
final Node recipient = subjectConfirmationDataNode.getAttributes().getNamedItem("Recipient");
final SubjectConfirmationIssue issue = validateRecipient(recipient, i);
if (issue != null) {
validationIssues.add(issue);
continue;
}

final Node inResponseTo = subjectConfirmationDataNodes.item(c).getAttributes().getNamedItem("InResponseTo");
final Node inResponseTo = subjectConfirmationDataNode.getAttributes().getNamedItem("InResponseTo");
if (inResponseTo == null && responseInResponseTo != null
|| inResponseTo != null && !inResponseTo.getNodeValue().equals(responseInResponseTo)) {
validationIssues.add(new SubjectConfirmationIssue(i, "SubjectConfirmationData has an invalid InResponseTo value"));
continue;
}

final Node notOnOrAfter = subjectConfirmationDataNodes.item(c).getAttributes().getNamedItem("NotOnOrAfter");
final Node notOnOrAfter = subjectConfirmationDataNode.getAttributes().getNamedItem("NotOnOrAfter");
if (notOnOrAfter == null) {
validationIssues
.add(new SubjectConfirmationIssue(i, "SubjectConfirmationData doesn't contain a NotOnOrAfter attribute"));
Expand All @@ -420,7 +421,7 @@ private void validateSubjectConfirmation(final String responseInResponseTo) {
continue;
}

final Node notBefore = subjectConfirmationDataNodes.item(c).getAttributes().getNamedItem("NotBefore");
final Node notBefore = subjectConfirmationDataNode.getAttributes().getNamedItem("NotBefore");
if (notBefore != null) {
Instant nb = Util.parseDateTime(notBefore.getNodeValue());
nb = ChronoUnit.SECONDS.addTo(nb, settings.getClockDrift() * -1);
Expand Down Expand Up @@ -460,7 +461,7 @@ public Map<String, String> getNameIdData() {
return this.nameIdData;
}
try {
final Map<String, String> nameIdData = new HashMap<>();
final Map<String, String> data = new HashMap<>();

final NodeList encryptedIDNodes = this.queryAssertion("/saml:Subject/saml:EncryptedID");
NodeList nameIdNodes;
Expand Down Expand Up @@ -498,25 +499,25 @@ public Map<String, String> getNameIdData() {
throw new ValidationException("An empty NameID value found", ValidationException.EMPTY_NAMEID);
}

nameIdData.put("Value", value);
data.put("Value", value);

if (nameIdElem.hasAttribute("Format")) {
nameIdData.put("Format", nameIdElem.getAttribute("Format"));
data.put("Format", nameIdElem.getAttribute("Format"));
}
if (nameIdElem.hasAttribute("SPNameQualifier")) {
final String spNameQualifier = nameIdElem.getAttribute("SPNameQualifier");
validateSpNameQualifier(spNameQualifier);
nameIdData.put("SPNameQualifier", spNameQualifier);
data.put("SPNameQualifier", spNameQualifier);
}
if (nameIdElem.hasAttribute("NameQualifier")) {
nameIdData.put("NameQualifier", nameIdElem.getAttribute("NameQualifier"));
data.put("NameQualifier", nameIdElem.getAttribute("NameQualifier"));
}
}
} else if (settings.getWantNameId()) {
throw new ValidationException("No name id found in Document.", ValidationException.NO_NAMEID);
}
this.nameIdData = nameIdData;
return nameIdData;
this.nameIdData = data;
return data;
} catch (SAMLException e) {
throw e;
} catch (DOMException e) {
Expand All @@ -531,13 +532,13 @@ public Map<String, String> getNameIdData() {
*
*/
public String getNameId() {
final Map<String, String> nameIdData = getNameIdData();
String nameID = null;
if (!nameIdData.isEmpty()) {
LOGGER.debug("SAMLResponse has NameID --> {}", nameIdData.get("Value"));
nameID = nameIdData.get("Value");
final Map<String, String> data = getNameIdData();
String nameId = null;
if (!data.isEmpty()) {
LOGGER.debug("SAMLResponse has NameID --> {}", data.get("Value"));
nameId = data.get("Value");
}
return nameID;
return nameId;
}

/**
Expand All @@ -547,13 +548,13 @@ public String getNameId() {
*
*/
public String getNameIdFormat() {
final Map<String, String> nameIdData = getNameIdData();
String nameidFormat = null;
if (!nameIdData.isEmpty() && nameIdData.containsKey("Format")) {
LOGGER.debug("SAMLResponse has NameID Format --> {}", nameIdData.get("Format"));
nameidFormat = nameIdData.get("Format");
final Map<String, String> data = getNameIdData();
String nameIdFormat = null;
if (!data.isEmpty() && data.containsKey("Format")) {
LOGGER.debug("SAMLResponse has NameID Format --> {}", data.get("Format"));
nameIdFormat = data.get("Format");
}
return nameidFormat;
return nameIdFormat;
}

/**
Expand All @@ -563,11 +564,11 @@ public String getNameIdFormat() {
*
*/
public String getNameIdNameQualifier() {
final Map<String, String> nameIdData = getNameIdData();
final Map<String, String> data = getNameIdData();
String nameQualifier = null;
if (!nameIdData.isEmpty() && nameIdData.containsKey("NameQualifier")) {
LOGGER.debug("SAMLResponse has NameID NameQualifier --> " + nameIdData.get("NameQualifier"));
nameQualifier = nameIdData.get("NameQualifier");
if (!data.isEmpty() && data.containsKey("NameQualifier")) {
LOGGER.debug("SAMLResponse has NameID NameQualifier --> {}", data.get("NameQualifier"));
nameQualifier = data.get("NameQualifier");
}
return nameQualifier;
}
Expand All @@ -579,11 +580,11 @@ public String getNameIdNameQualifier() {
*
*/
public String getNameIdSPNameQualifier() {
final Map<String, String> nameIdData = getNameIdData();
final Map<String, String> data = getNameIdData();
String spNameQualifier = null;
if (!nameIdData.isEmpty() && nameIdData.containsKey("SPNameQualifier")) {
LOGGER.debug("SAMLResponse has NameID NameQualifier --> " + nameIdData.get("SPNameQualifier"));
spNameQualifier = nameIdData.get("SPNameQualifier");
if (!data.isEmpty() && data.containsKey("SPNameQualifier")) {
LOGGER.debug("SAMLResponse has NameID NameQualifier --> {}", data.get("SPNameQualifier"));
spNameQualifier = data.get("SPNameQualifier");
}
return spNameQualifier;
}
Expand All @@ -609,17 +610,17 @@ public Map<String, List<String>> getAttributes() {
ValidationException.DUPLICATED_ATTRIBUTE_NAME_FOUND);
}

final NodeList childrens = nodes.item(i).getChildNodes();
final NodeList children = nodes.item(i).getChildNodes();

List<String> attrValues = null;
if (attributes.containsKey(attName) && settings.isAllowRepeatAttributeName()) {
attrValues = attributes.get(attName);
} else {
attrValues = new ArrayList<>();
}
for (int j = 0; j < childrens.getLength(); j++) {
if ("AttributeValue".equals(childrens.item(j).getLocalName())) {
String attrValue = childrens.item(j).getTextContent();
for (int j = 0; j < children.getLength(); j++) {
if ("AttributeValue".equals(children.item(j).getLocalName())) {
String attrValue = children.item(j).getTextContent();
if (attrValue != null && settings.isTrimAttributeValues()) {
attrValue = attrValue.trim();
}
Expand All @@ -629,7 +630,7 @@ public Map<String, List<String>> getAttributes() {

attributes.put(attName, attrValues);
}
LOGGER.debug("SAMLResponse has attributes: {}", attributes.toString());
LOGGER.debug("SAMLResponse has attributes: {}", attributes);
} else {
LOGGER.debug("SAMLResponse has no attributes");
}
Expand Down Expand Up @@ -685,10 +686,7 @@ public static SamlResponseStatus getStatus(final Document dom) {
*/
public Boolean checkOneCondition() {
final NodeList entries = this.queryAssertion("/saml:Conditions");
if (entries.getLength() == 1) {
return true;
}
return false;
return entries.getLength() == 1;
}

/**
Expand All @@ -700,10 +698,7 @@ public Boolean checkOneCondition() {
*/
public Boolean checkOneAuthnStatement() {
final NodeList entries = this.queryAssertion("/saml:AuthnStatement");
if (entries.getLength() == 1) {
return true;
}
return false;
return entries.getLength() == 1;
}

/**
Expand Down Expand Up @@ -933,8 +928,8 @@ public boolean validateNumAssertions() {
*/
public ArrayList<String> processSignedElements() {
final ArrayList<String> signedElements = new ArrayList<>();
final ArrayList<String> verifiedSeis = new ArrayList<>();
final ArrayList<String> verifiedIds = new ArrayList<>();
final List<String> verifiedSeis = new ArrayList<>();
final List<String> verifiedIds = new ArrayList<>();

final NodeList signNodes = query("//ds:Signature", null);
for (int i = 0; i < signNodes.getLength(); i++) {
Expand Down Expand Up @@ -1011,7 +1006,7 @@ public boolean validateSignedElements(final ArrayList<String> signedElements) {
final Map<String, Integer> occurrences = new HashMap<>();
for (final String e : signedElements) {
if (occurrences.containsKey(e)) {
occurrences.put(e, occurrences.get(e).intValue() + 1);
occurrences.put(e, occurrences.get(e) + 1);
} else {
occurrences.put(e, 1);
}
Expand All @@ -1027,7 +1022,7 @@ public boolean validateSignedElements(final ArrayList<String> signedElements) {
}

// check that the signed elements found here, are the ones that will be verified
// by org.codelibs.saml2.core.core.util.Util.validateSign()
// by org.codelibs.saml2.core.util.Util.validateSign()
if (occurrences.containsKey(responseTag)) {
final NodeList expectedSignatureNode = query(Util.RESPONSE_SIGNATURE_XPATH, null);
if (expectedSignatureNode.getLength() != 1) {
Expand Down Expand Up @@ -1189,7 +1184,6 @@ protected NodeList queryAssertion(final String assertionXpath) {
* @return DOMNodeList The queried nodes
*/
protected NodeList query(final String nameQuery, final Node context) {
// LOGGER.debug("Executing query " + nameQuery);
return Util.query(getSAMLResponseDocument(), nameQuery, context);
}

Expand Down Expand Up @@ -1225,19 +1219,18 @@ private Document decryptAssertion(final Document dom) {
}

// We need to Remove the saml:EncryptedAssertion Node
final NodeList AssertionDataNodes = Util.query(dom, "/samlp:Response/saml:EncryptedAssertion/saml:Assertion");
if (AssertionDataNodes.getLength() == 0) {
final NodeList assertionDataNodes = Util.query(dom, "/samlp:Response/saml:EncryptedAssertion/saml:Assertion");
if (assertionDataNodes.getLength() == 0) {
throw new ValidationException("No /samlp:Response/saml:EncryptedAssertion/saml:Assertion element found",
ValidationException.MISSING_ENCRYPTED_ELEMENT);
}
final Node assertionNode = AssertionDataNodes.item(0);
final Node assertionNode = assertionDataNodes.item(0);
assertionNode.getParentNode().getParentNode().replaceChild(assertionNode, assertionNode.getParentNode());

// In order to avoid Signature Validation errors we need to rebuild the dom.
// https://groups.google.com/forum/#!topic/opensaml-users/gpXvwaZ53NA
final String xmlStr = Util.convertDocumentToString(dom);

// LOGGER.debug("Decrypted SAMLResponse --> " + xmlStr);
return Util.convertStringToDocument(xmlStr);
}

Expand Down
Loading
Loading