Skip to content

Commit b66e772

Browse files
committed
more review fixes
1 parent 3f94b32 commit b66e772

32 files changed

Lines changed: 482 additions & 487 deletions

.github/workflows/messages.yml

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,46 @@ jobs:
9595
- name: Build frontend
9696
run: make build-front
9797

98+
lint-client-bridge:
99+
runs-on: ubuntu-latest
100+
steps:
101+
- name: Checkout repository
102+
uses: actions/checkout@v6
103+
- name: Create env files
104+
run: make create-env-files
105+
- name: Run client-bridge linting
106+
run: make lint-client-bridge
107+
108+
test-client-bridge:
109+
runs-on: ubuntu-latest
110+
steps:
111+
- name: Checkout repository
112+
uses: actions/checkout@v6
113+
- name: Create env files
114+
run: make create-env-files
115+
- name: Run client-bridge tests
116+
run: make test-client-bridge
117+
118+
test-mta-in:
119+
runs-on: ubuntu-latest
120+
steps:
121+
- name: Checkout repository
122+
uses: actions/checkout@v6
123+
- name: Create env files
124+
run: make create-env-files
125+
- name: Run mta-in tests
126+
run: make test-mta-in
127+
128+
test-socks-proxy:
129+
runs-on: ubuntu-latest
130+
steps:
131+
- name: Checkout repository
132+
uses: actions/checkout@v6
133+
- name: Create env files
134+
run: make create-env-files
135+
- name: Run socks-proxy tests
136+
run: make test-socks-proxy
137+
98138
check-api-state:
99139
runs-on: ubuntu-latest
100140
steps:

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ It features a [MTA](https://en.wikipedia.org/wiki/Message_transfer_agent) based
6060
### Based on standards
6161
* 🔑 OpenID Connect for all user accounts as the primary authentication method. Plug any identity provider, including Keycloak.
6262
* 📬 SMTP in and out (server-to-server).
63-
* ✅ JMAP-inspired data model. JMAP-compilant endpoint [in progress](https://github.com/suitenumerique/messages/pull/479).
63+
* ✅ JMAP-inspired data model. JMAP-compliant endpoint [in progress](https://github.com/suitenumerique/messages/pull/479).
6464
* 📮 Optional IMAP and SMTP client access via the [client bridge](/src/client-bridge/), for users who prefer traditional email clients like Thunderbird or mobile phones. Uses app-specific passwords with configurable roles.
6565

6666

src/backend/core/api/openapi.json

Lines changed: 22 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -124,34 +124,6 @@
124124
}
125125
}
126126
},
127-
"/api/v1.0/client-bridge/auth/": {
128-
"post": {
129-
"operationId": "client_bridge_auth_create",
130-
"description": "Authenticate a client-bridge channel by email username and app-specific password.\n\nPOST /api/v1.0/client-bridge/auth/\nInput: {\"username\": \"<mailbox email>\", \"password\": \"...\"}\nReturns: {\"token\": \"<JWT>\", \"channel_id\", \"mailbox_id\", \"mailbox_email\", \"role\",\n \"expires_at\": \"<ISO 8601>\"}\n\nThe request must carry ``X-Service-Auth: Bearer <CLIENTBRIDGE_API_SECRET>``\nto prove it comes from the client-bridge service.\n\nThe JWT is signed with CLIENTBRIDGE_API_SECRET and contains the channel_id,\nmailbox_id, role, and expiration. The client-bridge passes it as\nX-Channel-Token on all subsequent requests.",
131-
"tags": [
132-
"client-bridge"
133-
],
134-
"responses": {
135-
"200": {
136-
"description": "No response body"
137-
}
138-
}
139-
}
140-
},
141-
"/api/v1.0/client-bridge/submit/": {
142-
"post": {
143-
"operationId": "client_bridge_submit_create",
144-
"description": "Submit an outbound message via the client-bridge.\n\nPOST /api/v1.0/client-bridge/submit/\nContent-Type: message/rfc822 (raw email)\nHeaders: X-Channel-Token (JWT), X-Mail-From, X-Rcpt-To\nReturns: {\"message_id\": \"...\", \"status\": \"accepted\"}\n\nUses ClientBridgeChannelAuthentication: the channel is resolved from\nthe JWT and available as request.auth.",
145-
"tags": [
146-
"client-bridge"
147-
],
148-
"responses": {
149-
"200": {
150-
"description": "No response body"
151-
}
152-
}
153-
}
154-
},
155127
"/api/v1.0/config/": {
156128
"get": {
157129
"operationId": "config_retrieve",
@@ -2879,7 +2851,17 @@
28792851
],
28802852
"responses": {
28812853
"200": {
2882-
"description": "New password generated"
2854+
"content": {
2855+
"application/json": {
2856+
"schema": {
2857+
"$ref": "#/components/schemas/RotatePasswordResponse"
2858+
}
2859+
}
2860+
},
2861+
"description": ""
2862+
},
2863+
"400": {
2864+
"description": "Channel type does not support password rotation"
28832865
},
28842866
"403": {
28852867
"description": "Permission denied"
@@ -7852,6 +7834,17 @@
78527834
"one_time_password"
78537835
]
78547836
},
7837+
"RotatePasswordResponse": {
7838+
"type": "object",
7839+
"properties": {
7840+
"password": {
7841+
"type": "string"
7842+
}
7843+
},
7844+
"required": [
7845+
"password"
7846+
]
7847+
},
78557848
"SendMessageRequest": {
78567849
"type": "object",
78577850
"description": "Serializer for sending messages.",

src/backend/core/api/serializers.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1517,11 +1517,12 @@ def _move_sensitive_settings(self, validated_data):
15171517
return validated_data
15181518

15191519
extracted = {
1520-
key: settings_data.pop(key)
1521-
for key in keys_to_encrypt
1522-
if key in settings_data
1520+
key: settings_data[key] for key in keys_to_encrypt if key in settings_data
15231521
}
15241522
if extracted:
1523+
# Remove extracted keys from settings without mutating during iteration
1524+
for key in extracted:
1525+
del settings_data[key]
15251526
existing = (self.instance.encrypted_settings or {}) if self.instance else {}
15261527
validated_data["encrypted_settings"] = {**existing, **extracted}
15271528

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
from drf_spectacular.utils import (
77
OpenApiResponse,
88
extend_schema,
9+
inline_serializer,
910
)
1011
from rest_framework import mixins, status, viewsets
12+
from rest_framework import serializers as drf_serializers
1113
from rest_framework.decorators import action
1214
from rest_framework.response import Response
1315

@@ -115,7 +117,13 @@ def destroy(self, request, *args, **kwargs):
115117
@extend_schema(
116118
request=None,
117119
responses={
118-
200: OpenApiResponse(description="New password generated"),
120+
200: inline_serializer(
121+
name="RotatePasswordResponse",
122+
fields={"password": drf_serializers.CharField()},
123+
),
124+
400: OpenApiResponse(
125+
description="Channel type does not support password rotation"
126+
),
119127
403: OpenApiResponse(description="Permission denied"),
120128
404: OpenApiResponse(description="Channel not found"),
121129
},

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from django.conf import settings
99

1010
import jwt
11+
from drf_spectacular.utils import extend_schema
1112
from rest_framework import status
1213
from rest_framework.permissions import IsAuthenticated
1314
from rest_framework.response import Response
@@ -67,6 +68,7 @@ class ClientBridgeAuthView(APIView):
6768
permission_classes = []
6869
throttle_classes = [ClientBridgeAuthThrottle]
6970

71+
@extend_schema(exclude=True)
7072
def post(self, request): # pylint: disable=missing-function-docstring
7173
# Validate service secret
7274
if not settings.FEATURE_CLIENTBRIDGE:
@@ -164,6 +166,7 @@ class ClientBridgeSubmitView(APIView):
164166
authentication_classes = [ClientBridgeChannelAuthentication]
165167
permission_classes = [IsAuthenticated]
166168

169+
@extend_schema(exclude=True)
167170
def post(self, request): # pylint: disable=missing-function-docstring
168171
channel = request.auth
169172
mail_from = request.META.get("HTTP_X_MAIL_FROM")
@@ -231,6 +234,7 @@ def post(self, request): # pylint: disable=missing-function-docstring
231234
message,
232235
"",
233236
"",
237+
user=request.user,
234238
raw_mime=raw_data,
235239
)
236240
if not prepared:
@@ -243,11 +247,9 @@ def post(self, request): # pylint: disable=missing-function-docstring
243247
send_message_task.delay(str(message.id))
244248

245249
logger.info(
246-
"Message submitted via client-bridge: channel=%s, message=%s, from=%s, to=%s",
250+
"Message submitted via client-bridge: channel=%s, message=%s",
247251
channel.id,
248252
message.id,
249-
mail_from,
250-
rcpt_to,
251253
)
252254

253255
return Response(

src/backend/core/mda/inbound_create.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -372,9 +372,9 @@ def _create_message_from_inbound( # pylint: disable=too-many-arguments
372372
mime_id=parsed_email.get("messageId", parsed_email.get("message_id"))
373373
or None,
374374
parent=parent_message,
375-
sent_at=parsed_email.get("date") or timezone.now()
376-
if not is_outbound
377-
else None,
375+
sent_at=(
376+
None if is_outbound else (parsed_email.get("date") or timezone.now())
377+
),
378378
is_draft=is_outbound, # Outbound: draft until prepare_outbound_message finalizes
379379
is_sender=is_sender,
380380
is_starred=False,

src/backend/core/mda/outbound.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ def prepare_outbound_message(
8989
if raw_mime is not None:
9090
# Client-bridge path: the email client already composed the MIME.
9191
# Just sign and store it.
92+
message.sender_user = user
9293
return _sign_and_store(mailbox_sender, message, raw_mime)
9394

9495
# --- Web/API path: compose MIME from text/html body --- #

src/backend/core/tests/api/test_client_bridge.py

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,7 @@ def test_rotate_password_rejects_non_client_bridge(self, mailbox):
595595

596596
assert response.status_code == status.HTTP_400_BAD_REQUEST
597597

598+
@override_settings(FEATURE_MAILBOX_ADMIN_CHANNELS=[], FEATURE_CLIENTBRIDGE=True)
598599
def test_client_bridge_type_rejected_when_not_in_admin_channels(self, mailbox):
599600
"""Creating a client-bridge channel should fail when not in FEATURE_MAILBOX_ADMIN_CHANNELS."""
600601
user = UserFactory()
@@ -882,13 +883,13 @@ def test_post_allowed(self, _make_channel, mailbox, role):
882883
"""Roles with CAN_EDIT should be allowed to POST."""
883884
channel = _make_channel(role)
884885
client = self._jwt_client(channel, mailbox, role=role)
885-
# POST to threads — will fail validation but should not be 403
886+
# POST to threads — will fail with 405 (no CreateModelMixin) but should not be 403
886887
response = client.post(
887888
"/api/v1.0/threads/",
888889
{},
889890
format="json",
890891
)
891-
assert response.status_code != status.HTTP_403_FORBIDDEN
892+
assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED
892893

893894
def test_post_blocked_reader(self, _make_channel, mailbox):
894895
"""reader should not be allowed to POST (not in CAN_EDIT or CAN_SEND)."""
@@ -913,8 +914,7 @@ def test_patch_allowed(self, _make_channel, mailbox, role):
913914
{},
914915
format="json",
915916
)
916-
# Not 403 — may be 404/405 depending on the endpoint
917-
assert response.status_code != status.HTTP_403_FORBIDDEN
917+
assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED
918918

919919
@pytest.mark.parametrize("role", ["reader", "sender_only"])
920920
def test_patch_blocked(self, _make_channel, mailbox, role):
@@ -938,7 +938,7 @@ def test_delete_allowed(self, _make_channel, mailbox, role):
938938
response = client.delete(
939939
"/api/v1.0/threads/",
940940
)
941-
assert response.status_code != status.HTTP_403_FORBIDDEN
941+
assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED
942942

943943
@pytest.mark.parametrize("role", ["reader", "sender_only"])
944944
def test_delete_blocked(self, _make_channel, mailbox, role):
@@ -961,8 +961,8 @@ def test_send_endpoint_allows_sender(self, _make_channel, mailbox):
961961
{"messageId": str(uuid.uuid4()), "senderId": str(mailbox.id)},
962962
format="json",
963963
)
964-
# Should fail on draft lookup, not on role enforcement
965-
assert response.status_code != status.HTTP_403_FORBIDDEN
964+
# Should fail on draft lookup (404), not on role enforcement (403)
965+
assert response.status_code == status.HTTP_404_NOT_FOUND
966966

967967
@pytest.mark.parametrize("role", ["reader", "sender_only"])
968968
def test_send_endpoint_blocked_no_post(self, _make_channel, mailbox, role):
@@ -1049,6 +1049,31 @@ def test_submit_allows_sender_roles(
10491049

10501050
assert response.status_code == status.HTTP_202_ACCEPTED
10511051

1052+
@patch("core.api.viewsets.client_bridge.send_message_task")
1053+
def test_submit_sets_sender_user(
1054+
self,
1055+
mock_send_task, # pylint: disable=unused-argument
1056+
_make_channel,
1057+
mailbox,
1058+
):
1059+
"""Submit should set message.sender_user to the channel's user."""
1060+
channel = _make_channel("sender")
1061+
client = self._jwt_client(channel, mailbox, role="sender")
1062+
mailbox_email = str(mailbox)
1063+
raw_email = _build_raw_email(mailbox_email, "recipient@example.com")
1064+
1065+
response = client.post(
1066+
"/api/v1.0/client-bridge/submit/",
1067+
raw_email,
1068+
content_type="message/rfc822",
1069+
HTTP_X_MAIL_FROM=mailbox_email,
1070+
HTTP_X_RCPT_TO="recipient@example.com",
1071+
)
1072+
1073+
assert response.status_code == status.HTTP_202_ACCEPTED
1074+
message = models.Message.objects.get(id=response.data["message_id"])
1075+
assert message.sender_user == channel.user
1076+
10521077
# ── Auth endpoint returns role ──────────────────────────────────────
10531078

10541079
@pytest.mark.parametrize("role", ["reader", "editor", "sender", "sender_only"])
@@ -1086,10 +1111,10 @@ def test_default_role_is_sender(self, mailbox, channel_user):
10861111
)
10871112
assert response.status_code == status.HTTP_200_OK
10881113

1089-
# POST should work (sender can edit)
1114+
# POST should work (sender can edit) — 405 because ThreadViewSet has no CreateModelMixin
10901115
response = client.post(
10911116
"/api/v1.0/threads/",
10921117
{},
10931118
format="json",
10941119
)
1095-
assert response.status_code != status.HTTP_403_FORBIDDEN
1120+
assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED

src/backend/messages/settings.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,13 +260,13 @@ class Base(Configuration):
260260
}
261261
# Client-bridge service authentication
262262
CLIENTBRIDGE_API_SECRET = values.Value(
263-
"my-shared-secret-clientbridge",
263+
"",
264264
environ_name="CLIENTBRIDGE_API_SECRET",
265265
environ_prefix=None,
266266
)
267267
# Session JWT lifetime in seconds (default: 1 hour).
268268
# IMAP/SMTP clients must re-authenticate when the token expires.
269-
CLIENTBRIDGE_SESSION_TIMEOUT = values.IntegerValue(
269+
CLIENTBRIDGE_SESSION_TIMEOUT = values.PositiveIntegerValue(
270270
3600,
271271
environ_name="CLIENTBRIDGE_SESSION_TIMEOUT",
272272
environ_prefix=None,
@@ -1077,6 +1077,12 @@ def post_setup(cls):
10771077
"OIDC_STORE_REFRESH_TOKEN is enabled."
10781078
)
10791079

1080+
if cls.FEATURE_CLIENTBRIDGE and not cls.CLIENTBRIDGE_API_SECRET:
1081+
raise ValueError(
1082+
"CLIENTBRIDGE_API_SECRET must be set when "
1083+
"FEATURE_CLIENTBRIDGE is enabled"
1084+
)
1085+
10801086

10811087
class Build(Base):
10821088
"""Settings used when the application is built.

0 commit comments

Comments
 (0)