Implementation not according to spec

[holiman]

I found a couple of things about the spec to be a bit ambiguous fellowship post, and checked out how this reference implementation handles it. Turns out, not according to spec.

Statement regex, https://github.com/spruceid/siwe/blob/main/lib/regex.ts#L4:

const STATEMENT = '((?<statement>[^\\n]+)\\n)?';

As opposed to

const STATEMENT = '((?<statement>[^\\n]*)\\n)?';

Resources

const RESOURCES = `(\\nResources:(?<resources>(\\n- ${URI}?)+))?`;

as opposed to

const RESOURCES = `(\\nResources:(?<resources>(\\n- ${URI}?)*))?`;

So siwe does not allow empty-but-present statement, nor empty-but-present resources. Which I think is fine, but it's not according to spec.

I hope that rather than fixing this issue by making it adhere to the spec, we can update the spec, to change

statement = *( reserved / unreserved / " " )

into

statement = 1*( reserved / unreserved / " " )

and

resources = *( LF resource )

into

resources = 1*( LF resource )

[wyc]

Thank you for the issue, we should review the regex for any discrepancies. It does seem that requiring the statement by specifying at least one character could create a more regular message format, which may be preferable. I think it's also a reasonable suggestion to require the presence resources when "Resources:\n" exists, but this could prevent users from expressing a resource section of zero resources as opposed to an omitted resources section.

I would also like to note that the in-production implementations use the ABNF grammar from the spec directly implemented here, so this shouldn't affect any applications relying on this library to create/validate SIWE messages for anyone following this issue. The regex is provided as an alternative for implementation that can be leveraged when no ABNF parsers are available or for performance.

[fernando-deka]

Also, this implementation does not allow for the schema to be present with the domain:

sign-in-with-ethereum =
    [ scheme "://" ] domain %s" wants you to sign in with your Ethereum account:" LF
    address LF
    LF
    [ statement LF ]
    LF
    %s"URI: " uri LF
    %s"Version: " version LF
    %s"Chain ID: " chain-id LF
    %s"Nonce: " nonce LF
    %s"Issued At: " issued-at
    [ LF %s"Expiration Time: " expiration-time ]
    [ LF %s"Not Before: " not-before ]
    [ LF %s"Request ID: " request-id ]
    [ LF %s"Resources:"
    resources ]

scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
    ; See RFC 3986 for the fully contextualized
    ; definition of "scheme".

localhost:3000 wants you to sign in with your Ethereum account: ...  ✅ http://localhost:3000 wants you to sign in with your Ethereum account: ... ❌

According to the spec, that should be parsed but it fails.

[sbihel]

Also, this implementation does not allow for the schema to be present with the domain:

This should have been fixed with #195 which was released as part of [email protected]. If you still have issues with the scheme, would you mind opening a new issue?