[GH-ISSUE #10] Folder ID should not be tied directly to Cipher #4

Closed
opened 2026-03-03 01:23:41 +03:00 by kerem · 5 comments
Owner

Originally created by @mprasil on GitHub (Apr 27, 2018).
Original GitHub issue: https://github.com/dani-garcia/vaultwarden/issues/10

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.

Originally created by @mprasil on GitHub (Apr 27, 2018). Original GitHub issue: https://github.com/dani-garcia/vaultwarden/issues/10 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.
kerem closed this issue 2026-03-03 01:23:41 +03:00
Author
Owner

@mprasil commented on GitHub (Apr 27, 2018):

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?

<!-- gh-comment-id:384950734 --> @mprasil commented on GitHub (Apr 27, 2018): 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?
Author
Owner

@dani-garcia commented on GitHub (Apr 27, 2018):

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.

<!-- gh-comment-id:384960535 --> @dani-garcia commented on GitHub (Apr 27, 2018): 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: ```sql 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.
Author
Owner

@mprasil commented on GitHub (Apr 27, 2018):

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?

<!-- gh-comment-id:384964664 --> @mprasil commented on GitHub (Apr 27, 2018): 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?
Author
Owner

@dani-garcia commented on GitHub (Apr 27, 2018):

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.

<!-- gh-comment-id:384966309 --> @dani-garcia commented on GitHub (Apr 27, 2018): 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.
Author
Owner

@dani-garcia commented on GitHub (May 4, 2018):

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

<!-- gh-comment-id:386732180 --> @dani-garcia commented on GitHub (May 4, 2018): This should be fixed now with your PR, so I'm closing it.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/vaultwarden#4
No description provided.