Skip to content
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.

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

Refactored out Servlet dependencies from core and toolkit #395

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<parent>
<groupId>com.onelogin</groupId>
<artifactId>java-saml-toolkit</artifactId>
<version>2.9.1-SNAPSHOT</version>
<version>3.0.0-SNAPSHOT</version>
</parent>

<packaging>jar</packaging>
Expand All @@ -27,15 +27,23 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>3.12.4</version>
<scope>test</scope>
</dependency>

<!-- for log -->
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>2.0.5</version>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<version>1.4.7</version>
<optional>true</optional>
</dependency>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.xpath.XPathExpressionException;

import com.onelogin.saml2.http.HttpRequest;
import com.onelogin.saml2.model.hsm.HSM;

import org.apache.commons.lang3.StringUtils;
Expand All @@ -28,7 +29,6 @@
import org.xml.sax.SAXException;
import com.onelogin.saml2.exception.SettingsException;
import com.onelogin.saml2.exception.ValidationError;
import com.onelogin.saml2.http.HttpRequest;
import com.onelogin.saml2.model.SamlResponseStatus;
import com.onelogin.saml2.model.SubjectConfirmationIssue;
import com.onelogin.saml2.settings.Saml2Settings;
Expand Down
203 changes: 28 additions & 175 deletions core/src/main/java/com/onelogin/saml2/http/HttpRequest.java
Original file line number Diff line number Diff line change
@@ -1,150 +1,56 @@
package com.onelogin.saml2.http;

import static com.onelogin.saml2.util.Preconditions.checkNotNull;
import static java.util.Collections.unmodifiableList;
import static java.util.Collections.unmodifiableMap;
import com.onelogin.saml2.util.Util;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.apache.commons.lang3.StringUtils;

import com.onelogin.saml2.util.Util;

/**
* Framework-agnostic representation of an HTTP request.
* Framework-agnostic definition of an HTTP request with a very minimal set of
* methods needed to support the SAML handshake.
*
* @since 2.0.0
* @since 3.0.0
*/
public final class HttpRequest {
public interface HttpRequest {
Copy link
Author

@markkolich markkolich Mar 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK calling this interface SamlHttpRequest or OneLoginSamlHttpRequest if the existing name is too generic.

Three implementors in this PR:

image


public static final Map<String, List<String>> EMPTY_PARAMETERS = Collections.<String, List<String>>emptyMap();
int getServerPort();

private final String requestURL;
private final Map<String, List<String>> parameters;
private final String queryString;
String getScheme();

/**
* Creates a new HttpRequest.
*
* @param requestURL the request URL (up to but not including query parameters)
* @throws NullPointerException if requestURL is null
* @deprecated Not providing a queryString can cause HTTP Redirect binding to fail.
*/
@Deprecated
public HttpRequest(String requestURL) {
this(requestURL, EMPTY_PARAMETERS);
}
String getServerName();

/**
* Creates a new HttpRequest.
*
* @param requestURL the request URL (up to but not including query parameters)
* @param queryString string that is contained in the request URL after the path
*/
public HttpRequest(String requestURL, String queryString) {
this(requestURL, EMPTY_PARAMETERS, queryString);
}
String getRequestURL();

/**
* Creates a new HttpRequest.
*
* @param requestURL the request URL (up to but not including query parameters)
* @param parameters the request query parameters
* @throws NullPointerException if any of the parameters is null
* @deprecated Not providing a queryString can cause HTTP Redirect binding to fail.
*/
@Deprecated
public HttpRequest(String requestURL, Map<String, List<String>> parameters) {
this(requestURL, parameters, null);
}
String getRequestURI();

/**
* Creates a new HttpRequest.
*
* @param requestURL the request URL (up to but not including query parameters)
* @param parameters the request query parameters
* @param queryString string that is contained in the request URL after the path
* @throws NullPointerException if any of the parameters is null
*/
public HttpRequest(String requestURL, Map<String, List<String>> parameters, String queryString) {
this.requestURL = checkNotNull(requestURL, "requestURL");
this.parameters = unmodifiableCopyOf(checkNotNull(parameters, "queryParams"));
this.queryString = StringUtils.trimToEmpty(queryString);
}

/**
* @param name the query parameter name
* @param value the query parameter value
* @return a new HttpRequest with the given query parameter added
* @throws NullPointerException if any of the parameters is null
*/
public HttpRequest addParameter(String name, String value) {
checkNotNull(name, "name");
checkNotNull(value, "value");
String getQueryString();

final List<String> oldValues = parameters.containsKey(name) ? parameters.get(name) : new ArrayList<String>();
final List<String> newValues = new ArrayList<>(oldValues);
newValues.add(value);
final Map<String, List<String>> params = new HashMap<>(parameters);
params.put(name, newValues);

return new HttpRequest(requestURL, params, queryString);
}
void invalidateSession();

/**
* @param name the query parameter name
* @return a new HttpRequest with the given query parameter removed
* @throws NullPointerException if any of the parameters is null
*/
public HttpRequest removeParameter(String name) {
checkNotNull(name, "name");
Map<String, String[]> getParameterMap();

final Map<String, List<String>> params = new HashMap<>(parameters);
params.remove(name);
default List<String> getParameters(String name) {
final Map<String, String[]> paramsAsArray = getParameterMap();
final Map<String, List<String>> paramsAsList = new HashMap<>();
for (Map.Entry<String, String[]> param : paramsAsArray.entrySet()) {
paramsAsList.put(param.getKey(), Arrays.asList(param.getValue()));
}

return new HttpRequest(requestURL, params, queryString);
}

/**
* The URL the client used to make the request. Includes a protocol, server name, port number, and server path, but
* not the query string parameters.
*
* @return the request URL
*/
public String getRequestURL() {
return requestURL;
return paramsAsList.get(name);
}

/**
* @param name the query parameter name
* @return the first value for the parameter, or null
*/
public String getParameter(String name) {
default String getParameter(String name) {
List<String> values = getParameters(name);
return values.isEmpty() ? null : values.get(0);
}

/**
* @param name the query parameter name
* @return a List containing all values for the parameter
*/
public List<String> getParameters(String name) {
List<String> values = parameters.get(name);
return values != null ? values : Collections.<String>emptyList();
return (values == null || values.isEmpty()) ? null : values.get(0);
}

/**
* @return a map of all query parameters
*/
public Map<String, List<String>> getParameters() {
return parameters;
default String getEncodedParameter(String name, String defaultValue) {
String value = getEncodedParameter(name);
return (value != null) ? value : Util.urlEncoder(defaultValue);
}

/**
Expand All @@ -155,7 +61,8 @@ public Map<String, List<String>> getParameters() {
* @param name
* @return the first value for the parameter, or null
*/
public String getEncodedParameter(String name) {
default String getEncodedParameter(String name) {
String queryString = getQueryString();
Matcher matcher = Pattern.compile(Pattern.quote(name) + "=([^&#]+)").matcher(queryString);
if (matcher.find()) {
return matcher.group(1);
Expand All @@ -164,58 +71,4 @@ public String getEncodedParameter(String name) {
}
}

/**
* Return an url encoded get parameter value
* Prefer to extract the original encoded value directly from queryString since url
* encoding is not canonical.
*
* @param name
* @param defaultValue
* @return the first value for the parameter, or url encoded default value
*/
public String getEncodedParameter(String name, String defaultValue) {
String value = getEncodedParameter(name);
return (value != null ? value : Util.urlEncoder(defaultValue));
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}

if (o == null || getClass() != o.getClass()) {
return false;
}

HttpRequest that = (HttpRequest) o;
return Objects.equals(requestURL, that.requestURL) &&
Objects.equals(parameters, that.parameters) &&
Objects.equals(queryString, that.queryString);
}

@Override
public int hashCode() {
return Objects.hash(requestURL, parameters, queryString);
}

@Override
public String toString() {
return "HttpRequest{" +
"requestURL='" + requestURL + '\'' +
", parameters=" + parameters +
", queryString=" + queryString +
'}';
}

private static Map<String, List<String>> unmodifiableCopyOf(Map<String, List<String>> orig) {
Map<String, List<String>> copy = new HashMap<>();
for (Map.Entry<String, List<String>> entry : orig.entrySet()) {
copy.put(entry.getKey(), unmodifiableList(new ArrayList<>(entry.getValue())));
}

return unmodifiableMap(copy);
}


}
83 changes: 83 additions & 0 deletions core/src/main/java/com/onelogin/saml2/http/HttpRequestUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package com.onelogin.saml2.http;

import org.apache.commons.lang3.StringUtils;

public class HttpRequestUtils {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see this class used anywhere in core. It should either be moved or removed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used in toolkit but not core.

However, I kept the utils in core so that they stay near the HttpRequest and HttpResponse interfaces given the close relationship between these classes and their usage:

image


private HttpRequestUtils() {
}

/**
* Returns the protocol + the current host + the port (if different than
* common ports).
*
* @param request
* HttpServletRequest object to be processed
*
* @return the HOST URL
*/
public static String getSelfURLhost(HttpRequest request) {
String hostUrl = StringUtils.EMPTY;
final int serverPort = request.getServerPort();
if ((serverPort == 80) || (serverPort == 443) || serverPort == 0) {
hostUrl = String.format("%s://%s", request.getScheme(), request.getServerName());
} else {
hostUrl = String.format("%s://%s:%s", request.getScheme(), request.getServerName(), serverPort);
}
return hostUrl;
}

/**
* Returns the URL of the current context + current view + query
*
* @param request
* HttpServletRequest object to be processed
*
* @return current context + current view + query
*/
public static String getSelfURL(HttpRequest request) {
String url = getSelfURLhost(request);

String requestUri = request.getRequestURI();
String queryString = request.getQueryString();

if (null != requestUri && !requestUri.isEmpty()) {
url += requestUri;
}

if (null != queryString && !queryString.isEmpty()) {
url += '?' + queryString;
}
return url;
}

/**
* Returns the URL of the current host + current view.
*
* @param request
* HttpServletRequest object to be processed
*
* @return current host + current view
*/
public static String getSelfURLNoQuery(HttpRequest request) {
return request.getRequestURL();
}

/**
* Returns the routed URL of the current host + current view.
*
* @param request
* HttpServletRequest object to be processed
*
* @return the current routed url
*/
public static String getSelfRoutedURLNoQuery(HttpRequest request) {
String url = getSelfURLhost(request);
String requestUri = request.getRequestURI();
if (null != requestUri && !requestUri.isEmpty()) {
url += requestUri;
}
return url;
}

}
15 changes: 15 additions & 0 deletions core/src/main/java/com/onelogin/saml2/http/HttpResponse.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.onelogin.saml2.http;

import java.io.IOException;

/**
* Framework-agnostic definition of an HTTP response with a very minimal set of
* methods needed to support the SAML handshake.
*
* @since 3.0.0
*/
public interface HttpResponse {
Copy link
Author

@markkolich markkolich Mar 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK calling this interface SamlHttpResponse or OneLoginSamlHttpResponse if the existing name is too generic.

Two implementors in this PR:

image


void sendRedirect(String location) throws IOException;

}
Loading