Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

statamic.api.middleware does not allow performing custom authentication #10010

Open
rburgst opened this issue May 7, 2024 · 3 comments
Open
Labels

Comments

@rburgst
Copy link

rburgst commented May 7, 2024

Bug description

I created a middleware group secure-api that contains a middleware that checks for an api key in the request header.
This works great until the response is cached. Once the response is cached, then the cached response will always be returned regardless of whether the request still has the api key header.
The reason for this is that the order of the middleware application is the wrong way round.

see
statamic.api.middleware

    Route::middleware([
        RequireStatamicPro::class,
        Cache::class,
    ])->group(function () {
        Route::middleware(config('statamic.api.middleware'))
            ->name('statamic.api.')
            ->prefix(config('statamic.api.route'))
            ->group(__DIR__.'/api.php');
    });

by first allowing the Cache to respond the cached response, my custom middlewares which are in the secure-api group never get the chance to do anything.

How to reproduce

  1. create a middleware that checks an arbitrary http header
  2. in Http/Kernel.php create a new middleware group
        protected $middlewareGroups = [
         'web' => [
             \App\Http\Middleware\EncryptCookies::class,
             \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
             \Illuminate\Session\Middleware\StartSession::class,
             \Illuminate\View\Middleware\ShareErrorsFromSession::class,
             \App\Http\Middleware\VerifyCsrfToken::class,
             \Illuminate\Routing\Middleware\SubstituteBindings::class,
         ],
    
         'api' => [
             // \Laravel\Sanctum\Http\Middleware\EnsureFrontendRequestsAreStateful::class,
             \Illuminate\Routing\Middleware\ThrottleRequests::class.':api',
             \Illuminate\Routing\Middleware\SubstituteBindings::class,
         ],
         'secure-api' => [
             CheckApiKeyHeader::class
         ]
     ];
  3. in .env configure statamic REST api to use the given middleware group
    STATAMIC_API_MIDDLEWARE=secure-api
    STATAMIC_API_ENABLED=true
    
  4. send a rest request to any api endpoint (without the header so that the middleware will reject) => request will be rejected
  5. send an authenticated request to the same endpoint => request will be answered
  6. send an unauthenticated request to the same endpoint => ❗ request will be answered (with the previously cached response)

Logs

No response

Environment

php please support:details

Environment
Application Name: Laravel
Laravel Version: 10.48.9
PHP Version: 8.3.6
Composer Version: 2.7.4
Environment: local
Debug Mode: ENABLED
URL: localhost:9999
Maintenance Mode: OFF

Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: CACHED

Drivers
Broadcasting: pusher
Cache: statamic
Database: mysql
Logs: stack / single
Mail: sendmail
Queue: database
Session: cookie

Statamic
Addons: 19
Antlers: regex
Sites: 1
Stache Watcher: Disabled
Static Caching: Disabled
Version: 4.57.2 PRO

Statamic Addons
aryehraber/statamic-impersonator: 2.6.0
statamic/collaboration: 0.8.1
statamic/eloquent-driver: 3.4.0

Statamic Eloquent Driver
Asset Containers: eloquent
Assets: eloquent
Blueprints: file
Collection Trees: eloquent
Collections: eloquent
Entries: eloquent
Forms: eloquent
Global Sets: eloquent
Global Variables: eloquent
Navigation Trees: eloquent
Navigations: eloquent
Revisions: eloquent
Taxonomies: eloquent
Terms: eloquent

Installation

Fresh statamic/statamic site via CLI

Antlers Parser

None

Additional details

No response

@mxaGianluca
Copy link

@rburgst you can use the middleware priority to control the order of execution. Adding this:

protected $middlewarePriority = [
    CheckApiKeyHeader::class,
    \Statamic\API\Middleware\Cache::class,
];

Should fix your issue.
I do agree that the order of execution can be improved though.

Hope this helps.

@rburgst
Copy link
Author

rburgst commented May 12, 2024

@mxaGianluca That does indeed work. Thanks.

@mxaGianluca
Copy link

@mxaGianluca That does indeed work. Thanks.

You're welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants