feat: use a composition token (#1592)

app/Http/Controllers/API/AuthController.php

@@ -10,6 +10,7 @@ use App\Services\TokenManager;
 use Illuminate\Contracts\Auth\Authenticatable;
 use Illuminate\Foundation\Auth\ThrottlesLogins;
 use Illuminate\Hashing\HashManager;
+use Illuminate\Http\Request;
 use Illuminate\Http\Response;
 
 class AuthController extends Controller
@@ -34,14 +35,19 @@ class AuthController extends Controller
             abort(Response::HTTP_UNAUTHORIZED, 'Invalid credentials');
         }
 
+        $token = $this->tokenManager->createCompositionToken($user);
+
         return response()->json([
-            'token' => $this->tokenManager->createToken($user)->plainTextToken,
+            'token' => $token->apiToken,
+            'audio-token' => $token->audioToken,
         ]);
     }
 
-    public function logout()
+    public function logout(Request $request)
     {
-        $this->user?->currentAccessToken()->delete(); // @phpstan-ignore-line
+        if ($this->user) {
+            attempt(fn () => $this->tokenManager->deleteCompositionToken($request->bearerToken()));
+        }
 
         return response()->noContent();
     }

app/Http/Controllers/API/ProfileController.php

@@ -48,7 +48,10 @@ class ProfileController extends Controller
         $response = UserResource::make($this->user)->response();
 
         if ($request->new_password) {
-            $response->header('Authorization', $this->tokenManager->refreshToken($this->user)->plainTextToken);
+            $response->header(
+                'Authorization',
+                $this->tokenManager->refreshApiToken($request->bearerToken() ?: '')->plainTextToken
+            );
         }
 
         return $response;

app/Http/Kernel.php

@@ -2,6 +2,7 @@
 
 namespace App\Http;
 
+use App\Http\Middleware\AudioAuthenticate;
 use App\Http\Middleware\Authenticate;
 use App\Http\Middleware\ForceHttps;
 use App\Http\Middleware\ObjectStorageAuthenticate;
@@ -49,6 +50,7 @@ class Kernel extends HttpKernel
      */
     protected $routeMiddleware = [
         'auth' => Authenticate::class,
+        'audio.auth' => AudioAuthenticate::class,
         'os.auth' => ObjectStorageAuthenticate::class,
         'bindings' => SubstituteBindings::class,
         'can' => Authorize::class,

app/Http/Middleware/AudioAuthenticate.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace App\Http\Middleware;
+
+use Closure;
+use Illuminate\Http\Request;
+use Illuminate\Http\Response;
+
+class AudioAuthenticate
+{
+    public function handle(Request $request, Closure $next) // @phpcs:ignore
+    {
+        abort_unless($request->user()?->tokenCan('audio'), Response::HTTP_UNAUTHORIZED);
+
+        return $next($request);
+    }
+}

app/Http/Middleware/Authenticate.php

@@ -3,25 +3,21 @@
 namespace App\Http\Middleware;
 
 use Closure;
-use Illuminate\Contracts\Auth\Guard;
 use Illuminate\Http\Request;
+use Illuminate\Http\Response;
 
 class Authenticate
 {
-    public function __construct(protected Guard $auth)
-    {
-    }
-
     public function handle(Request $request, Closure $next) // @phpcs:ignore
     {
-        if ($this->auth->guest()) {
-            if ($request->ajax() || $request->wantsJson() || $request->route()->getName() === 'play') {
-                return response('Unauthorized.', 401);
-            } else {
-                return redirect()->guest('/');
-            }
+        if ($request->user()?->tokenCan('*')) {
+            return $next($request);
         }
 
-        return $next($request);
+        if ($request->ajax() || $request->wantsJson()) {
+            abort(Response::HTTP_UNAUTHORIZED);
+        }
+
+        return redirect()->guest('/');
     }
 }

app/Models/User.php

@@ -11,6 +11,7 @@ use Illuminate\Database\Eloquent\Relations\HasMany;
 use Illuminate\Foundation\Auth\User as Authenticatable;
 use Illuminate\Notifications\Notifiable;
 use Laravel\Sanctum\HasApiTokens;
+use Laravel\Sanctum\PersonalAccessToken;
 
 /**
  * @property UserPreferences $preferences
@@ -23,6 +24,7 @@ use Laravel\Sanctum\HasApiTokens;
  * @property-read string $avatar
  * @property Collection|array<array-key, Playlist> $playlists
  * @property Collection|array<array-key, PlaylistFolder> $playlist_folders
+ * @property PersonalAccessToken $currentAccessToken
  */
 class User extends Authenticatable
 {

app/Providers/AuthServiceProvider.php

@@ -30,7 +30,9 @@ class AuthServiceProvider extends ServiceProvider
             /** @var TokenManager $tokenManager */
             $tokenManager = app(TokenManager::class);
 
-            return $tokenManager->getUserFromPlainTextToken($request->api_token ?: '');
+            $token = $request->get('api_token') ?: $request->get('t');
+
+            return $tokenManager->getUserFromPlainTextToken($token ?: '');
         });
 
         $this->setPasswordDefaultRules();

app/Services/TokenManager.php

@@ -3,16 +3,47 @@
 namespace App\Services;
 
 use App\Models\User;
+use App\Values\CompositionToken;
+use Illuminate\Cache\Repository as Cache;
 use Laravel\Sanctum\NewAccessToken;
 use Laravel\Sanctum\PersonalAccessToken;
 
 class TokenManager
 {
+    public function __construct(private Cache $cache)
+    {
+    }
+
     public function createToken(User $user, array $abilities = ['*']): NewAccessToken
     {
         return $user->createToken(config('app.name'), $abilities);
     }
 
+    public function createCompositionToken(User $user): CompositionToken
+    {
+        $token = CompositionToken::fromAccessTokens(
+            api: $this->createToken($user),
+            audio: $this->createToken($user, ['audio'])
+        );
+
+        $this->cache->rememberForever("app.composition-tokens.$token->apiToken", static fn () => $token->audioToken);
+
+        return $token;
+    }
+
+    public function deleteCompositionToken(string $plainTextApiToken): void
+    {
+        /** @var string $audioToken */
+        $audioToken = $this->cache->get("app.composition-tokens.$plainTextApiToken");
+
+        if ($audioToken) {
+            self::deleteTokenByPlainTextToken($audioToken);
+            $this->cache->forget("app.composition-tokens.$plainTextApiToken");
+        }
+
+        self::deleteTokenByPlainTextToken($plainTextApiToken);
+    }
+
     public function destroyTokens(User $user): void
     {
         $user->tokens()->delete();
@@ -28,10 +59,11 @@ class TokenManager
         return PersonalAccessToken::findToken($plainTextToken)?->tokenable;
     }
 
-    public function refreshToken(User $user): NewAccessToken
+    public function refreshApiToken(string $currentPlainTextToken): NewAccessToken
     {
-        $this->destroyTokens($user);
+        $newToken = $this->createToken($this->getUserFromPlainTextToken($currentPlainTextToken));
+        $this->deleteTokenByPlainTextToken($currentPlainTextToken);
 
-        return $this->createToken($user);
+        return $newToken;
     }
 }

app/Values/CompositionToken.php

@@ -0,0 +1,26 @@
+<?php
+
+namespace App\Values;
+
+use Laravel\Sanctum\NewAccessToken;
+
+/**
+ * A "composition token" consists of two tokens:
+ *
+ * - an API token, which has all abilities
+ * - an audio token, which has only the "audio" ability i.e. to play and download audio files. This token is used for
+ * the audio player on the frontend as part of the GET query string, and thus has limited privileges.
+ *
+ * This approach helps prevent the API token from being logged by servers and proxies.
+ */
+final class CompositionToken
+{
+    private function __construct(public string $apiToken, public string $audioToken)
+    {
+    }
+
+    public static function fromAccessTokens(NewAccessToken $api, NewAccessToken $audio): self
+    {
+        return new self($api->plainTextToken, $audio->plainTextToken);
+    }
+}

resources/assets/js/App.vue

@@ -80,7 +80,7 @@ const onUserLoggedIn = async () => {
 
 onMounted(async () => {
   // The app has just been initialized, check if we can get the user data with an already existing token
-  if (authService.hasToken()) {
+  if (authService.hasApiToken()) {
     authenticated.value = true
     await init()
   }

resources/assets/js/components/album/AlbumTrackListItem.vue

@@ -36,7 +36,7 @@ const fmtLength = computed(() => secondsToHis(track.value.length))
 const active = computed(() => matchedSong.value && matchedSong.value.playback_state !== 'Stopped')
 
 const iTunesUrl = computed(() => {
-  return `${window.BASE_URL}itunes/song/${album.value.id}?q=${encodeURIComponent(track.value.title)}&api_token=${authService.getToken()}`
+  return `${window.BASE_URL}itunes/song/${album.value.id}?q=${encodeURIComponent(track.value.title)}&api_token=${authService.getApiToken()}`
 })
 
 const play = () => {

resources/assets/js/components/profile-preferences/LastfmIntegration.vue

@@ -65,7 +65,7 @@ const connected = computed(() => Boolean(currentUser.value.preferences!.lastfm_s
  * Koel will reload once the connection is successful.
  */
 const connect = () => window.open(
-  `${window.BASE_URL}lastfm/connect?api_token=${authService.getToken()}`,
+  `${window.BASE_URL}lastfm/connect?api_token=${authService.getApiToken()}`,
   '_blank',
   'toolbar=no,titlebar=no,location=no,width=1024,height=640'
 )

resources/assets/js/remote/App.vue

@@ -199,7 +199,7 @@ const maxRetriesReached = computed(() => retries.value >= MAX_RETRIES)
 
 onMounted(async () => {
   // The app has just been initialized, check if we can get the user data with an already existing token
-  if (authService.hasToken()) {
+  if (authService.hasApiToken()) {
     authenticated.value = true
     await init()
   }

resources/assets/js/services/authService.spec.ts

@@ -7,18 +7,18 @@ new class extends UnitTestCase {
   protected test () {
     it('gets the token', () => {
       const mock = this.mock(localStorageService, 'get')
-      authService.getToken()
+      authService.getApiToken()
       expect(mock).toHaveBeenCalledWith('api-token')
     })
 
     it.each([['foo', true], [null, false]])('checks if the token exists', (token, exists) => {
       this.mock(localStorageService, 'get', token)
-      expect(authService.hasToken()).toBe(exists)
+      expect(authService.hasApiToken()).toBe(exists)
     })
 
     it('sets the token', () => {
       const mock = this.mock(localStorageService, 'set')
-      authService.setToken('foo')
+      authService.setApiToken('foo')
       expect(mock).toHaveBeenCalledWith('api-token', 'foo')
     })
 

resources/assets/js/services/authService.ts

@@ -1,14 +1,26 @@
 import { localStorageService } from '@/services'
 
-const STORAGE_KEY = 'api-token'
+const API_TOKEN_STORAGE_KEY = 'api-token'
+const AUDIO_TOKEN_STORAGE_KEY = 'audio-token'
 
 export const authService = {
-  getToken: () => localStorageService.get<string | null>(STORAGE_KEY),
+  getApiToken: () => localStorageService.get(API_TOKEN_STORAGE_KEY),
 
-  hasToken () {
-    return Boolean(this.getToken())
+  hasApiToken () {
+    return Boolean(this.getApiToken())
   },
 
-  setToken: (token: string) => localStorageService.set(STORAGE_KEY, token),
-  destroy: () => localStorageService.remove(STORAGE_KEY)
+  setApiToken: (token: string) => localStorageService.set(API_TOKEN_STORAGE_KEY, token),
+
+  destroy: () => {
+    localStorageService.remove(API_TOKEN_STORAGE_KEY)
+    localStorageService.remove(AUDIO_TOKEN_STORAGE_KEY)
+  },
+
+  setAudioToken: (token: string) => localStorageService.set(AUDIO_TOKEN_STORAGE_KEY, token),
+
+  getAudioToken: () => {
+    // for backward compatibility, we first try to get the audio token, and fall back to the (full-privileged) API token
+    return localStorageService.get(AUDIO_TOKEN_STORAGE_KEY) || localStorageService.get(API_TOKEN_STORAGE_KEY)
+  }
 }

resources/assets/js/services/downloadService.ts

@@ -34,7 +34,7 @@ export const downloadService = {
    */
   trigger: (uri: string) => {
     const sep = uri.includes('?') ? '&' : '?'
-    const url = `${window.BASE_URL}download/${uri}${sep}api_token=${authService.getToken()}`
+    const url = `${window.BASE_URL}download/${uri}${sep}t=${authService.getAudioToken()}`
 
     const iframe = document.createElement('iframe')
     iframe.style.display = 'none'

resources/assets/js/services/http.ts

@@ -50,7 +50,7 @@ class Http {
     // Intercept the request to make sure the token is injected into the header.
     this.client.interceptors.request.use(config => {
       Http.setProgressBar()
-      config.headers.Authorization = `Bearer ${authService.getToken()}`
+      config.headers.Authorization = `Bearer ${authService.getApiToken()}`
       return config
     })
 
@@ -60,7 +60,10 @@ class Http {
 
       // …get the token from the header or response data if exists, and save it.
       const token = response.headers.authorization || response.data.token
-      token && authService.setToken(token)
+      token && authService.setApiToken(token)
+
+      const audioToken = response.data['audio-token']
+      audioToken && authService.setAudioToken(audioToken)
 
       return response
     }, error => {

resources/assets/js/services/socketService.ts

@@ -16,7 +16,7 @@ export const socketService = {
       authEndpoint: `${window.BASE_URL}api/broadcasting/auth`,
       auth: {
         headers: {
-          Authorization: `Bearer ${authService.getToken()}`
+          Authorization: `Bearer ${authService.getApiToken()}`
         }
       },
       cluster: window.PUSHER_APP_CLUSTER,

resources/assets/js/stores/songStore.spec.ts

@@ -147,13 +147,13 @@ new class extends UnitTestCase {
     it('gets source URL', () => {
       commonStore.state.cdn_url = 'http://test/'
       const song = factory<Song>('song', { id: 'foo' })
-      this.mock(authService, 'getToken', 'hadouken')
+      this.mock(authService, 'getAudioToken', 'hadouken')
 
-      expect(songStore.getSourceUrl(song)).toBe('http://test/play/foo?api_token=hadouken')
+      expect(songStore.getSourceUrl(song)).toBe('http://test/play/foo?t=hadouken')
 
       isMobile.any = true
       preferenceStore.transcodeOnMobile = true
-      expect(songStore.getSourceUrl(song)).toBe('http://test/play/foo/1/128?api_token=hadouken')
+      expect(songStore.getSourceUrl(song)).toBe('http://test/play/foo/1/128?t=hadouken')
     })
 
     it('gets shareable URL', () => {

resources/assets/js/stores/songStore.ts

@@ -113,8 +113,8 @@ export const songStore = {
 
   getSourceUrl: (song: Song) => {
     return isMobile.any && preferenceStore.transcodeOnMobile
-      ? `${commonStore.state.cdn_url}play/${song.id}/1/128?api_token=${authService.getToken()}`
-      : `${commonStore.state.cdn_url}play/${song.id}?api_token=${authService.getToken()}`
+      ? `${commonStore.state.cdn_url}play/${song.id}/1/128?t=${authService.getAudioToken()}`
+      : `${commonStore.state.cdn_url}play/${song.id}?t=${authService.getAudioToken()}`
   },
 
   getShareableUrl: (song: Song) => `${window.BASE_URL}#/song/${song.id}`,

routes/web.base.php

@@ -17,8 +17,6 @@ Route::middleware('web')->group(static function (): void {
     Route::get('remote', static fn () => view('remote'));
 
     Route::middleware('auth')->group(static function (): void {
-        Route::get('play/{song}/{transcode?}/{bitrate?}', [PlayController::class, 'show'])->name('song.play');
-
         Route::prefix('lastfm')->group(static function (): void {
             Route::get('connect', [LastfmController::class, 'connect'])->name('lastfm.connect');
             Route::get('callback', [LastfmController::class, 'callback'])->name('lastfm.callback');
@@ -27,6 +25,10 @@ Route::middleware('web')->group(static function (): void {
         if (ITunes::used()) {
             Route::get('itunes/song/{album}', [ITunesController::class, 'viewSong'])->name('iTunes.viewSong');
         }
+    });
+
+    Route::middleware('audio.auth')->group(static function (): void {
+        Route::get('play/{song}/{transcode?}/{bitrate?}', [PlayController::class, 'show'])->name('song.play');
 
         Route::prefix('download')->group(static function (): void {
             Route::get('songs', [SongDownloadController::class, 'show']);

tests/Feature/AuthTest.php

@@ -0,0 +1,55 @@
+<?php
+
+namespace Tests\Feature;
+
+use App\Models\User;
+use Illuminate\Support\Facades\Hash;
+
+class AuthTest extends TestCase
+{
+    public function testLogIn(): void
+    {
+        User::factory()->create([
+            'email' => 'koel@koel.dev',
+            'password' => Hash::make('secret'),
+        ]);
+
+        $this->post('api/me', [
+            'email' => 'koel@koel.dev',
+            'password' => 'secret',
+        ])
+            ->assertOk()
+            ->assertJsonStructure([
+                'token',
+                'audio-token',
+            ]);
+
+        $this->post('api/me', [
+            'email' => 'koel@koel.dev',
+            'password' => 'wrong-secret',
+        ])
+            ->assertUnauthorized();
+    }
+
+    public function testLogOut(): void
+    {
+        /** @var User $user */
+        $user = User::factory()->create([
+            'email' => 'koel@koel.dev',
+            'password' => Hash::make('secret'),
+        ]);
+
+        $response = $this->post('api/me', [
+            'email' => 'koel@koel.dev',
+            'password' => 'secret',
+        ]);
+
+        self::assertSame(2, $user->tokens()->count()); // 1 for API, 1 for audio token
+
+        $this->withToken($response->json('token'))
+            ->delete('api/me')
+            ->assertNoContent();
+
+        self::assertSame(0, $user->tokens()->count());
+    }
+}

tests/Feature/DownloadTest.php

@@ -35,7 +35,7 @@ class DownloadTest extends TestCase
             ->never();
 
         $this->get("download/songs?songs[]=$song->id")
-            ->assertRedirect('/');
+            ->assertUnauthorized();
     }
 
     public function testDownloadOneSong(): void

tests/Integration/Services/TokenManagerTest.php

@@ -0,0 +1,116 @@
+<?php
+
+namespace Tests\Integration\Services;
+
+use App\Models\User;
+use App\Services\TokenManager;
+use Illuminate\Support\Facades\Cache;
+use Laravel\Sanctum\PersonalAccessToken;
+use Tests\TestCase;
+
+class TokenManagerTest extends TestCase
+{
+    private TokenManager $tokenManager;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+
+        $this->tokenManager = app(TokenManager::class);
+    }
+
+    public function testCreateTokenWithAllAbilities(): void
+    {
+        /** @var User $user */
+        $user = User::factory()->create();
+        $token = $this->tokenManager->createToken($user);
+
+        self::assertTrue($token->accessToken->can('*'));
+    }
+
+    public function testCreateTokenWithSpecificAbilities(): void
+    {
+        /** @var User $user */
+        $user = User::factory()->create();
+        $token = $this->tokenManager->createToken($user, ['audio']);
+
+        self::assertTrue($token->accessToken->can('audio'));
+        self::assertFalse($token->accessToken->can('video'));
+        self::assertFalse($token->accessToken->can('*'));
+    }
+
+    public function testCreateCompositionToken(): void
+    {
+        /** @var User $user */
+        $user = User::factory()->create();
+        $token = $this->tokenManager->createCompositionToken($user);
+
+        self::assertModelExists(PersonalAccessToken::findToken($token->apiToken));
+
+        $audioTokenInstance = PersonalAccessToken::findToken($token->audioToken);
+        self::assertModelExists($audioTokenInstance);
+
+        /** @var string $cachedAudioToken */
+        $cachedAudioToken = Cache::get("app.composition-tokens.$token->apiToken");
+        self::assertTrue($audioTokenInstance->is(PersonalAccessToken::findToken($cachedAudioToken)));
+    }
+
+    public function testDeleteCompositionToken(): void
+    {
+        /** @var User $user */
+        $user = User::factory()->create();
+        $token = $this->tokenManager->createCompositionToken($user);
+
+        $this->tokenManager->deleteCompositionToken($token->apiToken);
+
+        self::assertNull(PersonalAccessToken::findToken($token->apiToken));
+        self::assertNull(PersonalAccessToken::findToken($token->audioToken));
+        self::assertNull(Cache::get("app.composition-tokens.$token->apiToken"));
+    }
+
+    public function testDestroyTokens(): void
+    {
+        /** @var User $user */
+        $user = User::factory()->create();
+        $user->createToken('foo');
+        $user->createToken('bar');
+
+        self::assertSame(2, $user->tokens()->count());
+
+        $this->tokenManager->destroyTokens($user);
+
+        self::assertSame(0, $user->tokens()->count());
+    }
+
+    public function testDeleteTokenByPlainTextToken(): void
+    {
+        /** @var User $user */
+        $user = User::factory()->create();
+        $token = $this->tokenManager->createToken($user);
+        self::assertModelExists($token->accessToken);
+
+        $this->tokenManager->deleteTokenByPlainTextToken($token->plainTextToken);
+
+        self::assertModelMissing($token->accessToken);
+    }
+
+    public function testGetUserFromPlainTextToken(): void
+    {
+        /** @var User $user */
+        $user = User::factory()->create();
+        $token = $this->tokenManager->createToken($user);
+
+        self::assertTrue($user->is($this->tokenManager->getUserFromPlainTextToken($token->plainTextToken)));
+    }
+
+    public function testReplaceApiToken(): void
+    {
+        /** @var User $user */
+        $user = User::factory()->create();
+        $oldToken = $this->tokenManager->createToken($user);
+        $newToken = $this->tokenManager->refreshApiToken($oldToken->plainTextToken);
+
+        self::assertModelMissing($oldToken->accessToken);
+        self::assertModelExists($newToken->accessToken);
+    }
+}