Accounts plugin support connection tokens #904
Conversation
…rt-connection-tokens
@Velua Odd, I can't reproduce this. At the top of that bindings.rs do you see |
I've seen the same error in a different file. It looks like a bug in rustfmt. |
|
Note: I manually edited the file that I saw it in to remove the trailing space, and then |
| } | ||
|
|
||
| fn prefix(&self) -> String { | ||
| // Do not namespace app data by protocol |
There was a problem hiding this comment.
Is this really a good idea? Most web storage APIs are partitioned by origin (The exception is Cookies, and they have to be secured in other ways). We definitely don't want a spoofed HTTP site to be able to grant permissions that affect the real HTTPS site.
There was a problem hiding this comment.
As discussed, comment has been changed
There was a problem hiding this comment.
If we care about limiting the key length, the scheme is probably the least significant part. I don't know that it's worth stripping it off by itself. If the scheme is redundant, the port and most of the host are also redundant.
There was a problem hiding this comment.
Okay, ended up making a change here.
The prefix is the app name rather than the domain. This means that only domains that are resolvable to an app name can be logged into. Apps served over a reverse proxy is still fine, as alternative domains can be looked up in the app registry in the future.
So the table is namespaced by the app name only (Namespaced automatically by protocol/port because of where supervisor is served from).
| let packed_token = &token.packed(); | ||
|
|
||
| tokens.add(token); | ||
| Keyvalue::set(&DbKeys::CONNECT_TOKENS, &tokens.packed()) |
There was a problem hiding this comment.
Normally I would expect tokens to be either encrypted (for privacy) or signed with a MAC (if the tokens are consumed by the same entity that created them) or asymmetric (if the token creator is separate from the consumer) key. This allows the token consumer to be stateless, and avoids having to worry about limits on the number of tokens.
There was a problem hiding this comment.
In particular, the current setup makes it possible for an attacker to guess a valid token. I don't know whether that's exploitable, but it makes me wonder why a token is needed at all.
There was a problem hiding this comment.
I'd like the entire client-side DB to eventually be encrypted.
Hmm, I was going to say the tokens can't be signed because there's nothing to sign with, but what if the accounts app generates it's own key in AuthSig::Keyvault client side, and signs the token.... that would probably actually work, wouldn't it
There was a problem hiding this comment.
In particular, the current setup makes it possible for an attacker to guess a valid token. I don't know whether that's exploitable
Malicious app could open a connection page for a third-party app, prompting the user to link their account to the third-party app. I think this would mostly just confuse the user? I'm not really sure what the motive would be for such an attacker.
I think the this token exists out of an abundance of caution. Makes it impractical to confuse the user in this way, and prevents honest actors from accidentally prompting users for a connection to a different app. Do you prefer a different approach @swatanabe?
Impracticality of guessing a token
While it's theoretically possible to guess a token, I don't think it's practical. The user would need to generate but fail to consume a connection token (uncommon). The mal app would need to somehow know the app for which the user has an available token (impossible), and would need to correctly guess it before expiry. Guessing is probably at best a 1/300 chance since the expiry time in seconds is baked into the token. And there's no way to check if a guessed token is valid (except with user interaction).
There was a problem hiding this comment.
I think I don't understand how the token is intended to be used. I don't see anything actually using it yet? Is there a write up of the intended flow somewhere?
There was a problem hiding this comment.
None of this is really used yet. Was prepping it for John to integrate
Just added more notes to the PR description for docs of this design.
| console.info( | ||
| "TODO: handle calls directly from the rootdomain itself", | ||
| ); | ||
| if (callerOrigin === siblingUrl(null, null, null, true)) { |
There was a problem hiding this comment.
I removed my origin rewrite and left the app explicitly set to homepage
…rt-connection-tokens
…rt-connection-tokens
James-Mart
added
the
System app
|
I believe all review topics have been addressed |
Changes
accountsapp into several modulesaccountsplugin api into different interfacesConnection tokens
The following flow shows an account creating a new connection token in response to a user clicking, "login" or "login with new account" or something
sequenceDiagram App->>Accounts: create_connection_token Accounts-->>App: return <token> note over App: redirect to accounts.psibase/login?token=<token> Accounts->>Accounts: decode_token Accounts-->>Accounts: login_direct(app, user) note over Accounts: redirect to app.psibaseAfter this, an account is connected to (and automatically logged-in to) the app. An app can switch the logged-in account between any connected accounts at will without redirection the accounts app.
Invite tokens
Currently, the flow for an invite is similar.
Consider a user wants to invite another person to an app.
Then the flow is something like:
sequenceDiagram actor User actor Friend participant App participant Accounts participant Invite User->>App: Click "generate invite link" App->>Invite: create_invite_token Invite-->>App: return <token> App-->>User: <accounts.psibase/login?token=token> note over User: Send to friend offline note over Friend: Navigate to invite link Accounts->>Accounts: decode_token alt existing account Accounts-->>Accounts: login_direct(app, user) else new account Accounts->>Invite: accept-create (creates new account) Accounts-->>Accounts: login_direct(app, user) end note over Accounts: redirect to app.psibase