Issue #11560

lachlan-roberts · 2024-06-06T15:07:20Z

Add new Jetty module called jetty-siwe, which implements EIP4361 Sign-In With Ethereum via the EthereumAuthenticator. This allows you to authenticate with a crypto wallet such as MetaMask.

See https://eips.ethereum.org/EIPS/eip-4361

lachlan-roberts · 2024-06-06T15:08:46Z

This is ready for early reviews as the implementation is complete.

To test this yourself you can install MetaMask browser extension and run org.eclipse.jetty.security.siwe.example.SignInWithEthereumEmbeddedExample.

TODOs:

~~Write Javadoc.~~
~~Add jetty-home module.~~
~~Add new section in the documentation.~~
~~Extract out the multipart changes to separate PR and use application/x-www-form-urlencoded instead.~~
~~Fix issues with module-info.java~~

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

sbordet

Please don't use LOG.warn() especially if you can default the value.

Also, try to make all classes internal if possible, and leave exported only those that are strictly necessary.

If possible, make it asynchronous.
I would prefer to avoid that half-sent messages to block threads for the whole idle timeout.

jetty-core/jetty-siwe/pom.xml

jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/AnyUserLoginService.java

jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java

...y-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/SignInWithEthereumToken.java

jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/SignedMessage.java

...-core/jetty-siwe/src/test/java/org/eclipse/jetty/security/siwe/util/EthereumCredentials.java

...tty-siwe/src/test/java/org/eclipse/jetty/security/siwe/util/SignInWithEthereumGenerator.java

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

janbartel

Need to do some doco as part of this pr too ;)

jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java

...e/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/internal/AnyUserLoginService.java

...y-siwe/src/main/java/org/eclipse/jetty/security/siwe/internal/EthereumSignatureVerifier.java

...e/jetty-siwe/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumParserTest.java

jetty-core/jetty-siwe/src/test/java/org/eclipse/jetty/security/siwe/SignInWithEthereumTest.java

...src/test/java/org/eclipse/jetty/security/siwe/example/SignInWithEthereumEmbeddedExample.java

jetty-core/jetty-siwe/src/test/resources/login.html

jetty-core/pom.xml

jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

gregw · 2024-07-08T05:22:57Z

@lachlan-roberts forced push????? what??!?!?!?!? Now I've lost all my review context! IMNSHO you should only ever force push when you've screwed up.... or put the other way, if I see a forced push I KNOW you've screwed up !)

We went looking everywhere, but couldn’t find those commits.
Sometimes commits can disappear after a force-push. Head back to the latest changes here.

gregw

Mostly good, but the agro from the forced push is making me set a high bar for javadoc, formatting, naming and layout.!

jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/SignedMessage.java

...y-siwe/src/main/java/org/eclipse/jetty/security/siwe/internal/EthereumSignatureVerifier.java

jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/internal/EthereumUtil.java

...ty-siwe/src/main/java/org/eclipse/jetty/security/siwe/internal/SignInWithEthereumParser.java

...tty-siwe/src/main/java/org/eclipse/jetty/security/siwe/internal/SignInWithEthereumToken.java

...e/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/internal/AnyUserLoginService.java

...src/test/java/org/eclipse/jetty/security/siwe/example/SignInWithEthereumEmbeddedExample.java

...-core/jetty-siwe/src/test/java/org/eclipse/jetty/security/siwe/util/EthereumCredentials.java

...tty-siwe/src/test/java/org/eclipse/jetty/security/siwe/util/SignInWithEthereumGenerator.java

jetty-core/jetty-siwe/src/test/resources/login.html

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

…SignInWithEthereum

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/SecurityHandler.java

jetty-core/jetty-siwe/src/main/java/module-info.java

jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java

janbartel · 2024-07-24T00:19:27Z

jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java

+            @SuppressWarnings("unchecked")
+            Set<String> attribute = (Set<String>)session.getAttribute(NONCE_SET_ATTR);
+            if (attribute == null)
+                session.setAttribute(NONCE_SET_ATTR, attribute = new FixedSizeSet<>(5));


Why do we need the FixedSizeSet class? Isn't this just setting a single element?

Probably, but I've had issues with openId where multiple authentication requests come in from the browser and then one of them fails because the nonce is forgotten and causes the other ones to fail.

...src/test/java/org/eclipse/jetty/security/siwe/example/SignInWithEthereumEmbeddedExample.java

jetty-core/jetty-siwe/src/test/resources/login.html

jetty-ee10/jetty-ee10-tests/jetty-ee10-test-siwe-webapp/src/main/webapp/login.html

...est-ee10-distribution/src/test/java/org/eclipse/jetty/ee10/tests/distribution/SiweTests.java

...e/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/internal/AnyUserLoginService.java

...ty-siwe/src/main/java/org/eclipse/jetty/security/siwe/internal/SignInWithEthereumParser.java

...tty-siwe/src/main/java/org/eclipse/jetty/security/siwe/internal/SignInWithEthereumToken.java

...y-siwe/src/main/resources/META-INF/services/org.eclipse.jetty.security.Authenticator$Factory

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

…SignInWithEthereum

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

gregw · 2024-08-21T05:37:02Z

documentation/jetty/modules/programming-guide/pages/security/index.adoc

+
+= Jetty Security
+
+TODO: introduction


Is that TODO shown in the resulting documentation, or is it treated like a comment?

It shows the todo in the documentation, but the documentation is very incomplete and doesn't yet have any other security modules. And a bunch of the other main headers have the same TODOs for the introduction page.

But I will remove this index file and just have a header without an intro page for this.

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

…SignInWithEthereum

lachlan-roberts · 2024-08-22T04:11:13Z

replaced by #12188

lachlan-roberts self-assigned this Jun 6, 2024

lachlan-roberts mentioned this pull request Jun 6, 2024

Implement Web3 OAuth/Sign-In Functionality #11560

Closed

lachlan-roberts added a commit that referenced this pull request Jun 7, 2024

PR #11883 - add support for login with application/x-www-form-urlencoded

478d6aa

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

lachlan-roberts requested review from gregw, sbordet and janbartel and removed request for gregw June 11, 2024 14:13

lachlan-roberts mentioned this pull request Jun 11, 2024

Improve support for MultiPart in jetty-core #11906

Merged

lachlan-roberts added a commit that referenced this pull request Jun 11, 2024

PR #11883 - remove Multipart usage from jetty-siwe

abba112

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

lachlan-roberts added a commit that referenced this pull request Jun 11, 2024

PR #11883 - remove Multipart usage from jetty-siwe

0974f96

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

sbordet requested changes Jun 17, 2024

View reviewed changes

lachlan-roberts added a commit that referenced this pull request Jun 19, 2024

PR #11883 - changes from review

5db8534

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

lachlan-roberts added a commit that referenced this pull request Jun 19, 2024

PR #11883 - fix jetty-siwe pom.xml

0904759

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

janbartel requested changes Jun 19, 2024

View reviewed changes

gregw requested changes Jun 20, 2024

View reviewed changes

jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java Show resolved Hide resolved

jetty-core/jetty-siwe/src/main/java/org/eclipse/jetty/security/siwe/EthereumAuthenticator.java Show resolved Hide resolved

lachlan-roberts added a commit that referenced this pull request Jun 20, 2024

PR #11883 - updated AnyUserLoginService to support nested login service

0040844

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

lachlan-roberts added a commit that referenced this pull request Jun 20, 2024

PR #11883 - changes from review

78403ec

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

gregw mentioned this pull request Jul 3, 2024

map Jakarta.servlet.http.HttpServletRequest to org.eclipse.jetty.server.Request #11964

Closed

Issue #11560 - Implement EIP-4361 Sign-In With Ethereum

436ca41

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

lachlan-roberts force-pushed the jetty-12.0.x-SignInWithEthereum branch from 78403ec to 436ca41 Compare July 5, 2024 05:47

lachlan-roberts added 3 commits July 5, 2024 17:13

PR #11883 - javadoc and code cleanup

90e5919

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

PR #11883 - test fixes and cleanup

ac5925a

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

PR #11883 - fixes to EthereumCredentials

70e6192

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

gregw requested changes Jul 8, 2024

View reviewed changes

lachlan-roberts and others added 3 commits July 8, 2024 16:21

Add openid and siwe documentation sections.

614025a

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12.0.x-…

4006228

…SignInWithEthereum

update version for jetty-siwe pom.xml

225e89c

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

lachlan-roberts added 4 commits July 15, 2024 12:03

add jetty module for siwe

aa945d5

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

add siwe.mod, distribution tests and documentation

52c6c88

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

changes from review

9ea7431

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

add javadoc for test classes

32b7043

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

lachlan-roberts marked this pull request as ready for review July 23, 2024 08:15

lachlan-roberts requested review from gregw, sbordet and janbartel July 23, 2024 08:16

janbartel requested changes Jul 24, 2024

View reviewed changes

gregw requested changes Jul 24, 2024

View reviewed changes

lachlan-roberts added 7 commits July 25, 2024 10:29

PR #11883 - changes from review

2f66835

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

PR #11883 - changes from review

44286fe

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

PR #11883 - changes from review

f1f1602

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12.0.x-…

9581ef1

…SignInWithEthereum

Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12.0.x-…

37af005

…SignInWithEthereum

update poms to 12.0.13-SNAPSHOT

cc61f78

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

Move SignInWithEthereumEmbeddedExample to code-examples in documentation

a525b70

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

lachlan-roberts requested review from janbartel and gregw August 21, 2024 04:22

PR #11883 - fix build issue with poms

063e3fc

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

gregw reviewed Aug 21, 2024

View reviewed changes

gregw previously approved these changes Aug 21, 2024

View reviewed changes

PR #11883 - remove experimental warning and fix to documentation

37aed0e

Signed-off-by: Lachlan Roberts <lachlan.p.roberts@gmail.com>

lachlan-roberts dismissed gregw’s stale review via 37aed0e August 21, 2024 13:56

Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12.0.x-…

b07a8c0

…SignInWithEthereum

lachlan-roberts mentioned this pull request Aug 22, 2024

Issue #11560 - Implement EIP-4361 Sign-In With Ethereum #12188

Merged

lachlan-roberts closed this Aug 22, 2024

lachlan-roberts deleted the jetty-12.0.x-SignInWithEthereum branch August 22, 2024 04:11

olamy mentioned this pull request Sep 29, 2024

Jetty 12.1.x add Eclipse dash too usagee #12325

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #11560 - Implement EIP-4361 Sign-In With Ethereum #11883

Issue #11560 - Implement EIP-4361 Sign-In With Ethereum #11883

lachlan-roberts commented Jun 6, 2024

lachlan-roberts commented Jun 6, 2024 •

edited

Loading

sbordet left a comment

janbartel left a comment

gregw commented Jul 8, 2024 •

edited

Loading

gregw left a comment

janbartel Jul 24, 2024

lachlan-roberts Jul 25, 2024

gregw Aug 21, 2024

lachlan-roberts Aug 21, 2024

lachlan-roberts commented Aug 22, 2024

Issue #11560 - Implement EIP-4361 Sign-In With Ethereum #11883

Issue #11560 - Implement EIP-4361 Sign-In With Ethereum #11883

Conversation

lachlan-roberts commented Jun 6, 2024

Issue #11560

lachlan-roberts commented Jun 6, 2024 • edited Loading

sbordet left a comment

Choose a reason for hiding this comment

janbartel left a comment

Choose a reason for hiding this comment

gregw commented Jul 8, 2024 • edited Loading

gregw left a comment

Choose a reason for hiding this comment

janbartel Jul 24, 2024

Choose a reason for hiding this comment

lachlan-roberts Jul 25, 2024

Choose a reason for hiding this comment

gregw Aug 21, 2024

Choose a reason for hiding this comment

lachlan-roberts Aug 21, 2024

Choose a reason for hiding this comment

lachlan-roberts commented Aug 22, 2024

lachlan-roberts commented Jun 6, 2024 •

edited

Loading

gregw commented Jul 8, 2024 •

edited

Loading