feat(ci): add DPoP nonce-challenge and enforcement inputs to test actions (DSPX-3397)#3667
Conversation
📝 WalkthroughWalkthroughThe startup composite actions now accept DPoP-related boolean inputs, validate them, and propagate them into generated OpenTDF and KAS configuration so DPoP enforcement settings can be applied during startup. ChangesDPoP startup action inputs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Summary of ChangesHello, 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! This pull request introduces configuration flexibility to the CI test actions, enabling developers to exercise DPoP (Demonstrating Proof-of-Possession) flows in end-to-end tests. By adding decoupled inputs for nonce-challenge and enforcement, the changes allow for granular control over security settings without modifying the underlying platform code, facilitating more robust testing of authentication features. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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 by creating a comment using either
Customization To customize the 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 Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. The tokens dance with DPoP grace, Security tightened in every place. With flags now set to true or false, The tests perform their steady waltz. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces configuration options to enable and enforce DPoP (Demonstrating Proof-of-Possession) in the test actions start-additional-kas and start-up-with-containers. It adds the dpop-challenge-enabled and dpop-enforce-required inputs, validates their values, and updates the configuration files using yq. The feedback suggests modifying the yq command in start-additional-kas/action.yaml to conditionally set require_nonce only when enabled, ensuring consistency and avoiding writing explicit false values to the configuration.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
d9fdadd to
925e626
Compare
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 `@test/start-additional-kas/action.yaml`:
- Line 43: The additional KAS action description is pointing to the wrong
runtime config key; update the DPoP enforcement reference in the action metadata
to use server.auth.enforceDPoP instead of server.auth.dpop.enforce. Make the
same correction anywhere the dpop-enforce-required flow references that setting
so the action actually enables enforcement, and verify the relevant
action/inputs definitions in action.yaml stay consistent.
In `@test/start-up-with-containers/action.yaml`:
- Line 40: The DPoP enforcement setting is being written to the wrong config
path, so update the startup/config mapping in the action and related config
writer to use AuthNConfig.EnforceDPoP -> server.auth.enforceDPoP instead of
server.auth.dpop.enforce; keep server.auth.dpop reserved for nonce-related
fields. Locate the mapping in the start-up-with-containers action and any
mirrored config serialization code, and ensure the generated config key matches
what the platform reads at runtime.
- Around line 38-41: The DPoP-capable Keycloak overlay is currently gated only
by the dpop-challenge-enabled input, so enforcement-only runs can still start
with a non-DPoP-capable issuer. Update the condition in the
start-up-with-containers action so the Keycloak 26.2 overlay is enabled when
either dpop-challenge-enabled or dpop-enforce-required is true, and keep the
existing DPoP settings aligned with the inputs. Use the existing
dpop-challenge-enabled and dpop-enforce-required symbols to locate the gating
logic.
🪄 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 UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 07f0f800-c7e4-40b4-9eb9-73d8144b8642
📒 Files selected for processing (2)
test/start-additional-kas/action.yamltest/start-up-with-containers/action.yaml
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
The DPoP nonce challenge only applies to DPoP-bound requests; without enforcement a plain Bearer token bypasses DPoP validation and never sees a challenge. When dpop-challenge-enabled is set, also set server.auth.dpop.enforce alongside require_nonce in both start actions. The flag only ever turns enforcement on: start-additional-kas uses with(select(...)) so it never writes enforce: false (preserving any base value), and start-up-with-containers sets it inside the step already gated on the flag. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
DPoP enforcement and the nonce-challenge flow are separate concerns. Replace the coupling (where dpop-challenge-enabled also set server.auth.dpop.enforce) with a dedicated dpop-enforce-required input (default false) that drives enforcement on its own. dpop-challenge-enabled again sets only require_nonce. The enforce knob only ever turns enforcement on: start-additional-kas uses with(select(...)) keyed on DPOP_ENFORCE_REQUIRED, and start-up-with-containers sets it in a new step gated on the flag, so enforce: false is never written. Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
925e626 to
edd1961
Compare
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
Summary
Part of DSPX-3397. Adds two independent inputs to the composite test actions so xtest can exercise DPoP end-to-end:
dpop-challenge-enabled(defaultfalse) → setsserver.auth.dpop.require_nonce: true.DPoPauthorization header, issue achallengeto make sure the requestor is in possession of the DPoP secret.start-up-with-containers, but adding tostart-additional-kasto allow testing in multi-kas scenarios, including making sure that the kases expect and require unique nonce values.dpop-enforce-required(defaultfalse) → setsserver.auth.dpop.enforce: true.BearerauthZ in favor of ONLY DPoP tokens. Useful for testing, but not ready for deployment.The two are decoupled: enforcement (reject non-DPoP tokens) is separate from the nonce-challenge feature.
Changes
test/start-additional-kas/action.yamlandtest/start-up-with-containers/action.yaml: new inputs + true/false validation + env wiring.enforceknob only ever turns enforcement on — it never writesenforce: false(so any base value is preserved): start-additional-kas useswith(select(...)), start-up-with-containers sets it in a step gated on the flag.Note on dependency
The
enforcesetting relies onserver.auth.dpop.enforce(introduced in #3666). Setting it before that lands is harmless (an older platform ignores the unknown config key), and these actions build the platform from the checked-out ref, so the field is honored within a PR run.Testing
yq.enforceuntouched (nofalsewritten); flag on →enforce: true;require_noncetracks onlydpop-challenge-enabled.Summary by CodeRabbit
Summary by CodeRabbit
New Features
Validation
trueorfalsefor the new DPoP options.