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(router): redesign JWK authentication logic #1498

Merged
merged 14 commits into from
Jan 30, 2025

Conversation

Noroth
Copy link
Contributor

@Noroth Noroth commented Jan 8, 2025

Motivation and Context

The current implementation for the authentication logic is not optimal. We are creating a token decoder per JWKs while the intention for the library is to have multiple sources where a keyset can be loaded and refreshed from.

This PR introduces a redesign of the current implementation. The configuration changes from a per JWKS basis to a global config with some options per JWKs (e.g. whitelisting algorithms when loading keys)

The token logic has been simplified to use a single token decoder, that can handle all keys and header/valueprefix combinations.

Warning

As part of the preparations for Cosmo V1, targeted for release in Q1 2025, this pull request introduces essential changes to enhance long-term stability and maintainability. While we strive to minimize breaking changes, they are sometimes necessary to lay the foundation for a more robust and scalable system.

Config change example:

Old New
authentication:
  providers:
    - name: "jwk1"
      jwks:
        refresh_interval: 1m
        url: "https://test1.io.well-known/jwks.json"
        header_value_prefixes:
          - "Bearer"
          - "Token"
          - "MyBearer"
        header_names:
          - "Authorization"
          - "X-Auth-Token"
    - name: "jwk2"
      jwks:
        refresh_interval: 2m
        url: "https://test2.io/.well-known/jwks.json"
        header_value_prefixes:
          - "Bearer"
          - "Token"
          - "MyBearer"
        header_names:
          - "Authorization"
          - "X-Auth-Token"
          - "X-Authorization"
authentication:
  jwt:
    jwks:
      - url: "https://test1.io/.well-known/jwks.json"
        refresh_interval: 1m
        # Leaving algorithms empty will allow all supported
        # algorithms from the config docs
      - url: "https://test2.io/.well-known/jwks.json"
        refresh_interval: 2m
        # optional list of allowed algorithms per JWKS
        algorithms: ["RS256", "EdDSA", "HS512"]
    header_name: Authorization # This is the default value
    header_value_prefix: Bearer # This is the default value
    # additional header/prefix combinations
    header_sources:
      - type: header 
        name: X-Auth-Token
        value_prefixes: [Token, MyToken]
      - type: header
        name: X-Authorization

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.
  • Documentation has been updated on https://github.com/wundergraph/cosmo-docs.
  • I have read the Contributors Guide.

@github-actions github-actions bot added the router label Jan 8, 2025
Copy link

github-actions bot commented Jan 9, 2025

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-8e78e803d5cbf7be2786774dad4cffbaaa3d970f

router/pkg/config/config.go Outdated Show resolved Hide resolved
@Noroth Noroth force-pushed the ludwig/eng-6230-redesign-jwk-authentication-logic branch from cc46adb to eadd615 Compare January 14, 2025 14:08
@Noroth Noroth force-pushed the ludwig/eng-6230-redesign-jwk-authentication-logic branch from b924099 to d9d341b Compare January 21, 2025 09:41
@Noroth Noroth marked this pull request as ready for review January 21, 2025 12:39
Copy link
Contributor

@StarpTech StarpTech left a comment

Choose a reason for hiding this comment

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

Overall great work. Just a few nuts.

router-tests/authentication_test.go Outdated Show resolved Hide resolved
router/pkg/authentication/validation_store.go Show resolved Hide resolved
router-tests/cmd/jwks-server/main.go Show resolved Hide resolved
@StarpTech
Copy link
Contributor

Please document how the migration will look like and mention that it is breaking.

Copy link
Contributor

@StarpTech StarpTech left a comment

Choose a reason for hiding this comment

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

LGTM

@Noroth
Copy link
Contributor Author

Noroth commented Jan 21, 2025

I will prepare the documentation changes before merging this PR

@Noroth Noroth merged commit ecdf74a into main Jan 30, 2025
13 checks passed
@Noroth Noroth deleted the ludwig/eng-6230-redesign-jwk-authentication-logic branch January 30, 2025 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants