feat: implement authentication system for API Gateway and FileService #34

Merged

conco merged 3 commits from claude/issue-17-add-authentication into master 2025-11-28 04:26:23 +00:00

Conversation 4 Commits 3 Files Changed 29 +408 -30

conco commented 2025-11-27 14:09:13 +00:00

Owner

Copy Link

• Add JWT Bearer token validation to API Gateway with restricted CORS
• Add cookie-based JWT validation to FileService for browser image requests
• Create shared authentication infrastructure in FictionArchive.Service.Shared
• Update frontend to set fa_session cookie after OIDC login
• Add [Authorize] attributes to GraphQL mutations with role-based restrictions
• Configure OIDC settings for both services in docker-compose

Implements FA-17: Authentication for microservices architecture

- Add JWT Bearer token validation to API Gateway with restricted CORS - Add cookie-based JWT validation to FileService for browser image requests - Create shared authentication infrastructure in FictionArchive.Service.Shared - Update frontend to set fa_session cookie after OIDC login - Add [Authorize] attributes to GraphQL mutations with role-based restrictions - Configure OIDC settings for both services in docker-compose Implements FA-17: Authentication for microservices architecture

conco added 1 commit 2025-11-27 14:09:13 +00:00

feat: implement authentication system for API Gateway and FileService ... 78612ea29d

- Add JWT Bearer token validation to API Gateway with restricted CORS
- Add cookie-based JWT validation to FileService for browser image requests
- Create shared authentication infrastructure in FictionArchive.Service.Shared
- Update frontend to set fa_session cookie after OIDC login
- Add [Authorize] attributes to GraphQL mutations with role-based restrictions
- Configure OIDC settings for both services in docker-compose

Implements FA-17: Authentication for microservices architecture

conco commented 2025-11-27 14:09:31 +00:00

Author

Owner

Copy Link

@claude please review this

@claude please review this

Claude commented 2025-11-27 14:11:06 +00:00

First-time contributor

Copy Link

Claude finished @conco's task —— View job

---

Code Review Progress

I'll review the authentication system implementation. Here's my progress:

• Read repository setup instructions (CLAUDE.md)
• Review shared authentication infrastructure
• Review API Gateway authentication changes
• Review FileService authentication changes
• Review frontend authentication updates
• Review GraphQL mutation authorization
• Review docker-compose OIDC configuration
• Analyze security implications and provide recommendations

Code Review Results

This is a solid authentication implementation with good separation of concerns. Here are my findings:

✅ Strengths

1. Shared Infrastructure Design

• Clean separation with AuthenticationExtensions.cs providing reusable authentication setup
• Proper configuration model in OidcConfiguration.cs with sensible defaults
• Good use of dependency injection patterns

2. Security Best Practices

• JWT validation with proper issuer, audience, lifetime, and signing key validation
• HTTPS requirement detection based on authority URL (FictionArchive.Service.Shared/Extensions/AuthenticationExtensions.cs:25)
• Appropriate clock skew tolerance (5 minutes) for token validation
• Cookie security attributes properly set based on environment (production vs development)

3. Authorization Implementation

• Role-based access control with admin-only operations (SchedulerService/GraphQL/Mutation.cs:14,21,28)
• Granular permissions (admin vs authenticated user) in NovelService (NovelService/GraphQL/Mutation.cs:16,22)
• FileService properly secured with [Authorize] attribute (FileService/Controllers/S3ProxyController.cs:14)

4. Frontend Integration

• Proper cookie management with environment-aware security settings (fictionarchive-web/src/auth/AuthContext.tsx:9-26)
• Cross-domain cookie support for production (.orfl.xyz domain)
• Automatic token refresh and cleanup on logout

⚠️ Areas for Improvement

1. Missing Error Handling

// FictionArchive.Service.Shared/Extensions/AuthenticationExtensions.cs:13
var oidcConfig = configuration.GetSection("OIDC").Get<OidcConfiguration>();
if (oidcConfig == null) {
    throw new InvalidOperationException("OIDC configuration is required but not found in app settings");
}

Recommendation: Add validation for required fields (Authority, ClientId, Audience) and provide more specific error messages.

2. CORS Configuration Concerns

// FictionArchive.API/Program.cs:33-36
policyBuilder.WithOrigins("https://fictionarchive.orfl.xyz", "http://localhost:5173")
    .AllowAnyMethod()
    .AllowAnyHeader()
    .AllowCredentials();

Recommendation: Consider restricting methods and headers to only what's needed. AllowAnyMethod() and AllowAnyHeader() may be overly permissive.

3. Cookie Security Hardening

// fictionarchive-web/src/auth/AuthContext.tsx:15
const cookieValue = `fa_session=${user.access_token}; path=/; ${secure ? 'secure; ' : ''}samesite=${sameSite}${domain ? `; domain=${domain}` : ''}`

Recommendation: Add HttpOnly flag to prevent XSS attacks, though this may require backend coordination for token extraction.

4. Token Validation Configuration
The OIDC configuration lacks some important security settings:

// Missing from appsettings.json
{
  "RequireSignedTokens": true,
  "RequireExpirationTime": true,
  "ValidateTokenReplay": true
}

5. FileService Missing OIDC Config

// FictionArchive.Service.FileService/appsettings.json missing OIDC section

Issue: FileService uses AddOidcCookieAuthentication but has no OIDC configuration in appsettings.json. This will cause runtime exceptions.

🔒 Security Analysis

Good Security Practices:

• JWT Bearer tokens with proper validation
• Role-based authorization preventing unauthorized admin operations
• Secure cookie handling with environment-appropriate flags
• HTTPS enforcement in production

Potential Security Risks:

1. Missing HttpOnly cookies - Access tokens in cookies are vulnerable to XSS
2. Overly permissive CORS - AllowAnyMethod/AllowAnyHeader increases attack surface
3. Missing rate limiting - Authentication endpoints should have rate limiting
4. Token logging risk - Ensure access tokens aren't logged in server logs

📋 Action Items

1. Critical: Add OIDC configuration to FileService appsettings.json
2. High: Implement HttpOnly cookies with backend token extraction
3. Medium: Restrict CORS to specific methods/headers needed
4. Medium: Add comprehensive OIDC configuration validation
5. Low: Consider adding rate limiting to authentication endpoints

🏗️ Architecture Notes

The microservices authentication approach is well-designed:

• API Gateway handles GraphQL authentication via Bearer tokens
• FileService handles browser requests via cookies
• Shared authentication library ensures consistency
• Environment variables properly configured in docker-compose

Overall, this is a solid foundation for the authentication system. The main concerns are around the missing FileService configuration and potential security hardening opportunities.

**Claude finished @conco's task** —— [View job](https://git.orfl.xyz/conco/FictionArchive/actions/runs/71) --- ### Code Review Progress I'll review the authentication system implementation. Here's my progress: - [x] Read repository setup instructions (CLAUDE.md) - [x] Review shared authentication infrastructure - [x] Review API Gateway authentication changes - [x] Review FileService authentication changes - [x] Review frontend authentication updates - [x] Review GraphQL mutation authorization - [x] Review docker-compose OIDC configuration - [x] Analyze security implications and provide recommendations ### Code Review Results This is a solid authentication implementation with good separation of concerns. Here are my findings: ### ✅ **Strengths** **1. Shared Infrastructure Design** - Clean separation with `AuthenticationExtensions.cs` providing reusable authentication setup - Proper configuration model in `OidcConfiguration.cs` with sensible defaults - Good use of dependency injection patterns **2. Security Best Practices** - JWT validation with proper issuer, audience, lifetime, and signing key validation - HTTPS requirement detection based on authority URL (`FictionArchive.Service.Shared/Extensions/AuthenticationExtensions.cs:25`) - Appropriate clock skew tolerance (5 minutes) for token validation - Cookie security attributes properly set based on environment (production vs development) **3. Authorization Implementation** - Role-based access control with admin-only operations (`SchedulerService/GraphQL/Mutation.cs:14,21,28`) - Granular permissions (admin vs authenticated user) in NovelService (`NovelService/GraphQL/Mutation.cs:16,22`) - FileService properly secured with `[Authorize]` attribute (`FileService/Controllers/S3ProxyController.cs:14`) **4. Frontend Integration** - Proper cookie management with environment-aware security settings (`fictionarchive-web/src/auth/AuthContext.tsx:9-26`) - Cross-domain cookie support for production (`.orfl.xyz` domain) - Automatic token refresh and cleanup on logout ### ⚠️ **Areas for Improvement** **1. Missing Error Handling** ```csharp // FictionArchive.Service.Shared/Extensions/AuthenticationExtensions.cs:13 var oidcConfig = configuration.GetSection("OIDC").Get<OidcConfiguration>(); if (oidcConfig == null) { throw new InvalidOperationException("OIDC configuration is required but not found in app settings"); } ``` **Recommendation**: Add validation for required fields (Authority, ClientId, Audience) and provide more specific error messages. **2. CORS Configuration Concerns** ```csharp // FictionArchive.API/Program.cs:33-36 policyBuilder.WithOrigins("https://fictionarchive.orfl.xyz", "http://localhost:5173") .AllowAnyMethod() .AllowAnyHeader() .AllowCredentials(); ``` **Recommendation**: Consider restricting methods and headers to only what's needed. `AllowAnyMethod()` and `AllowAnyHeader()` may be overly permissive. **3. Cookie Security Hardening** ```typescript // fictionarchive-web/src/auth/AuthContext.tsx:15 const cookieValue = `fa_session=${user.access_token}; path=/; ${secure ? 'secure; ' : ''}samesite=${sameSite}${domain ? `; domain=${domain}` : ''}` ``` **Recommendation**: Add `HttpOnly` flag to prevent XSS attacks, though this may require backend coordination for token extraction. **4. Token Validation Configuration** The OIDC configuration lacks some important security settings: ```json // Missing from appsettings.json { "RequireSignedTokens": true, "RequireExpirationTime": true, "ValidateTokenReplay": true } ``` **5. FileService Missing OIDC Config** ```json // FictionArchive.Service.FileService/appsettings.json missing OIDC section ``` **Issue**: FileService uses `AddOidcCookieAuthentication` but has no OIDC configuration in appsettings.json. This will cause runtime exceptions. ### 🔒 **Security Analysis** **Good Security Practices:** - JWT Bearer tokens with proper validation - Role-based authorization preventing unauthorized admin operations - Secure cookie handling with environment-appropriate flags - HTTPS enforcement in production **Potential Security Risks:** 1. **Missing HttpOnly cookies** - Access tokens in cookies are vulnerable to XSS 2. **Overly permissive CORS** - AllowAnyMethod/AllowAnyHeader increases attack surface 3. **Missing rate limiting** - Authentication endpoints should have rate limiting 4. **Token logging risk** - Ensure access tokens aren't logged in server logs ### 📋 **Action Items** 1. **Critical**: Add OIDC configuration to FileService appsettings.json 2. **High**: Implement HttpOnly cookies with backend token extraction 3. **Medium**: Restrict CORS to specific methods/headers needed 4. **Medium**: Add comprehensive OIDC configuration validation 5. **Low**: Consider adding rate limiting to authentication endpoints ### 🏗️ **Architecture Notes** The microservices authentication approach is well-designed: - API Gateway handles GraphQL authentication via Bearer tokens - FileService handles browser requests via cookies - Shared authentication library ensures consistency - Environment variables properly configured in docker-compose Overall, this is a solid foundation for the authentication system. The main concerns are around the missing FileService configuration and potential security hardening opportunities.

conco commented 2025-11-27 16:17:50 +00:00

Author

Owner

Copy Link

@claude Please make the following adjustments to the branch code:

Address points 1, 4, and 5
The build currently fails due to the Authorize attribute on GraphQL endpoints requiring string[] instead of string for roles
The admin role should be removed from the ImportNovel endpoint

@claude Please make the following adjustments to the branch code: Address points 1, 4, and 5 The build currently fails due to the Authorize attribute on GraphQL endpoints requiring string[] instead of string for roles The admin role should be removed from the ImportNovel endpoint

Claude commented 2025-11-27 16:18:09 +00:00

First-time contributor

Copy Link

Claude finished @conco's task —— View job

---

Authentication System Issues - Complete ✅

Successfully addressed all requested issues and pushed the fixes to the branch.

Todo List

• Check out the existing branch and read setup instructions
• Review current authentication code and identify issues
• Fix GraphQL authorization attributes (string[] instead of string for roles)
• Remove admin role from ImportNovel endpoint
• Add OIDC configuration validation (Point 1: Better error handling)
• Add comprehensive OIDC configuration validation (Point 4)
• Verify FileService OIDC configuration (Point 5) - Already complete
• Commit and push changes

Changes Made:

1. Fixed GraphQL Authorization Syntax

• SchedulerService/GraphQL/Mutation.cs: Changed [Authorize(Roles = "admin")] to [Authorize(Roles = new[] { "admin" })] for all three endpoints
• NovelService/GraphQL/Mutation.cs: Removed admin role from ImportNovel endpoint (now just [Authorize])

2. Enhanced OIDC Configuration Validation

• Added comprehensive validation in AuthenticationExtensions.cs
• Validates required fields: Authority, ClientId, Audience
• Ensures Authority is a valid URI
• Enforces HTTPS requirement (except localhost for development)
• Provides specific, actionable error messages

3. Validation Features Added:

private static void ValidateOidcConfiguration(OidcConfiguration config)
{
    // Validates Authority, ClientId, Audience are not null/empty
    // Validates Authority is valid URI with HTTPS (except localhost)
    // Throws descriptive errors listing all issues found
}

The build failures should now be resolved. The authorization attributes use the correct syntax, and the OIDC configuration will provide clear error messages if misconfigured during startup.

---

Branch: claude/issue-17-add-authentication

**Claude finished @conco's task** —— [View job](https://git.orfl.xyz/conco/FictionArchive/actions/runs/73) --- ### Authentication System Issues - Complete ✅ Successfully addressed all requested issues and pushed the fixes to the branch. ### Todo List - [x] Check out the existing branch and read setup instructions - [x] Review current authentication code and identify issues - [x] Fix GraphQL authorization attributes (string[] instead of string for roles) - [x] Remove admin role from ImportNovel endpoint - [x] Add OIDC configuration validation (Point 1: Better error handling) - [x] Add comprehensive OIDC configuration validation (Point 4) - [x] Verify FileService OIDC configuration (Point 5) - Already complete - [x] Commit and push changes ### Changes Made: **1. Fixed GraphQL Authorization Syntax** - `SchedulerService/GraphQL/Mutation.cs`: Changed `[Authorize(Roles = "admin")]` to `[Authorize(Roles = new[] { "admin" })]` for all three endpoints - `NovelService/GraphQL/Mutation.cs`: Removed admin role from ImportNovel endpoint (now just `[Authorize]`) **2. Enhanced OIDC Configuration Validation** - Added comprehensive validation in `AuthenticationExtensions.cs` - Validates required fields: Authority, ClientId, Audience - Ensures Authority is a valid URI - Enforces HTTPS requirement (except localhost for development) - Provides specific, actionable error messages **3. Validation Features Added:** ```csharp private static void ValidateOidcConfiguration(OidcConfiguration config) { // Validates Authority, ClientId, Audience are not null/empty // Validates Authority is valid URI with HTTPS (except localhost) // Throws descriptive errors listing all issues found } ``` The build failures should now be resolved. The authorization attributes use the correct syntax, and the OIDC configuration will provide clear error messages if misconfigured during startup. --- **Branch**: [claude/issue-17-add-authentication](https://git.orfl.xyz/conco/FictionArchive/src/branch/claude/issue-17-add-authentication)

gitea-actionsbot added 1 commit 2025-11-27 16:20:11 +00:00

fix: address authentication system issues ... 9c82d648cd

- Fix GraphQL authorization attributes to use string[] instead of string for roles
- Remove admin role requirement from ImportNovel endpoint
- Add comprehensive OIDC configuration validation with specific error messages
- Validate Authority, ClientId, and Audience are properly configured
- Ensure HTTPS requirement except for localhost development

Co-authored-by: conco <conco@users.noreply.local>

conco added 1 commit 2025-11-28 04:23:06 +00:00

[FA-17] Update auth 75e96cbee5

conco merged commit bc83bffb4b into master 2025-11-28 04:26:23 +00:00

conco referenced this issue from a commit 2025-11-28 04:26:25 +00:00

Merge pull request 'feat: implement authentication system for API Gateway and FileService' (#34) from claude/issue-17-add-authentication into master

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

source

New novel source

wontfix

This won't be fixed

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

conco

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: conco/FictionArchive#34