Taskprov: fix authorization bug in aggregate init call #2023
Conversation
|
I think you'll need to rebase now that #2019 has landed, and maybe some more as I will be landing changes to |
|
No problem, I can hang onto this until the PR churn is done. |
inahga
force-pushed
the
inahga/fix-taskprov-bug
branch
2 times, most recently
from
c423295
to
d6c1f6f
Compare
|
Rebase complete, should be ready for review. |
| ); | ||
| let aggregate_resp: AggregationJobResp = decode_response_body(&mut test_conn).await; | ||
|
|
||
| assert_eq!(aggregate_resp.prepare_resps().len(), 1, "{}", name); |
There was a problem hiding this comment.
nit:
| assert_eq!(aggregate_resp.prepare_resps().len(), 1, "{}", name); | |
| assert_eq!(aggregate_resp.prepare_resps().len(), 1, "{name}"); |
and in a few other places
| } | ||
| } | ||
|
|
||
| fn generate_helper_report_share( |
There was a problem hiding this comment.
nit: this looks a lot like what aggregator::aggregate_init_tests::PrepareInitGenerator does. It goes and constructs a PrepareInit, but on the way there it does generate a VDAF transcript and generate a report share. I wonder if you could add a PrepareInitGenerator::next_report_share_with_metadata and use that here.
There was a problem hiding this comment.
WDYT about the latest commit? It's not really a simplification IMO, but I like that it expands on PrepareInitGenerator to make other kinds of tests easier to write.
inahga
force-pushed
the
inahga/fix-taskprov-bug
branch
from
4a5b826
to
cac69ae
Compare
Aggregate init calls subsequent to a call that provisioned a task would fail, since we were using the wrong (non-existent) task-specific auth tokens for authorization.
inahga
force-pushed
the
inahga/fix-taskprov-bug
branch
from
46118fc
to
51b79b1
Compare
Calls to /aggregate subsequent to taskprov task provisioning would fail for the same task, because /aggregate was not using the right authorization tokens (that is, it was attempting to use non-existent authorization tokens and failing).
The first commit is just a refactoring PR to make the test implementation cleaner. The second commit is the operative one.