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

refactor!: Configuration of signer moved into jwt finalizer #1534

Merged
merged 36 commits into from
Jun 18, 2024

Conversation

dadrus
Copy link
Owner

@dadrus dadrus commented Jun 11, 2024

Related issue(s)

relates to #1493
relates to #1507
closes #1536

Checklist

  • I agree to follow this project's Code of Conduct.
  • I have read, and I am following this repository's Contributing Guidelines.
  • I have read the Security Policy.
  • I have referenced an issue describing the bug/feature request.
  • I have added tests that prove the correctness of my implementation.
  • I have updated the documentation.

Description

Until #1493, there was just a single mechanism, which was making use of key material for signature creation purposes - the jwt finalizer. With that FR the situation has changed and there is a need to have independent key material configurations for different mechanisms.

This requirement has been addressed by this PR, which refactors the existing configuration (which introduces a breaking change) and makes it more future proof.

Old Configuraiton & Behavior

Before this PR, heimdall generated an ECDSA key pair if the global signer property was not configured. The jwt fnalizer did then make use of it.

# in heimdall's config file
signer:
   # optional. defaults to "heimdall"
  name: foobar
  # optional. if not specified, an ECDSA key pair is generated
  key_store:
    path: /opt/heimdall/keystore.pem
  # optional. if not specified the first entry from the key store is used
  key_id: foo

New Configuration & Behavior

This PR has moved the configuration of the signer into the jwt finalizer and made the configuration of the corresponding key material mandatory.

# in heimdall's config file
mechanisms:
  finalizers:
    - id: my-finalizer
      type: jwt
      config:
        # mandatory. cannot be overwritten in a rule
        signer:
          # optional. defaults to "heimdall"
          name: foobar
          # mandatory
          key_store:
            path: /opt/heimdall/keystore.pem
          # optional. if not specified the first entry from the key store is used
          key_id: foo
        # other properties defined by jwt finalizer
        # ...

The configured key material is still exposed via the JWKS endpoint as it was previously the case. The behavior related to certificate expiration metrics is preserved as well.

Other topics

This PR fixes the #1536 bug as well


BEGIN_COMMIT_OVERRIDE
refactor!: Configuration of signer moved into jwt finalizer (#1534)
fix: Taking updates of certificates into account while collecting metrics (#1534)
END_COMMIT_OVERRIDE

@dadrus dadrus changed the title refactor!: Configuration of signer moved into jwt finalizer refactor!: Configuration of signer moved into jwt finalizer Jun 11, 2024
Copy link

codecov bot commented Jun 11, 2024

Codecov Report

Attention: Patch coverage is 95.08772% with 14 lines in your changes missing coverage. Please review.

Project coverage is 89.63%. Comparing base (8236eba) to head (ef07677).

Files Patch % Lines
...ernal/rules/mechanisms/finalizers/jwt_finalizer.go 90.00% 3 Missing and 1 partial ⚠️
cmd/validate/ruleset.go 81.81% 2 Missing ⚠️
internal/otel/metrics/certificate/observer.go 94.87% 1 Missing and 1 partial ⚠️
internal/watcher/noop_watcher.go 33.33% 2 Missing ⚠️
internal/otel/metrics/module.go 0.00% 1 Missing ⚠️
...al/rules/mechanisms/finalizers/cookie_finalizer.go 90.90% 1 Missing ⚠️
...al/rules/mechanisms/finalizers/header_finalizer.go 90.90% 1 Missing ⚠️
internal/x/tlsx/options.go 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1534      +/-   ##
==========================================
+ Coverage   89.58%   89.63%   +0.04%     
==========================================
  Files         271      272       +1     
  Lines        9084     9012      -72     
==========================================
- Hits         8138     8078      -60     
+ Misses        702      696       -6     
+ Partials      244      238       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dadrus dadrus merged commit 4475745 into main Jun 18, 2024
27 checks passed
@dadrus dadrus deleted the refactor/jwt_signer_moved_to_jwt_finalizer branch June 18, 2024 15:00
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.

Metrics exposed about certificate expiration can become stale
1 participant