From d80a19ba7066cf283651deb970f1c3d2c4807080 Mon Sep 17 00:00:00 2001 From: Phan An Date: Sun, 31 Mar 2024 19:19:03 +0200 Subject: [PATCH] feat: support reverse proxy authentication --- .env.example | 12 +++ app/Http/Controllers/IndexController.php | 25 +++++ .../SSO/GoogleCallbackController.php | 3 +- app/Repositories/UserRepository.php | 10 +- app/Services/AuthenticationService.php | 5 - app/Services/ProxyAuthService.php | 37 ++++++++ app/Services/UserService.php | 26 +++-- app/Values/SSOUser.php | 49 ++++++++++ config/koel.php | 7 ++ resources/assets/js/App.vue | 6 ++ .../assets/js/components/auth/LoginForm.vue | 4 +- .../profile-preferences/ProfileForm.vue | 7 +- resources/assets/js/services/authService.ts | 5 - resources/assets/js/types.d.ts | 10 +- resources/views/base.blade.php | 3 - resources/views/index.blade.php | 5 + routes/web.base.php | 3 +- tests/Feature/KoelPlus/ProxyAuthTest.php | 95 +++++++++++++++++++ .../KoelPlus/Services/UserServiceTest.php | 28 +++--- 19 files changed, 288 insertions(+), 52 deletions(-) create mode 100644 app/Http/Controllers/IndexController.php create mode 100644 app/Services/ProxyAuthService.php create mode 100644 app/Values/SSOUser.php create mode 100644 tests/Feature/KoelPlus/ProxyAuthTest.php diff --git a/.env.example b/.env.example index 199e4733..e2264357 100644 --- a/.env.example +++ b/.env.example @@ -157,6 +157,18 @@ SSO_GOOGLE_CLIENT_SECRET= SSO_GOOGLE_HOSTED_DOMAIN=yourdomain.com +# Koel can be configured to authenticate users via a reverse proxy. +# To enable this feature, set PROXY_AUTH_ENABLED to true and provide the necessary configuration below. +PROXY_AUTH_ENABLED=false +# The header name that contains the unique identifer for the user +PROXY_AUTH_USER_HEADER=remote-user +# The header name that contains the user's preferred, humanly-readable name +PROXY_AUTH_PREFERRED_NAME_HEADER=remote-preferred-name +# A comma-separated list of allowed proxy IPs or CIDRs, for example 10.10.1.0/24 or 2001:0db8:/32 +# If empty, NO requests will be allowed (which means proxy authentication is disabled). +PROXY_AUTH_ALLOW_LIST= + + # Sync logs can be found under storage/logs/. Valid options are: # all: Log everything (errored-, skipped-, and successfully processed file). # error: Log errors only. This is the default. diff --git a/app/Http/Controllers/IndexController.php b/app/Http/Controllers/IndexController.php new file mode 100644 index 00000000..6b478976 --- /dev/null +++ b/app/Http/Controllers/IndexController.php @@ -0,0 +1,25 @@ + null]; + + if (License::isPlus() && config('koel.proxy_auth.enabled')) { + $data['token'] = optional( + $proxyAuthService->tryGetProxyAuthenticatedUserFromRequest($request), + static fn ($user) => $auth->logUserIn($user)->toArray() + ); + } + + return view('index', $data); + } +} diff --git a/app/Http/Controllers/SSO/GoogleCallbackController.php b/app/Http/Controllers/SSO/GoogleCallbackController.php index 205f7748..3873cba1 100644 --- a/app/Http/Controllers/SSO/GoogleCallbackController.php +++ b/app/Http/Controllers/SSO/GoogleCallbackController.php @@ -6,6 +6,7 @@ use App\Facades\License; use App\Http\Controllers\Controller; use App\Services\AuthenticationService; use App\Services\UserService; +use App\Values\SSOUser; use Laravel\Socialite\Facades\Socialite; class GoogleCallbackController extends Controller @@ -15,7 +16,7 @@ class GoogleCallbackController extends Controller assert(License::isPlus()); $user = Socialite::driver('google')->user(); - $user = $userService->createOrUpdateUserFromSocialiteUser($user, 'Google'); + $user = $userService->createOrUpdateUserFromSSO(SSOUser::fromSocialite($user, 'Google')); return view('sso-callback')->with('token', $auth->logUserIn($user)->toArray()); } diff --git a/app/Repositories/UserRepository.php b/app/Repositories/UserRepository.php index 9654ce96..1ef84659 100644 --- a/app/Repositories/UserRepository.php +++ b/app/Repositories/UserRepository.php @@ -5,7 +5,7 @@ namespace App\Repositories; use App\Models\User; -use Laravel\Socialite\Contracts\User as SocialiteUser; +use App\Values\SSOUser; class UserRepository extends Repository { @@ -19,12 +19,12 @@ class UserRepository extends Repository return User::query()->firstWhere('email', $email); } - public function findOneBySocialiteUser(SocialiteUser $socialiteUser, string $provider): ?User + public function findOneBySSO(SSOUser $ssoUser): ?User { // we prioritize the SSO ID over the email address, but still resort to the latter return User::query()->firstWhere([ - 'sso_id' => $socialiteUser->getId(), - 'sso_provider' => $provider, - ]) ?? $this->findOneByEmail($socialiteUser->getEmail()); + 'sso_id' => $ssoUser->id, + 'sso_provider' => $ssoUser->provider, + ]) ?? $this->findOneByEmail($ssoUser->email); } } diff --git a/app/Services/AuthenticationService.php b/app/Services/AuthenticationService.php index b0ea664d..955082b6 100644 --- a/app/Services/AuthenticationService.php +++ b/app/Services/AuthenticationService.php @@ -53,11 +53,6 @@ class AuthenticationService return $this->passwordBroker->sendResetLink(['email' => $email]) === Password::RESET_LINK_SENT; } - public function generatePasswordResetToken(User $user): string - { - return $this->passwordBroker->createToken($user); - } - public function tryResetPasswordUsingBroker(string $email, string $password, string $token): bool { $credentials = [ diff --git a/app/Services/ProxyAuthService.php b/app/Services/ProxyAuthService.php new file mode 100644 index 00000000..1029184d --- /dev/null +++ b/app/Services/ProxyAuthService.php @@ -0,0 +1,37 @@ +userService->createOrUpdateUserFromSSO(SSOUser::fromProxyAuthRequest($request)); + } catch (Throwable $e) { + Log::error($e->getMessage(), ['exception' => $e]); + } + + return null; + } + + private static function validateProxyIp(Request $request): bool + { + return IpUtils::checkIp($request->ip(), config('koel.proxy_auth.allow_list')); + } +} diff --git a/app/Services/UserService.php b/app/Services/UserService.php index 8f221925..cc9a7e10 100644 --- a/app/Services/UserService.php +++ b/app/Services/UserService.php @@ -6,11 +6,10 @@ use App\Exceptions\UserProspectUpdateDeniedException; use App\Facades\License; use App\Models\User; use App\Repositories\UserRepository; +use App\Values\SSOUser; use Illuminate\Contracts\Hashing\Hasher; use Illuminate\Support\Facades\File; use Illuminate\Support\Str; -use Laravel\Socialite\Contracts\User as SocialiteUser; -use Webmozart\Assert\Assert; class UserService { @@ -33,7 +32,7 @@ class UserService ): User { if ($ssoProvider) { License::requirePlus(); - Assert::oneOf($ssoProvider, ['Google']); + SSOUser::assertValidProvider($ssoProvider); } return User::query()->create([ @@ -47,31 +46,30 @@ class UserService ]); } - public function createOrUpdateUserFromSocialiteUser(SocialiteUser $socialiteUser, string $provider): User + public function createOrUpdateUserFromSSO(SSOUser $ssoUser): User { License::requirePlus(); - Assert::oneOf($provider, ['Google']); - $existingUser = $this->repository->findOneBySocialiteUser($socialiteUser, $provider); + $existingUser = $this->repository->findOneBySSO($ssoUser); if ($existingUser) { $existingUser->update([ - 'avatar' => $existingUser->has_custom_avatar ? $existingUser->avatar : $socialiteUser->getAvatar(), - 'sso_id' => $socialiteUser->getId(), - 'sso_provider' => $provider, + 'avatar' => $existingUser->has_custom_avatar ? $existingUser->avatar : $ssoUser->avatar, + 'sso_id' => $ssoUser->id, + 'sso_provider' => $ssoUser->provider, ]); return $existingUser; } return $this->createUser( - name: $socialiteUser->getName(), - email: $socialiteUser->getEmail(), + name: $ssoUser->name, + email: $ssoUser->email, plainTextPassword: '', isAdmin: false, - avatar: $socialiteUser->getAvatar(), - ssoId: $socialiteUser->getId(), - ssoProvider: $provider + avatar: $ssoUser->avatar, + ssoId: $ssoUser->id, + ssoProvider: $ssoUser->provider, ); } diff --git a/app/Values/SSOUser.php b/app/Values/SSOUser.php new file mode 100644 index 00000000..e4c4ea89 --- /dev/null +++ b/app/Values/SSOUser.php @@ -0,0 +1,49 @@ +getId(), + email: $socialiteUser->getEmail(), + name: $socialiteUser->getName(), + avatar: $socialiteUser->getAvatar(), + ); + } + + public static function fromProxyAuthRequest(Request $request): self + { + $identifier = $request->header(config('koel.proxy_auth.user_header')); + + return new self( + provider: 'Reverse Proxy', + id: $identifier, + email: "$identifier@reverse-proxy", + name: $request->header(config('koel.proxy_auth.preferred_name_header')) ?: $identifier, + avatar: null, + ); + } + + public static function assertValidProvider(string $provider): void + { + Assert::oneOf($provider, ['Google', 'Reverse Proxy']); + } +} diff --git a/config/koel.php b/config/koel.php index d450a3ac..386e1b3c 100644 --- a/config/koel.php +++ b/config/koel.php @@ -141,6 +141,13 @@ return [ 'sync_log_level' => env('SYNC_LOG_LEVEL', 'error'), + 'proxy_auth' => [ + 'enabled' => env('PROXY_AUTH_ENABLED', false), + 'user_header' => env('PROXY_AUTH_USER_HEADER', 'remote-user'), + 'preferred_name_header' => env('PROXY_AUTH_PREFERRED_NAME_HEADER', 'remote-preferred-name'), + 'allow_list' => array_map(static fn ($entry) => trim($entry), explode(',', env('PROXY_AUTH_ALLOW_LIST', ''))), + ], + 'misc' => [ 'home_url' => 'https://koel.dev', 'docs_url' => 'https://docs.koel.dev', diff --git a/resources/assets/js/App.vue b/resources/assets/js/App.vue index a0e931fa..83716ec6 100644 --- a/resources/assets/js/App.vue +++ b/resources/assets/js/App.vue @@ -85,6 +85,12 @@ const onUserLoggedIn = async () => { } onMounted(async () => { + // If the user is authenticated via a proxy, we have the token in the window object. + // Simply forward it to the authService and continue with the normal flow. + if (window.AUTH_TOKEN) { + authService.setTokensUsingCompositeToken(window.AUTH_TOKEN) + } + // The app has just been initialized, check if we can get the user data with an already existing token if (authService.hasApiToken()) { await init() diff --git a/resources/assets/js/components/auth/LoginForm.vue b/resources/assets/js/components/auth/LoginForm.vue index acd8229a..15b73d37 100644 --- a/resources/assets/js/components/auth/LoginForm.vue +++ b/resources/assets/js/components/auth/LoginForm.vue @@ -34,9 +34,9 @@ diff --git a/resources/views/index.blade.php b/resources/views/index.blade.php index dfcfdb24..33db5139 100644 --- a/resources/views/index.blade.php +++ b/resources/views/index.blade.php @@ -3,5 +3,10 @@ @section('title', 'Koel') @push('scripts') + @vite(['resources/assets/js/app.ts']) @endpush diff --git a/routes/web.base.php b/routes/web.base.php index 3c799692..62c897d3 100644 --- a/routes/web.base.php +++ b/routes/web.base.php @@ -7,6 +7,7 @@ use App\Http\Controllers\Download\DownloadArtistController; use App\Http\Controllers\Download\DownloadFavoritesController; use App\Http\Controllers\Download\DownloadPlaylistController; use App\Http\Controllers\Download\DownloadSongsController; +use App\Http\Controllers\IndexController; use App\Http\Controllers\LastfmController; use App\Http\Controllers\PlayController; use App\Http\Controllers\SSO\GoogleCallbackController; @@ -15,7 +16,7 @@ use Illuminate\Support\Facades\Route; use Laravel\Socialite\Facades\Socialite; Route::middleware('web')->group(static function (): void { - Route::get('/', static fn () => view('index')); + Route::get('/', IndexController::class); Route::get('remote', static fn () => view('remote')); diff --git a/tests/Feature/KoelPlus/ProxyAuthTest.php b/tests/Feature/KoelPlus/ProxyAuthTest.php new file mode 100644 index 00000000..59d88a23 --- /dev/null +++ b/tests/Feature/KoelPlus/ProxyAuthTest.php @@ -0,0 +1,95 @@ + true, + 'koel.proxy_auth.allow_list' => ['192.168.1.0/24'], + 'koel.proxy_auth.user_header' => 'remote-user', + 'koel.proxy_auth.preferred_name_header' => 'remote-preferred-name', + ]); + } + + protected function tearDown(): void + { + config([ + 'koel.proxy_auth.enabled' => false, + 'koel.proxy_auth.allow_list' => [], + 'koel.proxy_auth.user_header' => 'remote-user', + 'koel.proxy_auth.preferred_name_header' => 'remote-preferred-name', + ]); + + parent::tearDown(); + } + + public function testProxyAuthenticateNewUser(): void + { + $response = $this->get('/', [ + 'REMOTE_ADDR' => '192.168.1.127', + 'remote-user' => '123456', + 'remote-preferred-name' => 'Bruce Dickinson', + ]); + + $response->assertOk(); + $response->assertViewHas('token'); + + /** @var array $token */ + $token = $response->viewData('token'); + + self::assertNotNull(PersonalAccessToken::findToken($token['token'])); + + self::assertDatabaseHas(User::class, [ + 'email' => '123456@reverse-proxy', + 'name' => 'Bruce Dickinson', + 'sso_id' => '123456', + 'sso_provider' => 'Reverse Proxy', + ]); + } + + public function testProxyAuthenticateExistingUser(): void + { + $user = create_user([ + 'sso_id' => '123456', + 'sso_provider' => 'Reverse Proxy', + ]); + + $response = $this->get('/', [ + 'REMOTE_ADDR' => '192.168.1.127', + 'remote-user' => '123456', + 'remote-preferred-name' => 'Bruce Dickinson', + ]); + + $response->assertOk(); + $response->assertViewHas('token'); + + /** @var array $token */ + $token = $response->viewData('token'); + + self::assertTrue($user->is(PersonalAccessToken::findToken($token['token'])->tokenable)); + } + + public function testProxyAuthenticateWithDisallowedIp(): void + { + $response = $this->get('/', [ + 'REMOTE_ADDR' => '255.168.1.127', + 'remote-user' => '123456', + 'remote-preferred-name' => 'Bruce Dickinson', + ]); + + $response->assertOk(); + + self::assertNull($response->viewData('token')); + } +} diff --git a/tests/Integration/KoelPlus/Services/UserServiceTest.php b/tests/Integration/KoelPlus/Services/UserServiceTest.php index 426bf60e..6669e418 100644 --- a/tests/Integration/KoelPlus/Services/UserServiceTest.php +++ b/tests/Integration/KoelPlus/Services/UserServiceTest.php @@ -4,6 +4,7 @@ namespace Tests\Integration\KoelPlus\Services; use App\Models\User; use App\Services\UserService; +use App\Values\SSOUser; use Illuminate\Support\Facades\Hash; use Laravel\Socialite\Two\User as SocialiteUser; use Mockery; @@ -40,7 +41,7 @@ class UserServiceTest extends PlusTestCase self::assertSame('https://lh3.googleusercontent.com/a/vatar', $user->avatar); } - public function testCreateUserFromSocialiteUser(): void + public function testCreateUserFromSSO(): void { self::assertDatabaseMissing(User::class, ['email' => 'bruce@iron.com']); @@ -51,7 +52,7 @@ class UserServiceTest extends PlusTestCase 'getAvatar' => 'https://lh3.googleusercontent.com/a/vatar', ]); - $user = $this->service->createOrUpdateUserFromSocialiteUser($socialiteUser, 'Google'); + $user = $this->service->createOrUpdateUserFromSSO(SSOUser::fromSocialite($socialiteUser, 'Google')); self::assertModelExists($user); @@ -66,6 +67,7 @@ class UserServiceTest extends PlusTestCase { $user = create_user([ 'email' => 'bruce@iron.com', + 'name' => 'Bruce Dickinson', 'sso_id' => '123', 'sso_provider' => 'Google', ]); @@ -77,30 +79,34 @@ class UserServiceTest extends PlusTestCase 'getAvatar' => 'https://lh3.googleusercontent.com/a/vatar', ]); - $this->service->createOrUpdateUserFromSocialiteUser($socialiteUser, 'Google'); + $this->service->createOrUpdateUserFromSSO(SSOUser::fromSocialite($socialiteUser, 'Google')); $user->refresh(); - self::assertSame('Steve Harris', $user->name); + self::assertSame('Bruce Dickinson', $user->name); // Name should not be updated self::assertSame('https://lh3.googleusercontent.com/a/vatar', $user->avatar); - self::assertSame('steve@iron.com', $user->email); + self::assertSame('bruce@iron.com', $user->email); // Email should not be updated self::assertSame('Google', $user->sso_provider); } public function testUpdateUserFromSSOEmail(): void { - $user = create_user(['email' => 'bruce@iron.com']); + $user = create_user([ + 'email' => 'bruce@iron.com', + 'name' => 'Bruce Dickinson', + ]); $socialiteUser = Mockery::mock(SocialiteUser::class, [ 'getId' => '123', 'getEmail' => 'bruce@iron.com', - 'getName' => 'Bruce Dickinson', + 'getName' => 'Steve Harris', 'getAvatar' => 'https://lh3.googleusercontent.com/a/vatar', + ]); - $this->service->createOrUpdateUserFromSocialiteUser($socialiteUser, 'Google'); + $this->service->createOrUpdateUserFromSSO(SSOUser::fromSocialite($socialiteUser, 'Google')); $user->refresh(); - self::assertSame('Bruce Dickinson', $user->name); + self::assertSame('Bruce Dickinson', $user->name); // Name should not be updated self::assertSame('https://lh3.googleusercontent.com/a/vatar', $user->avatar); self::assertSame('Google', $user->sso_provider); } @@ -110,7 +116,6 @@ class UserServiceTest extends PlusTestCase $user = create_user([ 'email' => 'bruce@iron.com', 'name' => 'Bruce Dickinson', - 'avatar' => 'https://lh3.googleusercontent.com/a/vatar', 'sso_provider' => 'Google', ]); @@ -120,15 +125,12 @@ class UserServiceTest extends PlusTestCase email: 'steve@iron.com', password: 'TheTrooper', isAdmin: true, - avatar: 'https://lh3.googleusercontent.com/a/vatar/2' ); $user->refresh(); - self::assertSame('Bruce Dickinson', $user->name); self::assertSame('bruce@iron.com', $user->email); self::assertFalse(Hash::check('TheTrooper', $user->password)); self::assertTrue($user->is_admin); - self::assertSame('https://lh3.googleusercontent.com/a/vatar', $user->avatar); } }