Repository navigation
Describe the LDAP and OIDC deployments, and fix the OIDC one - #356
Merged
Merged
Conversation
|
Warning Review limit reachedNext included review available in 23 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
allamiro
force-pushed
the
docs/deployment-guides
branch
from
August 28, 2026 08:30
9f6cbab to
7687ee0
Compare
Closes lixmal#319. The README named LDAP and OIDC as supported backends and stopped there. The repo has had a working OpenLDAP and Keycloak setup for a while, but knowing that meant reading docker-compose.yml, .env.example and config.example.yml and piecing it together. CONFIGURATION also still said "see config.yml" after config.example.yml arrived, and the deployment section never mentioned that the app has no TLS listener at all. DEPLOYMENT now walks through each backend against the bundled service, and CONFIGURATION points at config.example.yml and calls out use_keyring and trust_proxy_headers, the two options that decide whether a deployment is sound rather than merely running. The OIDC instructions did not work as they stood. The compose file suggested issuer: 'http://keycloak:8080/realms/keepass', which the app accepts happily, but the browser is then redirected to the authorization endpoint on the same host and cannot resolve a compose service name. Keycloak is now pinned with --hostname so it names one issuer whichever address it is asked on, the issuer stays the public URL, and discovery_url points at the internal one. Verified by running the stack: the provider redirect lands on the published address, the token exchange goes over the compose network, and the login completes. The LDAP path was run the same way, wrong password included. Configuring a test backend takes two files, because two programs are being configured: .env sets the service up, config.yml tells the app how to reach it. config.yml does no variable expansion, so where they overlap the value has to be written out in both. That was implied by the walkthroughs but never stated, and is now a table of what each variable sets and what it has to be copied into, naming the three that do not follow the variable at all — the realm, client id and secret describe what tests/keycloak/realm.json already contains, and its redirect URI is fixed at port 8080. Two smaller corrections. The from-source build passed --target-dir release and then told the reader to run target/release/keepass4web-rs, which is not where that puts the binary; the RUSTFLAGS above it are x86-only and break an aarch64 build, so they are now shown separately with that said. And config.yml is not read as a set of appendable fragments: telling people to append an auth_backend block produces a duplicate key and the app refuses to start, so the instructions say to edit the existing lines. Ports, credentials and the Keycloak URL move into .env.example, which every variable now explains, and the compose file reads them instead of hard-coding.
allamiro
force-pushed
the
docs/deployment-guides
branch
from
August 28, 2026 08:37
7687ee0 to
c54f2c9
Compare
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 #319.
The README named LDAP and OIDC as supported backends and left it there. The OpenLDAP and Keycloak services have been in the repo for a while, but finding out how to use them meant reading
docker-compose.yml,.env.exampleandconfig.example.ymland piecing it together.CONFIGURATIONalso still said "seeconfig.yml" afterconfig.example.ymllanded, and nothing anywhere said the app has no TLS listener.The OIDC instructions did not work
Worth reviewing first, since it is a behaviour change and not just prose. The compose file suggested:
The app accepts that happily — it fetches the document over the compose network and the issuer matches. Then the browser is redirected to the authorization endpoint on that same host and cannot resolve
keycloak, so the login never starts.Keycloak is now started with
--hostname=${KC_PUBLIC_URL}so it names one issuer whichever address it was asked on,issuerstays the public URL the browser can reach, anddiscovery_urlpoints at the internal one. The app already rewrites the endpoints it calls itself back to the internal host, so the token exchange and the JWKS fetch still go over the compose network.Checked by running it, not by reading it: the redirect lands on the published address, the callback returns a code, and the app logs
user login ... successful. The LDAP path was run the same way, including a wrong password being rejected.Two files, and which holds what
Configuring a test backend takes both
.envandconfig.yml, because two programs are being configured — one sets the service up, the other tells the app how to reach it.config.ymldoes no variable expansion, so where they overlap the value has to be written out in both. There is now a table of what each variable sets and what it has to be copied into, including the three that do not follow the variable at all: the realm, client id and secret describe whattests/keycloak/realm.jsonalready contains, and its redirect URI is fixed at port 8080.Durations
db_session_timeoutandsession_lifetimesaid what they were but not what may be written in them or how to choose. Both now list the accepted syntax — verified againsthumantime_serderather than assumed, including thatmonthsandyearsare averages and that there is no value meaning "never" — with a few worked examples and a note that the two are independent: the database can close while the session is still valid, and then only the master password is asked for again. No values changed.Two smaller corrections
--target-dir releaseand then told the reader to runtarget/release/keepass4web-rs, which is not where that puts the binary. TheRUSTFLAGSabove it are x86-only and fail an aarch64 build, so they are now shown separately with that said, matching what the Dockerfile does per target triple.config.ymlis not a set of appendable fragments. Instructions that append anauth_backendblock produceduplicate fieldand the app panics on startup, so the text says to edit the lines already there. I walked into this while testing, which is the only reason it is called out.Parameterised rather than hard-coded
Ports, credentials and the Keycloak URL live in
.env.exampleand the compose file reads them.${...}is used only where compose actually expands it; theconfig.ymlsnippets show literal values, because that file has no interpolation and writing variables there would be misleading.TLS
A short section, because the answer is currently "there isn't any":
listen/portgo straight toHttpServer::bindand there is nowhere to give it a certificate, so anything beyond localhost belongs behind a proxy. It also notes what follows forcookie_samesiteandtrust_proxy_headers, thatldaps://andldapi://are accepted, and that OIDC and the HTTP database backend verify against a bundled root store rather than the system one — so an internal provider on a private CA is rejected even when the host trusts that CA. Happy to drop this section if you would rather it lived elsewhere.The TOC is regenerated with doctoc.
version:is dropped from the compose file, which Compose v2 warns about as obsolete.