Feat/read permissions #509
Conversation
| server_data = client.get(f"/publications/v1/{response.json()['identifier']}").json() | ||
| server_data = client.get( | ||
| f"/publications/v1/{response.json()['identifier']}", | ||
| headers={"Authorization": "Fake token"}, |
There was a problem hiding this comment.
You'll see this in several places: to access draft assets we now need to authenticate, which requires mocking the keycloak introspection (done above with with logged_in_user(...):) and provide a fake token.
| def test_entry_status_can_update_on_put( | ||
| def test_entry_status_does_not_update_on_put( |
There was a problem hiding this comment.
This test was updated in a previous PR and the name was not updated to reflect the new behavior. Entry Status can only be updated through the review process endpoints currently.
| status=EntryStatus.DRAFT, | ||
| date_deleted=deletion_time, | ||
| ), | ||
| factory_test_resource(title="test_resource_to_keep", status=EntryStatus.DRAFT, |
There was a problem hiding this comment.
it's the same function, but renamed. factory did not say what it was a factory function of (factory functions are helper functions to produce an object). normally I would name this test_resource_factory but that test_ prefix would make Pytest think it is a test function. Somewhat unfortunate.
| resource_name: str, | ||
| auto_publish: None, |
There was a problem hiding this comment.
auto_publish is a new fixture (please skip ahead and look at it then continue). Basically: it automatically makes any aiod entry have status published, which avoids us having to either 1) fake the review process every time we post an asset and want it to be published or 2) update tests by adding the required authentication for GET requests on a draft asset.
| response = client.get(default_endpoint, allow_redirects=False) | ||
| response = client.get(default_endpoint, follow_redirects=False) |
There was a problem hiding this comment.
allow_redirects was deprecated, otherwise unrelated
| factory_test_resource(title="My third test resource", status=EntryStatus.DRAFT, | ||
| platform_resource_identifier="4"), |
There was a problem hiding this comment.
Note that the new list has one more asset, but its in draft. So here we are now automatically also checking if draft assets are included in get_all requests (and later similar in count requests).
| register_asset(person) | ||
| register_asset(publication) | ||
| register_asset(Publication(name="2")) | ||
| register_asset(Publication(name="3")) | ||
| register_asset(contact) |
There was a problem hiding this comment.
register_asset helper function makes sure there is also an associated user, permission, and, if required, review entry.
|
|
||
| @pytest.mark.parametrize( | ||
| "title", | ||
| ["\"'é:?", "!@#$%^&*()`~", "Ω≈ç√∫˜µ≤≥÷", "田中さんにあげて下さい", " أي بعد, ", "𝑻𝒉𝒆 𝐪𝐮𝐢𝐜𝐤", "گچپژ"], | ||
| ) | ||
| def test_unicode(client_test_resource: TestClient, title: str, mocked_privileged_token: Mock): |
There was a problem hiding this comment.
Ideally we phase out the mocked_privileged_token in favor of the logged_in_user. It's more specific about to which calls it applies and which roles apply (if any).
There was a problem hiding this comment.
and allows faking multiple users in the same test
| original = keycloak_openid.introspect | ||
| user = user or kc_user_with_roles() |
There was a problem hiding this comment.
It became a pretty common pattern not to care about what user was logged in, just that is was a particular user. It made sense to me to make that default behavior.
| def bypass_reviewer_publish_everything() -> None: | ||
| """Function to set the AIoD entry to published without a review. | ||
| This function is a *test utility* to simplify test setups, and avoid | ||
| e.g., authentication for get requests. | ||
| """ | ||
| with DbSession() as session: | ||
| session.exec(update(AIoDEntryORM).values(status=EntryStatus.PUBLISHED)) | ||
| session.commit() |
There was a problem hiding this comment.
Basically what auto_publish also does, except on a manual invocation. There were some situations where the update trigger lead to failure for a reason I could not figure out, or where you do not want the asset published immediately but rather after some checks, in those cases we can use this manual override.
Change
Make draft assets private, only visible with explicit permission.
That means:
ASSET/{identifier}if the user as read permissions for that assetget_submissionendpoint, but not directly.How to Test
Primarily try to think if there are permutations which do not work with these changes.
In particular ones that inadvertently may expose the draft asset.
More generally, tests are updated and it can be manually tested.
Checklist
Related Issues
Related to #449