miniflux / v2

Minimalist and opinionated feed reader
https://miniflux.app
Apache License 2.0
6.99k stars 727 forks source link

Add option to disable local auth form #2752

Closed thefinn93 closed 3 months ago

thefinn93 commented 4 months ago

Do you follow the guidelines?

What: This adds a config option to indicate that user auth should only happen via oauth2. Setting DISABLE_LOCAL_AUTH=1 will remove the username/password form from the login screen, and remove the password change/oauth link/unlink options from the user settings screen. It also makes the login form target refuse to validate credentials, and makes the oauth2 unlink endpoint refuse to unlink, instead redirecting back to the login screen.

Why: I'd like my users not to be confused about how to login. The (non-functional) form asking for a username and password is very confusing. My users don't know how the login system works and they shouldn't need to. #2649 discusses some other reasons this may be a desirable configuration. I decided to hide the password change and oauth account unlink settings to prevent users from thinking they changed their oauth password or accidentally breaking their miniflux account by unlinking it.

Other things to note:

thefinn93 commented 3 months ago

Done

57194 commented 3 months ago

This only checks against if an OAuth provider is configured…what about AUTH_PROXY_HEADER?

Edit: Probably out of scope of this PR, but it would be nice if there was a way to clear a password if one had been set in the past, that way only API keys can work.

Edit 2: Never run random SQL you find online, but if you're brave, password-clearing isn't too bad:

UPDATE users SET password='' WHERE username='theUsernameToClearPasswordFor';
thefinn93 commented 3 months ago

I've never used miniflux with AUTH_PROXY_HEADER, but I'd imagine adding support for that would just mean updating the safety check in internal/cli/cli.go that currently ensures that an oauth provider is configured. Are there other considerations? That check is easy enough to update.

Clearing a previously set password is definitely out of scope for this PR.

57194 commented 3 months ago

I've never used miniflux with AUTH_PROXY_HEADER, […]. Are there other considerations?

There's nothing I can think of off the top of my head (but I am by no means an expert).

The only two keys for proxy-based authentication are:

Much like OIDC/OAuth…if you don't have the authentication configured right elsewhere, you're not going to be able to get in anyway. (Or you'll let anyone in if you didn't configure your reverse proxy to require authentication for Miniflux. :-P )


Edit: If you like reading too much, here are some pages that provide some idea on how ForwardAuth/proxy-based auth would be setup:

thefinn93 commented 3 months ago

I definitely understand how AUTH_PROXY_HEADER works broadly, just not sure if there are any quirks of how Miniflux handles it. Maybe @fguillot can chime in? Happy to update the safety check to add that support.

fguillot commented 3 months ago

This is a good point. At least one external authentication must be configured when DISABLE_LOCAL_AUTH=1, either OIDC/OAuth2 or the proxy auth.

I'm also assuming that OAUTH2_USER_CREATION/AUTH_PROXY_USER_CREATION must be set to true when DISABLE_LOCAL_AUTH=1 because the entire authentication section is removed on the settings page, which prevent people to associate their local account to a third-party provider.

image

thefinn93 commented 3 months ago

I have expanded the check to allow HTTP_PROXY_HEADER but require user creation to also be enabled.