Add role certificate validity check when loading manifest in core #734
Conversation
Codecov Report
@@ Coverage Diff @@
## master #734 +/- ##
==========================================
- Coverage 86.51% 86.18% -0.33%
==========================================
Files 159 159
Lines 11183 11641 +458
==========================================
+ Hits 9675 10033 +358
- Misses 1508 1608 +100
Continue to review full report at Codecov.
|
touilleMan
force-pushed
the
check-workspace-role-remote-loader
branch
from
b496320
to
c070128
Compare
| @@ -79,7 +79,8 @@ | |||
| @pytest.mark.trio | |||
| @pytest.mark.parametrize("type", ["file", "folder"]) | |||
| async def test_new_empty_entry(type, running_backend, alice_user_fs, alice2_user_fs): | |||
| wid = await create_shared_workspace("w", alice_user_fs, alice2_user_fs) | |||
| with freeze_time("2000-01-01"): | |||
| wid = await create_shared_workspace("w", alice_user_fs, alice2_user_fs) | |||
There was a problem hiding this comment.
Because otherwise shared workspace is created with current time, but after that we do a workspace.sync() with time frozen at 2000-01-03.
So the timestamp consistency test failed given there is no role certificate available at the time we upload the data
| elif role_at_timestamp == RealmRole.READER: | ||
| raise FSError( | ||
| f"Manifest was created at {expected_timestamp} by `{expected_author}` " | ||
| "which had write right on the workspace at that time" |
There was a problem hiding this comment.
which had no write right? which had no right to write in the workspace?
There was a problem hiding this comment.
wrong wording or poor style ?
| # Also test timestamped workspace | ||
| with pytest.raises(FSError) as exc: | ||
| await workspace.to_timestamped(options["backend_timestamp"]) | ||
| str(exc.value) == exc_msg |
There was a problem hiding this comment.
Maybe parametrize the testbed fixture and let the test catch the FSError exception? e.g:
@pytest.mark.trio
async def test_empty_blob(testbed):
with pytest.raises(FSError) as context:
await testbed.run(blob=b"")
assert context.value.message == "Cannot decrypt vlob: The nonce must be exactly 24 bytes long"There was a problem hiding this comment.
That was exactly what I was doing before adding the timestamped part ^^
Before that we had only one call raising an exception so it was logical to let the caller test handle it
But with two errors being raised, I should have two methods (well 3 in fact: init, run_workspace, run_timestamped_workspace) that have to be called by the test with exception handling. This is much more cumbersome (especially considering the methods may not be easily idempotent) so I just KISS
fix #727