Add GOTIFY_LOCALAUTH_ENABLED to disable local authentication - #1027
Closed
MichielMak wants to merge 1 commit into
Closed
Add GOTIFY_LOCALAUTH_ENABLED to disable local authentication#1027MichielMak wants to merge 1 commit into
MichielMak wants to merge 1 commit into
Conversation
Adds a config option to turn off the built-in username/password authentication, so a Gotify instance can rely on OIDC only. When GOTIFY_LOCALAUTH_ENABLED is false: - username/password credentials are no longer accepted for HTTP basic auth on any endpoint, so local credentials cannot be used to bypass the disabled login, - POST /auth/local/login responds with 403, - the default admin user is not created on startup, - the WebUI hides the login form and the register button. Client tokens, application tokens and OIDC sessions are unaffected. The option defaults to true, so existing installations do not change. Gotify refuses to start when both local authentication and OIDC are disabled, because no way to authenticate would remain. Closes gotify#1007 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Member
|
Hey thanks for the contribution. Another user created a PR for this feature: #1020, and this PR is already reviewed and nearly merged. Therefore, I'll close this one. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1007
Adds an option to turn off Gotify's built-in username/password authentication, so an instance can rely on OIDC only. Implemented following @jmattheis' design in #1007.
The option
GOTIFY_LOCALAUTH_ENABLED(localauth.enabled), defaulttrue. Existing installations are unaffected.When it is
false:POST /auth/local/loginreturns403with a clear error.Gotify refuses to start when both local auth and OIDC are disabled, since that would leave no way to log in.
Why basic auth had to change
Blocking the login endpoint alone is not enough — Gotify also accepts a username and password directly via HTTP basic auth on the regular API endpoints. If only the login route were blocked, someone could keep using local credentials there and the option would be cosmetic.
The fix is a single guard in
auth/authentication.go.handleUser()is the only place a password is checked for basic auth, and all six middlewares (RequireAdmin,RequireClient,RequireElevatedClient,RequireApplicationToken,RequireApplicationOrClient,Optional) go through it, so one guard covers them all.handleClientandhandleApplicationare untouched, which is why token auth is unaffected.Only two places in the codebase verify a password: this one, and
SessionAPI.Loginbehind the now-403 route. So with the option off there is no remaining password login path. There is a test (TestLocalAuthDisabledRejectsBasicAuth) that fails if the guard is removed./gotifyinfoGained an additive
localauthboolean so the UI knows whether to show the login form. Clients that ignore it are unaffected.docs/spec.jsonis regenerated withmake update-swagger.Two notes for review
1. Refusing to start. You asked for
log.Fatal(). I used the existingfutureFatalmechanism inconfig.Get()instead, which is how other config errors are reported — same result (fatal message, exit 1), but it stays unit-testable, whereaslog.Fatal()callsos.Exit. Happy to switch to a literallog.Fatal()if you prefer.2. Deliberately left alone. Two things are out of scope here, tell me if you want either included:
GOTIFY_REGISTRATION=trueand local auth off,POST /userstill creates accounts with a password that can never be used. Arguably it should be refused for the same reason the default admin is skipped.The docs repo probably needs a matching entry for the new option.
Testing
go build ./...andgo test ./...pass, plustsc,eslintandprettieron the UI. New tests cover the config parsing and start-up refusal, the403on the login route, basic auth rejection, and that no default user is created.