Skip to content

Various improvements for SignInWithEthereum Authentication #15649

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

Open
lachlan-roberts wants to merge 1 commit into
base: jetty-12.1.x
Choose a base branch
from
Open

Various improvements for SignInWithEthereum Authentication #15649

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
2 changes: 1 addition & 1 deletion documentation/jetty/modules/programming-guide/pages/security/siwe-support.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Authenticate New Users::

Domains::
* Init Param: org.eclipse.jetty.security.siwe.domains
* Description: This list of allowed domains to be declared in the `domain` field of the SIWE Message. If left blank this will allow all domains.
* Description: The list of allowed domains to be declared in the `domain` field of the SIWE Message. If left unconfigured, the `domain` field must match the authority (`host[:port]`) of the request being authenticated.

Chain IDs::
* Init Param: org.eclipse.jetty.security.siwe.chainIds
Expand Down
16 changes: 2 additions & 14 deletions jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,21 +94,9 @@ public void setLoginPath(String loginPath)
}
}

public boolean isLoginPage(String uri)
public boolean isLoginPage(String pathInContext)
{
return matchURI(uri, _loginPath);
}

private boolean matchURI(String uri, String path)
{
int jsc = uri.indexOf(path);
if (jsc < 0)
return false;
int e = jsc + path.length();
if (e == uri.length())
return true;
char c = uri.charAt(e);
return c == ';' || c == '#' || c == '/' || c == '?';
return pathInContext != null && pathInContext.equals(_loginPath);
}

public void setDispatch(boolean dispatch)
Expand Down
70 changes: 46 additions & 24 deletions ...s/jetty-ethereum/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,14 @@
import java.io.InputStream;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.function.Function;
import java.util.function.Predicate;

import org.eclipse.jetty.http.HttpException;
import org.eclipse.jetty.http.HttpHeader;
Expand Down Expand Up @@ -49,12 +52,12 @@
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.Session;
import org.eclipse.jetty.util.AsciiLowerCaseSet;
import org.eclipse.jetty.util.Blocker;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.CharsetStringBuilder.Iso88591StringBuilder;
import org.eclipse.jetty.util.Fields;
import org.eclipse.jetty.util.IncludeExcludeSet;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
import org.eclipse.jetty.util.UrlEncoded;
Expand Down Expand Up @@ -86,8 +89,8 @@ public class EthereumAuthenticator extends LoginAuthenticator implements Dumpabl
private static final String DEFAULT_NONCE_PATH = "/auth/nonce";
private static final String NONCE_SET_ATTR = "org.eclipse.jetty.security.siwe.nonce";

private final IncludeExcludeSet<String, String> _chainIds = new IncludeExcludeSet<>();
private final IncludeExcludeSet<String, String> _domains = new IncludeExcludeSet<>();
private final Set<String> _chainIds = new HashSet<>();
private final Set<String> _domains = new AsciiLowerCaseSet();

private String _loginPath;
private String _authenticationPath = DEFAULT_AUTHENTICATION_PATH;
Expand All @@ -105,12 +108,12 @@ public EthereumAuthenticator()

public void includeDomains(String... domains)
{
_domains.include(domains);
_domains.addAll(Arrays.asList(domains));
}

public void includeChainIds(String... chainIds)
{
_chainIds.include(chainIds);
_chainIds.addAll(Arrays.asList(chainIds));
}

@Override
Expand Down Expand Up @@ -522,7 +525,7 @@ private AuthenticationState validateSignInWithEthereumToken(SignInWithEthereumTo

try
{
siwe.validate(signedMessage, nonce -> redeemNonce(session, nonce), _domains, _chainIds);
siwe.validate(signedMessage, nonce -> redeemNonce(session, nonce), getDomainValidator(request), getChainIdValidator());
}
catch (Throwable t)
{
Expand All @@ -532,15 +535,36 @@ private AuthenticationState validateSignInWithEthereumToken(SignInWithEthereumTo
return null;
}

private Predicate<String> getChainIdValidator()
{
if (_chainIds.isEmpty())
return chainId -> true;
return _chainIds::contains;
}

private Predicate<String> getDomainValidator(Request request)
{
if (!_domains.isEmpty())
return _domains::contains;

// Default to use the authority of the request.
String host = Request.getServerName(request);
int port = Request.getServerPort(request);
int defaultPort = request.isSecure() ? 443 : 80;
String authority = StringUtil.isBlank(host) ? null
: (port <= 0 || port == defaultPort) ? host : host + ":" + port;
return domain -> authority != null && StringUtil.asciiEqualsIgnoreCase(authority, domain);
}

@Override
public AuthenticationState validateRequest(Request request, Response response, Callback callback) throws ServerAuthException
{
if (LOG.isDebugEnabled())
LOG.debug("validateRequest({},{})", request, response);

String uri = request.getHttpURI().toString();
if (uri == null)
uri = "/";
String pathInContext = Request.getPathInContext(request);
if (pathInContext == null)
pathInContext = "/";

try
{
Expand All @@ -552,9 +576,9 @@ public AuthenticationState validateRequest(Request request, Response response, C
return sendError(request, response, callback, "session could not be created");
}

if (isNonceRequest(uri))
if (isNonceRequest(pathInContext))
return handleNonceRequest(request, response, callback);
if (isAuthenticationRequest(uri))
if (isAuthenticationRequest(pathInContext))
{
if (LOG.isDebugEnabled())
LOG.debug("authentication request");
Expand Down Expand Up @@ -704,31 +728,29 @@ protected Fields getParameters(Request request)
}
}

public boolean isLoginPage(String uri)
public boolean isLoginPage(String pathInContext)
{
return matchURI(uri, _loginPath);
return pathInContext != null && pathInContext.equals(_loginPath);
}

public boolean isAuthenticationRequest(String uri)
public boolean isAuthenticationRequest(String pathInContext)
{
return matchURI(uri, _authenticationPath);
return matchURI(pathInContext, _authenticationPath);
}

public boolean isNonceRequest(String uri)
public boolean isNonceRequest(String pathInContext)
{
return matchURI(uri, _noncePath);
return matchURI(pathInContext, _noncePath);
}

private boolean matchURI(String uri, String path)
private boolean matchURI(String pathInContext, String path)
{
int jsc = uri.indexOf(path);
if (jsc < 0)
if (pathInContext == null || !pathInContext.startsWith(path))
return false;
int e = jsc + path.length();
if (e == uri.length())
int e = path.length();
if (e == pathInContext.length())
return true;
char c = uri.charAt(e);
return c == ';' || c == '#' || c == '/' || c == '?';
return pathInContext.charAt(e) == '/';
}

public boolean isErrorPage(String pathInContext)
Expand Down
16 changes: 8 additions & 8 deletions ...ereum/src/main/java/org/eclipse/jetty/security/siwe/internal/SignInWithEthereumToken.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import org.eclipse.jetty.security.ServerAuthException;
import org.eclipse.jetty.security.siwe.EthereumAuthenticator;
import org.eclipse.jetty.util.IncludeExcludeSet;
import org.eclipse.jetty.util.StringUtil;

/**
Expand Down Expand Up @@ -104,13 +103,14 @@ public static SignInWithEthereumToken from(String message)
/**
* @param signedMessage the {@link EthereumAuthenticator.SignedMessage}.
* @param validateNonce a {@link Predicate} used to validate the nonce.
* @param domains the {@link IncludeExcludeSet} used to validate the domain.
* @param chainIds the {@link IncludeExcludeSet} used to validate the chainId.
* @param validateDomain a {@link Predicate} used to validate the domain.
* @param validateChainId a {@link Predicate} used to validate the chainId.
* @throws ServerAuthException if the {@link EthereumAuthenticator.SignedMessage} fails validation.
*/
public void validate(EthereumAuthenticator.SignedMessage signedMessage, Predicate<String> validateNonce,
IncludeExcludeSet<String, String> domains,
IncludeExcludeSet<String, String> chainIds) throws ServerAuthException
public void validate(EthereumAuthenticator.SignedMessage signedMessage,
Predicate<String> validateNonce,
Predicate<String> validateDomain,
Predicate<String> validateChainId) throws ServerAuthException
{
if (validateNonce != null && !validateNonce.test(nonce()))
throw new ServerAuthException("invalid nonce " + nonce);
Expand All @@ -136,9 +136,9 @@ public void validate(EthereumAuthenticator.SignedMessage signedMessage, Predicat
throw new ServerAuthException("SIWE message not yet valid");
}

if (domains != null && !domains.test(domain()))
if (validateDomain != null && !validateDomain.test(domain()))
throw new ServerAuthException("unregistered domain: " + domain());
if (chainIds != null && !chainIds.test(chainId()))
if (validateChainId != null && !validateChainId.test(chainId()))
throw new ServerAuthException("unregistered chainId: " + chainId());
}
}
47 changes: 45 additions & 2 deletions .../jetty-ethereum/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,26 +232,69 @@ public void testEnforceDomain() throws Exception
assertThat(response.getContentAsString(), containsString("UserPrincipal: " + _credentials.getAddress()));
}

@Test
public void testDomainBoundToRequestByDefault() throws Exception
{
// A message asserting an unknown domain is rejected.
String nonce = getNonce();
String siweMessage = SignInWithEthereumGenerator.generateMessage(null, "example.com", _credentials.getAddress(), nonce);
ContentResponse response = sendAuthRequest(_credentials.signMessage(siweMessage));
assertThat(response.getStatus(), equalTo(HttpStatus.FORBIDDEN_403));
assertThat(response.getContentAsString(), containsString("unregistered domain"));

// A message asserting the request authority is accepted.
nonce = getNonce();
siweMessage = SignInWithEthereumGenerator.generateMessage(null, "localhost:" + _connector.getLocalPort(), _credentials.getAddress(), nonce);
response = sendAuthRequest(_credentials.signMessage(siweMessage));
assertThat(response.getStatus(), equalTo(HttpStatus.OK_200));
assertThat(response.getContentAsString(), containsString("UserPrincipal: " + _credentials.getAddress()));
}

@Test
public void testEnforceChainId() throws Exception
{
_authenticator.includeChainIds("1");
String domain = "localhost:" + _connector.getLocalPort();

// Test login with invalid chainId.
String nonce = getNonce();
String siweMessage = SignInWithEthereumGenerator.generateMessage(null, "localhost", _credentials.getAddress(), nonce, "2");
String siweMessage = SignInWithEthereumGenerator.generateMessage(null, domain, _credentials.getAddress(), nonce, "2");
ContentResponse response = sendAuthRequest(_credentials.signMessage(siweMessage));
assertThat(response.getStatus(), equalTo(HttpStatus.FORBIDDEN_403));
assertThat(response.getContentAsString(), containsString("unregistered chainId"));

// Test login with valid chainId.
nonce = getNonce();
siweMessage = SignInWithEthereumGenerator.generateMessage(null, "localhost", _credentials.getAddress(), nonce, "1");
siweMessage = SignInWithEthereumGenerator.generateMessage(null, domain, _credentials.getAddress(), nonce, "1");
response = sendAuthRequest(_credentials.signMessage(siweMessage));
assertThat(response.getStatus(), equalTo(HttpStatus.OK_200));
assertThat(response.getContentAsString(), containsString("UserPrincipal: " + _credentials.getAddress()));
}

@Test
public void testPathMatching() throws Exception
{
_client.setFollowRedirects(false);

// Protected resources containing substring of a protected path must redirect to /login.
for (String path : new String[]{"/admin/login", "/login/secret", "/admin/auth/login", "/admin/auth/nonce", "/admin/error"})
{
ContentResponse response = _client.GET("http://localhost:" + _connector.getLocalPort() + path);
assertTrue(HttpStatus.isRedirection(response.getStatus()), path + " HttpStatus was not redirect: " + response.getStatus());
assertThat(path, response.getHeaders().get(HttpHeader.LOCATION), equalTo("/login"));
}

// The exact login path is allowed without authentication.
ContentResponse response = _client.GET("http://localhost:" + _connector.getLocalPort() + "/login");
assertThat(response.getStatus(), equalTo(HttpStatus.OK_200));
assertThat(response.getContentAsString(), equalTo("Please Login"));

// The exact error path is allowed without authentication.
response = _client.GET("http://localhost:" + _connector.getLocalPort() + "/error?" + EthereumAuthenticator.ERROR_PARAMETER + "=oops");
assertThat(response.getStatus(), equalTo(HttpStatus.FORBIDDEN_403));
assertThat(response.getContentAsString(), equalTo("oops"));
}

private ContentResponse sendAuthRequest(EthereumAuthenticator.SignedMessage signedMessage) throws ExecutionException, InterruptedException, TimeoutException
{
MultiPartRequestContent content = new MultiPartRequestContent();
Expand Down
Loading