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

Support optional scheme being provided in msg header #195

Merged
merged 3 commits into from
Apr 10, 2024
Merged

Support optional scheme being provided in msg header #195

merged 3 commits into from
Apr 10, 2024

Conversation

NicholasEllul
Copy link
Contributor

@NicholasEllul NicholasEllul commented Mar 19, 2024

Context

The EIP-4361 specification outlines the abnf message format that corresponds to a valid SiWE message. As seen below, this format indicates that there is an optional scheme that can pe provided alongside the domain in the first part of the message

sign-in-with-ethereum =
    [ scheme "://" ] domain %s" wants you to sign in with your Ethereum account:" LF
    ...

Problem

The siwe-parser currently fails when a scheme is provided alongside the domain. This is due to the fact that our abnf grammar string currently lacks this [ scheme "://" ] declaration in the grammar, resulting in valid SiWE messages containing this scheme to result in an error being thrown.

Solution

This pull request does two things to solve this:

  • Adds [ scheme "://" ] to the grammar string
  • Adds logic to parse the scheme from a given message
  • Add support for the optional scheme being provided in a SiweMessage
  • Adds a test case containing the example containing a scheme that is specified in the EIP-4361 spec. This will ensure we do not have any regressions in the future.

@sbihel
Copy link
Member

sbihel commented Mar 23, 2024

Thank you for the PR! Would you mind adding support for the other packages as well?

@NicholasEllul
Copy link
Contributor Author

@sbihel thanks for the callout - I've now made the respective changes

@NicholasEllul NicholasEllul changed the title feat(siwe-parser): support optional scheme being provided for domain Support optional scheme being provided in msg header Mar 30, 2024
@sbihel
Copy link
Member

sbihel commented Apr 10, 2024

I'm on the fence to have scheme be part of or separate from domain. But it think this way is a more natural addition

@sbihel sbihel merged commit 9ae4850 into spruceid:main Apr 10, 2024
6 checks passed
@sbihel
Copy link
Member

sbihel commented Apr 10, 2024

Published as part of @spruceid/[email protected] and [email protected].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants