Skip to content

Conversation

@Kyemate
Copy link
Contributor

@Kyemate Kyemate commented Oct 21, 2025

closes #11469

Summary by CodeRabbit

  • Refactor
    • Reorganized request pipeline architecture by migrating request validation components from filters to middleware pattern across API and Web servers.

@coderabbitai
Copy link

coderabbitai bot commented Oct 21, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The PR migrates app version validation from an action filter to HTTP middleware, moving it before authentication to prevent unauthorized errors on outdated clients. Related filters are reorganized into a RequestPipeline namespace, and the filter is removed entirely.

Changes

Cohort / File(s) Summary
Project Configuration
Boilerplate.Server.Api.csproj
Updated public using declarations from Boilerplate.Server.Api.Filters to Boilerplate.Server.Api.RequestPipeline
Removed Filter
Filters/ForceUpdateActionFilter.cs
Deleted action filter that validated app version, read X-App-Version and X-App-Platform headers, and threw ClientNotSupportedException for outdated clients
New Middleware
RequestPipeline/ForceUpdateMiddleware.cs
Added middleware class with InvokeAsync method performing identical app version validation, executed before authentication in the request pipeline
Namespace Reorganization
RequestPipeline/HangfireDashboardAuthorizationFilter.cs, RequestPipeline/ODataOperationFilter.cs
Moved classes from Boilerplate.Server.Api.Filters to Boilerplate.Server.Api.RequestPipeline namespace
Service Configuration
Program.Services.cs
Removed conditional ForceUpdateActionFilter registration from AddControllers options delegate; changed to plain AddControllers() call
API Middleware Pipeline
Program.Middlewares.cs
Added conditional registration of ForceUpdateMiddleware after UseCors, gated by settings.SupportedAppVersions being non-null
Web Middleware Pipeline
src/Server/Boilerplate.Server.Web/Program.Middlewares.cs
Added ServerApiSettings binding and conditional registration of ForceUpdateMiddleware; updated import from Filters to RequestPipeline

Sequence Diagram

sequenceDiagram
    participant Client
    participant Server

    rect rgb(200, 220, 255)
    note over Client, Server: OLD: Action Filter (runs after auth)
    Client->>Server: Request (X-App-Version, X-App-Platform)
    Server->>Server: CORS Middleware
    Server->>Server: Authentication Middleware
    Server->>Server: ForceUpdateActionFilter ❌<br/>(version check)
    alt Version outdated
        Server-->>Client: 401 Unauthorized<br/>(auth failure masks version issue)
    else Version OK
        Server->>Server: Action execution
        Server-->>Client: 200 OK
    end
    end

    rect rgb(220, 255, 220)
    note over Client, Server: NEW: Middleware (runs before auth)
    Client->>Server: Request (X-App-Version, X-App-Platform)
    Server->>Server: CORS Middleware
    Server->>Server: ForceUpdateMiddleware ✓<br/>(version check)
    alt Version outdated
        Server-->>Client: ClientNotSupportedException<br/>(clear error, before auth)
    else Version OK
        Server->>Server: Authentication Middleware
        Server->>Server: Action execution
        Server-->>Client: 200 OK
    end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

The changes involve understanding both filter and middleware execution order, verifying the logic migration is correct, and ensuring proper configuration in two Program.cs files. While heterogeneous (multiple file types affected), the changes follow a consistent refactoring pattern with clear intent to fix an authorization issue.

Poem

🐰 Hoppy migration, middleware's the way!
Filters caught auth'd requests, gone astray,
Now version checks hop first in the line,
Before auth middleware—the fix so divine!
Outdated clients get caught quick and clean,
No more "unauthorized" masquerading the scene.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Replaced the ForceUpdateActionFilter with ForceUpdateMiddleware" directly and accurately summarizes the main architectural change in the changeset. The changes clearly show the removal of the action filter class, addition of a new middleware class, and updates to both Program.Services.cs and Program.Middlewares.cs to reflect this shift. The title is concise, specific, and clearly communicates the primary change to someone scanning the commit history.
Linked Issues Check ✅ Passed The code changes directly address the core issue #11469, which reports that version checking via ForceUpdateActionFilter occurs after authentication middleware, causing unauthorized errors when auth fails. By converting from an action filter to middleware registered before authentication, the version check now executes in the request pipeline before authentication/authorization runs. The ForceUpdateMiddleware performs the same version validation logic as the original filter but is registered earlier in the pipeline (after UseCors, before auth), eliminating the authorization error when the app version is below minimum and refresh token calls fail.
Out of Scope Changes Check ✅ Passed The core changes directly address the linked issue by moving version checking from action filter to middleware. However, the PR also includes namespace reorganization changes where HangfireDashboardAuthorizationFilter and ODataOperationFilter are moved from the Filters namespace to RequestPipeline, along with updates to the .csproj global using declarations. While these changes appear to be part of a cohesive architectural shift from a "Filters" pattern to "RequestPipeline" terminology, the movement of these other authorization and OData filters is not explicitly required by issue #11469 and could be considered tangential refactoring beyond the stated objective of replacing ForceUpdateActionFilter with ForceUpdateMiddleware.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between f568119 and 89ee34e.

📒 Files selected for processing (8)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Filters/ForceUpdateActionFilter.cs (0 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Middlewares.cs (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/RequestPipeline/ForceUpdateMiddleware.cs (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/RequestPipeline/HangfireDashboardAuthorizationFilter.cs (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/RequestPipeline/ODataOperationFilter.cs (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs (3 hunks)
💤 Files with no reviewable changes (1)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Filters/ForceUpdateActionFilter.cs
🔇 Additional comments (7)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/RequestPipeline/ODataOperationFilter.cs (1)

5-5: LGTM! Namespace refactoring aligns with the broader reorganization.

The namespace change from Boilerplate.Server.Api.Filters to Boilerplate.Server.Api.RequestPipeline appropriately groups request pipeline components together.

src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Services.cs (1)

190-190: LGTM! Correctly removes filter registration in favor of middleware approach.

The simplification from AddControllers(options => { ... }) to AddControllers() is appropriate since version validation has been moved to middleware, which executes earlier in the request pipeline and solves the unauthorized error issue described in the PR objectives.

src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Middlewares.cs (1)

46-49: LGTM! Correct middleware placement solves the unauthorized error issue.

The ForceUpdateMiddleware is properly positioned after CORS (line 44) but before authentication (line 50). This ensures that version validation happens before authentication, preventing the unauthorized errors that occurred when the old ForceUpdateActionFilter executed after authentication middleware.

src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/RequestPipeline/HangfireDashboardAuthorizationFilter.cs (1)

4-4: LGTM! Namespace refactoring aligns with the broader reorganization.

The namespace change appropriately groups request pipeline components together.

src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Boilerplate.Server.Api.csproj (1)

78-78: LGTM! Global using directive updated to match namespace refactoring.

The change ensures types from the new Boilerplate.Server.Api.RequestPipeline namespace are available throughout the project.

src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs (2)

14-14: LGTM! Import updated to match namespace refactoring.

The using directive correctly references the new Boilerplate.Server.Api.RequestPipeline namespace.


32-33: LGTM! Correct settings binding and middleware placement.

The ServerApiSettings binding (lines 32-33) enables the conditional registration of ForceUpdateMiddleware (lines 106-109), which is properly positioned after CORS but before authentication. This mirrors the Server.Api implementation and ensures consistent version validation across both server projects.

Also applies to: 106-109

@ysmoradi ysmoradi merged commit 687fd62 into bitfoundation:develop Oct 29, 2025
1 check passed
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.

App update middleware causing unauthorized errors

2 participants