Skip to content

[18.0][IMP] vault: add User security group to control module access - #975

Open
jans23 wants to merge 2 commits into
OCA:18.0from
Nitrokey:18.0-imp-vault-user-group
Open

[18.0][IMP] vault: add User security group to control module access#975
jans23 wants to merge 2 commits into
OCA:18.0from
Nitrokey:18.0-imp-vault-user-group

Conversation

@jans23

@jans23 jans23 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Introduce a new visible User security group under a dedicated "Vault" category. The whole module (menus, model access and record rules) is now gated behind this group instead of base.group_user. The group is implied by base.group_user so every internal user keeps access by default; removing the group from a user denies access to the vault without touching their keys.

@jans23
jans23 force-pushed the 18.0-imp-vault-user-group branch 3 times, most recently from 61e3baf to 50357ce Compare July 23, 2026 06:11
@jans23 jans23 changed the title [IMP] vault: add "Vault User" security group to control module access [IMP] vault: add User security group to control module access Jul 23, 2026
@jans23
jans23 force-pushed the 18.0-imp-vault-user-group branch from 50357ce to 4b4ed0d Compare July 23, 2026 06:48

@cvinh cvinh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes of course

@jans23
jans23 force-pushed the 18.0-imp-vault-user-group branch 3 times, most recently from 215295c to 8f68e5b Compare August 23, 2026 16:42
@cvinh

cvinh commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

The group is implied by base.group_user so every internal user keeps access by default;

This is not a good idea... but I think you force pushed to default user

@cvinh

cvinh commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Maybe you can update readme to describe the feature

@jans23
jans23 force-pushed the 18.0-imp-vault-user-group branch from 8f68e5b to ff55e0e Compare August 24, 2026 04:07
@jans23

jans23 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

@cvinh I addressed your feedback. Thank you for that.

@jans23 jans23 changed the title [IMP] vault: add User security group to control module access [18.0][IMP] vault: add User security group to control module access Aug 24, 2026
@jans23

jans23 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

@CarlosRoca13 could you review this, please?

@OCA-git-bot OCA-git-bot added the mod:vault_share Module vault_share label Aug 24, 2026
@jans23
jans23 force-pushed the 18.0-imp-vault-user-group branch from e02681d to d96a4f2 Compare August 25, 2026 08:32

@fkantelberg fkantelberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a breaking change for existing databases. If you include the fix in an already created database the users which had vaults before can't access them because the group isn't set on the migration. I would advise some migration code to set the newly created group to all existing internal users.

This isn't a security improvement because the rights control the access it only blocks the features from users. I don't fully understand the concept. You add it to the defaults but want to explicitly remove the rights from users. Afterwards when they already created their private vaults? I have some concerns about this workflow because the person who removes the access doesn't see if the user has a vault which he uses exclusivly.

@jans23
jans23 force-pushed the 18.0-imp-vault-user-group branch from d96a4f2 to cfb6c23 Compare September 4, 2026 17:00
Gate the module (menus, model access and record rules) behind a new
"User" group instead of base.group_user. The group is granted to new
users via base.default_user; removing the group from a specific user
denies access to the vault without touching their keys. Existing users
must be added to the group manually (or via shell) on upgrade.
@jans23
jans23 force-pushed the 18.0-imp-vault-user-group branch from cfb6c23 to 85427ea Compare September 4, 2026 17:04
@jans23

jans23 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

@fkantelberg Migration code added. This PR adds a way to manage which users can use Vault. Most other apps in Odoo have such option too. Same as not all users have access to--say--the Sales module but the administrator can control this. Of course if an access is removed later on the administrator should be aware of the consequences which is the case for all other modules too.

@jans23
jans23 force-pushed the 18.0-imp-vault-user-group branch from 85427ea to 3f60d8f Compare September 4, 2026 17:17
@fkantelberg

Copy link
Copy Markdown
Member

@jans23 The comparison with sales is off because the access model isn't relying on the groups and in sales the groups are the only possible access. If you want to compare why not with Calendar? Every one gets access but you only see those private events you are assigned to. Or Discuss. A basic feature with channels (vaults) to discuss (share secrets).

The reasoning with the admin isn't convincing because in all other cases the admin (or the person who manages access) can see which documents a person has access to. For vault and especially private vault those aren't shown anywhere inside of the UI except for the specific user. How do you want to address it?

If I'm honest I would leave the vault open to all internal users because it gives the all internal users an option to store secrets securely and otherwise people tend to use more unsecure methods to achieve it. I don't see how this is an improvement it looks more like a restriction without benefits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:vault_share Module vault_share mod:vault Module vault series:18.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants