feat(notification): add JWT auth + E2E smoke test to Notification service#52
feat(notification): add JWT auth + E2E smoke test to Notification service#52devin-ai-integration[bot] wants to merge 2 commits into
Conversation
- Fix Shared project reference paths (../../Shared -> ../../../Shared) - Add Microsoft.AspNetCore.Authentication.JwtBearer package - Configure JWT Bearer authentication in Program.cs - Add [Authorize] attribute to NotificationController - /healthz endpoint remains public (mapped outside controllers)
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| namespace Notification.API.Controllers; | ||
|
|
||
| [ApiController] | ||
| [Authorize] |
There was a problem hiding this comment.
🔴 Internal event-receiving endpoint is blocked by authentication, breaking inter-service communication
The order-event ingestion endpoint is now gated by JWT authentication ([Authorize] at src/Services/Notification/Notification.API/Controllers/NotificationController.cs:10) without an [AllowAnonymous] override, so other services that post events here will always receive a 401 Unauthorized response.
Impact: The notification service can no longer receive order-placed events from other microservices, completely breaking notification delivery.
Controller-level Authorize blocks service-to-service endpoint
The [Authorize] attribute is applied at the controller level (line 10), which means every action inherits it — including ReceiveOrderPlacedEvent at line 92. This endpoint is explicitly documented as the inter-service communication mechanism: "In production this would be a RabbitMQ consumer; this endpoint enables local testing without a message broker."
The Order service (src/Services/Order/Order.API/Program.cs) has no JWT authentication configured whatsoever — it doesn't reference the JwtBearer package, doesn't configure token generation, and has no mechanism to obtain a bearer token. Therefore, when the Order service (or any integration test tool) attempts to POST to /api/notification/events/order-placed, it will receive a 401 Unauthorized.
The fix is to add [AllowAnonymous] to the ReceiveOrderPlacedEvent action method so that it bypasses the controller-level authorization requirement.
Prompt for agents
The [Authorize] attribute at the controller level (line 10 of NotificationController.cs) blocks the ReceiveOrderPlacedEvent endpoint which is designed for service-to-service communication. The Order service has no JWT authentication and cannot obtain a token, so posting order-placed events will fail with 401.
Fix: Add [AllowAnonymous] attribute to the ReceiveOrderPlacedEvent method (line 92) so it can still be called by other services without a JWT token. You'll need to add 'using Microsoft.AspNetCore.Authorization;' if not already imported (it is already imported at line 1). Simply add [AllowAnonymous] above [HttpPost("events/order-placed")] on line 92.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Valid observation — the ReceiveOrderPlacedEvent endpoint is indeed now gated behind auth. However, the scope of this PR explicitly requires adding [Authorize] at the controller level without modifying methods or other controller code. Adding [AllowAnonymous] to the event endpoint would be a reasonable follow-up to preserve inter-service communication.
| "Jwt": { | ||
| "Secret": "QuickApp_Microservices_SuperSecret_Key_For_Dev_Only_Min_32_Chars!", | ||
| "Issuer": "quickapp-identity", | ||
| "Audience": "quickapp-api" | ||
| } |
There was a problem hiding this comment.
🚩 Docker-compose missing JWT configuration environment variables for notification-service
The docker-compose.yml does not pass Jwt__Secret, Jwt__Issuer, or Jwt__Audience environment variables to the notification-service container. The service will still start because these values are present in appsettings.json which gets baked into the Docker image. However, this means the production/dev secret is hardcoded in the image rather than being injected at runtime, which deviates from 12-factor app practices. Other services in the compose file also don't receive JWT config, which compounds the inter-service auth issue.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — this is a valid 12-factor concern. However, modifying docker-compose.yml is explicitly out of scope for this PR. The JWT config in appsettings.json serves as the development default; runtime injection via environment variables can be addressed in a follow-up.
| namespace Notification.API.Controllers; | ||
|
|
||
| [ApiController] | ||
| [Authorize] |
There was a problem hiding this comment.
🚩 Controller-level [Authorize] also gates the event ingestion endpoint
The [Authorize] attribute at NotificationController.cs:10 applies to all endpoints including ReceiveOrderPlacedEvent (line 92). This endpoint is documented as a stand-in for RabbitMQ message consumption during local testing. When this is eventually replaced by an actual message broker consumer, the auth requirement is irrelevant. However, if another microservice (e.g., Order service) needs to call this HTTP endpoint directly for service-to-service communication before RabbitMQ is set up, it would need a valid JWT. Consider whether [AllowAnonymous] on the event endpoint would be more appropriate, or whether service-to-service auth is intended.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is the same concern raised in the earlier review — the [Authorize] at controller level is intentional per the PR scope. Whether to add [AllowAnonymous] on the event endpoint is a follow-up decision for the maintainer. See the earlier thread for more context.
Summary
Adds JWT Bearer authentication to the Notification service and an E2E smoke test validating auth enforcement + gateway routing.
JWT Auth
Notification.API.csproj: addedMicrosoft.AspNetCore.Authentication.JwtBearer, fixed broken Shared project reference paths (../../Shared→../../../Shared)Program.cs: configured JWT Bearer withValidateIssuer/Audience/Lifetime/IssuerSigningKeyfromJwt:Secret/Issuer/Audienceconfig section; addedapp.UseAuthentication(); app.UseAuthorization();beforeMapControllers()NotificationController: added[Authorize]attribute — all endpoints require a valid JWT;/healthzremains public (mapped separately viaapp.MapHealthChecks)appsettings.json: added sharedJwtconfig block (quickapp-identity/quickapp-api/ HS256)E2E Smoke Test (
tests/notification-e2e-smoke.sh)Bash script that generates an HS256 JWT and runs 6 assertions:
GET /healthz→ 200GET /api/notification(no token) → 401POST /api/notification/events/order-placed(with JWT) → 201GET /api/notification(with JWT) → 200, response contains created notificationGET /api/notification/{id}/preview(with JWT) → 200, HTML contentGET /api/notifications/api/notificationwithout token) → 401 (proves gateway routes to service with auth enforced)Validation
dotnet buildsucceeds with 0 errorsLink to Devin session: https://partner-workshops.devinenterprise.com/sessions/e5cec196f7f7471f95b74e4999b03550
Requested by: @mbatchelor81