Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permission protection #861

Open
wants to merge 22 commits into
base: stretch-unstable
from
Open

Permission protection #861

wants to merge 22 commits into from

Conversation

@Josue-T
Copy link
Contributor

Josue-T commented Dec 22, 2019

The problem

There are no way to protect a permission.

YunoHost/issues#1486
https://forum.yunohost.org/t/yunohost-3-7-spooky-testing-call-for-feedback/9385/31?u=josue

Solution

  • Add an attribute in LDAP to know if this permission is protected.

PR Status

  • Implement a migration
  • Manage backup/restore
  • Implement the unit test
  • Test the code (done by the unit test)

How to test

  • Create a protected permission (in a app) or just use the mail permission (which should be protected).
  • Try to add or remove the visitors group in this permission.

Note that to test this with the unit test you need this branch : YunoHost/test_apps#5

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :
Josue-T added 4 commits Dec 22, 2019
data/helpers.d/setting Outdated Show resolved Hide resolved
Josue-T and others added 7 commits Dec 23, 2019
Co-Authored-By: Kayou <pierre.moltess@gmail.com>
…ermission creation
Josue-T added 7 commits Dec 23, 2019
@Josue-T Josue-T removed the work needed label Dec 29, 2019
Josue-T added 3 commits Dec 29, 2019
@Josue-T Josue-T requested review from alexAubin and YunoHost/core-dev Dec 29, 2019
if permission.split('.')[0] in SYSTEM_PERMS:
ldap.update('cn=%s,ou=permission' % permission, {'isProtected': "TRUE"})
elif permission.endswith(".main"):
ldap.update('cn=%s,ou=permission' % permission, {'isProtected': "FALSE"})

This comment has been minimized.

Copy link
@kay0u

kay0u Jan 14, 2020

Member

Some apps should be protected by default (for example phpmyadmin? pi-hole...) What is happening if isProtected is not defined?

This comment has been minimized.

Copy link
@Josue-T

Josue-T Jan 14, 2020

Author Contributor

Well, here we just define if a user is allowed to add/remove the visitors group to this permission. So why for phpmyadmin need to protected ?

What is happening if isProtected is not defined?

As I defined this attribute as mandatory slapd won't be happy when you try to update this object if this attribute is not defined.

This comment has been minimized.

Copy link
@kay0u

kay0u Jan 14, 2020

Member

Perhaps phpmyadmin was not a good example, anyway, there i no magic solution here, and the packager should update the permission if needed.

As I defined this attribute as mandatory slapd won't be happy when you try to update this object if this attribute is not defined.

Ok, no problem then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.