OSAC-1663: Add LDAP + Keycloak integration setup guide - #38
Conversation
Signed-off-by: Ofer Bochan <obochan@obochan-thinkpadp1gen7.rmtil.csb>
|
Warning Review limit reached
More reviews will be available in 17 minutes and 9 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughAdds ChangesLDAP–Keycloak Integration Guide
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@guides/ldap-keycloak-integration.md`:
- Around line 277-281: Remove the `-k` and `-ks` flags from all curl commands in
the Keycloak integration examples that disable TLS verification, particularly in
the token fetch command at the KC_TOKEN assignment and all subsequent curl
commands to the KC_URL endpoint. Either ensure proper trusted CA certificate
configuration is in place and documented, or explicitly mark these commands as
lab-only examples with appropriate warnings about security implications. The
affected sections include the token retrieval curl command and all admin API
calls throughout the guide at the specified line ranges.
- Around line 112-154: The prerequisites section lists OpenLDAP as a supported
LDAP Server Software option, but all the subsequent setup steps are specific to
389-ds/RHDS (using commands like dscreate, configuration parameters like
nsAccountLock and nsuniqueid, and references to RHDS). Remove OpenLDAP from the
supported LDAP server software list in the prerequisites section, or update it
to clarify that this guide provides setup instructions specifically for 389
Directory Server and Red Hat Directory Server only. This will prevent OpenLDAP
users from following an incomplete setup path.
- Around line 472-479: The recommended procedure in the LDAP removal section
incorrectly states that triggering a full sync will remove orphaned Keycloak
records for deleted LDAP users. This is factually incorrect. Keycloak's
full-sync does not automatically remove deleted LDAP users; orphan removal is
actually reactive and only occurs when "Remove invalid users during searches" is
enabled AND that specific user is accessed (login, admin search, or API lookup).
Rewrite the procedure section to accurately explain either the reactive "Remove
invalid users during searches" mechanism (noting it requires per-user access to
trigger removal), or document the manual "Remove imported" action in the Admin
Console (with a warning that this removes all imported users, not just deleted
ones), or provide the actual working procedure for achieving deletion.
Additionally, update the synchronization behavior table that follows to ensure
it correctly reflects Keycloak's actual LDAP federation behavior regarding
orphan handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: a3b91352-b886-460c-978a-14a3ff30e57c
📒 Files selected for processing (1)
guides/ldap-keycloak-integration.md
| KC_TOKEN=$(curl -ks "${KC_URL}/realms/master/protocol/openid-connect/token" \ | ||
| -d 'client_id=admin-cli' \ | ||
| -d 'grant_type=password' \ | ||
| -d 'username=admin' \ | ||
| -d 'password=<admin-password>' | jq -r '.access_token') |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Stop using curl -k in the Keycloak examples.
These snippets disable TLS verification (-k/-ks) while fetching admin tokens and calling the admin API. That normalizes an avoidable MITM risk on the control plane; if these are lab-only commands, label them as such. Otherwise, show the same flow without -k and require a trusted CA.
Suggested fix
- KC_TOKEN=$(curl -ks "${KC_URL}/realms/master/protocol/openid-connect/token" \
+ KC_TOKEN=$(curl -s "${KC_URL}/realms/master/protocol/openid-connect/token" \Also applies to: 289-319, 348-355, 362-372, 397-415, 438-441, 513-519
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@guides/ldap-keycloak-integration.md` around lines 277 - 281, Remove the `-k`
and `-ks` flags from all curl commands in the Keycloak integration examples that
disable TLS verification, particularly in the token fetch command at the
KC_TOKEN assignment and all subsequent curl commands to the KC_URL endpoint.
Either ensure proper trusted CA certificate configuration is in place and
documented, or explicitly mark these commands as lab-only examples with
appropriate warnings about security implications. The affected sections include
the token retrieval curl command and all admin API calls throughout the guide at
the specified line ranges.
- Remove OpenLDAP from supported LDAP software (guide is 389-ds specific) - Add TLS note explaining -k flag is for lab/self-signed environments - Fix incorrect claim that full sync removes orphaned users - Document correct Keycloak behavior for LDAP user deletion Signed-off-by: Ofer Bochan <obochan@obochan-thinkpadp1gen7.rmtil.csb>
* NO-ISSUE: Add baremetal fulfillment repos * NO-ISSUE: Add new baremetal repos to CLAUDE.md Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Adrien Gentil <agentil@redhat.com> --------- Signed-off-by: Adrien Gentil <agentil@redhat.com>
1 participant
Summary
Test plan
Summary by CodeRabbit
Documentation