feat(core): DSPX-609 - add cli-client to keycloak provisioning #2396
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @alkalescent, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello! Gemini here, providing a summary of this pull request. This PR, titled feat(core): DSPX-609 - add cli-client to keycloak provisioning, aims to enhance the Keycloak provisioning setup by adding a dedicated client configuration specifically for command-line interface (CLI) applications. The changes involve updating the Keycloak data configuration file to include the new client definition and making a corresponding update in the documentation to reflect a deprecated configuration option.
Highlights
- Add CLI Client to Keycloak Provisioning: A new client named
cli-clientis added to the Keycloak provisioning YAML configuration. This client is configured as a public client, enabled, and includes redirect URIs suitable for local development/testing (http://localhost:*/*). It also utilizes thecustomAudMapper. - Deprecate Documentation Setting: The
public_client_idsetting in thedocs/Configuring.mdfile is commented out and explicitly marked as DEPRECATED, likely indicating that specific client configurations like the newcli-clientare now the preferred approach.
Changelog
- docs/Configuring.md
- Commented out the
public_client_idconfiguration line and added a DEPRECATED comment.
- Commented out the
- service/cmd/keycloak_data.yaml
- Added a new client definition for
cli-clientincluding its ID, name, enabled status, public client flag, redirect URIs (http://localhost:*/*), and protocol mappers.
- Added a new client definition for
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A client for the command line,
In YAML, its config does shine.
Public and free,
For localhost, you see,
Authentication, now quite fine.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a new cli-client to the Keycloak provisioning configuration, which is a positive step for enabling command-line interface interactions with the platform.
The primary concern identified is related to the security of the redirectUris configured for this new client. It currently uses overly broad wildcards, which should be addressed. Additionally, the pull request checklist (tests, documentation) is currently unchecked, and these aspects are important to complete before merging.
No explicit style guide was provided, so the review is based on general best practices for YAML and Markdown.
Summary of Findings
- Security: Overly Permissive Redirect URI for
cli-client: TheredirectUrisfor the newcli-clientinservice/cmd/keycloak_data.yamlis configured ashttp://localhost:*/*. This use of wildcards for both port and path presents a security risk, potentially allowing malicious applications on the user's local machine to intercept authentication tokens. It is strongly recommended to use specific ports and paths for redirect URIs. - Process: PR Checklist Incomplete: The pull request checklist (unit tests, integration tests, documentation) is currently unchecked. It's important to add or update these to ensure the changes are well-tested and documented before merging.
- Configuration: Deprecated
public_client_idCommented Out: Indocs/Configuring.md, thepublic_client_idconfiguration line was commented out. This change reflects its deprecated status and is a minor cleanup. No detailed review comment was added for this specific change as its severity is considered low and falls outside the requested feedback levels ofmedium,high, orcritical.
Merge Readiness
Due to the high-severity security concern regarding the redirectUris for the cli-client and the incomplete pull request checklist (tests and documentation), I recommend that these issues be addressed before this PR is merged. Addressing the redirect URI will significantly improve the security posture of the CLI authentication flow.
As a reviewer, I am not authorized to approve pull requests. Please ensure further review and approval from the designated maintainers once the suggested changes have been implemented.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
Error Summary:
Standard Benchmark Metrics Skipped or Failed |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Bulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
Error Summary:
Standard Benchmark Metrics Skipped or Failed |
🤖 I have created a release *beep* *boop* --- ## [0.7.0](service/v0.6.0...service/v0.7.0) (2025-06-24) ### ⚠ BREAKING CHANGES * **policy:** disable kas grants in favor of key mappings ([#2220](#2220)) ### Features * **authz:** Add caching to keycloak ERS ([#2466](#2466)) ([f5b0a06](f5b0a06)) * **authz:** auth svc registered resource GetDecision support ([#2392](#2392)) ([5405674](5405674)) * **authz:** authz v2 GetBulkDecision ([#2448](#2448)) ([0da3363](0da3363)) * **authz:** cache entitlement policy within authorization service ([#2457](#2457)) ([c16361c](c16361c)) * **authz:** ensure logging parity between authz v2 and v1 ([#2443](#2443)) ([ef68586](ef68586)) * **core:** add cache manager ([#2449](#2449)) ([2b062c5](2b062c5)) * **core:** consume RPC interceptor request context metadata in logging ([#2442](#2442)) ([2769c48](2769c48)) * **core:** DSPX-609 - add cli-client to keycloak provisioning ([#2396](#2396)) ([48e7489](48e7489)) * **core:** ERS cache setup, fix cache initialization ([#2458](#2458)) ([d0c6938](d0c6938)) * inject logger and cache manager to key managers ([#2461](#2461)) ([9292162](9292162)) * **kas:** expose provider config from key details. ([#2459](#2459)) ([0e7d39a](0e7d39a)) * **main:** Add Close() method to cache manager ([#2465](#2465)) ([32630d6](32630d6)) * **policy:** disable kas grants in favor of key mappings ([#2220](#2220)) ([30f8cf5](30f8cf5)) * **policy:** Restrict deletion of pc with used key. ([#2414](#2414)) ([3b40a46](3b40a46)) * **sdk:** allow Connect-Protocol-Version RPC header for cors ([#2437](#2437)) ([4bf241e](4bf241e)) ### Bug Fixes * **core:** remove generics on new platform cache manager and client ([#2456](#2456)) ([98c3c16](98c3c16)) * **core:** replace opentdf-public client with cli-client ([#2422](#2422)) ([fb18525](fb18525)) * **deps:** bump github.com/casbin/casbin/v2 from 2.106.0 to 2.107.0 in /service in the external group ([#2416](#2416)) ([43afd48](43afd48)) * **deps:** bump github.com/opentdf/platform/protocol/go from 0.4.0 to 0.5.0 in /service ([#2470](#2470)) ([3a73fc9](3a73fc9)) * **deps:** bump github.com/opentdf/platform/sdk from 0.4.7 to 0.5.0 in /service ([#2473](#2473)) ([ad37476](ad37476)) * **deps:** bump the external group across 1 directory with 2 updates ([#2450](#2450)) ([9d8d1f1](9d8d1f1)) * **deps:** bump the external group across 1 directory with 2 updates ([#2472](#2472)) ([d45b3c8](d45b3c8)) * only request a token when near expiration ([#2370](#2370)) ([556d95e](556d95e)) * **policy:** fix casing bug and get provider config on update. ([#2403](#2403)) ([a52b8f9](a52b8f9)) * **policy:** properly formatted pem in test fixtures ([#2409](#2409)) ([54ffd23](54ffd23)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Proposed Changes
Checklist
Testing Instructions