fix: add access control to set_risk_tier + initialize admin#54
Closed
woydarko wants to merge 1 commit into
Closed
Conversation
Fixes mericcintosun#50 ## Problem set_risk_tier had no authorization check — any address could overwrite any user's risk score, making the on-chain credit score trivially manipulable and blocking mainnet deployment. ## Changes ### risk_score/src/lib.rs - Added DataKey enum with Admin variant for typed storage - Added initialize(admin) — sets trusted admin once, panics if called again - Added get_admin() — returns current admin address - set_risk_tier now takes a caller parameter and enforces: - Contract must be initialized (panics with clear message if not) - caller must be admin OR the user themselves - Unauthorized callers panic with 'unauthorized' - update_chosen_tier now calls user.require_auth() for self-service updates ### risk_score/Cargo.toml - Added dev-dependencies with soroban-sdk testutils feature for tests ### Tests (20 tests, 0 failed) New auth tests: - test_initialize_sets_admin - test_initialize_twice_panics - test_admin_can_set_risk_tier_for_any_user - test_user_can_set_own_risk_tier - test_third_party_cannot_set_risk_tier (should panic) - test_set_risk_tier_requires_initialization (should panic) All 14 existing tests adapted to new signature and passing.
|
@woydarko is attempting to deploy a commit to the mericcintosun Team on Vercel. A member of the Team first needs to authorize it. |
Owner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #50
set_risk_tierhad no authorization check — any address could overwrite any user's risk score, making the on-chain credit score trivially manipulable and blocking mainnet deployment (v1.1 roadmap).Changes
risk_score/src/lib.rsAdded
initialize(admin)"already initialized"if called againDataKey::AdminAdded
get_admin()Noneif not yet initializedFixed
set_risk_tier— now takes acallerparameter and enforces:"contract not initialized — call initialize() first"if not)callermust be the admin OR the user themselves — both requirerequire_auth()"unauthorized: caller must be admin or the user themselves"Fixed
update_chosen_tieruser.require_auth()so only the user themselves can change their chosen tierrisk_score/Cargo.toml[dev-dependencies]withsoroban-sdk = { version = "22.0.8", features = ["testutils"] }to enableAddress::generatein testsTests (20 passed, 0 failed)
New auth tests added:
test_initialize_sets_admin— admin is correctly storedtest_initialize_twice_panics— double initialization rejectedtest_admin_can_set_risk_tier_for_any_user— admin can set scores for otherstest_user_can_set_own_risk_tier— user can set their own scoretest_third_party_cannot_set_risk_tier— attacker cannot overwrite victim's scoretest_set_risk_tier_requires_initialization— uninitialized contract rejects callsAll 14 existing tests adapted to new signature and passing.