Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent keycloak requests #29

Merged
merged 4 commits into from
Sep 12, 2024

Conversation

Tockra
Copy link
Contributor

@Tockra Tockra commented Sep 12, 2024

Overview

This pull request addresses the issue of unnecessary OIDC discovery calls when token validation fails due to common errors like expired tokens or malformed tokens. The previous implementation triggered an OIDC request to Keycloak under various error conditions, which may not always be necessary.

Previous Implementation

The middleware previously made OIDC requests for the following error conditions:

Error Conditions

InvalidToken: Token doesn't have a valid JWT shape.
InvalidSignature: Signature doesn't match.
InvalidEcdsaKey: Secret is not a valid ECDSA key.
InvalidRsaKey: Secret is not a valid RSA key.
RsaFailedSigning: Failed signing with the given key.
InvalidAlgorithmName: Algorithm name mismatch.
InvalidKeyFormat: Key provided in an invalid format.
MissingRequiredClaim: Required claim not present.
ExpiredSignature: Token's exp claim indicates expiration.
InvalidIssuer: Token’s iss claim does not match the expected issuer.
InvalidAudience: Token’s aud claim does not match expected values.
InvalidSubject: Token’s sub claim does not match expected values.
ImmatureSignature: Token’s nbf claim is in the future.
InvalidAlgorithm: Algorithm mismatch in header or key.
MissingAlgorithm: Validation struct lacks algorithms.
Base64: Error decoding base64 text.
Json: Serialization/deserialization error.
Utf8: Invalid UTF-8 text.
Crypto: Unspecified crypto error.

Proposed Changes

The OIDC discovery should only occur for error conditions where it can potentially resolve the issue:

  • InvalidRsaKey: While rare, if this occurs, a valid key can be retrieved from Keycloak.
  • InvalidEcdsaKey: Added for completeness, though its relevance is uncertain.
  • RsaFailedSigning: May occur after a private key change in Keycloak. However, such changes are infrequent, and without rate limiting, this can lead to excessive requests to the Keycloak server through our Axum backend.

By refining these conditions, we reduce unnecessary load on both our system and the Keycloak server.

Decode Error:
- InvalidRsaKey
- InvalidEcdsaKey
- RsaFailedSigning
@lpotthast
Copy link
Owner

Hey, great change. Was very much needed. Would have probably implemented some sort of throttling.., but going by the error types seems very reasonable. Thanks!

@lpotthast lpotthast merged commit 55457dd into lpotthast:main Sep 12, 2024
@Tockra Tockra deleted the prevent-keycloak-requests branch September 13, 2024 08:22
@Tockra
Copy link
Contributor Author

Tockra commented Sep 13, 2024

Hey, great change. Was very much needed. Would have probably implemented some sort of throttling.., but going by the error types seems very reasonable. Thanks!

I believe we still need some form of throttling here.

If I understand correctly, the key validation checks whether your key was signed by the stored RSA key. If not, it requests the public certs endpoint of the Keycloak server and searches for a key with the kid passed in your JWK token.

image

To prevent multiple unnecessary requests, we could store the kids that have previously caused requests to Keycloak. This way, the same invalid JWT with a different kid won't repeatedly trigger cross-requests to Keycloak.

Last but not least a rate limit would be nice which reduces the request against keycloak to something like:
maximum X requests per second.

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.

2 participants