Skip to content

Commit 8c6197f

Browse files
AchoArnoldCopilot
AchoArnold
and
Copilot
authored
feat(api): sign HTTP adapter notifications with phone-ID JWT (NdoleStudio#992)
* feat(api): sign HTTP adapter notifications with phone-ID JWT Sign HTTPS adapter notification requests with a JWT the same way webhook requests are signed, using the phone ID (a UUID) as the HMAC secret instead of a per-webhook signing key. - FCMClient.Send now takes the sending phone's ID so HTTP-transport clients can generate a bearer token; Firebase/emulator clients ignore it. - HTTPNotificationSender signs a JWT (10 min validity, audience is the endpoint URL with any userinfo stripped) and sends it via the X-Httpsms-Signature header rather than Authorization, so adapters can still use HTTP basic auth embedded in the endpoint URL. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(api): send adapter JWT via Authorization header Send the phone-signed notification JWT via the standard Authorization header, matching webhook requests exactly, instead of a dedicated X-Httpsms-Signature header. Adapter endpoint URLs are no longer expected to carry HTTP basic auth credentials, since Authorization is now always used for the bearer JWT; update the corresponding test to assert basic auth from the URL is ignored. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: validate adapter notification JWT auth in emulator The adapter emulator now requires and validates the phone-ID-signed JWT (Authorization: Bearer) that the API sends with every FCM-compatible notification, mirroring the webhook JWT validation already used in integration tests. - adapter-emulator: gateway registration now requires phone_id; notification_handler verifies the JWT (HS256, sub==phone_id, iss==api.httpsms.com) before recording/processing, rejecting invalid/missing tokens with 401. - emulator_test.go: updated existing tests to register phone_id and send valid tokens; added negative tests for missing auth and wrong signing secret. - helpers_test.go: setupAdapterPhone now upserts the phone before registering the gateway (so phone_id is known), and adds an assertAdapterNotificationJWT helper mirroring assertWebhookJWT. - adapter_integration_test.go: asserts the JWT on recorded message and heartbeat notifications. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 45ed9de9-a3ad-41cf-ad32-ebec28d9771c * fix(api): stop embedding phone ID in adapter notification JWT claims Address PR review: the token used phoneID as both the readable sub claim and the HS256 signing secret, so anyone who saw one token could read the secret and forge further ones. The sub claim is unnecessary since the adapter already knows which phone ID to verify against from its own gateway registration, so it is removed; the phone ID remains the signing secret only. - http_notification_sender.go: getAuthToken no longer sets Subject. - adapter-emulator/notification_handler.go: verifyNotificationAuth no longer checks claims.Subject. - Updated tests in api/pkg/services and tests/ to assert sub is empty instead of equal to the phone ID. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 45ed9de9-a3ad-41cf-ad32-ebec28d9771c * fix(api): address remaining PR review comments - fcm_client.go: fix Send doc comment - phoneID identifies the receiving/target phone, not the sending phone. - adapter-emulator/notification_handler.go: verifyNotificationAuth now requires the HS256 signing method specifically instead of accepting any HMAC variant, matching what the API actually signs with. - emulator_test.go: added TestNotificationHandlerRejectsNonHS256SigningMethod covering the HS256-only check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 45ed9de9-a3ad-41cf-ad32-ebec28d9771c --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 45ed9de9-a3ad-41cf-ad32-ebec28d9771c
1 parent 20adcdf commit 8c6197f

14 files changed

Lines changed: 350 additions & 44 deletions

‎api/pkg/services/emulator_fcm_client.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"firebase.google.com/go/messaging"
1212
"github.com/NdoleStudio/httpsms/pkg/telemetry"
1313
"github.com/NdoleStudio/stacktrace"
14+
"github.com/google/uuid"
1415
)
1516

1617
// EmulatorFCMClient sends FCM messages to the phone emulator via HTTP.
@@ -50,7 +51,7 @@ type emulatorFCMResponse struct {
5051
}
5152

5253
// Send sends a message to the emulator's FCM endpoint.
53-
func (c *EmulatorFCMClient) Send(ctx context.Context, message *messaging.Message) (string, error) {
54+
func (c *EmulatorFCMClient) Send(ctx context.Context, message *messaging.Message, _ uuid.UUID) (string, error) {
5455
payload := &emulatorFCMRequest{
5556
Message: &emulatorFCMMessage{
5657
Token: message.Token,

‎api/pkg/services/fcm_client.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@ import (
44
"context"
55

66
"firebase.google.com/go/messaging"
7+
"github.com/google/uuid"
78
)
89

910
// FCMClient sends Firebase-compatible messages through a phone notification transport.
1011
type FCMClient interface {
1112
// Send sends a message and returns the transport's delivery identifier on success.
12-
Send(ctx context.Context, message *messaging.Message) (string, error)
13+
// phoneID identifies the receiving phone (the notification's target) and is used by HTTP
14+
// adapter transports to sign the request.
15+
Send(ctx context.Context, message *messaging.Message, phoneID uuid.UUID) (string, error)
1316
}
1417

1518
// FirebaseFCMClient wraps the real Firebase messaging.Client.
@@ -23,6 +26,6 @@ func NewFirebaseFCMClient(client *messaging.Client) *FirebaseFCMClient {
2326
}
2427

2528
// Send sends a message via the real Firebase SDK.
26-
func (c *FirebaseFCMClient) Send(ctx context.Context, message *messaging.Message) (string, error) {
29+
func (c *FirebaseFCMClient) Send(ctx context.Context, message *messaging.Message, _ uuid.UUID) (string, error) {
2730
return c.client.Send(ctx, message)
2831
}

‎api/pkg/services/http_notification_sender.go‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"context"
66
"encoding/json"
77
"errors"
8+
"fmt"
89
"io"
910
"net/http"
1011
"net/url"
@@ -14,13 +15,17 @@ import (
1415
"github.com/NdoleStudio/httpsms/pkg/telemetry"
1516
"github.com/NdoleStudio/stacktrace"
1617
"github.com/avast/retry-go/v5"
18+
"github.com/golang-jwt/jwt/v5"
19+
"github.com/google/uuid"
1720
)
1821

1922
const (
2023
maxNotificationResponseDiscardBytes = 4 * 1024
2124
notificationHTTPAttempts = 3
2225
notificationHTTPTimeout = 5 * time.Second
2326
notificationHTTPRetryDelay = 250 * time.Millisecond
27+
notificationJWTIssuer = "api.httpsms.com"
28+
notificationJWTValidity = 10 * time.Minute
2429
)
2530

2631
// HTTPNotificationSender sends FCM-compatible gateway notifications to HTTPS adapters.
@@ -62,6 +67,7 @@ func newHTTPNotificationSenderWithRetrier(
6267
func (sender *HTTPNotificationSender) Send(
6368
ctx context.Context,
6469
message *messaging.Message,
70+
phoneID uuid.UUID,
6571
) (string, error) {
6672
if message == nil {
6773
return "", sender.notificationError("", "notification message is nil")
@@ -78,8 +84,13 @@ func (sender *HTTPNotificationSender) Send(
7884
return "", sender.notificationError(hostname, "cannot encode notification")
7985
}
8086

87+
authToken, err := sender.getAuthToken(endpoint, phoneID)
88+
if err != nil {
89+
return "", sender.notificationError(hostname, "cannot generate notification auth token")
90+
}
91+
8192
err = sender.retrier.Do(func() error {
82-
return sender.deliver(ctx, endpoint, body)
93+
return sender.deliver(ctx, endpoint, body, authToken)
8394
})
8495
if err == nil {
8596
return "http/success", nil
@@ -91,6 +102,27 @@ func (sender *HTTPNotificationSender) Send(
91102
return "", sender.notificationError(hostname, "notification request failed")
92103
}
93104

105+
// getAuthToken generates a JWT bearer token for the HTTPS adapter, signed with the phone ID
106+
// the same way webhook requests are signed with the webhook signing key. The phone ID is only
107+
// used as the HMAC secret and is intentionally not embedded in any claim: the adapter already
108+
// knows which phone ID to verify against from its own gateway registration, and putting the
109+
// phone ID in a readable claim would let anyone who intercepts one token read the signing
110+
// secret and forge further tokens.
111+
func (sender *HTTPNotificationSender) getAuthToken(endpoint *url.URL, phoneID uuid.UUID) (string, error) {
112+
audience := *endpoint
113+
audience.User = nil
114+
115+
now := time.Now().UTC()
116+
token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.RegisteredClaims{
117+
Audience: []string{audience.String()},
118+
ExpiresAt: jwt.NewNumericDate(now.Add(notificationJWTValidity)),
119+
IssuedAt: jwt.NewNumericDate(now),
120+
Issuer: notificationJWTIssuer,
121+
NotBefore: jwt.NewNumericDate(now.Add(-notificationJWTValidity)),
122+
})
123+
return token.SignedString([]byte(phoneID.String()))
124+
}
125+
94126
func encodeHTTPNotificationPayload(message *messaging.Message) ([]byte, error) {
95127
return json.Marshal(map[string]any{
96128
"message": message,
@@ -101,6 +133,7 @@ func (sender *HTTPNotificationSender) deliver(
101133
ctx context.Context,
102134
endpoint *url.URL,
103135
body []byte,
136+
authToken string,
104137
) error {
105138
if err := ctx.Err(); err != nil {
106139
return terminalNotificationRequestError{cause: err}
@@ -109,7 +142,7 @@ func (sender *HTTPNotificationSender) deliver(
109142
attemptCtx, cancel := context.WithTimeout(ctx, sender.timeout)
110143
defer cancel()
111144

112-
request, err := createHTTPNotificationRequest(attemptCtx, endpoint, body)
145+
request, err := createHTTPNotificationRequest(attemptCtx, endpoint, body, authToken)
113146
if err != nil {
114147
return terminalNotificationRequestError{cause: err}
115148
}
@@ -125,6 +158,7 @@ func createHTTPNotificationRequest(
125158
ctx context.Context,
126159
endpoint *url.URL,
127160
body []byte,
161+
authToken string,
128162
) (*http.Request, error) {
129163
request, err := http.NewRequestWithContext(
130164
ctx,
@@ -136,6 +170,7 @@ func createHTTPNotificationRequest(
136170
return nil, err
137171
}
138172
request.Header.Set("Content-Type", "application/json")
173+
request.Header.Set("Authorization", fmt.Sprintf("Bearer %s", authToken))
139174
return request, nil
140175
}
141176

‎api/pkg/services/http_notification_sender_test.go‎

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,21 @@ import (
88
"net/http"
99
"net/url"
1010
"reflect"
11+
"strings"
1112
"testing"
1213
"time"
1314

1415
"firebase.google.com/go/messaging"
1516
"github.com/NdoleStudio/httpsms/pkg/telemetry"
17+
"github.com/golang-jwt/jwt/v5"
18+
"github.com/google/uuid"
1619
"github.com/stretchr/testify/assert"
1720
"github.com/stretchr/testify/require"
1821
"go.opentelemetry.io/otel/trace"
1922
)
2023

24+
var testNotificationPhoneID = uuid.New()
25+
2126
type roundTripFunc func(*http.Request) (*http.Response, error)
2227

2328
func (roundTrip roundTripFunc) RoundTrip(request *http.Request) (*http.Response, error) {
@@ -57,10 +62,20 @@ func TestHTTPNotificationSenderSendsFCMCompatiblePayload(t *testing.T) {
5762
assert.Equal(t, "high", payload.Message.Android.Priority)
5863
assert.Equal(t, "600s", payload.Message.Android.TTL)
5964

65+
token, err := jwt.Parse(strings.TrimPrefix(request.Header.Get("Authorization"), "Bearer "), func(*jwt.Token) (interface{}, error) {
66+
return []byte(testNotificationPhoneID.String()), nil
67+
})
68+
require.NoError(t, err)
69+
assert.True(t, token.Valid)
70+
claims, ok := token.Claims.(jwt.MapClaims)
71+
require.True(t, ok)
72+
assert.Empty(t, claims["sub"], "phone ID must not be embedded in a claim since it is also the signing secret")
73+
assert.Equal(t, "api.httpsms.com", claims["iss"])
74+
6075
return response(http.StatusNoContent, http.NoBody), nil
6176
}))
6277

63-
result, err := sender.Send(context.Background(), message)
78+
result, err := sender.Send(context.Background(), message, testNotificationPhoneID)
6479

6580
require.NoError(t, err)
6681
assert.Equal(t, "http/success", result)
@@ -150,6 +165,7 @@ func TestHTTPNotificationSenderRetriesOnlyTransientFailures(t *testing.T) {
150165
result, err := sender.Send(
151166
context.Background(),
152167
&messaging.Message{Token: "https://adapter.example.com/notify"},
168+
testNotificationPhoneID,
153169
)
154170

155171
if test.wantErr {
@@ -179,6 +195,7 @@ func TestHTTPNotificationSenderReusesRetrierAcrossSends(t *testing.T) {
179195
_, err := sender.Send(
180196
context.Background(),
181197
&messaging.Message{Token: "https://adapter.example.com/notify"},
198+
testNotificationPhoneID,
182199
)
183200
require.NoError(t, err)
184201
}
@@ -206,6 +223,7 @@ func TestHTTPNotificationSenderCreatesFreshRequestAndBodyForEveryAttempt(t *test
206223
Token: "https://adapter.example.com/notify",
207224
Data: map[string]string{"KEY_MESSAGE_ID": "message-1"},
208225
},
226+
testNotificationPhoneID,
209227
)
210228

211229
require.NoError(t, err)
@@ -227,6 +245,7 @@ func TestHTTPNotificationSenderBoundsResponseBodyDiscard(t *testing.T) {
227245
_, err := sender.Send(
228246
context.Background(),
229247
&messaging.Message{Token: "https://adapter.example.com/notify"},
248+
testNotificationPhoneID,
230249
)
231250

232251
require.NoError(t, err)
@@ -253,6 +272,7 @@ func TestHTTPNotificationSenderOmitsTTLForHeartbeat(t *testing.T) {
253272
Priority: "high",
254273
},
255274
},
275+
testNotificationPhoneID,
256276
)
257277

258278
require.NoError(t, err)
@@ -274,12 +294,29 @@ func TestHTTPNotificationSenderUsesInjectedHTTPClientUnchanged(t *testing.T) {
274294
assert.Equal(t, time.Minute, sender.client.Timeout)
275295
}
276296

277-
func TestHTTPNotificationSenderAllowsEndpointUserInformation(t *testing.T) {
297+
func TestHTTPNotificationSenderIgnoresEndpointUserInformation(t *testing.T) {
298+
// Adapter endpoints must not rely on HTTP basic auth embedded in the URL; the Authorization
299+
// header always carries the phone-signed JWT instead.
278300
sender := newHTTPNotificationSender(t, roundTripFunc(func(request *http.Request) (*http.Response, error) {
279-
username, password, ok := request.BasicAuth()
280-
assert.True(t, ok)
281-
assert.Equal(t, "adapter-user", username)
282-
assert.Equal(t, "adapter-password", password)
301+
_, _, ok := request.BasicAuth()
302+
assert.False(t, ok)
303+
304+
authorization := request.Header.Get("Authorization")
305+
assert.True(t, strings.HasPrefix(authorization, "Bearer "))
306+
307+
token, err := jwt.Parse(strings.TrimPrefix(authorization, "Bearer "), func(*jwt.Token) (interface{}, error) {
308+
return []byte(testNotificationPhoneID.String()), nil
309+
})
310+
require.NoError(t, err)
311+
assert.True(t, token.Valid)
312+
claims, ok := token.Claims.(jwt.MapClaims)
313+
require.True(t, ok)
314+
audience, err := claims.GetAudience()
315+
require.NoError(t, err)
316+
require.Len(t, audience, 1)
317+
assert.NotContains(t, audience[0], "adapter-user")
318+
assert.NotContains(t, audience[0], "adapter-password")
319+
283320
return response(http.StatusNoContent, http.NoBody), nil
284321
}))
285322
endpoint := &url.URL{
@@ -292,6 +329,7 @@ func TestHTTPNotificationSenderAllowsEndpointUserInformation(t *testing.T) {
292329
_, err := sender.Send(
293330
context.Background(),
294331
&messaging.Message{Token: endpoint.String()},
332+
testNotificationPhoneID,
295333
)
296334

297335
require.NoError(t, err)
@@ -309,6 +347,7 @@ func TestHTTPNotificationSenderBoundsEveryAttemptByTimeout(t *testing.T) {
309347
_, err := sender.Send(
310348
context.Background(),
311349
&messaging.Message{Token: "https://adapter.example.com/notify"},
350+
testNotificationPhoneID,
312351
)
313352

314353
require.Error(t, err)
@@ -328,6 +367,7 @@ func TestHTTPNotificationSenderStopsRetriesWhenParentContextIsCancelled(t *testi
328367
_, err := sender.Send(
329368
ctx,
330369
&messaging.Message{Token: "https://adapter.example.com/notify"},
370+
testNotificationPhoneID,
331371
)
332372

333373
require.Error(t, err)
@@ -339,7 +379,7 @@ func TestHTTPNotificationSenderRejectsNilMessage(t *testing.T) {
339379
return response(http.StatusNoContent, http.NoBody), nil
340380
}))
341381

342-
_, err := sender.Send(context.Background(), nil)
382+
_, err := sender.Send(context.Background(), nil, testNotificationPhoneID)
343383

344384
require.Error(t, err)
345385
assert.Contains(t, err.Error(), "notification message is nil")

‎api/pkg/services/phone_notification_service.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ func (service *PhoneNotificationService) sendPhoneNotification(
215215
}
216216

217217
message.Token = strings.TrimSpace(*phone.FcmToken)
218-
result, err := client.Send(ctx, message)
218+
result, err := client.Send(ctx, message, phone.ID)
219219
if err != nil {
220220
return "", transport, stacktrace.Propagatef(
221221
err,

‎api/pkg/services/phone_notification_service_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ func (logger *phoneNotificationLogger) Printf(string, ...interface{}) {}
9090

9191
type recordingPhoneNotificationClient struct {
9292
message *messaging.Message
93+
phoneID uuid.UUID
9394
result string
9495
err error
9596
calls int
@@ -98,9 +99,11 @@ type recordingPhoneNotificationClient struct {
9899
func (client *recordingPhoneNotificationClient) Send(
99100
_ context.Context,
100101
message *messaging.Message,
102+
phoneID uuid.UUID,
101103
) (string, error) {
102104
client.calls++
103105
client.message = message
106+
client.phoneID = phoneID
104107
return client.result, client.err
105108
}
106109

@@ -122,6 +125,7 @@ func TestPhoneNotificationServiceSendPhoneNotificationUsesMappedClient(t *testin
122125
assert.Equal(t, entities.NotificationTransportHTTP, transport)
123126
assert.Equal(t, "https://adapter.example.com/notify", message.Token)
124127
assert.Same(t, message, httpClient.message)
128+
assert.Equal(t, phone.ID, httpClient.phoneID)
125129
assert.Equal(t, 1, httpClient.calls)
126130
}
127131

‎tests/adapter-emulator/control_handler.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const maxControlBodyBytes = 1024 * 1024
1414
type gatewayRegistration struct {
1515
PhoneNumber string `json:"phone_number"`
1616
PhoneAPIKey string `json:"phone_api_key"`
17+
PhoneID string `json:"phone_id"`
1718
}
1819

1920
type incomingMessageRequest struct {
@@ -39,8 +40,9 @@ func (instance *emulator) handleGatewayRegistration(writer http.ResponseWriter,
3940
}
4041
registration.PhoneNumber = strings.TrimSpace(registration.PhoneNumber)
4142
registration.PhoneAPIKey = strings.TrimSpace(registration.PhoneAPIKey)
42-
if registration.PhoneNumber == "" || registration.PhoneAPIKey == "" {
43-
writeControlError(writer, http.StatusBadRequest, errors.New("phone_number and phone_api_key are required"))
43+
registration.PhoneID = strings.TrimSpace(registration.PhoneID)
44+
if registration.PhoneNumber == "" || registration.PhoneAPIKey == "" || registration.PhoneID == "" {
45+
writeControlError(writer, http.StatusBadRequest, errors.New("phone_number, phone_api_key and phone_id are required"))
4446
return
4547
}
4648

0 commit comments

Comments
 (0)