search

wwWallet / wallet-frontend

BSD 2-Clause "Simplified" License
24 stars 7 forks source link

Passing user data on password unlocking #296

Closed kkmanos closed 3 months ago

kkmanos commented 3 months ago

The initial commit seems to be creating cached users. Instead, I am now setting only the userHandleB64u

emlun commented 3 months ago

Was that not the intent? What else is the motivation for this?

kkmanos commented 3 months ago

@emlun I noticed that the userHandle is not stored in the session storage when logged-in or signed-up with username/password, making it inaccessible through the keystore.

Instead of this PR, do you think that it would be better to fetch the user's info (including the userHandle) through the /user/session/account-info endpoint? The primary reason of introducing this feature, is because we need the userHandle for other features (such as passing it as a 'state' parameter in the authorization request of OpenID4VCI flow)

emlun commented 3 months ago

Ah, I see. So the problem was that the user would show up as a "Log in as..." button, but the user has no passkeys so clicking that button always fails? I think we can fix that with a simple filter in the cachedUsers updater effect.

For the other part, I think it might be best to just expose the setUserHandleB64u function in the LocalStorageKeystore API. Calling keystore.initPassword twice is dangerous because it'll re-initialize the keystore with a new wallet in the local copy of the privateData, overwriting the wallet that was sent to the server, so there's an immediate desync between local and server state.

Shall I open another PR showing what I mean?

kkmanos commented 3 months ago

If you think that exposing setUserHandleB64u will be useful in other cases as well in the future, then you can proceed with opening a PR for it. Otherwise, I think that we can avoid it for know.

What I am thinking is that I should close this PR and instead solve the issue of the initial use-case I had in mind by accessing the account info.

What is your opinion?

kkmanos commented 3 months ago

Closing this PR because the alternative PR https://github.com/wwWallet/wallet-frontend/pull/300 is prefered