Prevent multiple user accounts with the same account id

[gr2m]

Today I can do this:

hoodie.account.signUp({username: 'user1', password: 'secret'})
hoodie.account.signUp({username: 'user2', password: 'secret'})

This creates two accounts with the same account id. That is a problem because GET /_users/_design/byId/_view/byId?key="<ID HERE>" will now return two results, but only the first one is used. If I sign in two the second account, my session will not be valid when using the byId view to find the account doc to validate it against.

We must prevent multiple account docs to be created that have the same account id. I don’t know what the best strategy for that could be. We might want to change how we store accounts entirely and instead of making the username part of the doc._id, we use the id instead, as the username can change while the account id cannot. We’ll have to guarantee both to be unique anyway.

ping @janl can you think of a good strategy to guarantee uniqueness of a CouchDB doc property value in a database?

[mbad0la]

What if we could use the generateId method during signUp while avoiding the new accounts to inherit the id from the Account instance?

[gr2m]

I’m not sure what you mean, could you explain it in a bit more detail?

[mbad0la]

We can update

body: internals.serialise('account', options, cache.id)

to

body: internals.serialise('account', options, generateId())

in signup.js

generateId could be imported from here.

From what I understand Account instances are stateful and are supposed to contain the data for the authenticated account. signUp in itself is a stateless action hence de-coupling it with the account id won't break anything.

What do you think about this @gr2m ?

[gr2m]

I fear that won’t work, the account ID we pass to the sign up must be the same that we have stored locally, as we will add it to all documents created & updated in the .hoodie.createdBy and .hoodie.updatedBy properties (this is still tbd).

We might do something else on the client side. We could keep track that a signup was already done successfully for the current account id and either prevent another signup or send a different request that would rename the document of the previous signup.

But either way, we cannot trust the client, people could send their own requests without even using the client, so we have to secure this on the server side anyway, by preventing that two account documents with the same account id can be created.

[mbad0la]

Won't you be able to create and add documents only when you have authorized the user?
Account ID can be overwritten on signIn isn't it?

But either way, we cannot trust the client, people could send their own requests without even using the client, so we have to secure this on the server side anyway, by preventing that two account documents with the same account id can be created

This should be an obvious validation on the server-side no doubt. id needs to be unique and hence checking for possible conflicts should be enforced.

[gr2m]

Account ID can be overwritten on signIn isn't it?

No :) account ids cannot be changed

[gr2m]

I think the we can solve this problem and at the same time prepare to properly implement custom user confirmation workflows.

What happens today

In the browser, I create an account with

hoodie.account.signUp({username: 'test', password: 'test'})

Which sends a PUT /session/account request to the server, which then creates an account in the route handler using @hoodie/account-server-api

accounts.add({
  username: username,
  password: password,
  createdAt: createdAt || currentTime,
  signedUpAt: currentTime,
  include: query.include,
  id: id
})

which creates a document in Hoodie’s users database which looks like

{
  "type": "user",
  "name": "test",
  "createdAt": "2017-07-30T03:24:45.700Z", // when account was created locally
  "signedUpAt": "2017-07-30T03:53:00.335Z", // when signed up on the server
  "roles": ["id:ow7eior"],
  "iterations": 10,
  "password_scheme": "pbkdf2",
  "salt": "1a71231d513017012313417913b1db1151fd1121fe1241fb",
  "derived_key": "a99ee677d8a59fdc4a00215b1eac68ce27428d53",
  "_id": "org.couchdb.user:test",
  "_rev": "1-6a23e40ab47bef398205241333889f18"
}

We don’t have any confirmation flow implemented at this point, once created, a user can sign in to the account. Because of that, the same user can create as many user accounts as they want, and they will all have the same "roles": ["id:ow7eior"] property.

Suggestion

When creating a user account, we store a new property createdBy which has the id of the current user. We don’t set "roles" to ["id:${accountId}"], we leave it empty which means the account is unconfirmed. We add a new method to @hoodie/account-server-api: api.accounts.confirm({id, username}). This method does two things

1. creates a new document with the _id property set to id/${accountId}, so for the example above, it would be set to id/ow7eior.
2. It would add set "roles" to ["id:${accountId}"] and confirmedAt to the current timestamp

Because the id/${accountId} document exists now, running api.accounts.confirm({id, username}) for an id that was already confirmed will fail with a conflict error.

I would remove the existing confirmation option for @hoodie/account-server which is currently not implemented anyway and instead replace it with autoConfirmation which defaults to true. If set to false than the developer is responsible to implement whatever confirmation routine they want. We can implement a plugin for an email confirmation flow for example, many will want to implement confirmation using Stripe and other payment providers, it’s up to them. That will also let us remove nodemailer from our dependencies which will resolve #774.

Process of implementation

• Start a PR on @hoodie/account-server-api
  • add accounts.confirm({id, username}) method which creates the id/${accountId} and adds the id role to the user account
  • adjust accounts.add to no longer add the id role
  • update README
  • Make sure to release with a breaking change version update
• Start a PR on @hoodie/account-server
  • update how-it-works.md
  • update plugin/README.md: remove options.confirmation and options.notifications, add options.autoConfirmation (Boolean) which defaults to true
  • Update the sign up route handler to check for the autoConfirmation option. If it is set, directly confirm the user account
  • Once the new version of @hoodie/account-server-api is released, update package.json
  • Make sure to release with a breaking change version update
• Start a PR on hoodie
  • Once the new version of @hoodie/account-server is released, update package.json

To be discussed

• As we now add addition documents besides user account documents, we should consider to prefix user accounts with user/, too. We might have group accounts in future as well, maybe more. This will be a breaking change which will require a migration.
• What shall we do when an account gets deleted? Should we delete the id/${accountId} document as well or leave it there so people can’t sign up reusing an existing account id, which might be a security issue, maybe?

Anything else?