YunoHost-Apps / navidrome_ynh

Navidrome package for YunoHost
https://www.navidrome.org/
GNU General Public License v3.0
17 stars 4 forks source link

Enable SSO auth (does not prevent Subsonic API on 'public' mode) #85

Closed selfhoster1312 closed 1 year ago

selfhoster1312 commented 1 year ago

Problem

I don't want to login to every single app. We have SSOWat for this.

Solution

Navidrome now supports header authentication. They don't yet support to define a custom "auth" redirection URL for unauthenticated clients.

From my personal tests it does not block login to Subsonic API when site is in public mode. However for security reasons we should require SSOWat v11.1.2.1 before releasing this change.

I'm not sure if it's possible to declare a SSOWat required version in the manifest or elsewhere in the install script.

PR Status

selfhoster1312 commented 1 year ago

!testme

ericgaspar commented 1 year ago

!testme

selfhoster1312 commented 1 year ago

!testme

ericgaspar commented 1 year ago

!testme

selfhoster1312 commented 1 year ago

Hmmm i just tried with user who never used Navidrome before and account does not get populated.

Logs say:

User passed in header not found

Upstream issue

selfhoster1312 commented 1 year ago

I opened a PR on navidrome here. It works but effectively disables Subsonic API usage because the password is random-generated. I'm waiting for feedback upstream.

selfhoster1312 commented 1 year ago

I added a new commit to disable password check when changing password when HTTP header auth is enabled. Upstream doesn't look happy to merge before they rework the auth system entirely.

The patch is simple and could be rebased easily on next releases (until the auth system is reworked)... but that means no more prebuilt binaries for Yunohost if we base the package on that branch... and oh it is slow to build this JS interface.

We could publish Github actions or other CI for the branch to publish our own pre-built binaries. What do you think?

selfhoster1312 commented 1 year ago

Quoting upstream:

Yeah, this may be ok as a workaround, as it is a contained change in the code. I'll do some testing and let you know.

ericgaspar commented 1 year ago

superseded by #88