OIDC Server by jmattheis · Pull Request #941 · gotify/server

jmattheis · 2026-03-29T21:19:32Z

This MR:

Refactors the auth handling by replacing the old requireToken / callback structure with a simpler approach.
Adds cookie-based token reading and /auth/local/{login,logout} endpoints for web UI authentication.
- Without this, we'd have to redirect to an url https://gotify-ui.net/#/oidc/callback?token=... to store the token in the local storage. Using cookies seems to be more easy than securing this against CSRF and stuff. Also with HttpOnly cookies we guard against pontential token exposures.
- Secure cookies are supported but disabled by default to avoid breaking existing HTTP installations. We could detect HTTPS via the Referer or X-Forwarded-Proto headers in the future.
Add /gotifyinfo endpoint. In the android app we need to know if oidc is enabled, and the android app also displays the version. I didn't want to add this information to the /version endpoint so it's a new endpoint. I don't really like the naming, but I wanted something unique so that the android app can be sure it's a gotify/server instance.

Web UI: Standard OAuth2 Authorization Code + PKCE

This is the standard OIDC implementation with a library.

A "Login with OIDC" button appears when OIDC is enabled (This is only visible in the production build, when the UI is served by gotify/server)
Clicking it redirects to GET /auth/oidc/login, which then redirects to the IdP.
After authentication the IdP redirects to /auth/oidc/callback, which:
1. Exchanges the authorization code for tokens.
2. Resolves the user via the userinfo endpoint (creating the user if configured).
3. Creates a Gotify client, sets the session cookie, and redirects to the UI.

Android App flow

Android's recommended App Links (verified domains) aren't usable here because the server is self-hosted at a user-chosen domain.

I guess most of the android app, are doing their own OIDC flow against the IdP server to then obtain a token from the IdP server to then access the resources with the token. In our case, we need a client token from gotify.

We could let the android app do the whole OIDC flow, but this would require us to have a public oauth2 client, as we don't want to store the client_secret in the android app. Requesting the client secret from the server seems like a bad idea.

Another simple implementation would be to do the same flow as the Web-UI, but in the end redirecting to a custom URI scheme like gotify://oidc/callback?token=C123123123 and then using this token for authentication. I've decided against this because the custom URI scheme can be set by any app, and therefore intercept the token.

PKCE was exactly made to prevent this, so I've searched for a way to still get the benefits from PKCE, but not having to reimplement a OIDC like handshake.

This flow is non standard but it's basically stolen from Vaultwarden. I'm not that deep in security, but I'd expect they spend some time designing this, and ensuring it's secure.

It works like this.

The app generates a PKCE code_verifier / code_challenge pair.
The app POST /auth/oidc/external/authorize with code_challenge, a custom-scheme redirect_uri, and a client name. The server stores a pending session keyed by a random state token, and then reponds with the the authorization URL + state. Vaultwarden 1, Vaultwarden 2,
The app opens the authorization URL in a browser. The IdP authenticates the user and redirects to the app's custom URI (gotify://oidc/callback) with code and state.
POST /auth/oidc/external/token with code, state, and code_verifier. The server gets the pending session, relays the code_verifier to the IdP for PKCE validation, exchanges the code for tokens, resolves the user, and returns a Gotify client token + user info. Vaultwarden 1, Vaultwarden 2

PKCE is required, and only verified by the IdP. We could support non PKCE IdP by manually validating the challange on the /auth/oidc/external/token endpoint, simliar to Vaultwarden Impl, but I think most of the Idp will support PKCE, so it's required until someone requests it.

For testing I've added Dex and Authelia config in ./test/oidc.

Related MRs:

I've manually tested this with:

Web-UI: Dex (on root, and subpath /gotify/ with a reverse proxy) and authelia
Android with Dex and Authelia

Disclaimer: this PR was created with AI assistance.

Add two new endpoints for native app OIDC authentication using the PKCE relay pattern (similar to Vaultwarden's SSO implementation): - POST /auth/oidc/external/authorize - accepts a PKCE code_challenge from the client, forwards it to the IdP, and returns the authorize URL - POST /auth/oidc/external/token - accepts the auth code and code_verifier, relays them to the IdP for token exchange, and returns a gotify client token The server never generates its own PKCE pair for this flow. It then relays the client's code_challenge to the IdP during authorization and the code_verifier during token exchange. The IdP validates the binding. Pending auth sessions are stored in memory with a 10-minute TTL. CSRF protection is provided by the state parameter, which contains a cryptographically random nonce and is validated on the token exchange. The state is single-use (deleted from the pending session map on lookup), preventing replay attacks. Even without single-use enforcement, replay would be harmless since the IdP's authorization code can only be exchanged once.

It's not easy to test this automatically without a real oidc server.

codecov · 2026-03-29T21:25:19Z

Codecov Report

❌ Patch coverage is 53.82353% with 157 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.92%. Comparing base (0610537) to head (c463d98).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
api/oidc.go	33.11%	99 Missing and 4 partials ⚠️
auth/authentication.go	77.88%	11 Missing and 12 partials ⚠️
api/session.go	55.31%	13 Missing and 8 partials ⚠️
router/router.go	52.38%	8 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #941      +/-   ##
==========================================
- Coverage   78.70%   75.92%   -2.79%     
==========================================
  Files          57       60       +3     
  Lines        2517     2758     +241     
==========================================
+ Hits         1981     2094     +113     
- Misses        397      514     +117     
- Partials      139      150      +11

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jmattheis · 2026-03-29T21:26:44Z

api/health.go

@@ -16,7 +16,7 @@ type HealthAPI struct {
 }


@eternal-flame-AD sorry this is kinda big, but would you mind having a look at this? Don't worry about declining if you don't want to review this, or don't have the time. I don't want to burden you with this :D.

Anyway, my plan is to create a snapshot docker build in the coming days and then let the people in #433 try out the functionality to improve the user-guide and find possible bugs with special IdP servers. So there is no rush reviewing this.

Sure, I can look at this tonight first to see if there are large architectural/security changes needed and we can do a final round for final check and nitpicks after the code settles. Probably won't review Android though because I'm not very familar with android dev.

Awesome thanks!

eternal-flame-AD · 2026-03-31T09:54:49Z

api/oidc.go

+func (a *OIDCAPI) storePendingSession(state string, session *pendingOIDCSession) {
+	a.pendingSessionsMu.Lock()
+	defer a.pendingSessionsMu.Unlock()
+	for s, sess := range a.pendingSessions {


I feel this could be done better, this has O(n^2) complexity over number of users who are authenticating at the same time.

If we are unconditionally iterating through the entire map as the first action of the flow anyways, might as well just use an array (with swap-delete).

A more complex solution is bulk cleanup like this: https://gist.github.com/eternal-flame-AD/e4a47a8a7e6b26bba365a7d24ab84221

My plan was to have a simple implementation as we probably don't have many parallel logins anyway.

Your decay map doesn't seem to complex, I'll use it, as it's not that much code overhead.

eternal-flame-AD · 2026-03-31T11:37:34Z

api/oidc.go

+			http.Error(w, err.Error(), status)
+			return
+		}
+		clientName, _, _ := strings.Cut(state, ":")


Can the name itself already have a colon in it?

True, I'll reverse the order of the random nonce and the client name and then split by the first colon as the nonce is hex encoded and never includes a colon.

eternal-flame-AD · 2026-03-31T12:04:23Z

api/oidc.go

+		ctx.AbortWithError(http.StatusBadRequest, err)
+		return
+	}
+	state, err := a.generateState(req.Name)


Is this how it works for VaultWarden (server generates state) ? I'm not super sure what this is for. From my cursory exploration it seems like in VaultWarden state is simply for the client's convenience (might be wrong..)

Okay this seems to be documented in RFC6749, I got it messed up. The flow is right although the bookkeeping ClientName could be clearer (don't keep them both in the server memory and in the token text at the same time).

No, this is different from vaultwarden, but as I understood is this for CSRF protection, so basically the android app has to check the initial state against the state that will be received on the redirect uri. So I don't think it matters.

Do you mean removing the client name from the pending session struct and then directly reading it from the state similar to how it's done in the web ui flow?

eternal-flame-AD · 2026-03-31T12:15:22Z

docs/spec.json

        }
      }
    },
+    "/gotifyinfo": {


I guess if we want to be standard we can use /.well-known/appspecific which is IANA registered

https://github.com/Vroo/well-known-uri-appspecific/blob/main/well-known-uri-for-application-specific-purposes.txt

I'm a little worried if users have badly configured reverse proxies and e.g. serve certbot/acme stuff directly under .well-known instead of .well-known/acme-challenge/ which then could block gotify/android from working.

What do you think?

eternal-flame-AD · 2026-03-31T12:19:27Z

config/config.go

+		Enabled       bool   `default:"false"`
+		Issuer        string `default:""`
+		ClientID      string `default:""`
+		ClientSecret  string `default:""`


I think it would be great if this can be read from a file as well for things like docker secrets

Yeah, I agree, but let's do this in a separate MR.

I'm thinking about dropping the YAML support and just support env vars. The current config library isn't really great for array values about which we have many issues.

Besides env vars, the user could specify a env file. Similar to this https://screego.net/#/config

eternal-flame-AD · 2026-03-31T12:21:40Z

auth/cookie.go

+)
+
+// CookieMaxAge is the lifetime of the session cookie in seconds (7 days).
+const CookieMaxAge = 7 * 24 * 60 * 60


Should we actually enforce this (delete or invalidate clients an expiration date in DB)?

I feel like we should otherwise I'm not very certain of the benefits of adding a lifetime. It's also useful for things like orgs who may want to automatically deprovision users after they leave.

I'd say there is still a benefit, I'd say if a user hasn't used the cookie in a longer time, it's better to just discard it, even if it's still valid.

I'll add a "source" string field to the client table which can be "user, local-login, oidc-login". And then periodically prune clients with type *-login that didn't had activity in the last 7 days.

Do you think 7 days is a good enough default for now?

jmattheis added 13 commits March 28, 2026 21:13

fix: simplify authentication handling

86df116

fix: docs indention

e7234fd

fix: read token from cookie

0efef49

feat: implement /auth/local/{login,logout}

1774781

fix: use cookie login in UI

862fa3b

fix: make secure cookie configurable

9a51dd7

feat: oidc

e072c64

fix: update example config to be in sync with website

c92b130

fix: add /gotifyconfig endpoint

9ca2623

fix: increase api version

83895b5

test: oidc tests

44bda75

It's not easy to test this automatically without a real oidc server.

test: oidc dex + authelia

c463d98

jmattheis requested a review from a team as a code owner March 29, 2026 21:19

jmattheis commented Mar 29, 2026

View reviewed changes

This was referenced Mar 29, 2026

OIDC Documentation gotify/website#100

Open

OIDC Android gotify/android#444

Open

eternal-flame-AD reviewed Mar 31, 2026

View reviewed changes

Uh oh!

Conversation

jmattheis commented Mar 29, 2026

Web UI: Standard OAuth2 Authorization Code + PKCE

Android App flow

Uh oh!

codecov bot commented Mar 29, 2026

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmattheis Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

jmattheis Apr 1, 2026 •

edited

Loading