'visitors' mechanics (replace the old public/private mechanic) + integrate the permission system with SSOwat

[alexAubin]

The problem

The whole is_public and unprotected_uris mechanic should be integrated in the permission system to have a consistent permission management mechanic

Solution

So that's a completely untested implementation of the visitors group and a mechanism to replace the weird "unprotected_uris = /" to allow public access.

This way, we get a consistent permission mechanism where the admin can choose for a given app to :

• make it "public", i.e. allow it for the group visitors
• make it "private", i.e. allow it for the group all (registered) users
• make it only available to specific groups and/or users

No need for apps to save and manage some is_public state outside the install script. And no need for admins to reinstall an app just to tweak the public/private status ...

Admins and packagers shall be able to reconfigure the publicness/privateness of the app using something like :

# cli 
yunohost user permission update wordpress --remove all_users --add visitors

# app helpers
ynh_permission_update --permission main --remove all_users --add visitors

... or through the webadmin once it gets implemented

This can easily be extended to subparts of the app (e.g. admin interface). For example eauchat recently was in a situation where he wanted to have the mailman admin interface accessible to visitors because he didn't feel like creating yunohost accounts just for this.

Soooo yup, at least that's the theory, 'cause the hard part ends up managing the legacy unprotected_uris (and all the variants protected vs. unprotected and uris vs. regexes). And still need to think about how this correlates with relative or absolute urls and regexes

PR Status

Not tested at all lol

How to test

Well uh there are some unit tests drafts for now

Validation

• Principle agreement 0/2 :
• Quick review 0/1 :
• Simple test 0/1 :
• Deep review 0/1 :

[alexAubin]

Digging into this, I noticed that many app use ynh_app_setting_set "$app" skipped_uris "/" instead of unprotected ... :/

Some do this to temporarily allow access to access the setup interface for the app, but some other do use it to make the app public ... dunno if that's just some copypasta they found or if there's a real motivation compared to unprotected

[alexAubin]

Soooo, made some progress on this and now I'm stuck ...

• I encountered this huge conceptual issue which can't be dealt with the current paradigm of SSOwat. TL;DR : we can't properly handle permissions with multiple urls or regexes right now. Though I really wonder if we really need to allow having multiple urls for one permission ...

• We should add an option tile_in_ssowat to the permission system to know if a given permission should be displayed on ssowat... Though (c.f. the previous issue), in the current SSOwat paradigm, having access implies to have the tile displayed and viceversa ...

• And last but not least (the biggest issue imho which really makes the whole thing stuck). Consider the following generated ssowat configuration :

    "protected_urls": [
        "yolo.test/urlpermissionapp", 
        "yolo.test/urlpermissionapp/admin"
    ], 
    "users": {
        "alice": {
            "yolo.test/urlpermissionapp": "Permissions Test", 
            "yolo.test/urlpermissionapp/admin": "Permissions Test (Admin)"
        }, 
        "bob": {
            "yolo.test/urlpermissionapp": "Permissions Test"
        }
    }

• alice and bob do have access to /urlpermissionapp as expected.
• visitors don't have access, as defined in protected_urls.
• alice has access to /admin as expected
• ... but bob does also have access to /admin, because this function in SSOwat will only checked if the requested uri starts with an authorized uri for this user.

This is in fact not a bug but a feature on which several apps rely on (though now that I actually dig into it, not so many ...) : lutim, bozon, lektor, dotclear2, lufi, and lstu

So one quick solution (apart rewriting the whole SSOwat paradigm and I really don't want to do that yet...) is to tweak the has_access function mentionned earlier to check for a longer match in the list of protected_urls/regex and deny access if so.

That will break the apps mentionned in a conservative way (meaning : even authentified user won't be able to access those uris) until they implement the new permission system but I guess we can live with this as they don't seem to be the most critical app ever ?

[Josue-T]

We should add an option tile_in_ssowat to the permission system to know if a given permission should be displayed on ssowat... Though (c.f. the previous issue), in the current SSOwat paradigm, having access implies to have the tile displayed and viceversa ...

I could push on this branch or on an other one a commit to add an attribute in LDAP for this if you want.

[Josue-T]

So one quick solution (apart rewriting the whole SSOwat paradigm and I really don't want to do that yet...) is to tweak the has_access function mentionned earlier to check for a longer match in the list of protected_urls/regex and deny access if so.

As I understand with your solution we need to do some change of SSOwat. So the minimum is:

• Add the protected_url key support
• tweak the has_access function to check for a longer match in the list of protected_urls/regex and deny access if so.

And as I see your example we have a duplication of information (between each user and the protected_url). I don't mind but maybe there are a cleaner way to do this 😄

So my personal preference is to change this concept but it still my point of view.

[alexAubin]

So my personal preference is to change this concept but it still my point of view.

Well yes I do agree that in the ideal world we should just rewrite the whole format of the ssowat config and handling of the un/protected uris/regexes and the user ACL thingies. But as always, things are less about agreeing and the principle and more about what are the practical implications. In this case, the fact that we gotta keep supporting the damn legacy, and that it's going to be a huge mess if we do this right now (and I don't really know when will be a good time actually, maybe in ~6months or 1 year when most of the app will have moved to the new permission system or idk...) But anyway this whole permission thing is already huge as fuck, so if we can find some pragmatic middle ground that'll be better :/

[Josue-T]

Ideally my purpose for the next SSOwat config for now would be by example:

{
    "/route1": {
        "permission": ["visitors", "user1", "user2", ...],   # Public route
        "label": "App1"
    },
    "/route2_with_a_regex$": {
        "permission": ["user1", "user2"],       # Private route
        "label": None,
   },
    "/route3": {
        "permission": "skipped",                   # Skipped route idk
        "label": None,
    },
    "/route4": {
        "permission": "unprotected",           # Unprotected route idk
        "label": "App3"
    }
}

But yes I know, implementing this on the SSOwat side is a work.

[Josue-T]

Are you sure of that ? :-/ At line 1506 OK, but here I don't understand why...

[alexAubin]

Hmmm, idk, don't remember exactly, but basically if add an url as protected, it shouldn't be "unprotected" by legacy stuff ... E.g. say that you install an app still using the legacy stuff as public :

• it will have declared unprotected_urls = / in its setting
• now the user use the permission system to declare that it shouldn't be public anymore
• so now the new permission system will set / as protected
• but without the line we're discussing, / will also be in the unprotected urls ...

[Josue-T]

Ok, but why we don't cleanup the settings and remove all old permission things ?

[Josue-T]

Ok, but why we don't cleanup the settings and remove all old permission things ?

Well reading your other answer I understand...

[Josue-T]

Hmmm, idk, don't remember exactly, but basically if add an url as protected, it shouldn't be "unprotected" by legacy stuff ... E.g. say that you install an app still using the legacy stuff as public :

• it will have declared unprotected_urls = / in its setting
• now the user use the permission system to declare that it shouldn't be public anymore
• so now the new permission system will set / as protected
• but without the line we're discussing, / will also be in the unprotected urls ...

For me there are 2 cases:

• The permission was migrated automatically. In this case we can use the permission to change the access to the app. And in this case we don't need this line.
• The permission wasn't migrated because it was too customized. in this case the user is not able to change the permission (because it was not migrated). So we don't have any permission for this custom case in LDAP. So this line in this case is also not needed.

[alexAubin]

The permission was migrated automatically. In this case we can use the permission to change the access to the app. And in this case we don't need this line.

It's fuzzy to me, but I think that we still need to keep the line, because as said in the previous comment, if the app install declares unprotected_urls = / (so the app is public) then the user changes the permission to private using the new permission system, then the line unprotected_urls = / is still there so we need to remove it from the "compiled" ssowat configuration if there's a conflict with the new permission system ...

If we want to avoid having to do this, we should remove the unprotected_urls from the settings entirely, but then we gotta be careful that it doesn't get reintroduce when running the upgrade script or restore script ...

[Josue-T]

Don't really understand why maintain the legacy permission here if you migrate the old unprotected_uris in the visitor group (in the migration).

[alexAubin]

Because the migration only covers the case where unprotected_uris is /, which is the classic use case, but there are many others that we can't migrate automagically where apps defined specific unprotected_uris and protected_uris

[alexAubin]

Sooo, implemented the remaining items from #795 (comment) . Not that I finally chose to drop the "multiple urls per permission" thing because I don't see any usecase where that'd be useful and that add unecessary complexity

I also fixed the inteface with SSOwat in YunoHost/SSOwat#147

Validated everything using unit tests etc.

So that PR and the permission system is finally pretty much complete to me 👍

Edit: updated the documentation draft accordingly : https://github.com/YunoHost/doc/blob/group-and-permissions/groups_and_permissions.md

[alexAubin]

Soooo planning merge this like during Tuesday's meeting to move forward and possibly have an alpha-stage release

[alexAubin]

Sooo, merging, moving on with heavy testing using the unstable app CI to validate the 3.7 release..