feat: Implement OIDC Single Sign-On support with configuration options #6161
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| router.get("/auth/oidc/login", async (req, res) => { | ||
| try { | ||
| cleanupExpiredSessions(); | ||
|
|
||
| const config = await getOIDCConfig(); | ||
| if (!config.enabled) { | ||
| throw createOIDCError("oidcDisabled", "OIDC is disabled"); | ||
| } | ||
|
|
||
| const client = await getClient(config); | ||
| const redirectURI = await resolveRedirectURI(req, config.redirectURI); | ||
|
|
||
| const state = generators.state(); | ||
| const nonce = generators.nonce(); | ||
| const returnTo = validateReturnTo(req.query.returnTo); | ||
|
|
||
| sessionStore.set(state, { | ||
| nonce, | ||
| redirectURI, | ||
| createdAt: Date.now(), | ||
| returnTo, | ||
| }); | ||
|
|
||
| const authorizationUrl = client.authorizationUrl({ | ||
| scope: config.scope, | ||
| redirect_uri: redirectURI, | ||
| state, | ||
| nonce, | ||
| }); | ||
|
|
||
| res.redirect(authorizationUrl); | ||
| } catch (error) { | ||
| log.error("oidc", error); | ||
| const errorCode = error.oidcCode || "oidcGenericError"; | ||
| res.redirect(`/oidc/callback?error=${encodeURIComponent(errorCode)}`); | ||
| } | ||
| }); |
Check failure
Code scanning / CodeQL
Missing rate limiting High
|
Thank you for your feedback. During authentication, if the user does not exist, they are then created in the users table. So I think with PR #3571 there won’t be any issues — they will show up in the list. The accounts just don’t have a password. |
|
As always, I hope this kind of pull requests should be discussed first, before coding. Since I have no knowledge of OIDC, it would be difficult for me to test and ensure the pull request whether it is 100% safe. I will not be able to maintain it in the future due to my skill issue. Also since I was casually created the auth logic at the beginning, my appoarch is acutally not following the best practice. Instead of fixing it, I am thinking maybe we should move to a better solution. Recently I have studied some Node.js auth libraries, one of them is Better Auth (20K+ starts). It basically provides many useful apis for further development like:
Also it provides a lot of plugins, one of them is OIDC: I have tried this library in my another project, I would say it is really better and easier to use. I kind of want to migrate it in Uptime Kuma too. |
|
@edwin-anne has not re-invented the wheel, the diff leans on the I would suggest here that perfection is the enemy of good (and the enemy of .. actually shipping features). If you have long-term ambitions to migrate to a more powerful auth library, great - but can we please have a patch release which includes community contributions like this first? |
|
@lol768 while your sentiment might be right in a company where moving fast and breaking things is acceptable, in OSS we don't really have this advantage. I don't have a refactoring/engineering budget for this project, so everything in master kind of stays the way it got merged. "does this make future changes harder" is thus critical. I am also not an expert on security, but CodeQL has found one hole above, so 🤷🏻♂️ |
|
The Semmle/CodeQL warning here is nonsense. You can see the intent behind the rule is to flag cases where:
The flagged code:
There would be little-to-no benefit from trying to introduce rate-limiting here, and a number of potential problems that it could cause (imagine if e.g. a user closes their browser tab by accident and then restores it). SAST tools like CodeQL do have their place, but their output needs to be taken with a pinch of salt and a bit of common sense applied to what is going on. |
As long as a stable username is resolved from the claims in the ID token in the same way as this PR does, you could bin-off all of the code in this PR a week after releasing it, pull in your preferred auth library, and ship a new release that allows all of the existing users to login in exactly the same way, without any sort of interruption to user's workflows. That is the beauty of OpenID Connect. |
Exactly, so the only thing we need to discuss here is where in the UI would the OIDC settings be (I'm perfectly fine with how this PR does it) - and when the decision is made to change the underlying library, everything will work as it did. From everything I'm seeing so far, Kuma is suffering from crippling release/feature fear and is stagnating - but to put my money where my mouth is - @edwin-anne could you build an image from your branch so we can test it out? (ghcr please, not dockerhub) .. Even though I really agree that it would be like 15 times easier just migrating to Better-Auth first, and then working on child changes (such as oidc) |
That is a big if. I don't feel comfortable accepting this risk, sorry. |
|
My guy, you're acting as if the fate of humanity depends on Kuma being bug free (which is not inherently bad, but live a little). Unsure where this is coming from, but it's definitely something to dwell about. Anyways, as lol explained, this way of getting the username is the de-facto standard and literally how oidc works - but then again, let's wait for an image from edwin, test it out, and see how it goes. |
|
@rwjack If you want to test Uptime Kuma with OIDC authentication, you can run the following Docker image: 👉 This image will automatically pull the latest code from my edwin-anne:master branch to include the OIDC connection changes. |
|
Now this is exactly why you don't space out releases over a year whilst making major changes, so now instead of testing oidc I have to waste time debugging dumb issues. For some reason the container cannot create any folder within the data directory. If I create "/data/upload" manually, after a restart, it then asks for "screenshots", then "docker-tls" - you get the idea. And once all that is done: Even though the permissions are fine on my end (if it's a docker volume, docker manages the perms itself), but for clarity: root in wall-e in /opt/kuma-test took 4s ❯ docker exec -it kuma sh
$ cd /app/data
$ ls -la
total 24
drwxr-xr-x 5 node node 4096 Oct 17 11:37 .
drwxr-xr-x 1 node root 4096 Oct 17 11:38 ..
drwxr-xr-x 2 node node 4096 Oct 17 11:37 docker-tls
drwxr-xr-x 2 node node 4096 Oct 17 11:36 screenshots
drwxr-xr-x 2 node node 4096 Oct 17 11:36 upload
$ touch db-config.json
$ ls -la
total 24
drwxr-xr-x 5 node node 4096 Oct 17 11:40 .
drwxr-xr-x 1 node root 4096 Oct 17 11:38 ..
-rw-r--r-- 1 node node 0 Oct 17 11:40 db-config.json
drwxr-xr-x 2 node node 4096 Oct 17 11:37 docker-tls
drwxr-xr-x 2 node node 4096 Oct 17 11:36 screenshots
drwxr-xr-x 2 node node 4096 Oct 17 11:36 upload
$
root in wall-e in /opt/kuma-test took 16s ❯ cd -
/var/lib/docker/volumes
root in wall-e in lib/docker/volumes ❯ ll kuma-test_data/_data
drwxr-xr-x - server server 17 Oct 13:37 docker-tls
drwxr-xr-x - server server 17 Oct 13:36 screenshots
drwxr-xr-x - server server 17 Oct 13:36 upload
.rw-r--r-- 0 server server 17 Oct 13:40 db-config.json
root in wall-e in lib/docker/volumes ❯ ll kuma-test_data
drwxr-xr-x - server server 17 Oct 13:40 _data
$ id
uid=1000(node) gid=1000(node) groups=1000(node)
root in wall-e in lib/docker/volumes ❯ id server
uid=1000(server) gid=1000(server) groups=1000(server),24(cdrom),25(floppy),29(audio),30(dip),44(video),46(plugdev),109(netdev)---
volumes:
data:
services:
kuma:
image: louislam/uptime-kuma:pr-test2
restart: unless-stopped
container_name: kuma
hostname: kuma
environment:
- PUID=1000
- PGID=1000
- NODE_EXTRA_CA_CERTS=/certs/HQ-ca.pem
- UPTIME_KUMA_GH_REPO=edwin-anne:master
ports:
- "127.0.0.1:15001:3001"
volumes:
- data:/app/data
- /etc/ssl/HQ-ca.pem:/certs/HQ-ca.pem:ro |
|
We skip the permissions issue if I don't use docker compose and run directly, but the issue with db-config. json persists: root in wall-e in /opt/kuma-test ❯ docker run --rm -it -p 127.0.0.1:15001:3001 -e UPTIME_KUMA_GH_REPO="edwin-anne:master" louislam/uptime-kuma:pr-test2
> [email protected] start-pr-test
> node extra/checkout-pr.mjs && npm install && npm run dev
Checking out PR from edwin-anne:master
remote: Enumerating objects: 727, done.
remote: Counting objects: 100% (559/559), done.
remote: Total 727 (delta 559), reused 559 (delta 559), pack-reused 168 (from 1)
Receiving objects: 100% (727/727), 667.72 KiB | 3.55 MiB/s, done.
Resolving deltas: 100% (609/609), completed with 139 local objects.
From https://github.com/edwin-anne/uptime-kuma
* branch master -> FETCH_HEAD
* [new branch] master -> edwin-anne/master
HEAD is now at e2b6b3fb Merge branch 'master' of https://github.com/edwin-anne/uptime-kuma
added 7 packages, removed 5 packages, changed 184 packages, and audited 1330 packages in 17s
285 packages are looking for funding
run `npm fund` for details
11 vulnerabilities (2 low, 3 moderate, 6 high)
To address issues that do not require attention, run:
npm audit fix
To address all issues possible (including breaking changes), run:
npm audit fix --force
Some issues need review, and may require choosing
a different dependency.
Run `npm audit` for details.
> [email protected] dev
> concurrently -k -r "wait-on tcp:3000 && npm run start-server-dev " "npm run start-frontend-dev"
> [email protected] start-frontend-dev
> cross-env NODE_ENV=development vite --host --config ./config/vite.config.js
The CJS build of Vite's Node API is deprecated. See https://vite.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details.
VITE v5.4.19 ready in 494 ms
➜ Local: http://localhost:3000/
➜ Network: http://172.20.0.2:3000/
➜ Vue DevTools: Open http://localhost:3000/__devtools__/ as a separate window
➜ Vue DevTools: Press Alt(⌥)+Shift(⇧)+D in App to toggle the Vue DevTools
➜ press h + enter to show help
> [email protected] start-server-dev
> cross-env NODE_ENV=development node server/server.js
Welcome to Uptime Kuma
Your Node.js version: 20.19.2
2025-10-17T11:53:14Z [SERVER] DEBUG: Arguments
2025-10-17T11:53:14Z [SERVER] DEBUG: {}
2025-10-17T11:53:14Z [SERVER] INFO: Env: development
2025-10-17T11:53:14Z [SERVER] DEBUG: Inside Container: false
2025-10-17T11:53:14Z [SERVER] INFO: Uptime Kuma Version: 2.0.0-beta.4
2025-10-17T11:53:14Z [SERVER] INFO: Loading modules
2025-10-17T11:53:14Z [SERVER] DEBUG: Importing express
2025-10-17T11:53:14Z [SERVER] DEBUG: Importing redbean-node
2025-10-17T11:53:14Z [SERVER] DEBUG: Importing jsonwebtoken
2025-10-17T11:53:14Z [SERVER] DEBUG: Importing http-graceful-shutdown
2025-10-17T11:53:14Z [SERVER] DEBUG: Importing prometheus-api-metrics
2025-10-17T11:53:14Z [SERVER] DEBUG: Importing 2FA Modules
2025-10-17T11:53:15Z [SERVER] INFO: Creating express and socket.io instance
2025-10-17T11:53:15Z [SERVER] INFO: Server Type: HTTP
2025-10-17T11:53:15Z [SERVER] DEBUG: Importing Monitor
2025-10-17T11:53:15Z [SERVER] DEBUG: Importing Settings
2025-10-17T11:53:15Z [SERVER] DEBUG: Importing Notification
2025-10-17T11:53:15Z [NOTIFICATION] DEBUG: Prepare Notification Providers
2025-10-17T11:53:15Z [SERVER] DEBUG: Importing Database
2025-10-17T11:53:15Z [SERVER] DEBUG: Importing Background Jobs
2025-10-17T11:53:15Z [SERVER] INFO: Data Dir: ./data/
2025-10-17T11:53:15Z [SETUP-DATABASE] INFO: db-config.json is not found or invalid: ENOENT: no such file or directory, open 'data/db-config.json'
2025-10-17T11:53:15Z [SETUP-DATABASE] INFO: Starting Setup Database on 3001
2025-10-17T11:53:15Z [SETUP-DATABASE] INFO: Open http://localhost:3001 in your browser
2025-10-17T11:53:15Z [SETUP-DATABASE] INFO: Waiting for user action... |
|
Sorry that you are having a bad time with that releases test infrastructure @rwjack
|
|
Sorry to say, but the manual openidc implementation in this PR has currently no chance of getting merged. Keeping it open is bad, sorry for you having wasted your time @edwin-anne I don't think I would be doing us a favor long term by merging if we are aligned that BetterAuth is the way we should go. Here are the things:
Could we make it work by marking it as "this feature is experimental with planned breaking changes, unadited and not tested in CI" and adding small e2e test? Not sure if we want to move into this direction though.
Not being able to login is extremely anoying and given how technical our users usually are (measured in issue quality) I do think a larger part could not recover from this. The v2.0 release (which was my fault for getting so delayed) has shown me that the "we fix it in main" really does not work in this OSS project. |
|
due note, you can always approach it like jellyseerr (now seerr) does, have some long lived branches of experimental features that get periodically brought into alignment with :latest, but latest is stable, whereas these are explicitly marked as "for-testing" |
|
We don't have capacity for this strategy and that would not really solve the core issue for this PR. |
|
As per #6276, it seems "BetterAuth" isn't going to be the... better choice given the incompatibilities with the current project - when is version 3.0.0 expected to be released? |
|
If i is just to bump the node version, that should be doable very soon-ish. We currently have lots of things on our plate. |
I think so. Maybe 2.1 -> 2.2 -> 3.0, I guess. I will mainly focus on #6276. For pull request reviews, it pretty much depends on community's help. I saw many pull requests was set in the 2.1.0 milestones, but also many of them are stalled at maybe 70-90% progress. If you would like to help:
https://github.com/louislam/uptime-kuma/pulls?q=is%3Aopen+is%3Apr+milestone%3A2.1.0 If pull requests that are eventually no one to help, I will postpone them in order to not blocking the release. |
|
So I guess #3571 will be merged to provide basic multi-admin support for the 2.x series, allowing anyone to use this feature and report any issues. This way, when v3.x arrives, a potential implementation using BetterAuth (or another lib) will benefit from that experience. wdyt @CommanderStorm @louislam? |
|
I would like to defer to @louislam on that merge decision if this makes betterauth harder/simpler |
8 participants
❗ Important Announcements
Click here for more details:
🚫 Please Avoid Unnecessary Pinging of Maintainers
We kindly ask you to refrain from pinging maintainers unless absolutely necessary. Pings are for critical/urgent pull requests that require immediate attention.
📋 Overview
What problem does this pull request address?
What features or functionality does this pull request introduce or enhance?
This pull request adds OpenID Connect (OIDC) authentication support.
With this integration, users can now log in to Uptime Kuma through an external identity provider (such as Keycloak, Auth0, or any other OIDC-compatible IdP).
Key features:
🛠️ Type of change
📄 Checklist
📷 Screenshots or Visual Changes