Skip to content

Commit 66553ee

Browse files
sampaccoudAntoLC
sampaccoud
authored and
AntoLC
committed
✨(backend) add subrequest auth view for collaboration server
We need to improve security on the access to The collaboration server We can use the same pattern as for media files leveraging the nginx subrequest feature.
1 parent 64674b6 commit 66553ee

File tree

12 files changed

+118
-18
lines changed

12 files changed

+118
-18
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ and this project adheres to
1616

1717
## Changed
1818

19+
- 🔒️(collaboration) increase collaboration access security #472
1920
- 🔨(frontend) encapsulated title to its own component #474
2021
- 🐛(frontend) Fix hidden menu on Firefox #468
2122
- ⚡️(backend) optimize number of queries on document list view #411

‎Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,8 @@ logs: ## display app-dev logs (follow mode)
122122

123123
run: ## start the wsgi (production) and development server
124124
@$(COMPOSE) up --force-recreate -d celery-dev
125-
@$(COMPOSE) up --force-recreate -d nginx
126125
@$(COMPOSE) up --force-recreate -d y-provider
126+
@$(COMPOSE) up --force-recreate -d nginx
127127
@echo "Wait for postgresql to be up..."
128128
@$(WAIT_DB)
129129
.PHONY: run

‎docker-compose.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ services:
118118
depends_on:
119119
- keycloak
120120
- app-dev
121+
- y-provider
121122

122123
frontend-dev:
123124
user: "${DOCKER_USER:-1000}"

‎docker/files/etc/nginx/conf.d/default.conf

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,49 @@ server {
44
server_name localhost;
55
charset utf-8;
66

7+
# Proxy auth for collaboration server
8+
location /collaboration/ws/ {
9+
# Collaboration Auth request configuration
10+
auth_request /collaboration-auth;
11+
auth_request_set $authHeader $upstream_http_authorization;
12+
auth_request_set $canEdit $upstream_http_x_can_edit;
13+
auth_request_set $userId $upstream_http_x_user_id;
14+
15+
# Pass specific headers from the auth response
16+
proxy_set_header Authorization $authHeader;
17+
proxy_set_header X-Can-Edit $canEdit;
18+
proxy_set_header X-User-Id $userId;
19+
20+
# Ensure WebSocket upgrade
21+
proxy_http_version 1.1;
22+
proxy_set_header Upgrade $http_upgrade;
23+
proxy_set_header Connection "Upgrade";
24+
25+
# Collaboration server
26+
proxy_pass http://y-provider:4444;
27+
28+
# Set appropriate timeout for WebSocket
29+
proxy_read_timeout 86400;
30+
proxy_send_timeout 86400;
31+
32+
# Preserve original host and additional headers
33+
proxy_set_header Host $host;
34+
}
35+
36+
location /collaboration-auth {
37+
proxy_pass http://app-dev:8000/api/v1.0/documents/collaboration-auth/;
38+
proxy_set_header Host $host;
39+
proxy_set_header X-Real-IP $remote_addr;
40+
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
41+
proxy_set_header X-Original-URL $request_uri;
42+
43+
# Prevent the body from being passed
44+
proxy_pass_request_body off;
45+
proxy_set_header Content-Length "";
46+
proxy_set_header X-Original-Method $request_method;
47+
}
48+
49+
# Proxy auth for media
750
location /media/ {
851
# Auth request configuration
952
auth_request /media-auth;

‎env.d/development/common.dist

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ AI_API_KEY=password
5353
AI_MODEL=llama
5454

5555
# Collaboration
56-
COLLABORATION_WS_URL=ws://localhost:4444
56+
COLLABORATION_SERVER_SECRET=my-secret
57+
COLLABORATION_WS_URL=ws://localhost:8083/collaboration/ws
5758

5859
# Frontend
5960
FRONTEND_THEME=dsfr

‎src/backend/core/api/viewsets.py

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
f"{settings.MEDIA_URL:s}(?P<pk>{UUID_REGEX:s})/"
4646
f"(?P<key>{ATTACHMENTS_FOLDER:s}/{UUID_REGEX:s}{FILE_EXT_REGEX:s})$"
4747
)
48+
COLLABORATION_WS_URL_PATTERN = re.compile(rf"(?:^|&)room=(?P<pk>{UUID_REGEX})(?:&|$)")
4849

4950
# pylint: disable=too-many-ancestors
5051

@@ -620,6 +621,10 @@ def _authorize_subrequest(self, request, pattern):
620621
parsed_url = urlparse(original_url)
621622
match = pattern.search(parsed_url.path)
622623

624+
# If the path does not match the pattern, try to extract the parameters from the query
625+
if not match:
626+
match = pattern.search(parsed_url.query)
627+
623628
if not match:
624629
logger.debug(
625630
"Subrequest URL '%s' did not match pattern '%s'",
@@ -645,17 +650,19 @@ def _authorize_subrequest(self, request, pattern):
645650
except models.Document.DoesNotExist as exc:
646651
logger.debug("Document with ID '%s' does not exist", pk)
647652
raise drf.exceptions.PermissionDenied() from exc
648-
print(document)
649-
if not document.get_abilities(request.user).get(self.action, False):
653+
654+
user_abilities = document.get_abilities(request.user)
655+
656+
if not user_abilities.get(self.action, False):
650657
logger.debug(
651658
"User '%s' lacks permission for document '%s'", request.user, pk
652659
)
653-
# raise drf.exceptions.PermissionDenied()
660+
raise drf.exceptions.PermissionDenied()
654661

655662
logger.debug(
656663
"Subrequest authorization successful. Extracted parameters: %s", url_params
657664
)
658-
return url_params
665+
return url_params, user_abilities, request.user.id
659666

660667
@drf.decorators.action(detail=False, methods=["get"], url_path="media-auth")
661668
def media_auth(self, request, *args, **kwargs):
@@ -668,14 +675,36 @@ def media_auth(self, request, *args, **kwargs):
668675
annotation. The request will then be proxied to the object storage backend who will
669676
respond with the file after checking the signature included in headers.
670677
"""
671-
url_params = self._authorize_subrequest(request, MEDIA_STORAGE_URL_PATTERN)
678+
url_params, _, _ = self._authorize_subrequest(
679+
request, MEDIA_STORAGE_URL_PATTERN
680+
)
672681
pk, key = url_params.values()
673682

674683
# Generate S3 authorization headers using the extracted URL parameters
675684
request = utils.generate_s3_authorization_headers(f"{pk:s}/{key:s}")
676685

677686
return drf.response.Response("authorized", headers=request.headers, status=200)
678687

688+
@drf.decorators.action(detail=False, methods=["get"], url_path="collaboration-auth")
689+
def collaboration_auth(self, request, *args, **kwargs):
690+
"""
691+
This view is used by an Nginx subrequest to control access to a document's
692+
collaboration server.
693+
"""
694+
_, user_abilities, user_id = self._authorize_subrequest(
695+
request, COLLABORATION_WS_URL_PATTERN
696+
)
697+
can_edit = user_abilities["partial_update"]
698+
699+
# Add the collaboration server secret token to the headers
700+
headers = {
701+
"Authorization": settings.COLLABORATION_SERVER_SECRET,
702+
"X-Can-Edit": str(can_edit),
703+
"X-User-Id": str(user_id),
704+
}
705+
706+
return drf.response.Response("authorized", headers=headers, status=200)
707+
679708
@drf.decorators.action(
680709
detail=True,
681710
methods=["post"],

‎src/backend/core/models.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,7 @@ def get_abilities(self, user):
520520
"ai_transform": can_update,
521521
"ai_translate": can_update,
522522
"attachment_upload": can_update,
523+
"collaboration_auth": can_get,
523524
"destroy": RoleChoices.OWNER in roles,
524525
"favorite": can_get and user.is_authenticated,
525526
"link_configuration": is_owner_or_admin,

‎src/backend/core/tests/documents/test_api_documents_retrieve.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ def test_api_documents_retrieve_anonymous_public():
2626
"ai_transform": document.link_role == "editor",
2727
"ai_translate": document.link_role == "editor",
2828
"attachment_upload": document.link_role == "editor",
29+
"collaboration_auth": True,
2930
"destroy": False,
3031
# Anonymous user can't favorite a document even with read access
3132
"favorite": False,
@@ -89,6 +90,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated(
8990
"ai_transform": document.link_role == "editor",
9091
"ai_translate": document.link_role == "editor",
9192
"attachment_upload": document.link_role == "editor",
93+
"collaboration_auth": True,
9294
"destroy": False,
9395
"favorite": True,
9496
"invite_owner": False,

‎src/backend/core/tests/test_models_documents.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ def test_models_documents_get_abilities_forbidden(is_authenticated, reach, role)
9898
"ai_transform": False,
9999
"ai_translate": False,
100100
"attachment_upload": False,
101+
"collaboration_auth": False,
101102
"destroy": False,
102103
"favorite": False,
103104
"invite_owner": False,
@@ -134,6 +135,7 @@ def test_models_documents_get_abilities_reader(is_authenticated, reach):
134135
"ai_transform": False,
135136
"ai_translate": False,
136137
"attachment_upload": False,
138+
"collaboration_auth": True,
137139
"destroy": False,
138140
"favorite": is_authenticated,
139141
"invite_owner": False,
@@ -170,6 +172,7 @@ def test_models_documents_get_abilities_editor(is_authenticated, reach):
170172
"ai_transform": True,
171173
"ai_translate": True,
172174
"attachment_upload": True,
175+
"collaboration_auth": True,
173176
"destroy": False,
174177
"favorite": is_authenticated,
175178
"invite_owner": False,
@@ -195,6 +198,7 @@ def test_models_documents_get_abilities_owner():
195198
"ai_transform": True,
196199
"ai_translate": True,
197200
"attachment_upload": True,
201+
"collaboration_auth": True,
198202
"destroy": True,
199203
"favorite": True,
200204
"invite_owner": True,
@@ -219,6 +223,7 @@ def test_models_documents_get_abilities_administrator():
219223
"ai_transform": True,
220224
"ai_translate": True,
221225
"attachment_upload": True,
226+
"collaboration_auth": True,
222227
"destroy": False,
223228
"favorite": True,
224229
"invite_owner": False,
@@ -246,6 +251,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries):
246251
"ai_transform": True,
247252
"ai_translate": True,
248253
"attachment_upload": True,
254+
"collaboration_auth": True,
249255
"destroy": False,
250256
"favorite": True,
251257
"invite_owner": False,
@@ -275,6 +281,7 @@ def test_models_documents_get_abilities_reader_user(django_assert_num_queries):
275281
"ai_transform": False,
276282
"ai_translate": False,
277283
"attachment_upload": False,
284+
"collaboration_auth": True,
278285
"destroy": False,
279286
"favorite": True,
280287
"invite_owner": False,
@@ -305,6 +312,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries):
305312
"ai_transform": False,
306313
"ai_translate": False,
307314
"attachment_upload": False,
315+
"collaboration_auth": True,
308316
"destroy": False,
309317
"favorite": True,
310318
"invite_owner": False,

‎src/backend/impress/settings.py

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,9 @@ class Base(Configuration):
372372
SENTRY_DSN = values.Value(None, environ_name="SENTRY_DSN", environ_prefix=None)
373373

374374
# Collaboration
375+
COLLABORATION_SERVER_SECRET = values.Value(
376+
None, environ_name="COLLABORATION_SERVER_SECRET", environ_prefix=None
377+
)
375378
COLLABORATION_WS_URL = values.Value(
376379
None, environ_name="COLLABORATION_WS_URL", environ_prefix=None
377380
)
@@ -465,9 +468,22 @@ class Base(Configuration):
465468
environ_prefix=None,
466469
)
467470

471+
USER_OIDC_FIELDS_TO_FULLNAME = values.ListValue(
472+
default=["first_name", "last_name"],
473+
environ_name="USER_OIDC_FIELDS_TO_FULLNAME",
474+
environ_prefix=None,
475+
)
476+
USER_OIDC_FIELD_TO_SHORTNAME = values.Value(
477+
default="first_name",
478+
environ_name="USER_OIDC_FIELD_TO_SHORTNAME",
479+
environ_prefix=None,
480+
)
481+
468482
ALLOW_LOGOUT_GET_METHOD = values.BooleanValue(
469483
default=True, environ_name="ALLOW_LOGOUT_GET_METHOD", environ_prefix=None
470484
)
485+
486+
# AI service
471487
AI_API_KEY = values.Value(None, environ_name="AI_API_KEY", environ_prefix=None)
472488
AI_BASE_URL = values.Value(None, environ_name="AI_BASE_URL", environ_prefix=None)
473489
AI_MODEL = values.Value(None, environ_name="AI_MODEL", environ_prefix=None)
@@ -483,17 +499,6 @@ class Base(Configuration):
483499
"day": 200,
484500
}
485501

486-
USER_OIDC_FIELDS_TO_FULLNAME = values.ListValue(
487-
default=["first_name", "last_name"],
488-
environ_name="USER_OIDC_FIELDS_TO_FULLNAME",
489-
environ_prefix=None,
490-
)
491-
USER_OIDC_FIELD_TO_SHORTNAME = values.Value(
492-
default="first_name",
493-
environ_name="USER_OIDC_FIELD_TO_SHORTNAME",
494-
environ_prefix=None,
495-
)
496-
497502
# Logging
498503
# We want to make it easy to log to console but by default we log production
499504
# to Sentry and don't want to log to console.

0 commit comments

Comments
 (0)