Skip to content

Commit

Permalink
fix: public routes cannot get guests users (#22)
Browse files Browse the repository at this point in the history
  • Loading branch information
BaaltRodrigo authored Oct 31, 2023
1 parent c779783 commit b28fe4d
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 18 deletions.
4 changes: 3 additions & 1 deletion app/Exceptions/Handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ public function register(): void
* Firebase error handling
*/
$this->renderable(function (FailedToVerifyToken $e, Request $request) {
return response()->json(['status' => 'error','message' => 'Invalid token'], Response::HTTP_UNAUTHORIZED);
return response()->json(
['status' => 'error', 'message' => 'Invalid or malformed token'],
Response::HTTP_UNAUTHORIZED);
});
}
}
21 changes: 15 additions & 6 deletions app/Http/Middleware/FirebaseAuth.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,32 @@ public function __construct()

/**
* Handle an incoming request.
* This middleware has
*
* @param \Illuminate\Http\Request $request
* @param string $mode strict|optional If the auth methods is optional, the user is not required to be authenticated
* @param \Closure(\Illuminate\Http\Request): (\Symfony\Component\HttpFoundation\Response) $next
*/
public function handle(Request $request, Closure $next): Response
public function handle(Request $request, Closure $next, ?string $mode = 'strict'): Response
{
// Check for the token in the header
$token = $request->bearerToken();

// If the auth is optional, the user is not required to be authenticated
$is_optional = $mode === 'optional';

if (!$token) {
return response()->json(['status' => 'error','message' => 'Token not provided'], Response::HTTP_UNAUTHORIZED);
return $is_optional
? $next($request)
: response()->json(['status' => 'error','message' => 'Token not provided'], Response::HTTP_UNAUTHORIZED);
}

// Verify the token

// From here, the token is present and we can proceed to verify it
// Verify the token as the route is without optional route
$verifiedIdToken = $this->auth->verifyIdToken(
$token,
$checkIfRevoked = true,
$leewayInSeconds = 3600 // Handle 1 hour of delay from the client
false, // Check if the token is revoked
3600 // Handle 1 hour of delay from the client
);

// Set the user in the request and proceed
Expand Down
15 changes: 11 additions & 4 deletions app/Policies/CalendarPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,19 @@ public function __construct()
//
}

public function view(User $user, Calendar $calendar)
public function view(?User $user, Calendar $calendar)
{
// Check if user owns calendar
$is_owner = $user->id === $calendar->user_id;
// Early exit to public calendar with or without user
if ($calendar->is_public) {
return Response::allow();
}

// If user is null, then it is a guest
if ($user === null) {
return Response::deny('You are not allowed to view this calendar');
}

return $is_owner || $calendar->is_public
return $user->id === $calendar->user_id
? Response::allow()
: Response::deny('You are not allowed to view this calendar');
}
Expand Down
4 changes: 3 additions & 1 deletion routes/api.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@
// Show for calendars needs to be outside middleware because there
// are public calendars that don't need to be authenticated
Route::prefix('/calendars/{calendar}')->group(function () {
Route::get('/', [CalendarController::class, 'show'])->name('calendars.show');
Route::get('/', [CalendarController::class, 'show'])
->middleware('auth.firebase:optional')
->name('calendars.show');
Route::get('/sections', [CalendarController::class, 'calendarSections'])->name('calendars.sections');
Route::match(['put', 'patch'], '/sections', [CalendarController::class, 'updateSections'])
->middleware('auth.firebase')
Expand Down
88 changes: 88 additions & 0 deletions tests/Feature/CalendarPrivacyTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
<?php

namespace Tests\Feature;

use App\Models\Calendar;
use App\Models\User;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Foundation\Testing\WithFaker;
use Tests\TestCase;
use Tests\Traits\UseFirebaseUser;

class CalendarPrivacyTest extends TestCase
{
use RefreshDatabase, UseFirebaseUser;

public function test_it_allows_guests_to_view_public_calendars(): void
{
$user = $this->createUser();
$calendar = Calendar::factory()->create([
'user_id' => $user->id,
'is_public' => true
]);
// Does not use any user to fetch the calendar

$response = $this->getJson(route('calendars.show', $calendar->uuid));

$response->assertOk();
}

public function test_it_allows_other_users_to_view_public_calendars(): void
{
$user1 = User::create(['id' => 'unused-user']);
$calendar = Calendar::factory()->create([
'user_id' => $user1->id,
'is_public' => true
]);
$firebaseUser = $this->createUser();
$this->actingAsFirebaseUser($firebaseUser);

$response = $this->getJson(route('calendars.show', $calendar->uuid));

$response->assertOk();
}

public function test_it_allows_owners_to_view_own_calendars(): void
{
$user = $this->createUser();
$calendar = Calendar::factory()->create([
'user_id' => $user->id,
'is_public' => false
]);
$this->actingAsFirebaseUser($user);

$response = $this->getJson(route('calendars.show', $calendar->uuid));

$response->assertOk();
}

public function test_it_denies_guests_to_see_private_calendars(): void
{
$user = $this->createUser();
$calendar = Calendar::factory()->create([
'user_id' => $user->id,
'is_public' => false
]);
// Does not use any user to fetch the calendar

$response = $this->getJson(route('calendars.show', $calendar->uuid));

$response->assertForbidden();
}

public function test_it_denies_other_users_to_see_private_calendars(): void
{
$user = User::create(['id' => 'owner-user']);
$calendar = Calendar::factory()->create([
'user_id' => $user->id,
'is_public' => false
]);
$firebaseUser = $this->createUser();
$this->actingAsFirebaseUser($firebaseUser);

$response = $this->getJson(route('calendars.show', $calendar->uuid));

$response->assertForbidden();
}

}
12 changes: 6 additions & 6 deletions tests/Feature/CalendarTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -183,16 +183,16 @@ public function test_it_allows_owners_to_delete_calendar_section(): void
]);
}

public function test_it_denies_access_to_private_calendars(): void
public function test_it_denies_when_guests_create_calendars(): void
{
$calendar = Calendar::factory()->create([
'user_id' => null,
'is_public' => false
$calendar = Calendar::factory()->make([
'user_id' => null
]);

$response = $this->getJson(route('calendars.show', $calendar->uuid));
$response = $this->postJson(route('calendars.store'), $calendar->toArray());

$response->assertStatus(Response::HTTP_FORBIDDEN);
$response->assertStatus(Response::HTTP_UNAUTHORIZED);
$this->assertDatabaseMissing('calendars', ['uuid' => $calendar->uuid]);
}

public function test_it_denies_delete_non_owned_calendars(): void
Expand Down

0 comments on commit b28fe4d

Please sign in to comment.