spruceid / siwe-py

A Python implementation of Sign-In with Ethereum
https://login.xyz
Apache License 2.0
66 stars 28 forks source link

Inconsistency between validity of URIs for `siwe` and `siwe-py` libraries #69

Closed derekpierre closed 1 month ago

derekpierre commented 1 month ago

In the siwe library (typescript) the following URIs within a SIWE message are valid when creating a SiweMessage object.

However, in siwe-py (python) these URIs are deemed invalid when creating a SiweMessage object.

To give the full context, the following EIP4361 message is valid for siwe, but not valid when using siwe-py:

localhost wants you to sign in with your Ethereum account:
0x2d1F436Ce1A7Ca3959f23D90981B2e65f46629e6

Give this application access to some of your data on Ceramic

URI: did:key:z6MkvkZ9sKvX8VYVhyMGZuEEnQpkor7fpK5T1WdsuTxznFKg
Version: 1
Chain ID: 80002
Nonce: mY7QV8NJ6x
Issued At: 2024-07-12T15:08:04.917Z
Expiration Time: 2024-07-19T15:08:04.917Z
Resources:
- ceramic://*

siwe-py raises the following:

ValidationError: 2 validation errors for SiweMessage
uri
  invalid or missing URL scheme (type=value_error.url.scheme)
resources -> 0
  URL host invalid (type=value_error.url.host)
mpeyfuss commented 1 month ago

I think this likely comes from the usage of AnyUrl from pydantic, which requires scheme://

The ABNF parser accounts for RFC 3986 properly.

https://github.com/spruceid/siwe-py/blob/2d5983412a9279b28db1712326cc2fe257b2e084/siwe/siwe.py#L174

mpeyfuss commented 1 month ago

Would be simple to create a custom validator for the fields, but also need to account for it in the regex validator, which probably requires some discussion as to why that's in there.

derekpierre commented 1 month ago

It seems that I was using a much older version that I expected, v2.4.1, 😅 . This was due to some sub-dependencies limitations.

v4.1.0 does not have this issue, both for abnf, and non-abnf. 👍

mpeyfuss commented 1 month ago

Cool! Wasn't sure what was throwing the validation error so my comments above were at best guesses lol. I should have just recreated :)