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

fix!: The term "scheme" is used properly as defined by RFC9110 #1042

Merged
merged 9 commits into from
Nov 17, 2023

Conversation

dadrus
Copy link
Owner

@dadrus dadrus commented Nov 16, 2023

Related issue(s)

closes #1044

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

As described in the issue referenced above, a property allowing to configure the Authorization header scheme is named schema (a at the end). This is unexpected and given the currently raised JSON schema validation error on start of heimdall not obvious. Because of that this PR changes the name of the property to what everybody expects it to be, namely scheme.

NOTE: Affected are all authenticator mechanisms, which define a property of Authentication Data Source type, namely (jwt, oauth2_interospection and generic).

The below jwt authenticator examples show the differences between the configuration before this PR and configuration changes introduced by this PR.

old configuration

id: jwt_authenticator
type: jwt
config:
  jwks_endpoint:
    url: http://foo/token
    jwt_source:
     - header: Authorization
       schema: Bearer  # property ends with 'a'

new configuration

id: jwt_authenticator
type: jwt
config:
  jwks_endpoint:
    url: http://foo/token
    jwt_source:
     - header: Authorization
       scheme: Bearer  # property ends with 'e' as it is actually expected

Copy link

codecov bot commented Nov 16, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (312ace1) 89.42% compared to head (7c37dcf) 89.42%.
Report is 2 commits behind head on main.

❗ Current head 7c37dcf differs from pull request most recent head 516e7c9. Consider uploading reports for the commit 516e7c9 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1042   +/-   ##
=======================================
  Coverage   89.42%   89.42%           
=======================================
  Files         248      248           
  Lines       10179    10179           
=======================================
  Hits         9103     9103           
  Misses        842      842           
  Partials      234      234           

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

@dadrus dadrus changed the title fix: The term "scheme" is used properly as defined by RFC9110 fix!: The term "scheme" is used properly as defined by RFC9110 Nov 17, 2023
@dadrus dadrus merged commit aaf4bd3 into main Nov 17, 2023
25 checks passed
@dadrus dadrus deleted the fix/header_schem_e_ branch November 17, 2023 17:03
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.

The property allowing configuration of the Authorization header scheme is named "schema"
1 participant