Skip to content

Add AuthzClientCryptoProvider for authorization client cryptographic operations - #8

Open
ShashankFC wants to merge 1 commit into
feature-authz-crypto-baselinefrom
feature-authz-crypto-implementation
Open

Add AuthzClientCryptoProvider for authorization client cryptographic operations#8
ShashankFC wants to merge 1 commit into
feature-authz-crypto-baselinefrom
feature-authz-crypto-implementation

Conversation

@ShashankFC

Copy link
Copy Markdown
Contributor

Test 3

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced cryptographic support with ECDSA algorithm implementation for authorization client operations.
    • Improved crypto provider selection with priority ordering mechanism.
  • Tests

    • Added test coverage for ECDSA algorithm variants (ES256, ES384, ES512).
  • Chores

    • Added testing framework dependencies.

✏️ Tip: You can customize this high-level summary in your review settings.


Replicated from ai-code-review-evaluation/keycloak-coderabbit#3

…tory

closes #33831

Signed-off-by: mposolda <mposolda@gmail.com>
@ShashankFC
ShashankFC requested a review from Copilot January 30, 2026 10:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 AuthzClientCryptoProvider with 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() + ", ");

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

String concatenation inside a loop should use StringBuilder.append() without the + operator. Replace with builder.append(foundProviders.get(i).getClass().getName()).append(\", \").

Copilot uses AI. Check for mistakes.
Comment on lines +114 to +116
ASN1Encoder.create().write(rBigInteger);
ASN1Encoder.create().write(sBigInteger);

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

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.

Suggested change
ASN1Encoder.create().write(rBigInteger);
ASN1Encoder.create().write(sBigInteger);

Copilot uses AI. Check for mistakes.
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.

3 participants