Folder ID should not be tied directly to Cipher

[mprasil]

When you read the documentation here: https://help.bitwarden.com/article/collections/

There's a note:

Collections are different than folders. Collections are a way to organize items and control user access within an organization’s vault while folders are a way for individual users to organize items within their own personal vault. An individual user may wish to further organize the items being shared with them in their own vault into a personalized folder structure that makes sense just for them.

This means, that Folders are per-user organization. This is only an issue once you start using Organization ciphers as they are shared and one user changing the folder would affect all other users. There probably needs to be some CipherInFolder mapping and we need to return folder ID of the Cipher depending on user as well.

[mprasil]

Shouldn't be too hard to implement, but there needs to be some migration SQL unless we're Ok with breaking the schema. Any thoughts on that?

[dani-garcia]

Yeah, you are right about that. We also need to change the Ciphers table because currently user_uuid can't be null, and that doesn't make sense when the cipher is owned by an organization.

I personally don't mind breaking the schema if necessary, but in this case I think a migration would be reasonably easy. I'm thinking something like this:

ALTER TABLE ciphers RENAME TO oldCiphers;

CREATE TABLE ciphers (
  uuid              TEXT     NOT NULL PRIMARY KEY,
  created_at        DATETIME NOT NULL,
  updated_at        DATETIME NOT NULL,
  user_uuid         TEXT     REFERENCES users (uuid), # Make this optional
  organization_uuid TEXT     REFERENCES organizations (uuid), # Add reference to orgs table
  # Remove folder_uuid
  type              INTEGER  NOT NULL,
  name              TEXT     NOT NULL,
  notes             TEXT,
  fields            TEXT,
  data              TEXT     NOT NULL,
  favorite          BOOLEAN  NOT NULL
);

CREATE TABLE users_ciphers (
  user_uuid   TEXT NOT NULL REFERENCES users (uuid),
  cipher_uuid TEXT NOT NULL REFERENCES ciphers (uuid),
  folder_uuid TEXT NOT NULL REFERENCES folders (uuid),

  PRIMARY KEY (user_uuid, cipher_uuid)
);

INSERT INTO ciphers (uuid, created_at, updated_at, user_uuid, organization_uuid, type, name, notes, fields, data, favorite) 
SELECT uuid, created_at, updated_at, user_uuid, organization_uuid, type, name, notes, fields, data, favorite FROM oldCiphers;

INSERT INTO users_ciphers (user_uuid, cipher_uuid, folder_uuid)
SELECT user_uuid, uuid, folder_uuid FROM oldCiphers WHERE folder_uuid IS NOT NULL;


DROP TABLE oldCiphers;

Something like that would be on the up.sql file, and the reverse process would be on the down.sql.

[mprasil]

I don't think we can really provide down.sql as you can map folders one way, but not the other way around (for example when two users have the cipher in their folders)

Is diesel fine with one way updates?

[dani-garcia]

Yeah, I didn't think of that.

I imagine diesel shoudn't have a problem as long as we don't choose to revert a migration, and I don't think we have a use for that.
In any case, if there is a problem, we could put folder_uuid to null in the down.sql and that would simply delete the folder assotiations, but we would still have the ciphers, which I guess is better than nothing.

[dani-garcia]

This should be fixed now with your PR, so I'm closing it.