mirror of
https://github.com/dani-garcia/vaultwarden.git
synced 2026-04-26 01:35:54 +03:00
[GH-ISSUE #10] Folder ID should not be tied directly to Cipher #4
Labels
No labels
SSO
Third party
better for forum
bug
bug
documentation
duplicate
enhancement
future Vault
future Vault
future Vault
good first issue
help wanted
low priority
notes
pull-request
question
troubleshooting
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/vaultwarden#4
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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:
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
CipherInFoldermapping and we need to return folder ID of theCipherdepending on user as well.@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?
@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:
Something like that would be on the up.sql file, and the reverse process would be on the down.sql.
@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?
@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.
@dani-garcia commented on GitHub (May 4, 2018):
This should be fixed now with your PR, so I'm closing it.