search

YunoHost / issues

General issue tracker for the YunoHost project
71 stars 7 forks source link

Bookworm: User cannot login to the portal before having access to an app #2384

Closed selfhoster1312 closed 1 month ago

selfhoster1312 commented 1 month ago

Apparently this is a known problem (cc @alexAubin)

When a user doesn't have permissions to a specific application, they can't login on the portal. This is because:

What is supposed to be the behavior here? I think it's reasonable that a user has access to the portal, pending access to specific apps. At the very least, the user should have access if their own domain is the same domain as the portal?

EDIT: A sample /etc/yunohost/portal/domain.tld.json

{
    "apps": {
        "nextcloud": {
            "description": {
                "en": "Online storage, file sharing platform and various other applications",
                "fr": "Stockage en ligne, plateforme de partage de fichiers et diverses autres applications"
            },
            "label": "TEST_NEXTCLOUD",
            "logo": "//domain.tld/yunohost/sso/applogos/7619f283fa6ce9e0c6ae30cef5bb7cb47b926e9abccda4009a70e54579aa915a.png",
            "public": false,
            "url": "domain.tld/nextcloud",
            "users": [
                "testuser3",
                "testuser2",
                "package_checker"
            ]
        }
    }
}

A user named testuser1 exists on the domain but is not listed there.

alexAubin commented 1 month ago

Hmmyeah I thought I implemented it but in fact not yet :

In this function https://github.com/YunoHost/yunohost/blob/bookworm/src/authenticators/ldap_ynhuser.py#L48 we should be checking that the domain part of the user's mail adresses (or just the main address ? idk)

selfhoster1312 commented 1 month ago

If we do it there, at this point we did a bind to the LDAP but we don't have the full user info. We would need to perform another LDAP query which is slow. Since we already read the portal/domain.json anyway, i would recommend putting it there in app_ssowatconf() in a special __portal__ permission. That's the easiest solution i can find without a redesign, or worse performance. What do you think?

Alternatively, we could just give permission to any user on the system to access portal by matching URI start with /yunohost/. Is that bad?

alexAubin commented 1 month ago

meh idk to me this shouldnt be in ssowatconf since this info would be used only by the portal api, not the SSO (though the SSO relies on the cookie delivered by the API).

I don't know if we should jump into premature optimization about ldap queries ... in the current code, we already do query the LDAP if the user is an admin. It's not like this code is called at every request, it's only called upon login and querying the portal API ?

alexAubin commented 1 month ago

Alternatively, we could just give permission to any user on the system to access portal by matching URI start with /yunohost/. Is that bad?

The whole point is that you should be able to have e.g. my_family_domain.tld and my_local_sport_club.tld on a single yunohost, and effectively "two different portals" (both in terms of who can access, and in terms of branding/customization)

Some people would only be to login on the first domain would not have access to the second domain and vice-versa. Maybe you as an admin can access both.

alexAubin commented 1 month ago

Fixed in https://github.com/YunoHost/yunohost/commit/5e406a55fa9a45069cb45093a915b0fe5fe18e10