Skip to content

feat(notification): add JWT auth + E2E smoke test to Notification service#52

Open
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/notification-service-auth
Open

feat(notification): add JWT auth + E2E smoke test to Notification service#52
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/notification-service-auth

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds JWT Bearer authentication to the Notification service and an E2E smoke test validating auth enforcement + gateway routing.

JWT Auth

  • Notification.API.csproj: added Microsoft.AspNetCore.Authentication.JwtBearer, fixed broken Shared project reference paths (../../Shared../../../Shared)
  • Program.cs: configured JWT Bearer with ValidateIssuer/Audience/Lifetime/IssuerSigningKey from Jwt:Secret/Issuer/Audience config section; added app.UseAuthentication(); app.UseAuthorization(); before MapControllers()
  • NotificationController: added [Authorize] attribute — all endpoints require a valid JWT; /healthz remains public (mapped separately via app.MapHealthChecks)
  • appsettings.json: added shared Jwt config 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:

  1. GET /healthz → 200
  2. GET /api/notification (no token) → 401
  3. POST /api/notification/events/order-placed (with JWT) → 201
  4. GET /api/notification (with JWT) → 200, response contains created notification
  5. GET /api/notification/{id}/preview (with JWT) → 200, HTML content
  6. Gateway route (GET /api/notifications/api/notification without token) → 401 (proves gateway routes to service with auth enforced)

Validation

  • dotnet build succeeds with 0 errors
  • All 6 E2E smoke tests pass via docker-compose (notification-service + notificationdb + api-gateway)
  • Zero changes to existing notification logic, routes, or response shapes

Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/e5cec196f7f7471f95b74e4999b03550
Requested by: @mbatchelor81


Open in Devin Review

- 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)
@mbatchelor81 mbatchelor81 self-assigned this Jun 26, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 2 additional findings in Devin Review.

Open in Devin Review

namespace Notification.API.Controllers;

[ApiController]
[Authorize]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +11 to 15
"Jwt": {
"Secret": "QuickApp_Microservices_SuperSecret_Key_For_Dev_Only_Min_32_Chars!",
"Issuer": "quickapp-identity",
"Audience": "quickapp-api"
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

namespace Notification.API.Controllers;

[ApiController]
[Authorize]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant