Skip to content

keep plugins settings in a readonlyrest.yml file - #105

Open
Dzuming wants to merge 6 commits into
masterfrom
epic/keep-plugins-settings-in-a-single-configuration
Open

Dzuming wants to merge 6 commits into
masterfrom
epic/keep-plugins-settings-in-a-single-configuration

Conversation

@Dzuming

@Dzuming Dzuming commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features

    • Added authentication support for Keycloak SAML, Keycloak OIDC, and LemonLDAP OIDC in the showcase environment.
    • Added configurable cookie authentication, logging, session signing, user mappings, and logout settings.
    • Updated the showcase stack to Elasticsearch and Kibana 9.5.0 with ROR binaries version 1.70.3.
  • Bug Fixes

    • Ensured Kibana loads the ReadonlyREST configuration in both binary installation modes.

Move ReadonlyREST Kibana configuration into the main ES configuration
file
and update plugin versions.
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: f3336aeb-d173-4ecf-a16f-b38ec61fe157

📝 Walkthrough

Walkthrough

The showcase now uses Elasticsearch and Kibana 9.5.0. ReadonlyREST contains Keycloak SAML/OIDC and LemonLDAP OIDC settings. Both Kibana image variants package the shared ReadonlyREST configuration.

Changes

Kibana authentication

Layer / File(s) Summary
ReadonlyREST authentication providers
ror-demo-cluster/conf/es/readonlyrest.yml, ror-demo-cluster/conf/kbn/enterprise-ror-newplatform-kibana.yml
Added Kibana settings and Keycloak SAML/OIDC and LemonLDAP OIDC providers to readonlyrest.yml. Removed the previous definitions from the Kibana-specific configuration.
Kibana image configuration wiring
ror-demo-cluster/.env-showcase, ror-demo-cluster/images/kbn/Dockerfile-use-ror-binaries-from-api, ror-demo-cluster/images/kbn/Dockerfile-use-ror-binaries-from-file
Activated Elasticsearch and Kibana 9.5.0 settings. Configured the ROR versions and copied readonlyrest.yml into both Kibana image variants.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Kibana
  participant ReadonlyREST
  participant Keycloak
  participant LemonLDAP
  Kibana->>ReadonlyREST: Load authentication configuration
  ReadonlyREST->>Keycloak: Use SAML or OIDC endpoints
  ReadonlyREST->>LemonLDAP: Use OIDC endpoints
Loading

Possibly related PRs

  • beshu-tech/ror-sandbox#85: Both PRs modify Kibana ReadonlyREST authentication settings for Keycloak and related identity providers.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: moving plugin settings into a single readonlyrest.yml configuration file.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch epic/keep-plugins-settings-in-a-single-configuration

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@ror-demo-cluster/conf/es/readonlyrest.yml`:
- Around line 90-91: Update the groupsParameter setting in the readonlyrest
configuration from Role to memberOf, while leaving usernameParameter unchanged,
so SAML group claims map correctly for ACL assignments.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c1cf036f-3920-489c-a0e4-bb1e0e84fffb

📥 Commits

Reviewing files that changed from the base of the PR and between e9664eb and 1c1513a.

⛔ Files ignored due to path filters (1)
  • ror-demo-cluster/readonlyrest_kbn_universal-1.71.0-pre4_es9.5.0.zip is excluded by !**/*.zip
📒 Files selected for processing (5)
  • ror-demo-cluster/.env-showcase
  • ror-demo-cluster/conf/es/readonlyrest.yml
  • ror-demo-cluster/conf/kbn/enterprise-ror-newplatform-kibana.yml
  • ror-demo-cluster/images/kbn/Dockerfile-use-ror-binaries-from-api
  • ror-demo-cluster/images/kbn/Dockerfile-use-ror-binaries-from-file
💤 Files with no reviewable changes (1)
  • ror-demo-cluster/conf/kbn/enterprise-ror-newplatform-kibana.yml

Comment on lines +90 to +91
usernameParameter: 'nameID'
groupsParameter: 'Role'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

jq '
  .clients[]
  | select(.clientId == "ror-saml")
  | {
      defaultClientScopes,
      protocolMappers: [
        .protocolMappers[]
        | {name, protocolMapper, config}
      ]
    }
' ror-demo-cluster/conf/keycloak/ror-realm.json

rg -n -C 3 'groupsParameter|external_group_ids|memberOf|Role' \
  ror-demo-cluster/conf/es/readonlyrest.yml \
  ror-demo-cluster/conf/keycloak/ror-realm.json

Repository: beshu-tech/ror-sandbox

Length of output: 7797


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import json
from pathlib import Path

p = Path("ror-demo-cluster/conf/keycloak/ror-realm.json")
data = json.loads(p.read_text())

client = next(c for c in data["clients"] if c.get("clientId") == "ror-saml")
print("client protocol:", client.get("protocol"))
print("client defaultClientScopes:", client.get("defaultClientScopes"))
print("client protocolMappers:")
for mapper in client.get("protocolMappers", []):
    print(json.dumps(mapper, sort_keys=True))

print("\nall protocol mappers mentioning Role, memberOf, or groups:")
for owner, obj in [("client", client)] + [
    (f"clientScope:{scope.get('name')}", scope)
    for scope in data.get("clientScopes", [])
]:
    for mapper in obj.get("protocolMappers", []):
        text = json.dumps(mapper, sort_keys=True)
        if any(term in text for term in ("Role", "memberOf", "groups")):
            print(owner, text)

print("\nuser groups:")
for user in data.get("users", []):
    print(user.get("username"), user.get("groups", []))
PY

rg -n -C 4 'groupsParameter|usernameParameter|memberOf|saml-group-membership-mapper|attribute.name.?["'\'']?[ :]?["'\'']?Role|name.?Role' \
  --glob '!*.lock' .

Repository: beshu-tech/ror-sandbox

Length of output: 12988


Set groupsParameter to memberOf.

The Keycloak SAML mapper emits memberOf, and no mapper emits Role. The current setting prevents group-based ACL assignments for SAML users.

🤖 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 `@ror-demo-cluster/conf/es/readonlyrest.yml` around lines 90 - 91, Update the
groupsParameter setting in the readonlyrest configuration from Role to memberOf,
while leaving usernameParameter unchanged, so SAML group claims map correctly
for ACL assignments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant