Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using Notification.API.Services;
using Notification.Domain.Interfaces;
Expand All @@ -6,6 +7,7 @@
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.

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.

[Route("api/[controller]")]
public class NotificationController : ControllerBase
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
<ItemGroup>
<ProjectReference Include="..\Notification.Domain\Notification.Domain.csproj" />
<ProjectReference Include="..\Notification.Infrastructure\Notification.Infrastructure.csproj" />
<ProjectReference Include="..\..\Shared\Shared.Contracts\Shared.Contracts.csproj" />
<ProjectReference Include="..\..\Shared\Shared.Infrastructure\Shared.Infrastructure.csproj" />
<ProjectReference Include="..\..\..\Shared\Shared.Contracts\Shared.Contracts.csproj" />
<ProjectReference Include="..\..\..\Shared\Shared.Infrastructure\Shared.Infrastructure.csproj" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="10.*" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="10.*" />
<PackageReference Include="Swashbuckle.AspNetCore" Version="7.*" />
</ItemGroup>
Expand Down
25 changes: 25 additions & 0 deletions src/Services/Notification/Notification.API/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
using Notification.Infrastructure.Data;
using Notification.Infrastructure.Repositories;
using Microsoft.EntityFrameworkCore;
using Microsoft.AspNetCore.Authentication.JwtBearer;
using Microsoft.IdentityModel.Tokens;
using System.Text;

var builder = WebApplication.CreateBuilder(args);

Expand All @@ -11,6 +14,25 @@
builder.Services.AddSwaggerGen();
builder.Services.AddHealthChecks();

var jwtSecret = builder.Configuration["Jwt:Secret"]
?? throw new InvalidOperationException("Jwt:Secret is not configured.");

builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
.AddJwtBearer(options =>
{
options.TokenValidationParameters = new TokenValidationParameters
{
ValidateIssuer = true,
ValidateAudience = true,
ValidateLifetime = true,
ValidateIssuerSigningKey = true,
ValidIssuer = builder.Configuration["Jwt:Issuer"],
ValidAudience = builder.Configuration["Jwt:Audience"],
IssuerSigningKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(jwtSecret))
};
});
builder.Services.AddAuthorization();

builder.Services.AddDbContext<NotificationDbContext>(options =>
options.UseNpgsql(builder.Configuration.GetConnectionString("DefaultConnection")));

Expand All @@ -32,6 +54,9 @@
app.UseSwaggerUI();
}

app.UseAuthentication();
app.UseAuthorization();

app.MapControllers();
app.MapHealthChecks("/healthz");

Expand Down
5 changes: 5 additions & 0 deletions src/Services/Notification/Notification.API/appsettings.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,10 @@
},
"ConnectionStrings": {
"DefaultConnection": "Host=localhost;Database=notificationdb;Username=postgres;Password=postgres"
},
"Jwt": {
"Secret": "QuickApp_Microservices_SuperSecret_Key_For_Dev_Only_Min_32_Chars!",
"Issuer": "quickapp-identity",
"Audience": "quickapp-api"
}
Comment on lines +11 to 15

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.

}
139 changes: 139 additions & 0 deletions tests/notification-e2e-smoke.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
#!/usr/bin/env bash
set -euo pipefail

# ─── Configuration ───────────────────────────────────────────────────────────
DIRECT_URL="${DIRECT_URL:-http://localhost:5005}"
GATEWAY_URL="${GATEWAY_URL:-http://localhost:5000}"
FAILURES=0

# ─── Colors ──────────────────────────────────────────────────────────────────
GREEN='\033[0;32m'
RED='\033[0;31m'
NC='\033[0m' # No Color

pass() { echo -e "${GREEN}[PASS]${NC} $1"; }
fail() { echo -e "${RED}[FAIL]${NC} $1"; FAILURES=$((FAILURES + 1)); }

# ─── Generate JWT ────────────────────────────────────────────────────────────
JWT_SECRET='QuickApp_Microservices_SuperSecret_Key_For_Dev_Only_Min_32_Chars!'

jwt_header=$(echo -n '{"alg":"HS256","typ":"JWT"}' | base64 -w0 | tr '+/' '-_' | tr -d '=')
jwt_payload=$(echo -n '{"iss":"quickapp-identity","aud":"quickapp-api","sub":"test-user","exp":'$(date -d "+1 hour" +%s)'}' | base64 -w0 | tr '+/' '-_' | tr -d '=')
jwt_signature=$(echo -n "${jwt_header}.${jwt_payload}" | openssl dgst -sha256 -hmac "${JWT_SECRET}" -binary | base64 -w0 | tr '+/' '-_' | tr -d '=')
TOKEN="${jwt_header}.${jwt_payload}.${jwt_signature}"

echo "======================================"
echo " Notification Service E2E Smoke Tests"
echo "======================================"
echo ""
echo "Direct URL: ${DIRECT_URL}"
echo "Gateway URL: ${GATEWAY_URL}"
echo ""

# ─── Test 1: Health check ────────────────────────────────────────────────────
echo "--- Test 1: GET /healthz (expect 200) ---"
HTTP_CODE=$(curl -s -o /tmp/response_body -w '%{http_code}' "${DIRECT_URL}/healthz")
if [ "$HTTP_CODE" = "200" ]; then
pass "GET /healthz returned ${HTTP_CODE}"
else
fail "GET /healthz returned ${HTTP_CODE} (expected 200)"
fi

# ─── Test 2: Unauthenticated GET /api/notification ───────────────────────────
echo "--- Test 2: GET /api/notification without auth (expect 401) ---"
HTTP_CODE=$(curl -s -o /tmp/response_body -w '%{http_code}' "${DIRECT_URL}/api/notification")
if [ "$HTTP_CODE" = "401" ]; then
pass "GET /api/notification without auth returned ${HTTP_CODE}"
else
fail "GET /api/notification without auth returned ${HTTP_CODE} (expected 401)"
fi

# ─── Test 3: POST /api/notification/events/order-placed ──────────────────────
echo "--- Test 3: POST /api/notification/events/order-placed with JWT (expect 201) ---"
HTTP_CODE=$(curl -s -o /tmp/response_body -w '%{http_code}' \
-X POST \
-H "Authorization: Bearer ${TOKEN}" \
-H "Content-Type: application/json" \
-d '{
"orderId": "11111111-1111-1111-1111-111111111111",
"customerId": "22222222-2222-2222-2222-222222222222",
"totalAmount": 9999,
"placedAt": "2025-01-15T10:30:00Z"
}' \
"${DIRECT_URL}/api/notification/events/order-placed")

if [ "$HTTP_CODE" = "201" ]; then
pass "POST /api/notification/events/order-placed returned ${HTTP_CODE}"
else
fail "POST /api/notification/events/order-placed returned ${HTTP_CODE} (expected 201)"
fi

# Extract the notification ID from the response
NOTIFICATION_ID=$(jq -r '.id' /tmp/response_body 2>/dev/null || echo "")
if [ -z "$NOTIFICATION_ID" ] || [ "$NOTIFICATION_ID" = "null" ]; then
fail "Could not extract notification ID from POST response"
echo " Response body: $(cat /tmp/response_body)"
fi

# ─── Test 4: GET /api/notification with JWT ──────────────────────────────────
echo "--- Test 4: GET /api/notification with JWT (expect 200, contains notification) ---"
HTTP_CODE=$(curl -s -o /tmp/response_body -w '%{http_code}' \
-H "Authorization: Bearer ${TOKEN}" \
"${DIRECT_URL}/api/notification")

if [ "$HTTP_CODE" = "200" ]; then
# Check that the response contains our notification ID
if echo "$(cat /tmp/response_body)" | jq -e ".[] | select(.id == \"${NOTIFICATION_ID}\")" > /dev/null 2>&1; then
pass "GET /api/notification returned ${HTTP_CODE} and contains created notification"
elif [ -z "$NOTIFICATION_ID" ] || [ "$NOTIFICATION_ID" = "null" ]; then
pass "GET /api/notification returned ${HTTP_CODE} (skipped body check — no ID from POST)"
else
fail "GET /api/notification returned ${HTTP_CODE} but response does not contain notification ${NOTIFICATION_ID}"
echo " Response body: $(cat /tmp/response_body | head -c 500)"
fi
else
fail "GET /api/notification returned ${HTTP_CODE} (expected 200)"
fi

# ─── Test 5: GET /api/notification/{id}/preview ──────────────────────────────
echo "--- Test 5: GET /api/notification/{id}/preview with JWT (expect 200, HTML) ---"
if [ -n "$NOTIFICATION_ID" ] && [ "$NOTIFICATION_ID" != "null" ]; then
HTTP_CODE=$(curl -s -o /tmp/response_body -w '%{http_code}' \
-H "Authorization: Bearer ${TOKEN}" \
"${DIRECT_URL}/api/notification/${NOTIFICATION_ID}/preview")

if [ "$HTTP_CODE" = "200" ]; then
if grep -qi "<html\|<div\|<body\|<head\|<p\|<table" /tmp/response_body; then
pass "GET /api/notification/${NOTIFICATION_ID}/preview returned ${HTTP_CODE} with HTML content"
else
fail "GET /api/notification/${NOTIFICATION_ID}/preview returned ${HTTP_CODE} but no HTML detected"
echo " Response body: $(cat /tmp/response_body | head -c 500)"
fi
else
fail "GET /api/notification/${NOTIFICATION_ID}/preview returned ${HTTP_CODE} (expected 200)"
fi
else
fail "Skipping preview test — no notification ID available"
fi

# ─── Test 6: Gateway route verification ──────────────────────────────────────
echo "--- Test 6: GET gateway /api/notifications/api/notification without token (expect 401) ---"
HTTP_CODE=$(curl -s -o /tmp/response_body -w '%{http_code}' \
"${GATEWAY_URL}/api/notifications/api/notification")

if [ "$HTTP_CODE" = "401" ]; then
pass "Gateway route returned ${HTTP_CODE} (routes to service, auth enforced)"
else
fail "Gateway route returned ${HTTP_CODE} (expected 401)"
fi

# ─── Summary ─────────────────────────────────────────────────────────────────
echo ""
echo "======================================"
if [ "$FAILURES" -eq 0 ]; then
echo -e "${GREEN}All tests passed!${NC}"
exit 0
else
echo -e "${RED}${FAILURES} test(s) failed.${NC}"
exit 1
fi