Add AuthzClientCryptoProvider for authorization client cryptographic operations - #8
Conversation
…tory closes #33831 Signed-off-by: mposolda <mposolda@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces cryptographic provider functionality for the authorization client, enabling it to handle ECDSA signature operations independently. The implementation adds a priority ordering mechanism to allow multiple crypto providers to coexist on the classpath.
Changes:
- Added
AuthzClientCryptoProviderwith ECDSA algorithm support and ASN.1 encoding/decoding utilities - Implemented priority ordering for crypto provider selection based on the
order()method - Added comprehensive test coverage for ECDSA algorithms (ES256, ES384, ES512)
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
authz/client/src/main/java/org/keycloak/authorization/client/util/crypto/AuthzClientCryptoProvider.java |
Implements new crypto provider with ECDSA operations for authorization client |
authz/client/src/main/java/org/keycloak/authorization/client/util/crypto/ASN1Encoder.java |
Provides ASN.1 DER encoding utilities for ECDSA signatures |
authz/client/src/main/java/org/keycloak/authorization/client/util/crypto/ASN1Decoder.java |
Provides ASN.1 DER decoding utilities for ECDSA signatures |
authz/client/src/test/java/org/keycloak/authorization/client/test/ECDSAAlgorithmTest.java |
Tests ECDSA algorithm variants with signature conversion operations |
common/src/main/java/org/keycloak/common/crypto/CryptoProvider.java |
Adds order() method to enable provider priority ordering |
common/src/main/java/org/keycloak/common/crypto/CryptoIntegration.java |
Updates provider detection to sort by order and handle multiple providers |
crypto/default/src/main/java/org/keycloak/crypto/def/DefaultCryptoProvider.java |
Implements order() returning 200 |
crypto/fips1402/src/main/java/org/keycloak/crypto/fips/FIPS1402Provider.java |
Implements order() returning 200 |
crypto/elytron/src/main/java/org/keycloak/crypto/elytron/WildFlyElytronProvider.java |
Implements order() returning 200 |
authz/client/src/main/resources/META-INF/services/org.keycloak.common.crypto.CryptoProvider |
Registers AuthzClientCryptoProvider as a service provider |
authz/client/src/main/java/org/keycloak/authorization/client/AuthzClient.java |
Initializes crypto integration on client creation |
authz/client/pom.xml |
Adds JUnit and Hamcrest test dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (foundProviders.size() > 1) { | ||
| StringBuilder builder = new StringBuilder("Ignored crypto providers: "); | ||
| for (int i = 1 ; i < foundProviders.size() ; i++) { | ||
| builder.append(foundProviders.get(i).getClass().getName() + ", "); |
There was a problem hiding this comment.
String concatenation inside a loop should use StringBuilder.append() without the + operator. Replace with builder.append(foundProviders.get(i).getClass().getName()).append(\", \").
| ASN1Encoder.create().write(rBigInteger); | ||
| ASN1Encoder.create().write(sBigInteger); | ||
|
|
There was a problem hiding this comment.
Lines 114-115 create encoder instances that are never used, resulting in dead code. These lines should be removed as the actual encoding is performed in lines 117-121.
| ASN1Encoder.create().write(rBigInteger); | |
| ASN1Encoder.create().write(sBigInteger); |
Test 3
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.
Replicated from ai-code-review-evaluation/keycloak-coderabbit#3