-
Notifications
You must be signed in to change notification settings - Fork 86
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
feat: impersonating sender of requests to a local PocketIC instance #4013
base: master
Are you sure you want to change the base?
Conversation
…6ed621-master' into mraszyk/pic-impersonate
e2e/tests-dfx/call.bash
Outdated
@@ -298,3 +298,82 @@ teardown() { | |||
assert_command dfx canister call inter2_mo read | |||
assert_match '(8 : nat)' | |||
} | |||
|
|||
@test "impersonate sender" { | |||
[[ ! "$USE_POCKETIC" ]] && skip "skipped for replica: impersonating sender is only supported for PocketIC" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have this test impersonate an identity other than (or in addition to) aaaaa-aa? It looks like these tests would pass if --impersonate
always used the anonymous identity rather than the impersonation mechanism.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The principal aaaaa-aa
is the management canister identity, not the anonymous identity 2vxsx-fae
, and thus --identity anonymous
would not make the tests pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, but that being the case, this test shows that --impersonate
can impersonate the management canister identity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test shows that --impersonate can impersonate the management canister identity
indeed and the impersonation mechanism is the only way how to make the management canister identity be the caller
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
retry_policy.reset(); | ||
request_accepted = true; | ||
let blob = if let Some(pocketic) = env.get_pocketic() { | ||
let msg_id = RawMessageId { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this change related to impersonation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. Because request status can only be retrieved by the same caller who submitted the update call before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new code path is always taken when env.get_pocketic.is_some()
, whether or not --impersonate
is involved. Doesn't this mean that when using PocketIC, dfx canister request_status
will work no matter which identity is used?
I'm adding a new test in #4047 to demonstrate this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this mean that when using PocketIC, dfx canister request_status will work no matter which identity is used?
Correct. If we aren't comfortable with this behavior, PocketIC would need to be extended with a check for matching callers (in production, the read state request handler takes care of it, but in PocketIC this was omitted since security is not a requirement and it didn't seem important for the sake of canister testing).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's how this could apply to canister testing:
- developer writes canister
- some of developer's maintenance processes include
dfx canister call --async
anddfx canister request-status
, but they forget to pass the same--identity
on both - this all works locally since they're using PocketIC, but fails on mainnet
This PR enables impersonating sender of requests to a local PocketIC instance:
dfx canister call
,dfx canister status
, anddfx canister update-settings
take an additional CLI argument--impersonate
to specify a principal on behalf of which requests to a local PocketIC instance are sent.