feat(call_server): configure JWT claims dynamically during runtime config #2461
Conversation
…alidation mechanism
|
""" WalkthroughThis change introduces support for user-defined JWT claims across the prover and call_server components. It adds configuration options for specifying JWT claims in YAML and TOML, updates environment variable handling, refactors JWT claim structures in Rust, and simplifies authentication logic and testing to validate claims against user-specified requirements. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Service (call_server/prover)
participant JWT Config
participant JWT Validator
User->>Service: Sends request with JWT token
Service->>JWT Config: Loads expected claims from config/env
Service->>JWT Validator: Passes JWT token and expected claims
JWT Validator->>JWT Validator: Decodes and validates claims
JWT Validator->>Service: Returns validation result
Service->>User: Processes request or rejects (401) based on result
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (7)
🔇 Additional comments (8)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The preview of the vlayer book is available at: https://vlayer-book-f5vb2i8lj-vlayer.vercel.app |
|
🐢 Incremental Compilation Time: 0:27.94. |
|
The preview of the vlayer book is available at: https://vlayer-book-qrngmuwbe-vlayer.vercel.app |
kubkon
force-pushed
the
kubkon+adam/call_server-jwt-claims-config
branch
from
2dc23de to
1e9cbf9
Compare
|
The preview of the vlayer book is available at: https://vlayer-book-rmsfr10na-vlayer.vercel.app |
|
The preview of the vlayer book is available at: https://vlayer-book-az67m6dlr-vlayer.vercel.app |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
rust/services/call/server_lib/src/gas_meter.rs (1)
88-101: 💡 Verification agent🧩 Analysis chain
Verify gas-meter logic aligns with PR objectives.
The PR objectives mention a TODO about updating gas-meter logic to accept the full JWT token and extract claims like
subinternally, since required claims are now defined at runtime. Please ensure this implementation properly handles the new dynamic claims structure.
🏁 Script executed:
#!/bin/bash # Description: Check if gas-meter logic properly handles dynamic JWT claims extraction # Expected: Find evidence of JWT claims extraction or validation logic # Search for JWT claim extraction or validation in gas meter related files rg -A 10 -B 5 "extract.*claim|validate.*claim|jwt.*claim" --type rust # Look for any TODO comments related to gas-meter and JWT rg -A 5 -B 2 "TODO.*gas.?meter|gas.?meter.*TODO" --type rust # Check if there are any references to 'sub' claim extraction rg -A 5 -B 2 "sub.*claim|claim.*sub" --type rustLength of output: 17141
Fix gas-meter JWT handling to extract and validate dynamic claims
The
gas_meter.rsimplementation currently only stores anOption<Token>but never decodes or validates the JWT or extracts runtime‐defined claims (e.g."sub"). To align with the PR’s objectives, you need to:
- In
rust/services/call/server_lib/src/gas_meter.rs(aroundRpcClient::new), decode the full JWT using the existing JWT middleware utilities (e.g.server_utils::jwt::axum::decode) and yourJwtConfig.- Call into
Config::validate(&token_data.claims)to enforce runtime claims.- Extract required claim values (like
sub) fromtoken_data.claims(using.pointer("/sub")or similar) and propagate them into your gas-meter logic.Without this, the gas meter never enforces or uses the dynamically configured claims. Please add the decode→validate→extract sequence so that the new dynamic‐claims feature is fully wired through the gas-meter.
🧹 Nitpick comments (1)
rust/services/dns/server/src/server.rs (1)
64-64: Consider using a more descriptive test secret.While
"deadbeef"is acceptable for testing, consider using a more descriptive value like"test-jwt-secret"to improve code readability.- pub const JWT_SECRET: &[u8] = b"deadbeef"; + pub const JWT_SECRET: &[u8] = b"test-jwt-secret";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
ansible/host_vars/nightly_fake_prover.yml(1 hunks)ansible/host_vars/stable_fake_prover.yml(1 hunks)ansible/host_vars/stable_prod_prover.yml(1 hunks)ansible/roles/prover/templates/vlayer.service.j2(1 hunks)book/src/appendix/architecture/prover.md(2 hunks)book/src/appendix/contributing/extension.md(1 hunks)book/src/appendix/contributing/rust.md(1 hunks)rust/cli/Cargo.toml(2 hunks)rust/cli/src/commands/jwt.rs(2 hunks)rust/jwt/Cargo.toml(0 hunks)rust/jwt/src/lib.rs(3 hunks)rust/server_utils/src/jwt.rs(1 hunks)rust/server_utils/src/jwt/axum.rs(3 hunks)rust/server_utils/src/jwt/cli.rs(2 hunks)rust/server_utils/src/jwt/config.rs(1 hunks)rust/services/call/server/src/main.rs(1 hunks)rust/services/call/server_lib/src/cli.rs(1 hunks)rust/services/call/server_lib/src/config.rs(5 hunks)rust/services/call/server_lib/src/gas_meter.rs(1 hunks)rust/services/call/server_lib/src/handlers.rs(1 hunks)rust/services/call/server_lib/src/jwt.rs(1 hunks)rust/services/call/server_lib/src/lib.rs(0 hunks)rust/services/call/server_lib/src/server.rs(2 hunks)rust/services/call/server_lib/src/token.rs(0 hunks)rust/services/call/server_lib/tests/integration_tests.rs(5 hunks)rust/services/call/server_lib/tests/test_helpers/mod.rs(1 hunks)rust/services/dns/server/src/config.rs(1 hunks)rust/services/dns/server/src/main.rs(1 hunks)rust/services/dns/server/src/server.rs(1 hunks)rust/services/dns/server/src/server/handlers/dns_query.rs(5 hunks)rust/services/dns/server/src/server/jwt.rs(1 hunks)
💤 Files with no reviewable changes (3)
- rust/jwt/Cargo.toml
- rust/services/call/server_lib/src/lib.rs
- rust/services/call/server_lib/src/token.rs
🧰 Additional context used
🧬 Code Graph Analysis (1)
rust/services/call/server_lib/src/jwt.rs (3)
rust/services/call/server_lib/src/server.rs (1)
server(62-79)rust/services/call/server_lib/tests/test_helpers/mod.rs (1)
server(95-112)rust/services/dns/server/src/server/jwt.rs (1)
from_ref(8-14)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build binaries
- GitHub Check: Build binaries
🔇 Additional comments (52)
rust/cli/Cargo.toml (2)
15-15: Workspace dependencyderive_builderadded
Thederive_buildercrate is now included to support builder-pattern generation for the new JWT claims struct.
26-26: Enablederivefeature onstrumcrate
Including thederivefeature flag forstrumallows procedural derivation of theEnvironmentenum used in dynamic claim validation.rust/services/dns/server/src/config.rs (1)
3-3: Update JWT config import path
Switching fromserver_utils::jwt::cli::Configtoserver_utils::jwt::config::Configensures we reference the relocated configuration type.book/src/appendix/contributing/extension.md (1)
62-62: Remove--proof fakeargument from server run command
The CLI invocation no longer requires the--proof fakeflag, reflecting the streamlined default proof mode and runtime-configurable JWT claims.rust/services/call/server_lib/tests/test_helpers/mod.rs (1)
11-11: Align JWT config import in test helpers
Updated the import to useserver_utils::jwt::config::Configso tests reference the new centralized JWT configuration struct.rust/services/dns/server/src/main.rs (1)
10-10: Use centralizedJwtConfigfromserver_utils::jwt::config
Ensures the DNS server reads its JWT settings from the newly refactored configuration module.rust/services/call/server/src/main.rs (1)
60-60: LGTM! Wildcard pattern correctly handles struct extension.The addition of
..in the destructuring pattern appropriately handles the newclaimsfield added toJwtOptionswhile maintaining the existing logging functionality forpublic_keyandalgorithm.rust/services/call/server_lib/src/handlers.rs (2)
8-8: LGTM! Import change aligns with JWT refactoring.The migration from local
token::Tokentoserver_utils::jwt::axum::Tokencorrectly reflects the refactoring to shared JWT utilities, supporting the new configurable claims functionality.
13-13: LGTM! Clean removal of local token import.Removing the local
tokenimport is appropriate since token handling has been moved to shared utilities inserver_utils.rust/services/call/server_lib/src/cli.rs (1)
38-38: LGTM! Environment variable parsing for JWT claims correctly implemented.The addition of
"auth.jwt.claims"as a list parse key enables parsing JWT claims from environment variables likeVLAYER_AUTH__JWT__CLAIMS="sub environment:test"as described in the PR objectives.ansible/host_vars/stable_fake_prover.yml (1)
8-10: LGTM! JWT claims configuration correctly implemented.The
vlayer_jwt_claimsconfiguration with"sub"and"environment:test"values aligns perfectly with the PR objectives, requiring thesubclaim to exist and theenvironmentclaim to match "test" exactly. This is appropriate for the stable fake prover environment.ansible/host_vars/stable_prod_prover.yml (1)
9-11: LGTM - JWT claims configuration for production environment.The configuration correctly specifies the required JWT claims for the production environment, with
subrequiring existence andenvironmentrequiring exact match to "production".rust/services/call/server_lib/src/gas_meter.rs (1)
9-12: Import reorganization looks good.The consolidation of JWT-related imports from
server_utilsis appropriate and aligns with removing the localTokenstruct.ansible/host_vars/nightly_fake_prover.yml (1)
8-10: LGTM - JWT claims configuration for test environment.The configuration correctly specifies the required JWT claims for the test environment, appropriately using "environment:test" to distinguish from production.
ansible/roles/prover/templates/vlayer.service.j2 (1)
44-44: Environment variable setup is correct.The template properly sets up the
VLAYER_AUTH__JWT__CLAIMSenvironment variable by joining the claims list with spaces, which matches the format specified in the PR objectives.book/src/appendix/contributing/rust.md (1)
122-123: LGTM! Command simplification improves usability.Removing the
--proof fakeargument aligns with the broader configuration changes and simplifies the profiling workflow.rust/services/dns/server/src/server.rs (2)
59-59: Import changes align with JWT refactoring.The explicit imports of
DecodingKeyandJwtConfigare consistent with the broader refactoring that centralizes JWT configuration handling.
73-75: Manual JWT configuration provides better test control.The explicit construction of JWT configuration with empty claims list is more transparent than using a default helper and aligns well with the new user-defined claims support.
rust/server_utils/src/jwt/cli.rs (2)
9-9: Import change aligns with JWT config module refactoring.Moving from local
Configtosuper::config::Configis consistent with centralizing JWT configuration logic into a dedicated module.
49-49: Empty claims vector supports new JWT claims feature.Adding
Vec::new()as the third parameter correctly initializes the JWT configuration with an empty claims list, which aligns with the new user-defined JWT claims support.book/src/appendix/architecture/prover.md (2)
84-90: Excellent documentation for JWT claims configuration.The documentation clearly explains both claim existence validation (name only) and value-specific validation with practical examples. This will help users understand how to configure custom JWT claims effectively.
129-129: Environment variable table properly updated.Adding
VLAYER_AUTH__JWT__CLAIMSto the environment variables table with correct type (list) and empty default maintains consistency with the configuration documentation.rust/services/dns/server/src/server/jwt.rs (2)
2-2: LGTM: Import update aligns with JWT config refactoring.The import change from
JwtStatetoJwtConfigis consistent with the centralized JWT configuration approach introduced in this PR.
6-15: LGTM: Clean implementation of FromRef trait.The implementation correctly extracts and clones the JWT configuration from the application state. The expect message provides clear guidance for configuration requirements.
rust/services/call/server_lib/src/server.rs (2)
14-14: LGTM: Updated import reflects new token extraction approach.The change from
ClaimsExtractortoTokenExtractoraligns with the centralized JWT validation strategy where claims validation is handled by theJwtConfig.
46-54: LGTM: Simplified authentication handler.The refactored
handle_with_authfunction is much cleaner by:
- Using
TokenExtractorinstead ofClaimsExtractor- Removing environment validation logic (now handled centrally)
- Directly passing the token to
Params::newThis aligns with the PR objective of centralizing JWT claims validation in the configuration layer.
rust/server_utils/src/jwt.rs (2)
3-3: LGTM: New config module supports centralized JWT validation.The addition of the
configsubmodule aligns with the PR objective of centralizing JWT claims configuration and validation logic.
5-5: LGTM: Simplified exports reflect architectural changes.The updated exports with the simplified
Claimstruct instead of complex claim types support the streamlined JWT handling approach.rust/services/call/server_lib/src/jwt.rs (2)
1-5: LGTM: Updated imports reflect JWT config centralization.The import changes from various JWT-related modules to the simplified
jwt::config::Configalign with the centralized JWT configuration approach.
7-16: LGTM: Simplified FromRef implementation aligns with architectural goals.The refactored implementation:
- Consistently returns
JwtConfiginstead ofJwtState- Removes complex validation logic in favor of centralized config-based validation
- Follows the same pattern as the DNS server implementation
This simplification supports the PR objective of centralizing JWT claims validation in the configuration layer, where the
JwtConfignow handles claims validation internally.rust/services/dns/server/src/server/handlers/dns_query.rs (1)
34-40: LGTM!The simplified token extraction aligns well with the new dynamic JWT claims configuration approach.
rust/cli/src/commands/jwt.rs (1)
97-109: Well-structured Claims definitionThe local Claims struct with builder pattern is a good design choice for the CLI's specific needs, allowing flexible JWT generation for testing while keeping the core jwt library generic.
rust/services/call/server_lib/tests/integration_tests.rs (7)
498-501: LGTM: Import changes align with JWT refactoring.The new imports replace previous abstractions with explicit JWT utilities, providing better control over token creation in tests.
506-506: LGTM: Hardcoded secret appropriate for tests.Using a fixed secret in test code ensures deterministic behavior and is a common testing pattern.
508-522: LGTM: Token creation function is well-implemented.The explicit JSON claims approach improves test clarity compared to previous abstractions. The timestamp calculation and JWT encoding logic are correct.
524-540: LGTM: JWT configuration correctly matches test tokens.The explicit claims configuration (
subwith any value,environmentmust be "test") aligns with the tokens created by thetoken()function.
586-595: LGTM: Tampered token test correctly validates signature verification.The test properly uses a different secret key to ensure signature validation fails, maintaining security.
624-647: LGTM: Environment mismatch test validates claims correctly.The test properly verifies that tokens with incorrect environment claims are rejected with descriptive error messages, ensuring robust claim validation.
649-700: LGTM: Gas meter authentication test correctly uses full token.The test properly validates JWT authentication flow with gas meter integration, using the full token string for bearer authentication as intended.
rust/server_utils/src/jwt/config.rs (6)
1-12: LGTM: Clean error handling and appropriate imports.The ValidationError type with descriptive messages will help with debugging JWT validation issues.
13-19: LGTM: Well-designed Config struct with security considerations.Skipping the public key in debug output is a good security practice to prevent accidental key exposure in logs.
21-52: LGTM: Robust and secure JWT claims validation logic.The implementation correctly handles:
- Nested claims via JSON pointer syntax
- Type safety (strings only for claim values)
- Value validation against allowlists
- Descriptive error messages for debugging
The validation approach is both secure and flexible for user-defined claims.
61-72: LGTM: Test correctly validates claim presence requirement.The test uses a realistic JWT structure and properly validates that required claims must exist.
74-87: LGTM: Excellent test coverage for nested claims validation.This test validates the crucial JSON pointer functionality, ensuring claims like
custom.hostcorrectly map to nested JSON structures like{"custom": {"host": "..."}}.
99-146: LGTM: Comprehensive error condition testing.The tests cover all major failure scenarios with detailed error message validation:
- Missing required claims
- Invalid claim values
- Unsupported claim types (non-strings)
This ensures robust error handling and good debugging experience.
rust/services/call/server_lib/src/config.rs (4)
16-16: LGTM: Import updates align with JWT refactoring.The alias
Claim as JwtClaimprevents naming conflicts, and the updated import path reflects the relocatedConfigstruct.Also applies to: 19-19
75-78: LGTM: Flexible claims configuration with backward compatibility.The optional claims field with
#[serde(default)]maintains backward compatibility while enabling user-defined JWT claims configuration.
80-96: LGTM: Flexible claim specification with proper error handling.The untagged enum design allows users to specify claims as either simple strings or structured objects, with proper error propagation for invalid string formats.
221-231: LGTM: Proper conversion from configuration to JWT validation config.The logic correctly converts flexible configuration claims to structured JWT claims with appropriate error propagation and passes them to the validation config.
rust/server_utils/src/jwt/axum.rs (3)
21-22: LGTM: Clean Token newtype with appropriate derives.The Token wrapper provides type safety for JWT tokens while remaining simple and efficient with appropriate trait derives.
24-37: LGTM: API simplification with proper error handling.Moving from generic
ClaimsExtractor<T>to concreteTokenExtractorsimplifies the API while maintaining type safety. TheValidationerror variant properly handles claim validation failures.
41-71: LGTM: Robust token extraction with flexible claim validation.The implementation correctly:
- Validates JWT header and signature
- Decodes claims as generic JSON for flexibility with user-defined claims
- Delegates validation to the Config's validate method
- Returns the raw token string for downstream use
This design supports the user-defined claims feature while maintaining security.
|
The preview of the vlayer book is available at: https://vlayer-book-entxopnm9-vlayer.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
rust/jwt/src/lib.rs (1)
88-103: Excellent implementation that addresses previous feedback!The
FromStrimplementation correctly handles the parsing requirements and appropriately addresses the empty claim name validation that was flagged in previous reviews. The use ofEmptyNameerror (instead ofEmptyString) is semantically better than the originally suggested fix.The parsing logic correctly handles the colon-separated format mentioned in the PR objectives (e.g., "sub environment:test").
🧹 Nitpick comments (1)
rust/jwt/src/lib.rs (1)
81-86: Consider adding documentation for the simplified Claim struct.The struct design is clean and appropriate for the new dynamic claim configuration approach. However, it would benefit from documentation explaining the purpose and usage of the
nameandvaluesfields.+/// Represents a JWT claim with a name and optional allowed values. +/// +/// When `values` is empty, the claim only needs to exist in the JWT. +/// When `values` contains items, the claim must match one of the specified values. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] pub struct Claim { + /// The name of the JWT claim (e.g., "sub", "environment") pub name: String, + /// Optional list of allowed values. Empty means any value is accepted. #[serde(default)] pub values: Vec<String>, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rust/jwt/src/lib.rs(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Build binaries
- GitHub Check: Test vlayer
- GitHub Check: Test Rust
- GitHub Check: Build binaries
- GitHub Check: Build examples
- GitHub Check: Lint TS
- GitHub Check: Lint Rust
- GitHub Check: Rust incremental compilation performance
🔇 Additional comments (2)
rust/jwt/src/lib.rs (2)
1-4: LGTM! Import additions support the new parsing functionality.The addition of
std::str::FromStrcorrectly supports the new string parsing capability for JWT claims.
16-19: LGTM! Well-defined error variants for claim parsing.The new error variants
EmptyStringandEmptyNameprovide clear, specific error messages for different parsing failure scenarios.
|
The preview of the vlayer book is available at: https://vlayer-book-ri2ubtnhg-vlayer.vercel.app |
Mirroring tlsnotary/tlsn#817, this PR makes it possible to configure user-defined JWT claims at runtime config (from either
config.tomlor env vars). So instead of having the expected claims hard-coded in the binary, we are now able to define them in the config file and have the authorisation middleware validate them at runtime.TOML
Env vars
Both of the examples above yield the same end result, namely, the server:
subclaim with any values - existence checkenvironmentwith exactly one valuetest- precise matchTODO
Before we can merge this PR, we need to make a tweak to the gas-meter logic since now we forward the JWT token in its entirety to the gas-meter rather than extracting anything in particular from the JWT claims. This is a necessary requirement since now knowledge of required JWT claims is deferred to runtime, therefore we cannot know uprfont what to expect in the extraction logic in the prover server. As such, it seems more natural to simply forward the JWT to the gas-meter which knows what to expect and can perform any relevant claim extraction on its own:
subclaim from the input JWTSummary by CodeRabbit
New Features
VLAYER_AUTH__JWT__CLAIMS) allows specifying required JWT claims for authentication.Bug Fixes
Documentation
Refactor