rainbowkit icon indicating copy to clipboard operation
rainbowkit copied to clipboard

[bug] SiWe as safe app does not call the authenticate function

Open malteish opened this issue 1 year ago • 12 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

RainbowKit Version

0.12.4

wagmi Version

0.12.4

Current Behavior

What works: Connecting (formerly gnosis) safe with WalletConnect

On connect modal select WalletConnect -> Official Modal -> copy link -> paste into app.safe.global WalletConnect app -> connection established -> sign login message -> approve in app.safe.global -> custom logic in pages/api/auth/[...nextauth].ts verifies the multisig has approved this message hash on-chain

What doesn't work: The same workflow as safe app

Rainbowkit support usage as (custom) safe app since version 0.12.4, so this is still brand new. We are excited about it, because we had been working on this very functionality for our dapp! But when I try the workflow above as safe app, I get stuck with an open modal claiming to be "Verifying signature ...", without ever calling the backend. grafik So the non-working flow is: Open app.safe.global -> open custom app -> paste dapp url -> accept disclaimers etc -> connect wallet (very smooth and nice!) -> sign message -> approve in safe -> get stuck at "Verifying signature ...", backend is never called.

Expected Behavior

Sign in workflow with safe app calls backend with 0x as signature, just as sign in workflow with WalletConnect does.

Steps To Reproduce

Not tested: enable an app to become a safe app by adding a manifest.json and configuring cors to allow loading of the same. Try signing the login message with WalletConnect and as safe app. Both should fail (unless the backend was modified to verify EIP1271), but only WalletConnect does fail whereas safe app gets stuck without ever proceeding to check the signature in the first place.

Link to Minimal Reproducible Example (CodeSandbox, StackBlitz, etc.)

No response

Anything else?

No response

malteish avatar Mar 27 '23 21:03 malteish

@malteish Good find. I'm able to reproduce the behavior you're seeing. I believe the issue is related to calling Wagmi's signMessage hook in the Safe environment, with RainbowKit awaiting a response before it proceeds to the next step. I'll continue trying to debug this; if you have any additional findings, please do share!

I've also started an initial look at adopting EIP-1271, but am receiving errors that 1271 is unsupported in the Safe browser upon attempts to enable with a safe_setSettings RPC call. Would definitely like to adopt gasless signatures for the SIWE flow.

DanielSinclair avatar Mar 28 '23 14:03 DanielSinclair

Thanks for looking into this, @DanielSinclair. We are very interested in gasless signatures, too, and happy to help exploring how it can be done. Fixing the on-chain signature should be much easier though, so I recommend doing that first. And I agree that it sounds like the return value of signMessage is not forwarded properly - possibly because it contains "0x" as signature (which is expected).

malteish avatar Mar 28 '23 15:03 malteish

After some digging I have to challenge our assumption that the bug starts with signMessageAsync. As the screenshot in the issue shows, I get stuck at "Verifying signature", which is only shown after the signature has been received. This suggests the problem starts in authAdapter.verify(). Which is weird because I log any call to authorize in [...nextauth].ts and don't see that it is executed. The glue between these 2 places, according to my current understanding, is rainbowkit-siwe-next-auth.

@DanielSinclair what do you make of this?

malteish avatar Mar 28 '23 20:03 malteish

@malteish Yes, you're right. Was able to isolate the issue to next-auth session handling. const { status } = useSession() is not updating to authenticated in the Safe environment. Still hunting down the exact reason, but it might be related to the iframe. It looks like the RainbowKit side is working as expected, so a custom authentication adapter that doesn't rely on next-auth sessions should continue to work.

DanielSinclair avatar Mar 29 '23 02:03 DanielSinclair

So we found out that CSRF and the nonce in the SiWe message were equal when using Metamask or WalletConnect, but different when using safe app. This is because next-auth enforces sameSite by default (which is a good choice!).

After updating cookie policy for testing, signing messages and authenticating them in the backend works. Added this to [...nextauth].ts:

cookies: {
      // WARNING: This disables the default cross-site protection for cookies!
      // These options were added to explore use of our app in a safe iframe.
      // https://www.npmjs.com/package/cookies-next
      // https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis-03#section-4.1.2.7
      csrfToken: {
        name: `__Host-next-auth.csrf-token`,
        options: {
          httpOnly: true,
          sameSite: 'none', //'lax',
          path: '/',
          secure: true,
        },
      },
      sessionToken: {
        name: `__Secure-next-auth.session-token`,
        options: {
          httpOnly: true,
          sameSite: 'none',
          path: '/',
          secure: true,
        },
      },
    },

** WARNING: THESE SETTINGS ARE A VERY BAD CHOICE FOR PRODUCTION** in my opinion. Maybe someone with deeper knowledge in this area of expertise can chime in to either prove me wrong or suggest a better configuration.

Background: https://github.com/nextauthjs/next-auth/issues/7051 https://github.com/nextauthjs/next-auth/issues/7077 https://next-auth.js.org/configuration/options#usesecurecookies

So this is not really a problem caused by Rainbowkit. Do you want to close the issue? I would like to keep it open a bit longer in hopes of someone finding a better solution.

malteish avatar Mar 29 '23 12:03 malteish

@malteish Will keep this issue open to see if a better solution is presented. I'll also relay to the Safe team to see if there is guidance on how to handle sessions/cookies in the Safe browser environment.

DanielSinclair avatar Mar 31 '23 06:03 DanielSinclair

We're also interested on this. We wanted to add support not only for Safe, but also for AA in general.

Do you know of other session manager (e.g. iron-session) that is able to properly handle sessions within an iframe?

@DanielSinclair , did the Safe team gave a proper answer? It may be something recurrent for them for the way they work with apps as iframes.

Regarding the signature, if been unable to make it work with Safe accounts using Wallet Connect. Instead of using the offchain signature with safe_setSettings, I was trying using the online EIP-1271 signature validation with no luck. But I've been able to make it work with other EIP-1271 compatible smart contracts accounts.

Why is that? I believe that is due to the way Safe works. It has to create an online TX to get the signature validation into the Safe account. However, the Safe returns the control to "us" before the TX is completed on the blockchain. This (I believe) is the flow:

  • Safe user connect to Rainbow app using Wallet Connect.
  • Rainbow app asks to sign to Safe user.
  • Safe app gets control.
  • Safe app asks the user(s) to sign.
  • User sign.
  • Safe app asks the user to send the TX into the blockchain to save the validation. *- Once the Tx is sent, control is sent back to Rainbow app via Wallet Connect. *- Rainbow app checks the signature, but it is not on-chain yet, as it is still on the mempool.

The two last steps are the possible ways to fix it.

@DanielSinclair , if you are talking to Safe team, ask them about this as well. You can create a Safe on testnet and test it yourself. If you need, using our dApp you can see a realworld example where is fails to sign. But other smart contract wallets can sign with no issues.

nfts2me avatar Apr 25 '23 09:04 nfts2me

@DanielSinclair curious if there was any feedback from the Safe on how to handle this correctly or a better workaround?

pauliusuza avatar May 23 '23 13:05 pauliusuza

Any news with regard to this issue?

nfts2me avatar Mar 07 '24 12:03 nfts2me

@nfts2me Our team is looking at this issue. We'll report back once we have a solution 🙏

magiziz avatar Mar 11 '24 19:03 magiziz

@nfts2me Our team is looking at this issue. We'll report back once we have a solution 🙏

Any news with regard to this issue @magiziz ?

nfts2me avatar May 21 '24 11:05 nfts2me

@nfts2me Yes. We're working on this right now, please bear with us 👍

magiziz avatar May 21 '24 14:05 magiziz