Skip to content
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

✨ [#4398] Check object ownership when creating submission #4696

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
35d6a17
:sparkles: [#4398] Pass initial_data_reference from auth start to ret…
stevenbal Oct 10, 2024
0aa3b9d
:sparkles: [#4398] Check object ownership during prefill and pre-regi…
stevenbal Oct 22, 2024
ebbf112
:white_check_mark: [#4398] Add tests for initial_data_reference owner…
stevenbal Oct 22, 2024
2046230
:recycle: [#4398] Modify ownership check behavior in case of errors
stevenbal Oct 29, 2024
27f7d4c
:recycle: [#4398] Move ownership check to proper location in prefill …
stevenbal Oct 29, 2024
9dc4fff
:sparkles: [#4398] Ensure logs from ownership check in pre-registrati…
stevenbal Oct 29, 2024
f8505ea
:white_check_mark: [#4398] Fix VCR issue in Objects API ownership tests
stevenbal Oct 29, 2024
3569555
:white_check_mark: [#4398] Fix failing tests due to new ownership check
stevenbal Nov 26, 2024
17358ba
:ok_hand: [#4398] Process PR feedback
stevenbal Nov 4, 2024
0b60071
:sparkles: [#4398] Configurable path to auth attribute for ownership …
stevenbal Nov 4, 2024
d9ab954
:fire: [#4398] Remove authentication view code to pass initial_data_r…
stevenbal Nov 5, 2024
191d09e
:loud_sound: [#4398] Add logevents for object ownership check
stevenbal Nov 5, 2024
12df2ff
:white_check_mark: [#4398] Update object ownership tests with log checks
stevenbal Nov 5, 2024
88d14da
:recycle: [#4398] Use submission.registration_backend in registration…
stevenbal Nov 12, 2024
116fa15
:sparkles: [#4398] Make auth_attribute_path required if update existi…
stevenbal Nov 12, 2024
3916207
:whale: Update objecttypes docker fixture
stevenbal Nov 25, 2024
3e1f834
:sparkles: [#4398] Add authAttributePath to objects prefill form
stevenbal Nov 25, 2024
3616e79
:globe_with_meridians: [#4398] Compile JS translations
stevenbal Nov 25, 2024
94a16e7
:sparkles: [#4398] Display errors for authAttributePath in modal
stevenbal Nov 25, 2024
055d89b
:recycle: [#4398] Use setValues instead of repeating setFieldValue
stevenbal Nov 26, 2024
bd1cc70
:ok_hand: [#4398] Process PR feedback
stevenbal Nov 26, 2024
04cee61
:recycle: [#4398] Use TargetPathSelect for auth attribute path
stevenbal Nov 28, 2024
00e1d16
:ok_hand: [#4398] Process PR feedback
stevenbal Nov 29, 2024
22b6430
:white_check_mark: [#4398] Re-record VCR cassettes for Objects API pr…
stevenbal Nov 29, 2024
1cd37e4
:white_check_mark: [#4398] Fix coverage for Objects API ownership val…
stevenbal Nov 29, 2024
addb139
:globe_with_meridians: [#4398] Update extracted translations (JS)
sergei-maertens Dec 2, 2024
215fa49
:recycle: [#4398] Refactor the TargetPathSelect component
sergei-maertens Dec 2, 2024
eec2eb3
:art: Use canvas instead of screen for DOM queries
sergei-maertens Dec 3, 2024
c77276c
:children_crossing: [#4398] Disable copy button until a backend is se…
sergei-maertens Dec 3, 2024
953d574
:recycle: [#4398] Refactor validation error handling to use context
sergei-maertens Dec 3, 2024
f17486d
:art: Apply fallback for missing form values
sergei-maertens Dec 3, 2024
6d45ca2
:recycle: [#4398] Clean up ownership validation in prefill plugins
sergei-maertens Dec 3, 2024
cc9a84d
:art: [#4398] Decouple variables endpoint validation behaviour from p…
sergei-maertens Dec 3, 2024
e6fe66f
:art: [#4398] Clean up test code
sergei-maertens Dec 4, 2024
cbc7bc9
:recycle: [#4398] Update ownership check interface in registration pl…
sergei-maertens Dec 4, 2024
794ca2e
:art: [#4398] Use dependency injection in unit tests
sergei-maertens Dec 4, 2024
20cb7d0
:label: [#4398] Fix type errors in Objects API tests
sergei-maertens Dec 4, 2024
e773941
:art: [#4398] Refactor registration plugin tests
sergei-maertens Dec 4, 2024
44c89d8
:pencil: Update release checklist with VCR tasks
sergei-maertens Dec 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/ISSUE_TEMPLATE/prepare-release.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ assignees: sergei-maertens
- Payment plugins
- [ ] Ogone legacy: `openforms.payments.contrib.ogone.tests.test_client`
- Prefill
- [ ] Endpoints: `openforms.prefill.contrib.objects_api.tests.test_endpoints`
- [ ] Config: `openforms.prefill.contrib.objects_api.tests.test_config`
- [ ] Prefill: `openforms.prefill.contrib.objects_api.tests.test_prefill`
- [ ] Objects API: `openforms.prefill.contrib.objects_api`
- [ ] Suwinet: `openforms.prefill.contrib.suwinet` (testenv access has been retracted and won't
be reinstated)
- Registration plugins:
- [ ] Objects API: `openforms.registrations.contrib.objects_api`
- [ ] ZGW APIs: `openforms.registrations.contrib.zgw_apis`
Expand Down
1 change: 1 addition & 0 deletions docker/objects-apis/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ docker compose -f docker-compose.objects-apis.yml run objecttypes-web \
--indent=4 \
--output /app/fixtures/objecttypes_api_fixtures.json \
core.objecttype \
core.objectversion \
token
```

Expand Down
73 changes: 69 additions & 4 deletions docker/objects-apis/fixtures/objecttypes_api_fixtures.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"documentation_url": "",
"labels": {},
"created_at": "2023-10-24",
"modified_at": "2024-02-08",
"modified_at": "2024-11-25",
"allow_geometry": true
}
},
Expand Down Expand Up @@ -210,8 +210,8 @@
"object_type": 1,
"version": 3,
"created_at": "2024-02-08",
"modified_at": "2024-02-08",
"published_at": "2024-02-08",
"modified_at": "2024-11-25",
"published_at": "2024-11-25",
"json_schema": {
"$id": "https://example.com/person.schema.json",
"type": "object",
Expand Down Expand Up @@ -262,7 +262,7 @@
}
}
},
"status": "draft"
"status": "published"
}
},
{
Expand Down Expand Up @@ -389,6 +389,71 @@
"status": "draft"
}
},
{
"model": "core.objectversion",
"pk": 9,
"fields": {
"object_type": 1,
"version": 4,
"created_at": "2024-11-25",
"modified_at": "2024-11-25",
"published_at": "2024-11-25",
"json_schema": {
"$id": "https://example.com/person.schema.json",
"type": "object",
"title": "Person",
"$schema": "https://json-schema.org/draft/2020-12/schema",
"properties": {
"age": {
"type": "integer",
"minimum": 18
},
"bsn": {
"type": "string"
},
"name": {
"type": "object",
"properties": {
"last.name": {
"type": "string"
}
}
},
"nested": {
"type": "object",
"properties": {
"unrelated": {
"type": "string"
},
"submission_payment_amount": {
"type": "number",
"multipleOf": 0.01
}
}
},
"submission_date": {
"type": "string",
"format": "date-time"
},
"submission_csv_url": {
"type": "string",
"format": "uri"
},
"submission_pdf_url": {
"type": "string",
"format": "uri"
},
"submission_payment_completed": {
"type": "boolean"
},
"submission_payment_public_ids": {
"type": "array"
}
}
},
"status": "draft"
}
},
{
"model": "token.tokenauth",
"pk": 1,
Expand Down
1 change: 1 addition & 0 deletions pyright.pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ include = [
"src/openforms/contrib/zgw/service.py",
"src/openforms/contrib/objects_api/",
# Registrations
"src/openforms/registrations/tasks.py",
"src/openforms/registrations/contrib/email/config.py",
"src/openforms/registrations/contrib/email/plugin.py",
"src/openforms/registrations/contrib/stuf_zds/options.py",
Expand Down
1 change: 0 additions & 1 deletion src/openforms/authentication/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,6 @@ def _handle_return(self, request: Request, slug: str, plugin_id: str):

if hasattr(request, "session") and FORM_AUTH_SESSION_KEY in request.session:
authentication_success.send(sender=self.__class__, request=request)

return response

def _handle_co_sign(self, form: Form, plugin: BasePlugin) -> None:
Expand Down
9 changes: 0 additions & 9 deletions src/openforms/conf/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1203,15 +1203,6 @@
"value": config("ZGW_APIS_INCLUDE_DRAFTS", default=False),
},
],
"REGISTRATION_OBJECTS_API_ENABLE_EXISTING_OBJECT_INTEGRATION": [
{
"condition": "boolean",
"value": config(
"REGISTRATION_OBJECTS_API_ENABLE_EXISTING_OBJECT_INTEGRATION",
default=False,
),
},
],
}

#
Expand Down
5 changes: 0 additions & 5 deletions src/openforms/conf/dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@

os.environ.setdefault("SENDFILE_BACKEND", "django_sendfile.backends.development")

# Feature flags for development
os.environ.setdefault(
"REGISTRATION_OBJECTS_API_ENABLE_EXISTING_OBJECT_INTEGRATION",
"1",
)

from .base import * # noqa isort:skip

Expand Down
81 changes: 81 additions & 0 deletions src/openforms/contrib/objects_api/ownership_validation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
from __future__ import annotations

import logging

from django.core.exceptions import PermissionDenied

from glom import Path, PathAccessError, glom
from requests.exceptions import RequestException

from openforms.contrib.objects_api.clients import ObjectsClient
from openforms.logging import logevent
from openforms.prefill.base import BasePlugin as BasePrefillPlugin
from openforms.registrations.base import BasePlugin as BaseRegistrationPlugin
from openforms.submissions.models import Submission

logger = logging.getLogger(__name__)


def validate_object_ownership(
submission: Submission,
client: ObjectsClient,
object_attribute: list[str],
plugin: BasePrefillPlugin | BaseRegistrationPlugin,
) -> None:
"""
Function to check whether the user associated with a Submission is the owner
of an Object in the Objects API, by comparing the authentication attribute.

This validation should only be done if the Submission has an `initial_data_reference`
"""
assert submission.initial_data_reference

if not submission.is_authenticated:
logger.warning(
"Cannot perform object ownership validation for reference %s with unauthenticated user",
submission.initial_data_reference,
)
logevent.object_ownership_check_anonymous_user(submission, plugin=plugin)
raise PermissionDenied("Cannot pass data reference as anonymous user")

auth_info = submission.auth_info

object = None
try:
object = client.get_object(submission.initial_data_reference)
except RequestException as e:
logger.exception(
"Something went wrong while trying to retrieve "
"object for initial_data_reference"
)
raise PermissionDenied from e

if not object_attribute:
logger.exception(
"No path for auth value configured: %s, cannot perform ownership check",
object_attribute,
)
raise PermissionDenied(
"Could not verify if user is owner of the referenced object"
)

try:
auth_value = glom(object["record"]["data"], Path(*object_attribute))
except PathAccessError as e:
logger.exception(
stevenbal marked this conversation as resolved.
Show resolved Hide resolved
"Could not retrieve auth value for path %s, it could be incorrectly configured",
object_attribute,
)
raise PermissionDenied(
"Could not verify if user is owner of the referenced object"
) from e

if auth_value != auth_info.value:
logger.warning(
"Submission with initial_data_reference did not pass ownership check for reference %s",
submission.initial_data_reference,
)
logevent.object_ownership_check_failure(submission, plugin=plugin)
raise PermissionDenied("User is not the owner of the referenced object")

logevent.object_ownership_check_success(submission, plugin=plugin)
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
interactions:
- request:
body: null
headers:
Accept:
- '*/*'
Accept-Encoding:
- gzip, deflate, br
Authorization:
- Token 7657474c3d75f56ae0abd0d1bf7994b09964dca9
Connection:
- keep-alive
Content-Crs:
- EPSG:4326
User-Agent:
- python-requests/2.32.2
method: GET
uri: http://localhost:8002/api/v2/objects/0122126f-4a7f-49d4-b131-b83786e15acf
response:
body:
string: '{"url":"http://objects-web:8000/api/v2/objects/0122126f-4a7f-49d4-b131-b83786e15acf","uuid":"0122126f-4a7f-49d4-b131-b83786e15acf","type":"http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e","record":{"index":1,"typeVersion":1,"data":{"bsn":"111222333","foo":"bar"},"geometry":null,"startAt":"2024-11-26","endAt":null,"registrationAt":"2024-11-26","correctionFor":null,"correctedBy":null}}'
headers:
Allow:
- GET, PUT, PATCH, DELETE, HEAD, OPTIONS
Connection:
- keep-alive
Content-Crs:
- EPSG:4326
Content-Length:
- '422'
Content-Type:
- application/json
Cross-Origin-Opener-Policy:
- same-origin
Date:
- Tue, 26 Nov 2024 13:21:59 GMT
Referrer-Policy:
- same-origin
Server:
- nginx/1.27.0
Vary:
- origin
X-Content-Type-Options:
- nosniff
X-Frame-Options:
- DENY
status:
code: 200
message: OK
version: 1
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
interactions:
- request:
body: null
headers:
Accept:
- '*/*'
Accept-Encoding:
- gzip, deflate, br
Authorization:
- Token 7657474c3d75f56ae0abd0d1bf7994b09964dca9
Connection:
- keep-alive
Content-Crs:
- EPSG:4326
User-Agent:
- python-requests/2.32.2
method: GET
uri: http://localhost:8002/api/v2/objects/0122126f-4a7f-49d4-b131-b83786e15acf
response:
body:
string: '{"url":"http://objects-web:8000/api/v2/objects/0122126f-4a7f-49d4-b131-b83786e15acf","uuid":"0122126f-4a7f-49d4-b131-b83786e15acf","type":"http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e","record":{"index":1,"typeVersion":1,"data":{"bsn":"111222333","foo":"bar"},"geometry":null,"startAt":"2024-11-26","endAt":null,"registrationAt":"2024-11-26","correctionFor":null,"correctedBy":null}}'
headers:
Allow:
- GET, PUT, PATCH, DELETE, HEAD, OPTIONS
Connection:
- keep-alive
Content-Crs:
- EPSG:4326
Content-Length:
- '422'
Content-Type:
- application/json
Cross-Origin-Opener-Policy:
- same-origin
Date:
- Tue, 26 Nov 2024 13:21:59 GMT
Referrer-Policy:
- same-origin
Server:
- nginx/1.27.0
Vary:
- origin
X-Content-Type-Options:
- nosniff
X-Frame-Options:
- DENY
status:
code: 200
message: OK
version: 1
Loading
Loading