feat: add custom issuer guard (jwt-guard)#64
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughReorganizes JWT guard crates under Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CustomIssuerGuard
participant BaseJwtGuard as base-jwt-guard (JwtGuard trait)
participant Storage
Client->>CustomIssuerGuard: init(config)
CustomIssuerGuard->>CustomIssuerGuard: init_acl(roles)
CustomIssuerGuard->>Storage: persist config, public_keys, roles
Client->>CustomIssuerGuard: storage_deposit()
CustomIssuerGuard->>Storage: register account, store placeholder claim
Client->>CustomIssuerGuard: claim_oidc(oidc_token_hash)
CustomIssuerGuard->>Storage: store OIDC hash for caller
Client->>CustomIssuerGuard: verify(issuer, jwt, sign_payload)
CustomIssuerGuard->>BaseJwtGuard: verify(issuer, jwt, sign_payload)
BaseJwtGuard->>CustomIssuerGuard: get_public_keys()
BaseJwtGuard->>CustomIssuerGuard: verify_custom_claims(..., predecessor)
CustomIssuerGuard->>Storage: fetch stored jwt claim for predecessor
BaseJwtGuard-->>CustomIssuerGuard: (bool, String) result
Client->>CustomIssuerGuard: set_public_keys(public_keys)
Note over CustomIssuerGuard: Access control check (DAO role)
CustomIssuerGuard->>Storage: update public_keys
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✏️ Tip: You can disable this entire section by setting Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @contracts/jwt-guards/jwt-guard/Cargo.toml:
- Line 45: The Cargo.toml currently pins crypto-bigint to a prerelease
"0.7.0-pre"; update the dependency to the latest stable release (e.g., "0.6.1")
or explicitly justify/lock the prerelease after validation — modify the
crypto-bigint entry in Cargo.toml to use version = "0.6.1" (or add a comment and
a tested git/release pin if you must keep 0.7.0-pre) and run cargo test/build to
confirm the contract compiles and passes all checks.
In @contracts/jwt-guards/jwt-guard/package.json:
- Around line 1-13: The package.json "name" field currently
"@contracts/custom-issuer-guard" mismatches the Cargo crate and directory named
"jwt-guard"; update the package name to "@contracts/jwt-guard" to match the
Cargo crate (jwt-guard) and directory (jwt-guard), or alternatively rename the
Cargo crate (Cargo.toml package name) and directory to "custom-issuer-guard" if
that better reflects intent—ensure the values in package.json "name", Cargo.toml
package name, and the folder name are consistent after the change.
In @contracts/jwt-guards/jwt-guard/README.md:
- Line 1: Update the README header to match the directory path by replacing the
current top-line title "contracts/custom-issuer-guard" with
"contracts/jwt-guards/jwt-guard" so the README accurately reflects the contract
name and location; ensure the first line/title in README.md matches the folder
name exactly.
In @contracts/jwt-guards/jwt-guard/src/error.rs:
- Around line 34-41: Update the doc comment example above the require_err macro
to reference the correct error enum for this crate: replace
`StakingDistributorError::InvalidAmount` with
`CustomIssuerGuardError::InvalidAmount` (or the appropriate variant on
CustomIssuerGuardError) so the example reflects the crate's error type when
demonstrating `require_err!(amount > 0,
CustomIssuerGuardError::InvalidAmount);`.
In @contracts/jwt-guards/jwt-guard/src/lib.rs:
- Around line 222-240: In verify_custom_claims update the doc comment to fix the
"fatxn" typo (replace with the correct term used elsewhere in the codebase —
e.g., "fast" or the actual claim name) so the docs are accurate, and replace the
non-idiomatic "".parse().unwrap() return with an idiomatic empty String like
String::new() (or "".to_string()) to return an empty subject; keep the rest of
the function logic and signatures the same.
- Around line 146-151: The claim_oidc function currently calls
internal_unwrap_jwt_claim(&account_id) which panics for accounts without an
existing entry, causing first-time callers to always fail; remove that unwrap
call (or replace it with a proper existence check using
self.jwt_claims.contains_key(&account_id) if you intend to enforce updates only)
and then simply insert the oidc_token_hash into self.jwt_claims; if update-only
behavior is desired, rename claim_oidc to update_oidc_claim and return or panic
with a clear message when contains_key is false.
🧹 Nitpick comments (2)
contracts/jwt-guards/auth0-guard/src/lib.rs (1)
5-6: LGTM!Import paths correctly updated to reference
base_jwt_guard. The contract logic remains unchanged.💡 Optional: Consolidate imports
-use base_jwt_guard::{JwtGuard, JwtPublicKey}; -use base_jwt_guard::assert_valid_public_key; +use base_jwt_guard::{assert_valid_public_key, JwtGuard, JwtPublicKey};contracts/jwt-guards/firebase-guard/src/lib.rs (1)
5-6: LGTM!Import paths correctly updated to reference
base_jwt_guard, consistent with the auth0-guard changes. The contract logic and attestation contract integration remain unchanged.💡 Optional: Consolidate imports
-use base_jwt_guard::{JwtGuard, JwtPublicKey}; -use base_jwt_guard::assert_valid_public_key; +use base_jwt_guard::{assert_valid_public_key, JwtGuard, JwtPublicKey};
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
contracts/Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (41)
contracts/Cargo.tomlcontracts/jwt-guards/auth0-guard/.gitignorecontracts/jwt-guards/auth0-guard/Cargo.tomlcontracts/jwt-guards/auth0-guard/README.mdcontracts/jwt-guards/auth0-guard/package.jsoncontracts/jwt-guards/auth0-guard/src/lib.rscontracts/jwt-guards/auth0-guard/tests/test_integration.rscontracts/jwt-guards/base-jwt-guard/.gitignorecontracts/jwt-guards/base-jwt-guard/Cargo.tomlcontracts/jwt-guards/base-jwt-guard/README.mdcontracts/jwt-guards/base-jwt-guard/package.jsoncontracts/jwt-guards/base-jwt-guard/src/core.rscontracts/jwt-guards/base-jwt-guard/src/jwt/codec.rscontracts/jwt-guards/base-jwt-guard/src/jwt/mod.rscontracts/jwt-guards/base-jwt-guard/src/lib.rscontracts/jwt-guards/base-jwt-guard/src/rsa/key.rscontracts/jwt-guards/base-jwt-guard/src/rsa/mod.rscontracts/jwt-guards/base-jwt-guard/src/rsa/rs256.rscontracts/jwt-guards/base-jwt-guard/src/utils/mod.rscontracts/jwt-guards/firebase-guard/.gitignorecontracts/jwt-guards/firebase-guard/Cargo.tomlcontracts/jwt-guards/firebase-guard/README.mdcontracts/jwt-guards/firebase-guard/package.jsoncontracts/jwt-guards/firebase-guard/src/config.rscontracts/jwt-guards/firebase-guard/src/error.rscontracts/jwt-guards/firebase-guard/src/lib.rscontracts/jwt-guards/firebase-guard/src/storage_impl.rscontracts/jwt-guards/firebase-guard/src/utils/mod.rscontracts/jwt-guards/firebase-guard/src/utils/utils.rscontracts/jwt-guards/firebase-guard/tests/test_integration.rscontracts/jwt-guards/jwt-guard/.gitignorecontracts/jwt-guards/jwt-guard/Cargo.tomlcontracts/jwt-guards/jwt-guard/README.mdcontracts/jwt-guards/jwt-guard/package.jsoncontracts/jwt-guards/jwt-guard/src/config.rscontracts/jwt-guards/jwt-guard/src/error.rscontracts/jwt-guards/jwt-guard/src/lib.rscontracts/jwt-guards/jwt-guard/src/storage_impl.rscontracts/jwt-guards/jwt-guard/src/utils/assert.rscontracts/jwt-guards/jwt-guard/src/utils/mod.rspnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- contracts/jwt-guards/firebase-guard/src/utils/utils.rs
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: GuillemGarciaDev
Repo: Peersyst/fast-auth PR: 34
File: contracts/auth0-guard/src/lib.rs:67-76
Timestamp: 2025-09-02T10:52:36.185Z
Learning: The init signature changes in contracts/auth0-guard/src/lib.rs have already been properly handled in the integration tests by adding the issuer parameter to the args_json in both contracts/auth0-guard/tests/test_integration.rs and examples/jwt-rs256-guard/tests/test_integration.rs.
Learnt from: GuillemGarciaDev
Repo: Peersyst/fast-auth PR: 34
File: contracts/auth0-guard/src/lib.rs:67-76
Timestamp: 2025-09-02T10:52:36.185Z
Learning: The init signature changes in contracts/auth0-guard/src/lib.rs have already been properly handled in both test files - contracts/auth0-guard/tests/test_integration.rs includes issuer values like "https://dev-gb1h5yrp85jsty.us.auth0.com/" and examples/jwt-rs256-guard/tests/test_integration.rs includes issuer as an empty string variable in all init calls.
📚 Learning: 2025-09-02T10:52:36.185Z
Learnt from: GuillemGarciaDev
Repo: Peersyst/fast-auth PR: 34
File: contracts/auth0-guard/src/lib.rs:67-76
Timestamp: 2025-09-02T10:52:36.185Z
Learning: The init signature changes in contracts/auth0-guard/src/lib.rs have already been properly handled in both test files - contracts/auth0-guard/tests/test_integration.rs includes issuer values like "https://dev-gb1h5yrp85jsty.us.auth0.com/" and examples/jwt-rs256-guard/tests/test_integration.rs includes issuer as an empty string variable in all init calls.
Applied to files:
contracts/jwt-guards/firebase-guard/tests/test_integration.rscontracts/jwt-guards/jwt-guard/.gitignorecontracts/jwt-guards/jwt-guard/src/utils/assert.rscontracts/jwt-guards/firebase-guard/src/config.rscontracts/jwt-guards/jwt-guard/src/error.rscontracts/jwt-guards/base-jwt-guard/README.mdcontracts/jwt-guards/auth0-guard/package.jsoncontracts/jwt-guards/firebase-guard/Cargo.tomlcontracts/jwt-guards/jwt-guard/package.jsoncontracts/jwt-guards/base-jwt-guard/Cargo.tomlcontracts/jwt-guards/auth0-guard/src/lib.rscontracts/jwt-guards/jwt-guard/src/storage_impl.rscontracts/jwt-guards/jwt-guard/src/utils/mod.rscontracts/jwt-guards/auth0-guard/Cargo.tomlcontracts/jwt-guards/firebase-guard/src/lib.rscontracts/jwt-guards/jwt-guard/README.mdcontracts/jwt-guards/jwt-guard/src/config.rscontracts/jwt-guards/jwt-guard/Cargo.tomlcontracts/Cargo.tomlcontracts/jwt-guards/auth0-guard/tests/test_integration.rspnpm-workspace.yamlcontracts/jwt-guards/jwt-guard/src/lib.rs
📚 Learning: 2025-09-02T10:52:36.185Z
Learnt from: GuillemGarciaDev
Repo: Peersyst/fast-auth PR: 34
File: contracts/auth0-guard/src/lib.rs:67-76
Timestamp: 2025-09-02T10:52:36.185Z
Learning: The init signature changes in contracts/auth0-guard/src/lib.rs have already been properly handled in the integration tests by adding the issuer parameter to the args_json in both contracts/auth0-guard/tests/test_integration.rs and examples/jwt-rs256-guard/tests/test_integration.rs.
Applied to files:
contracts/jwt-guards/firebase-guard/tests/test_integration.rscontracts/jwt-guards/jwt-guard/.gitignorecontracts/jwt-guards/jwt-guard/src/utils/assert.rscontracts/jwt-guards/firebase-guard/src/config.rscontracts/jwt-guards/jwt-guard/src/error.rscontracts/jwt-guards/base-jwt-guard/README.mdcontracts/jwt-guards/auth0-guard/package.jsoncontracts/jwt-guards/firebase-guard/Cargo.tomlcontracts/jwt-guards/jwt-guard/package.jsoncontracts/jwt-guards/base-jwt-guard/Cargo.tomlcontracts/jwt-guards/auth0-guard/src/lib.rscontracts/jwt-guards/jwt-guard/src/storage_impl.rscontracts/jwt-guards/jwt-guard/src/utils/mod.rscontracts/jwt-guards/auth0-guard/Cargo.tomlcontracts/jwt-guards/firebase-guard/src/lib.rscontracts/jwt-guards/jwt-guard/README.mdcontracts/jwt-guards/jwt-guard/src/config.rscontracts/jwt-guards/jwt-guard/Cargo.tomlcontracts/Cargo.tomlcontracts/jwt-guards/auth0-guard/tests/test_integration.rscontracts/jwt-guards/jwt-guard/src/lib.rs
🧬 Code graph analysis (6)
contracts/jwt-guards/firebase-guard/src/config.rs (1)
contracts/jwt-guards/base-jwt-guard/src/utils/mod.rs (1)
assert_valid_public_key(10-26)
contracts/jwt-guards/auth0-guard/src/lib.rs (1)
contracts/jwt-guards/base-jwt-guard/src/utils/mod.rs (1)
assert_valid_public_key(10-26)
contracts/jwt-guards/jwt-guard/src/storage_impl.rs (1)
contracts/jwt-guards/firebase-guard/src/storage_impl.rs (6)
internal_storage_unregister(8-23)internal_storage_balance_of(26-35)internal_register_account(38-42)internal_storage_deposit(45-69)internal_storage_withdraw(77-92)internal_storage_balance_bounds(96-100)
contracts/jwt-guards/firebase-guard/src/lib.rs (1)
contracts/jwt-guards/base-jwt-guard/src/utils/mod.rs (1)
assert_valid_public_key(10-26)
contracts/jwt-guards/jwt-guard/src/config.rs (2)
contracts/jwt-guards/base-jwt-guard/src/utils/mod.rs (1)
assert_valid_public_key(10-26)contracts/jwt-guards/firebase-guard/src/config.rs (2)
assert_valid(24-31)assert_valid(48-66)
contracts/jwt-guards/jwt-guard/src/lib.rs (2)
contracts/jwt-guards/base-jwt-guard/src/utils/mod.rs (1)
assert_valid_public_key(10-26)contracts/jwt-guards/base-jwt-guard/src/core.rs (2)
get_public_keys(30-30)verify_custom_claims(62-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-base / Build
- GitHub Check: integration-packages / Build
🔇 Additional comments (22)
contracts/jwt-guards/jwt-guard/.gitignore (1)
1-1: LGTM!Standard .gitignore pattern for Rust projects.
contracts/jwt-guards/jwt-guard/README.md (1)
3-49: LGTM!The build, test, and deployment instructions are clear and follow standard NEAR contract patterns. The useful links section provides appropriate resources.
contracts/jwt-guards/base-jwt-guard/README.md (1)
1-1: LGTM!The header correctly reflects the renamed crate. This update aligns with the workspace reorganization that renamed
jwt-guardtobase-jwt-guard.pnpm-workspace.yaml (1)
3-4: LGTM!The workspace patterns correctly accommodate the new directory structure, with
contracts/*covering top-level contracts andcontracts/jwt-guards/*covering the reorganized JWT guard crates.contracts/jwt-guards/auth0-guard/package.json (1)
2-2: LGTM!The package name now correctly reflects its directory and crate identity (
@contracts/auth0-guard).contracts/jwt-guards/base-jwt-guard/Cargo.toml (1)
2-3: LGTM!The crate rename to
base-jwt-guardclearly indicates its role as the foundational JWT guard that other specialized guards depend on.contracts/jwt-guards/firebase-guard/src/config.rs (1)
3-3: LGTM!The import path correctly updated to reference
base_jwt_guard, consistent with the crate rename. The imported items (JwtPublicKey,assert_valid_public_key) match the signatures in the base crate.contracts/jwt-guards/firebase-guard/Cargo.toml (1)
36-36: LGTM!The dependency path update correctly references the renamed
base-jwt-guardcrate within the newjwt-guards/directory structure.contracts/Cargo.toml (1)
4-7: LGTM!Clean workspace reorganization grouping JWT guard crates under a dedicated
jwt-guards/subdirectory. This improves project structure and makes the relationship between related crates clearer.contracts/jwt-guards/auth0-guard/Cargo.toml (1)
36-36: LGTM!Dependency path correctly updated to reference
base-jwt-guard, consistent with the workspace reorganization.contracts/jwt-guards/firebase-guard/tests/test_integration.rs (1)
4-4: LGTM!Import path correctly updated to reference
base_jwt_guardfollowing the crate restructuring.contracts/jwt-guards/auth0-guard/tests/test_integration.rs (2)
2-2: LGTM!Import path correctly updated to reference
base_jwt_guardfollowing the crate restructuring.
307-307: LGTM!Method name updated to
set_public_keys(plural), consistent with the API changes in this PR.contracts/jwt-guards/jwt-guard/src/storage_impl.rs (1)
1-101: LGTM!The storage management implementation follows the established pattern from
firebase-guard/src/storage_impl.rs. The use oflet _ =to suppress unusedPromiseresult warnings is a reasonable approach for fire-and-forget transfers in this context.contracts/jwt-guards/jwt-guard/src/utils/mod.rs (1)
1-3: LGTM!Standard module declaration and re-export pattern.
contracts/jwt-guards/jwt-guard/src/utils/assert.rs (1)
1-14: LGTM!Implementation follows the same pattern as
firebase-guard/src/utils.rswith appropriate error type substitution forCustomIssuerGuardError.contracts/jwt-guards/jwt-guard/src/error.rs (1)
1-32: LGTM!Error enum and Display implementation follow the established pattern from
firebase-guard/src/error.rs. The variant set is appropriately scoped for the custom issuer guard's functionality.contracts/jwt-guards/jwt-guard/src/config.rs (1)
1-67: LGTM!The configuration module follows the established patterns from
firebase-guard/src/config.rs. The validation logic correctly ensures:
- All public keys are validated via
assert_valid_public_key- Super admins set is non-empty
- All account IDs across super_admins, admins, and grantees are validated
contracts/jwt-guards/jwt-guard/src/lib.rs (4)
1-39: LGTM!The imports, module declarations, and Role enum with iterator are well-structured. The use of
access_controlandUpgradablemacros follows the plugin patterns correctly.
68-122: LGTM!The initialization methods are well-implemented:
init()properly checks for existing state, validates config, and initializes ACLinit_acl()correctly iterates through all role categories and adds them via the ACL interface with appropriate error handlingBased on learnings, ensure integration tests include the new config structure with roles when calling init.
165-175: LGTM!The
set_public_keysmethod correctly:
- Restricts access to the DAO role via
#[access_control_any(roles(Role::DAO))]- Validates each public key before updating the contract state
244-320: LGTM!The
StorageManagementtrait implementation correctly delegates to internal methods and follows NEAR storage management patterns. The doc comments clearly describe each method's purpose and behavior.
| schemars = "0.8" | ||
| serde_with = { version = "3.0", features = ["base64"] } | ||
| sha2 = "0.10.8" | ||
| crypto-bigint = { version = "0.7.0-pre", default-features = false, features = ["zeroize", "alloc"] } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
crypto-bigint crate latest stable version releases
💡 Result:
The latest stable release is crypto-bigint 0.6.1 (released 2025-02-14). [1][2]
Sources:
- CHANGELOG / docs.rs for v0.6.1. [1]
- crate listing / releases (pre-0.7 entries) on lib.rs. [2]
Consider using stable crypto-bigint version.
The crypto-bigint dependency is pinned to 0.7.0-pre, a prerelease version. The latest stable release is 0.6.1 (released February 2025). For production use in a NEAR contract, consider using the stable version or ensure the prerelease has been thoroughly tested and approved for production deployment.
🤖 Prompt for AI Agents
In @contracts/jwt-guards/jwt-guard/Cargo.toml at line 45, The Cargo.toml
currently pins crypto-bigint to a prerelease "0.7.0-pre"; update the dependency
to the latest stable release (e.g., "0.6.1") or explicitly justify/lock the
prerelease after validation — modify the crypto-bigint entry in Cargo.toml to
use version = "0.6.1" (or add a comment and a tested git/release pin if you must
keep 0.7.0-pre) and run cargo test/build to confirm the contract compiles and
passes all checks.
| @@ -0,0 +1,49 @@ | |||
| # contracts/custom-issuer-guard | |||
There was a problem hiding this comment.
Fix the header to match the directory structure.
The README header says "contracts/custom-issuer-guard" but the file is located at contracts/jwt-guards/jwt-guard/. This mismatch will confuse users about the actual contract name and path.
📝 Proposed fix
-# contracts/custom-issuer-guard
+# contracts/jwt-guards/jwt-guard📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # contracts/custom-issuer-guard | |
| # contracts/jwt-guards/jwt-guard |
🤖 Prompt for AI Agents
In @contracts/jwt-guards/jwt-guard/README.md at line 1, Update the README header
to match the directory path by replacing the current top-line title
"contracts/custom-issuer-guard" with "contracts/jwt-guards/jwt-guard" so the
README accurately reflects the contract name and location; ensure the first
line/title in README.md matches the folder name exactly.
| /// Use typed errors with NEAR's `require!` internally. | ||
| /// Example: `require_err!(amount > 0, StakingDistributorError::InvalidAmount);` | ||
| #[macro_export] | ||
| macro_rules! require_err { | ||
| ($cond:expr, $err:expr) => {{ | ||
| near_sdk::require!($cond, $err.to_string()); | ||
| }}; | ||
| } |
There was a problem hiding this comment.
Fix the documentation example.
The doc comment references StakingDistributorError which appears to be copy-pasted from another crate. Update to reference CustomIssuerGuardError for accuracy.
📝 Suggested fix
-/// Use typed errors with NEAR's `require!` internally.
-/// Example: `require_err!(amount > 0, StakingDistributorError::InvalidAmount);`
+/// Use typed errors with NEAR's `require!` internally.
+/// Example: `require_err!(amount > 0, CustomIssuerGuardError::InvalidAccountId);`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Use typed errors with NEAR's `require!` internally. | |
| /// Example: `require_err!(amount > 0, StakingDistributorError::InvalidAmount);` | |
| #[macro_export] | |
| macro_rules! require_err { | |
| ($cond:expr, $err:expr) => {{ | |
| near_sdk::require!($cond, $err.to_string()); | |
| }}; | |
| } | |
| /// Use typed errors with NEAR's `require!` internally. | |
| /// Example: `require_err!(amount > 0, CustomIssuerGuardError::InvalidAccountId);` | |
| #[macro_export] | |
| macro_rules! require_err { | |
| ($cond:expr, $err:expr) => {{ | |
| near_sdk::require!($cond, $err.to_string()); | |
| }}; | |
| } |
🤖 Prompt for AI Agents
In @contracts/jwt-guards/jwt-guard/src/error.rs around lines 34 - 41, Update the
doc comment example above the require_err macro to reference the correct error
enum for this crate: replace `StakingDistributorError::InvalidAmount` with
`CustomIssuerGuardError::InvalidAmount` (or the appropriate variant on
CustomIssuerGuardError) so the example reflects the crate's error type when
demonstrating `require_err!(amount > 0,
CustomIssuerGuardError::InvalidAmount);`.
| pub fn claim_oidc(&mut self, oidc_token_hash: Vec<u8>) { | ||
| assert_eq!(oidc_token_hash.len(), 32, "OIDC token hash must be 32 bytes"); | ||
| let account_id = env::predecessor_account_id(); | ||
| self.internal_unwrap_jwt_claim(&account_id); | ||
| self.jwt_claims.insert(account_id, oidc_token_hash); | ||
| } |
There was a problem hiding this comment.
Bug: claim_oidc will always panic for first-time claims.
Line 149 calls internal_unwrap_jwt_claim() which panics if the account has no existing entry in jwt_claims. Since this method is intended to store a new OIDC token hash, first-time callers will always fail because they have no prior entry.
The returned value from internal_unwrap_jwt_claim is also unused, suggesting this was intended as a registration check but uses the wrong mechanism.
🐛 Suggested fix: Remove the unwrap call or check storage registration instead
If the intent is to allow first-time claims:
pub fn claim_oidc(&mut self, oidc_token_hash: Vec<u8>) {
assert_eq!(oidc_token_hash.len(), 32, "OIDC token hash must be 32 bytes");
let account_id = env::predecessor_account_id();
- self.internal_unwrap_jwt_claim(&account_id);
+ // Optionally check storage registration if needed
self.jwt_claims.insert(account_id, oidc_token_hash);
}If the intent is to only allow updates to existing claims, rename the method and update the documentation to reflect that behavior.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn claim_oidc(&mut self, oidc_token_hash: Vec<u8>) { | |
| assert_eq!(oidc_token_hash.len(), 32, "OIDC token hash must be 32 bytes"); | |
| let account_id = env::predecessor_account_id(); | |
| self.internal_unwrap_jwt_claim(&account_id); | |
| self.jwt_claims.insert(account_id, oidc_token_hash); | |
| } | |
| pub fn claim_oidc(&mut self, oidc_token_hash: Vec<u8>) { | |
| assert_eq!(oidc_token_hash.len(), 32, "OIDC token hash must be 32 bytes"); | |
| let account_id = env::predecessor_account_id(); | |
| // Optionally check storage registration if needed | |
| self.jwt_claims.insert(account_id, oidc_token_hash); | |
| } |
🤖 Prompt for AI Agents
In @contracts/jwt-guards/jwt-guard/src/lib.rs around lines 146 - 151, The
claim_oidc function currently calls internal_unwrap_jwt_claim(&account_id) which
panics for accounts without an existing entry, causing first-time callers to
always fail; remove that unwrap call (or replace it with a proper existence
check using self.jwt_claims.contains_key(&account_id) if you intend to enforce
updates only) and then simply insert the oidc_token_hash into self.jwt_claims;
if update-only behavior is desired, rename claim_oidc to update_oidc_claim and
return or panic with a clear message when contains_key is false.
| /// Verifies custom claims in the JWT payload | ||
| /// # Arguments | ||
| /// * `jwt` - JWT token | ||
| /// * `jwt_payload` - Decoded JWT payload as bytes | ||
| /// * `sign_payload` - Payload to verify against the JWT fatxn claim | ||
| /// # Returns | ||
| /// * Tuple containing: | ||
| /// * Boolean indicating if verification succeeded | ||
| /// * String containing either the subject claim or error message | ||
| #[private] | ||
| fn verify_custom_claims(&self, jwt: String, _jwt_payload: Vec<u8>, _sign_payload: Vec<u8>, predecessor: AccountId) -> (bool, String) { | ||
| // Parse the payload into CustomClaims | ||
| let claim_hash = self.internal_unwrap_jwt_claim(&predecessor); | ||
| let jwt_hash = sha256(jwt); | ||
| if !jwt_hash.eq(claim_hash) { | ||
| return (false, format!("Claim for user {} not matching hash {:?}", predecessor, jwt_hash)); | ||
| } | ||
| (true, "".parse().unwrap()) | ||
| } |
There was a problem hiding this comment.
Minor: Typo and non-idiomatic string creation.
- Line 226: "fatxn" appears to be a typo - should this be "fast" or another term?
- Line 239:
"".parse().unwrap()is an unusual way to create an empty string.
✏️ Suggested fixes
- /// * `sign_payload` - Payload to verify against the JWT fatxn claim
+ /// * `sign_payload` - Payload to verify against the JWT claim- (true, "".parse().unwrap())
+ (true, String::new())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Verifies custom claims in the JWT payload | |
| /// # Arguments | |
| /// * `jwt` - JWT token | |
| /// * `jwt_payload` - Decoded JWT payload as bytes | |
| /// * `sign_payload` - Payload to verify against the JWT fatxn claim | |
| /// # Returns | |
| /// * Tuple containing: | |
| /// * Boolean indicating if verification succeeded | |
| /// * String containing either the subject claim or error message | |
| #[private] | |
| fn verify_custom_claims(&self, jwt: String, _jwt_payload: Vec<u8>, _sign_payload: Vec<u8>, predecessor: AccountId) -> (bool, String) { | |
| // Parse the payload into CustomClaims | |
| let claim_hash = self.internal_unwrap_jwt_claim(&predecessor); | |
| let jwt_hash = sha256(jwt); | |
| if !jwt_hash.eq(claim_hash) { | |
| return (false, format!("Claim for user {} not matching hash {:?}", predecessor, jwt_hash)); | |
| } | |
| (true, "".parse().unwrap()) | |
| } | |
| /// Verifies custom claims in the JWT payload | |
| /// # Arguments | |
| /// * `jwt` - JWT token | |
| /// * `jwt_payload` - Decoded JWT payload as bytes | |
| /// * `sign_payload` - Payload to verify against the JWT claim | |
| /// # Returns | |
| /// * Tuple containing: | |
| /// * Boolean indicating if verification succeeded | |
| /// * String containing either the subject claim or error message | |
| #[private] | |
| fn verify_custom_claims(&self, jwt: String, _jwt_payload: Vec<u8>, _sign_payload: Vec<u8>, predecessor: AccountId) -> (bool, String) { | |
| // Parse the payload into CustomClaims | |
| let claim_hash = self.internal_unwrap_jwt_claim(&predecessor); | |
| let jwt_hash = sha256(jwt); | |
| if !jwt_hash.eq(claim_hash) { | |
| return (false, format!("Claim for user {} not matching hash {:?}", predecessor, jwt_hash)); | |
| } | |
| (true, String::new()) | |
| } |
🤖 Prompt for AI Agents
In @contracts/jwt-guards/jwt-guard/src/lib.rs around lines 222 - 240, In
verify_custom_claims update the doc comment to fix the "fatxn" typo (replace
with the correct term used elsewhere in the codebase — e.g., "fast" or the
actual claim name) so the docs are accurate, and replace the non-idiomatic
"".parse().unwrap() return with an idiomatic empty String like String::new() (or
"".to_string()) to return an empty subject; keep the rest of the function logic
and signatures the same.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @contracts/jwt-guards/base-jwt-guard/package.json:
- Around line 1-13: The package's build script currently uses "cargo build"
which won't produce the expected WebAssembly output; update the scripts.build
entry in package.json to use the repository standard "cargo near build
non-reproducible-wasm" so the contract produces a non-reproducible WASM artifact
(leave scripts.test and scripts.lint unchanged).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
contracts/jwt-guards/base-jwt-guard/package.jsoncontracts/jwt-guards/jwt-guard/package.json
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: GuillemGarciaDev
Repo: Peersyst/fast-auth PR: 34
File: contracts/auth0-guard/src/lib.rs:67-76
Timestamp: 2025-09-02T10:52:36.185Z
Learning: The init signature changes in contracts/auth0-guard/src/lib.rs have already been properly handled in the integration tests by adding the issuer parameter to the args_json in both contracts/auth0-guard/tests/test_integration.rs and examples/jwt-rs256-guard/tests/test_integration.rs.
Learnt from: GuillemGarciaDev
Repo: Peersyst/fast-auth PR: 34
File: contracts/auth0-guard/src/lib.rs:67-76
Timestamp: 2025-09-02T10:52:36.185Z
Learning: The init signature changes in contracts/auth0-guard/src/lib.rs have already been properly handled in both test files - contracts/auth0-guard/tests/test_integration.rs includes issuer values like "https://dev-gb1h5yrp85jsty.us.auth0.com/" and examples/jwt-rs256-guard/tests/test_integration.rs includes issuer as an empty string variable in all init calls.
📚 Learning: 2025-09-02T10:52:36.185Z
Learnt from: GuillemGarciaDev
Repo: Peersyst/fast-auth PR: 34
File: contracts/auth0-guard/src/lib.rs:67-76
Timestamp: 2025-09-02T10:52:36.185Z
Learning: The init signature changes in contracts/auth0-guard/src/lib.rs have already been properly handled in the integration tests by adding the issuer parameter to the args_json in both contracts/auth0-guard/tests/test_integration.rs and examples/jwt-rs256-guard/tests/test_integration.rs.
Applied to files:
contracts/jwt-guards/base-jwt-guard/package.json
📚 Learning: 2025-09-02T10:52:36.185Z
Learnt from: GuillemGarciaDev
Repo: Peersyst/fast-auth PR: 34
File: contracts/auth0-guard/src/lib.rs:67-76
Timestamp: 2025-09-02T10:52:36.185Z
Learning: The init signature changes in contracts/auth0-guard/src/lib.rs have already been properly handled in both test files - contracts/auth0-guard/tests/test_integration.rs includes issuer values like "https://dev-gb1h5yrp85jsty.us.auth0.com/" and examples/jwt-rs256-guard/tests/test_integration.rs includes issuer as an empty string variable in all init calls.
Applied to files:
contracts/jwt-guards/base-jwt-guard/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-base / Build
- GitHub Check: integration-packages / Build
- GitHub Check: integration-contracts / integration
[TA-XXXX]: Title
Changes 🛠️
app/package
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.