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

Yunohost 12 Auth header removed #2445

Closed
Josue-T opened this issue Sep 13, 2024 · 6 comments
Closed

Yunohost 12 Auth header removed #2445

Josue-T opened this issue Sep 13, 2024 · 6 comments
Assignees
Labels
🔑 Authentication 👾 bug Something isn't working

Comments

@Josue-T
Copy link

Josue-T commented Sep 13, 2024

Describe the bug

Some authentication header was removed on Yunohost 12 and it's annoying because some apps need it to have the SSO working correctly. From what I know here are the list of the impacted apps (that I'm maintaining):

  • PgAdmin
  • Seafile
  • XWiki

And here are the list of the authentication header which was removed:
Remote-User, Email, Name, Auth-User.

Context

  • Hardware: Any
  • YunoHost version: 12.x

To reproduce

We can see easly the difference this way:

See the difference between Yunohost 11 and Yunohost 12.

Yunohost 11

...
some other headers
...
Authorization: Basic am9zdWU6cHdk
Remote-User: josue
Email: [email protected]
Name: =?utf-8?b?Sm9zdcODwqkgam9zdWU=?=
Auth-User: josue

Yunohost 12

...
some other headers
...
Authorization: Basic am9zdWU6cHdk

Expected behavior

We should have the same header between Yunohost 12 and Yunohost 11.

@Josue-T Josue-T added 👾 bug Something isn't working 🔑 Authentication labels Sep 13, 2024
@alexAubin
Copy link
Member

Regarding pgadmin :

  • it uses WEBSERVER_REMOTE_USER = 'REMOTE_USER' in the config, but it's probably easy to define REMOTE_USER from the nginx conf similar to what we do for php apps

Regarding seafile :

  • it uses REMOTE_USER_HEADER = 'HTTP_EMAIL' in combination with LDAP_LOGIN_ATTR = 'mail' but it sounds perfectly feasible to use the classic user uid (similar to pgadmin)

Regarding xwiki:

  • it uses xwiki.authentication.ldap.httpHeader=REMOTE_USER, so same as before, this header an probably be created by some nginx conf snippet ?

@alexAubin
Copy link
Member

Or alternatively we should decide to always populate some YNH_USER header which would only come from ssowat (contrarily to the basic auth header or remote_user where there's always some sort of possible-spoofing ambiguity) and deprecate using $remote_user to populate REMOTE_USER in nginx conf, especially php ones)

@Josue-T
Copy link
Author

Josue-T commented Sep 13, 2024

Hello,

Well I could agree for pgadmin and XWiki but the issue with seafile is that there are no other solution than to use email because it's hard coded and so we can't just say let's use the username instead cf YunoHost-Apps/seafile_ynh#5 note I searched many time a way to fix this but for now I only concluded that we still need to use the email as it's done by design.

Note also that to me the issue while using the basic auth instead of the remote user header is that we have the basic auth spoofing issue. The remote_user header don't have this security issue as it can't be set by the client (from what I understood). But well it's a bit a different discussion.

And to me in general case having theses HEADER was really good as by this way we was able to have a full authentication on some app without the need connect to LDAP. By example some app could support the authentication by the header but not by LDAP. And with theses header we was able to retrieve most of the account info like the name, username and email. This was completely removed and it's a shame.

Note that maybe some other apps use this, but as I'm not the maintainer I have less an overview of the usage.

@Josue-T
Copy link
Author

Josue-T commented Sep 13, 2024

Or alternatively we should decide to always populate some YNH_USER header which would only come from ssowat (contrarily to the basic auth header or remote_user where there's always some sort of possible-spoofing ambiguity) and deprecate using $remote_user to populate REMOTE_USER in nginx conf, especially php ones)

Yes this sound me really a good idea. I also used the $remote_user to populate the REMOTE_USER in nginx conf but following the issue with the basic auth spoofing it might be better to drop this practice.

@Josue-T Josue-T self-assigned this Oct 21, 2024
@Josue-T
Copy link
Author

Josue-T commented Oct 21, 2024

Hello,

@alexAubin I'll work on this theses few next days.

After some investigation of how it works, the idea is to populate some headers like YNH_USER, YNH_EMAIL, ... And depreciate the current REMOTE_USER header which is quite unsafe if populated from the auth basic header (like with php).

@alexAubin
Copy link
Member

YunoHost/SSOwat#231 got merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔑 Authentication 👾 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants