PR #11883 - changes from review

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
jetty · Jun 19, 2024 · 5db8534 · 5db8534
1 parent 0974f96
commit 5db8534
Show file tree

Hide file tree

Showing 14 changed files with 65 additions and 67 deletions.
diff --git a/jetty-core/jetty-siwe/pom.xml b/jetty-core/jetty-siwe/pom.xml
@@ -15,26 +15,6 @@
     <bundle-symbolic-name>${project.groupId}.siwe</bundle-symbolic-name>
   </properties>
 
-  <dependencyManagement>
-    <dependencies>
-      <dependency>
-        <groupId>org.jetbrains.kotlin</groupId>
-        <artifactId>kotlin-stdlib-common</artifactId>
-        <version>1.9.10</version>
-      </dependency>
-      <dependency>
-        <groupId>org.jetbrains.kotlin</groupId>
-        <artifactId>kotlin-stdlib-jdk7</artifactId>
-        <version>1.9.10</version>
-      </dependency>
-      <dependency>
-        <groupId>org.jetbrains.kotlin</groupId>
-        <artifactId>kotlin-stdlib-jdk8</artifactId>
-        <version>1.9.10</version>
-      </dependency>
-    </dependencies>
-  </dependencyManagement>
-
   <dependencies>
     <dependency>
       <groupId>org.eclipse.jetty</groupId>

diff --git a/jetty-core/jetty-siwe/src/main/java/module-info.java b/jetty-core/jetty-siwe/src/main/java/module-info.java
@@ -0,0 +1,21 @@
+//
+// ========================================================================
+// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+module org.eclipse.jetty.siwe
+{
+    requires transitive org.eclipse.jetty.security;
+    requires crypto;
+    requires org.bouncycastle.provider;
+
+    exports org.eclipse.jetty.security.siwe;
+}
diff --git a/...-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java b/...-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java
@@ -39,6 +39,10 @@
 import org.eclipse.jetty.security.UserIdentity;
 import org.eclipse.jetty.security.authentication.LoginAuthenticator;
 import org.eclipse.jetty.security.authentication.SessionAuthentication;
+import org.eclipse.jetty.security.siwe.internal.AnyUserLoginService;
+import org.eclipse.jetty.security.siwe.internal.EthereumUtil;
+import org.eclipse.jetty.security.siwe.internal.SignInWithEthereumParser;
+import org.eclipse.jetty.security.siwe.internal.SignInWithEthereumToken;
 import org.eclipse.jetty.server.FormFields;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Response;
@@ -61,7 +65,7 @@ public class EthereumAuthenticator extends LoginAuthenticator
     private static final Logger LOG = LoggerFactory.getLogger(EthereumAuthenticator.class);
 
     public static final String LOGIN_PATH_PARAM = "org.eclipse.jetty.security.siwe.login_path";
-    public static final String AUTH_PATH_PARAM = "org.eclipse.jetty.security.siwe.auth_path";
+    public static final String AUTHENTICATION_PATH_PARAM = "org.eclipse.jetty.security.siwe.authentication_path";
     public static final String NONCE_PATH_PARAM = "org.eclipse.jetty.security.siwe.nonce_path";
     public static final String MAX_MESSAGE_SIZE_PARAM = "org.eclipse.jetty.security.siwe.max_message_size";
     public static final String LOGOUT_REDIRECT_PARAM = "org.eclipse.jetty.security.siwe.logout_redirect_path";
@@ -71,7 +75,7 @@ public class EthereumAuthenticator extends LoginAuthenticator
     public static final String J_POST = "org.eclipse.jetty.security.siwe.POST";
     public static final String J_METHOD = "org.eclipse.jetty.security.siwe.METHOD";
     public static final String ERROR_PARAMETER = "error_description_jetty";
-    private static final String DEFAULT_AUTH_PATH = "/auth/login";
+    private static final String DEFAULT_AUTHENTICATION_PATH = "/auth/login";
     private static final String DEFAULT_NONCE_PATH = "/auth/nonce";
     private static final String NONCE_SET_ATTR = "org.eclipse.jetty.security.siwe.nonce";
 
@@ -80,7 +84,7 @@ public class EthereumAuthenticator extends LoginAuthenticator
     private final IncludeExcludeSet<String, String> _domains = new IncludeExcludeSet<>();
 
     private String _loginPath;
-    private String _authPath = DEFAULT_AUTH_PATH;
+    private String _authenticationPath = DEFAULT_AUTHENTICATION_PATH;
     private String _noncePath = DEFAULT_NONCE_PATH;
     private long _maxMessageSize = 4 * 1024;
     private String _logoutRedirectPath;
@@ -115,9 +119,9 @@ public void setConfiguration(Authenticator.Configuration authConfig)
         if (loginPath != null)
             setLoginPath(loginPath);
 
-        String authPath = authConfig.getParameter(AUTH_PATH_PARAM);
-        if (authPath != null)
-            setAuthPath(authPath);
+        String authenticationPath = authConfig.getParameter(AUTHENTICATION_PATH_PARAM);
+        if (authenticationPath != null)
+            setAuthenticationPath(authenticationPath);
 
         String noncePath = authConfig.getParameter(NONCE_PATH_PARAM);
         if (noncePath != null)
@@ -166,7 +170,7 @@ public void setLoginPath(String loginPath)
     {
         if (loginPath == null)
         {
-            LOG.warn("login path must not be null, defaulting to " + _loginPath);
+            LOG.warn("login path must not be null, defaulting to {}", _loginPath);
             loginPath = _loginPath;
         }
         else if (!loginPath.startsWith("/"))
@@ -178,33 +182,33 @@ else if (!loginPath.startsWith("/"))
         _loginPath = loginPath;
     }
 
-    public void setAuthPath(String authPath)
+    public void setAuthenticationPath(String authenticationPath)
     {
-        if (authPath == null)
+        if (authenticationPath == null)
         {
-            authPath = _authPath;
-            LOG.warn("login path must not be null, defaulting to " + authPath);
+            authenticationPath = _authenticationPath;
+            LOG.warn("authentication path must not be null, defaulting to {}", authenticationPath);
         }
-        else if (!authPath.startsWith("/"))
+        else if (!authenticationPath.startsWith("/"))
         {
-            authPath = "/" + authPath;
-            LOG.warn("login path must start with /");
+            authenticationPath = "/" + authenticationPath;
+            LOG.warn("authentication path must start with /");
         }
 
-        _authPath = authPath;
+        _authenticationPath = authenticationPath;
     }
 
     public void setNoncePath(String noncePath)
     {
         if (noncePath == null)
         {
             noncePath = _noncePath;
-            LOG.warn("login path must not be null, defaulting to " + noncePath);
+            LOG.warn("nonce path must not be null, defaulting to {}", noncePath);
         }
         else if (!noncePath.startsWith("/"))
         {
             noncePath = "/" + noncePath;
-            LOG.warn("login path must start with /");
+            LOG.warn("nonce path must start with /");
         }
 
         _noncePath = noncePath;
@@ -222,12 +226,7 @@ public void setDispatch(boolean dispatch)
 
     public void setLogoutRedirectPath(String logoutRedirectPath)
     {
-        if (logoutRedirectPath == null)
-        {
-            LOG.warn("logout redirect path must not be null, defaulting to /");
-            logoutRedirectPath = "/";
-        }
-        else if (!logoutRedirectPath.startsWith("/"))
+        if (logoutRedirectPath != null && !logoutRedirectPath.startsWith("/"))
         {
             LOG.warn("logout redirect path must start with /");
             logoutRedirectPath = "/" + logoutRedirectPath;
@@ -692,7 +691,7 @@ public boolean isLoginPage(String uri)
 
     public boolean isAuthenticationRequest(String uri)
     {
-        return matchURI(uri, _authPath);
+        return matchURI(uri, _authenticationPath);
     }
 
     public boolean isNonceRequest(String uri)

diff --git a/jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/SignedMessage.java b/jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/SignedMessage.java
@@ -13,6 +13,8 @@
 
 package org.eclipse.jetty.security.siwe;
 
+import org.eclipse.jetty.security.siwe.internal.EthereumSignatureVerifier;
+
 public record SignedMessage(String message, String signature)
 {
     public String recoverAddress()

diff --git a/...ty/security/siwe/AnyUserLoginService.java → ...ty/siwe/internal/AnyUserLoginService.java b/...ty/security/siwe/AnyUserLoginService.java → ...ty/siwe/internal/AnyUserLoginService.java
@@ -11,7 +11,7 @@
 // ========================================================================
 //
 
-package org.eclipse.jetty.security.siwe;
+package org.eclipse.jetty.security.siwe.internal;
 
 import java.util.function.Function;
 import javax.security.auth.Subject;

diff --git a/...urity/siwe/EthereumSignatureVerifier.java → ...e/internal/EthereumSignatureVerifier.java b/...urity/siwe/EthereumSignatureVerifier.java → ...e/internal/EthereumSignatureVerifier.java
@@ -11,7 +11,7 @@
 // ========================================================================
 //
 
-package org.eclipse.jetty.security.siwe;
+package org.eclipse.jetty.security.siwe.internal;
 
 import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;

diff --git a/...pse/jetty/security/siwe/EthereumUtil.java → .../security/siwe/internal/EthereumUtil.java b/...pse/jetty/security/siwe/EthereumUtil.java → .../security/siwe/internal/EthereumUtil.java
@@ -11,7 +11,7 @@
 // ========================================================================
 //
 
-package org.eclipse.jetty.security.siwe;
+package org.eclipse.jetty.security.siwe.internal;
 
 import java.security.SecureRandom;
 
@@ -20,6 +20,10 @@ public class EthereumUtil
     private static final String NONCE_CHARACTERS = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
     private static final SecureRandom RANDOM = new SecureRandom();
 
+    private EthereumUtil()
+    {
+    }
+
     public static String createNonce()
     {
         StringBuilder builder = new StringBuilder(8);

diff --git a/...curity/siwe/SignInWithEthereumParser.java → ...we/internal/SignInWithEthereumParser.java b/...curity/siwe/SignInWithEthereumParser.java → ...we/internal/SignInWithEthereumParser.java
@@ -11,7 +11,7 @@
 // ========================================================================
 //
 
-package org.eclipse.jetty.security.siwe;
+package org.eclipse.jetty.security.siwe.internal;
 
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;

diff --git a/...ecurity/siwe/SignInWithEthereumToken.java → ...iwe/internal/SignInWithEthereumToken.java b/...ecurity/siwe/SignInWithEthereumToken.java → ...iwe/internal/SignInWithEthereumToken.java
@@ -11,13 +11,14 @@
 // ========================================================================
 //
 
-package org.eclipse.jetty.security.siwe;
+package org.eclipse.jetty.security.siwe.internal;
 
 import java.time.LocalDateTime;
 import java.time.format.DateTimeFormatter;
 import java.util.function.Predicate;
 
 import org.eclipse.jetty.security.ServerAuthException;
+import org.eclipse.jetty.security.siwe.SignedMessage;
 import org.eclipse.jetty.util.IncludeExcludeSet;
 import org.eclipse.jetty.util.StringUtil;
 
@@ -72,20 +73,4 @@ public void validate(SignedMessage signedMessage, Predicate<String> validateNonc
         if (chainIds != null && !chainIds.test(chainId()))
             throw new ServerAuthException("unregistered chainId");
     }
-
-    @Override
-    public String toString()
-    {
-        return String.format(
-            "Scheme: %s" +
-                "%nDomain: %s" +
-                "%nAddress: %s" +
-                "%nURI: %s" +
-                "%nVersion: %s" +
-                "%nChainID: %s" +
-                "%nNonce: %s" +
-                "%nIssuedAt: %s" +
-                "%nStatement: %s",
-            scheme, domain, address, uri, version, chainId, nonce, issuedAt, statement);
-    }
 }
diff --git a/...etty-siwe/src/test/java/org/eclipse/jetty/security/siwe/SightInWithEthereumTokenTest.java b/...etty-siwe/src/test/java/org/eclipse/jetty/security/siwe/SightInWithEthereumTokenTest.java
@@ -16,6 +16,9 @@
 import java.time.LocalDateTime;
 import java.util.function.Predicate;
 
+import org.eclipse.jetty.security.siwe.internal.EthereumUtil;
+import org.eclipse.jetty.security.siwe.internal.SignInWithEthereumParser;
+import org.eclipse.jetty.security.siwe.internal.SignInWithEthereumToken;
 import org.eclipse.jetty.security.siwe.util.EthereumCredentials;
 import org.eclipse.jetty.security.siwe.util.SignInWithEthereumGenerator;
 import org.eclipse.jetty.util.IncludeExcludeSet;

diff --git a/...etty-siwe/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumParserTest.java b/...etty-siwe/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumParserTest.java
@@ -19,6 +19,9 @@
 import java.util.List;
 import java.util.stream.Stream;
 
+import org.eclipse.jetty.security.siwe.internal.EthereumUtil;
+import org.eclipse.jetty.security.siwe.internal.SignInWithEthereumParser;
+import org.eclipse.jetty.security.siwe.internal.SignInWithEthereumToken;
 import org.eclipse.jetty.security.siwe.util.SignInWithEthereumGenerator;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;

diff --git a/...e/jetty-siwe/src/test/java/org/eclipse/jetty/security/siwe/SignatureVerificationTest.java b/...e/jetty-siwe/src/test/java/org/eclipse/jetty/security/siwe/SignatureVerificationTest.java
@@ -13,6 +13,7 @@
 
 package org.eclipse.jetty.security.siwe;
 
+import org.eclipse.jetty.security.siwe.internal.EthereumSignatureVerifier;
 import org.eclipse.jetty.security.siwe.util.EthereumCredentials;
 import org.eclipse.jetty.security.siwe.util.SignInWithEthereumGenerator;
 import org.junit.jupiter.api.Test;

diff --git a/...re/jetty-siwe/src/test/java/org/eclipse/jetty/security/siwe/util/EthereumCredentials.java b/...re/jetty-siwe/src/test/java/org/eclipse/jetty/security/siwe/util/EthereumCredentials.java
@@ -15,8 +15,8 @@
 
 import java.nio.charset.StandardCharsets;
 
-import org.eclipse.jetty.security.siwe.EthereumSignatureVerifier;
 import org.eclipse.jetty.security.siwe.SignedMessage;
+import org.eclipse.jetty.security.siwe.internal.EthereumSignatureVerifier;
 import org.web3j.crypto.Credentials;
 import org.web3j.crypto.ECKeyPair;
 import org.web3j.crypto.Keys;

diff --git a/...-siwe/src/test/java/org/eclipse/jetty/security/siwe/util/SignInWithEthereumGenerator.java b/...-siwe/src/test/java/org/eclipse/jetty/security/siwe/util/SignInWithEthereumGenerator.java
@@ -16,7 +16,7 @@
 import java.time.LocalDateTime;
 import java.time.format.DateTimeFormatter;
 
-import org.eclipse.jetty.security.siwe.EthereumUtil;
+import org.eclipse.jetty.security.siwe.internal.EthereumUtil;
 
 public class SignInWithEthereumGenerator
 {