swisnl/json-api-server

Authenticate tests not working

Closed this issue · 7 comments

Tests use WithoutMiddleware trait

use DatabaseTransactions, WithoutMiddleware;

This effectively cancels all middleware, including auth:api, thus all *_unauthenticated() tests fail, because no authentication is required and operations complete same way they would for authenticated user.

I see that middleware is disabled, because tests do not make requests in application/vnd.api+json format. This should not be so.

Nevertheless I experimented with AuthenticationTest.php:

  • removing WithoutMiddleware - all tests fail because requests are not in application/vnd.api+json format
  • removing WithoutMiddleware and adding
        $this->withoutMiddleware([
             \Swis\JsonApi\Server\Http\Middleware\InspectContentType::class,
        ]);

so auth:api is not disabled.
Unauthenticated tests still fail, because instead of HTTP 401, they receive HTTP 302 (a redirection to login page)

Hmm, interesting, that is a bit of an oversight i recon. I would need to check how we test in our implementations, perhaps we made some changes there which were not migrated to the package. @Arnaudvz perhaps you could check this?

Some of simple tests

$response = $this->get($this->baseUrl);`

can be rewritten like

$response = $this->withHeaders(['Accept'=>'application/vnd.api+json'])->get($this->baseUrl);`

Or maybe set the withHeaders() for all tests, if possible and suits all of them.

As for 401 and 302 responses, I should double check, maybe Laravel by default does not do it and I have made it do so, somewhere. :)

Answer to 302 instead of 401 response also lies with #12
Laravel Exception handler responds with 302 by default and 401 only if InteractsWithContentTypes::expectsJson() is true, which is true for ajax calls or calls with header "Accept" which contains either "/json" or "+json".
Never worked with "Content-Type"

The middleware is tested, but i agree it it nicer if the headers are checked.

Interesting find on the Accept header there, it seems the specification doesnt really force sending the Accept header though...

Merged the change which adds the withHeaders calls. And released 0.3.2

Awesome, thank You!