diff --git a/config.local/src/.env b/config.local/src/.env --- a/config.local/src/.env +++ b/config.local/src/.env @@ -204,7 +204,3 @@ PASSPORT_PROXY_OAUTH_CLIENT_ID=942edef5-3dbd-4a14-8e3e-d5d59b727bee PASSPORT_PROXY_OAUTH_CLIENT_SECRET=L6L0n56ecvjjK0cJMjeeV1pPAeffUBO0YSSH63wf -#Generated by php artisan passport:client --password, but can be left hardcoded (the seeder will pick it up) -PASSPORT_COMPANIONAPP_OAUTH_CLIENT_ID=9566e018-f05d-425c-9915-420cdb9258bb -PASSPORT_COMPANIONAPP_OAUTH_CLIENT_SECRET=XjgV6SU9shO0QFKaU6pQPRC5rJpyRezDJTSoGLgz - diff --git a/config.local/src/database/seeds/OauthClientSeeder.php b/config.local/src/database/seeds/OauthClientSeeder.php --- a/config.local/src/database/seeds/OauthClientSeeder.php +++ b/config.local/src/database/seeds/OauthClientSeeder.php @@ -28,22 +28,6 @@ ]); $client->id = \config('auth.proxy.client_id'); - $client->save(); - - $companionAppClient = Passport::client()->forceFill([ - 'user_id' => null, - 'name' => "CompanionApp Password Grant Client", - 'secret' => \config('auth.companion_app.client_secret'), - 'provider' => 'users', - 'redirect' => 'https://' . \config('app.website_domain'), - 'personal_access_client' => 0, - 'password_client' => 1, - 'revoked' => false, - ]); - - $companionAppClient->id = \config('auth.companion_app.client_id'); - - $companionAppClient->save(); } } diff --git a/config.localhost/src/.env b/config.localhost/src/.env --- a/config.localhost/src/.env +++ b/config.localhost/src/.env @@ -194,7 +194,3 @@ #Generated by php artisan passport:client --password, but can be left hardcoded (the seeder will pick it up) PASSPORT_PROXY_OAUTH_CLIENT_ID=942edef5-3dbd-4a14-8e3e-d5d59b727bee PASSPORT_PROXY_OAUTH_CLIENT_SECRET=L6L0n56ecvjjK0cJMjeeV1pPAeffUBO0YSSH63wf - -#Generated by php artisan passport:client --password, but can be left hardcoded (the seeder will pick it up) -PASSPORT_COMPANIONAPP_OAUTH_CLIENT_ID=9566e018-f05d-425c-9915-420cdb9258bb -PASSPORT_COMPANIONAPP_OAUTH_CLIENT_SECRET=XjgV6SU9shO0QFKaU6pQPRC5rJpyRezDJTSoGLgz diff --git a/src/app/CompanionApp.php b/src/app/CompanionApp.php --- a/src/app/CompanionApp.php +++ b/src/app/CompanionApp.php @@ -3,6 +3,7 @@ namespace App; use Illuminate\Database\Eloquent\Model; +use App\Traits\UuidIntKeyTrait; /** * The eloquent definition of a CompanionApp. @@ -11,6 +12,8 @@ */ class CompanionApp extends Model { + use UuidIntKeyTrait; + /** @var array The attributes that are mass assignable */ protected $fillable = [ 'name', @@ -81,4 +84,14 @@ self::pushFirebaseNotification($notificationTokens, $data); return true; } + + /** + * Returns whether this companion app is paired with a device. + * + * @return bool + */ + public function isPaired(): bool + { + return !empty($this->device_id); + } } diff --git a/src/app/Http/Controllers/API/AuthController.php b/src/app/Http/Controllers/API/AuthController.php --- a/src/app/Http/Controllers/API/AuthController.php +++ b/src/app/Http/Controllers/API/AuthController.php @@ -46,7 +46,7 @@ 'grant_type' => 'password', 'client_id' => \config('auth.proxy.client_id'), 'client_secret' => \config('auth.proxy.client_secret'), - 'scopes' => '[*]', + 'scope' => 'api', 'secondfactor' => $secondFactor ]); $proxyRequest->headers->set('X-Client-IP', request()->ip()); diff --git a/src/app/Http/Controllers/API/V4/CompanionAppsController.php b/src/app/Http/Controllers/API/V4/CompanionAppsController.php --- a/src/app/Http/Controllers/API/V4/CompanionAppsController.php +++ b/src/app/Http/Controllers/API/V4/CompanionAppsController.php @@ -5,15 +5,74 @@ use App\Http\Controllers\ResourceController; use App\Utils; use App\Tenant; -use Laravel\Passport\Token; -use Laravel\Passport\TokenRepository; -use Laravel\Passport\RefreshTokenRepository; +use Laravel\Passport\Passport; +use Laravel\Passport\ClientRepository; use Illuminate\Http\Request; use Illuminate\Support\Facades\Validator; +use Illuminate\Support\Str; use BaconQrCode; class CompanionAppsController extends ResourceController { + /** + * Remove the specified companion app. + * + * @param string $id Companion app identifier + * + * @return \Illuminate\Http\JsonResponse + */ + public function destroy($id) + { + $companion = \App\CompanionApp::find($id); + if (!$companion) { + return $this->errorResponse(404); + } + + $user = $this->guard()->user(); + if ($user->id != $companion->user_id) { + return $this->errorResponse(403); + } + + // Revoke client and tokens + if ($companion->oauth_client) { + $clientRepository = app(ClientRepository::class); + $clientRepository->delete(\App\PassportClient::find($companion->oauth_client)); + } + + $companion->delete(); + + return response()->json([ + 'status' => 'success', + 'message' => \trans('app.companion-delete-success'), + ]); + } + + /** + * Create a companion app. + * + * @param \Illuminate\Http\Request $request + * + * @return \Illuminate\Http\JsonResponse + */ + public function store(Request $request) + { + $user = $this->guard()->user(); + + $name = \strtolower(request()->input('name')); + + \App\CompanionApp::create([ + 'name' => $name, + 'user_id' => $user->id, + 'mfa_enabled' => false, + 'device_id' => "", + ]); + + return response()->json([ + 'status' => 'success', + 'message' => \trans('app.companion-create-success'), + ]); + } + /** * Register a companion app. * @@ -30,6 +89,7 @@ [ 'notificationToken' => 'required|min:4|max:512', 'deviceId' => 'required|min:4|max:64', + 'companionId' => 'required|max:64', 'name' => 'required|max:512', ] ); @@ -40,32 +100,30 @@ $notificationToken = $request->notificationToken; $deviceId = $request->deviceId; + $companionId = $request->companionId; $name = $request->name; \Log::info("Registering app. Notification token: {$notificationToken} Device id: {$deviceId} Name: {$name}"); - $app = \App\CompanionApp::where('device_id', $deviceId)->first(); + $app = \App\CompanionApp::find($companionId); if (!$app) { - $app = new \App\CompanionApp(); - $app->user_id = $user->id; - $app->device_id = $deviceId; - $app->mfa_enabled = true; - $app->name = $name; - } else { - //FIXME this allows a user to probe for another users deviceId - if ($app->user_id != $user->id) { - \Log::warning("User mismatch on device registration. Expected {$user->id} but found {$app->user_id}"); - return $this->errorResponse(403); - } + return $this->errorResponse(404); } + if ($app->user_id != $user->id) { + \Log::warning("User mismatch on device registration. Expected {$user->id} but found {$app->user_id}"); + return $this->errorResponse(403); + } + + $app->device_id = $deviceId; + $app->mfa_enabled = true; + $app->name = $name; $app->notification_token = $notificationToken; $app->save(); return response()->json(['status' => 'success']); } - /** * Generate a QR-code image for a string * @@ -83,34 +141,6 @@ return 'data:image/svg+xml;base64,' . base64_encode($writer->writeString($data)); } - /** - * Revoke all companion app devices. - * - * @return \Illuminate\Http\JsonResponse The response - */ - public function revokeAll() - { - $user = $this->guard()->user(); - \App\CompanionApp::where('user_id', $user->id)->delete(); - - // Revoke all companion app tokens - $clientIdentifier = \App\Tenant::getConfig($user->tenant_id, 'auth.companion_app.client_id'); - $tokens = Token::where('user_id', $user->id)->where('client_id', $clientIdentifier)->get(); - - $tokenRepository = app(TokenRepository::class); - $refreshTokenRepository = app(RefreshTokenRepository::class); - - foreach ($tokens as $token) { - $tokenRepository->revokeAccessToken($token->id); - $refreshTokenRepository->revokeRefreshTokensByAccessTokenId($token->id); - } - - return response()->json([ - 'status' => 'success', - 'message' => \trans("app.companion-deleteall-success"), - ]); - } - /** * List devices. * @@ -139,7 +169,9 @@ // Process the result $result = $result->map( function ($device) { - return $device->toArray(); + return array_merge($device->toArray(), [ + 'isReady' => $device->isPaired() + ]); } ); @@ -171,7 +203,11 @@ return $this->errorResponse(403); } - return response()->json($result->toArray()); + return response()->json(array_merge($result->toArray(), [ + 'statusInfo' => [ + 'isReady' => $result->isPaired() + ] + ])); } /** @@ -179,9 +215,17 @@ * * @return \Illuminate\Http\JsonResponse */ - public function pairing() + public function pairing($id) { + $result = \App\CompanionApp::find($id); + if (!$result) { + return $this->errorResponse(404); + } + $user = $this->guard()->user(); + if ($user->id != $result->user_id) { + return $this->errorResponse(403); + } $clientIdentifier = \App\Tenant::getConfig($user->tenant_id, 'auth.companion_app.client_id'); $clientSecret = \App\Tenant::getConfig($user->tenant_id, 'auth.companion_app.client_secret'); @@ -190,11 +234,32 @@ return $this->errorResponse(500); } + if (!$result->oauth_client) { + $client = Passport::client()->forceFill([ + 'user_id' => $user->id, + 'name' => "CompanionApp Password Grant Client", + 'secret' => Str::random(40), + 'provider' => 'users', + 'redirect' => 'https://' . \config('app.website_domain'), + 'personal_access_client' => 0, + 'password_client' => 1, + 'revoked' => false, + 'allowed_scopes' => "mfa" + ]); + + $client->save(); + + $result->oauth_client = $client->id; + $result->save(); + } else { + $client = \App\PassportClient::find($result->oauth_client); + } $response['qrcode'] = self::generateQRCode( json_encode([ "serverUrl" => Utils::serviceUrl('', $user->tenant_id), - "clientIdentifier" => \App\Tenant::getConfig($user->tenant_id, 'auth.companion_app.client_id'), - "clientSecret" => \App\Tenant::getConfig($user->tenant_id, 'auth.companion_app.client_secret'), + "clientIdentifier" => $client->id, + "clientSecret" => $client->secret, + "companionId" => $id, "username" => $user->email ]) ); diff --git a/src/app/Http/Kernel.php b/src/app/Http/Kernel.php --- a/src/app/Http/Kernel.php +++ b/src/app/Http/Kernel.php @@ -69,6 +69,8 @@ 'signed' => \Illuminate\Routing\Middleware\ValidateSignature::class, 'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class, 'verified' => \Illuminate\Auth\Middleware\EnsureEmailIsVerified::class, + 'scopes' => \Laravel\Passport\Http\Middleware\CheckScopes::class, + 'scope' => \Laravel\Passport\Http\Middleware\CheckForAnyScope::class, ]; /** diff --git a/src/app/Observers/TokenObserver.php b/src/app/Observers/TokenObserver.php new file mode 100644 --- /dev/null +++ b/src/app/Observers/TokenObserver.php @@ -0,0 +1,23 @@ +client; + $scopes = $token->scopes; + if ($scopes) { + $allowedScopes = $client->getAllowedScopes(); + if (!empty($allowedScopes)) { + $scopes = array_intersect($scopes, $allowedScopes); + } + $scopes = array_unique($scopes, SORT_REGULAR); + $token->scopes = $scopes; + } + } +} diff --git a/src/app/PassportClient.php b/src/app/PassportClient.php new file mode 100644 --- /dev/null +++ b/src/app/PassportClient.php @@ -0,0 +1,24 @@ + The attributes that should be cast */ + protected $casts = [ + 'allowed_scopes' => 'array', + ]; + + public function getAllowedScopes(): array + { + if ($this->allowed_scopes) { + return $this->allowed_scopes; + } + return []; + } +} diff --git a/src/app/Providers/AuthServiceProvider.php b/src/app/Providers/AuthServiceProvider.php --- a/src/app/Providers/AuthServiceProvider.php +++ b/src/app/Providers/AuthServiceProvider.php @@ -42,8 +42,16 @@ } ); + Passport::tokensCan([ + 'api' => 'Access API', + 'mfa' => 'Access MFA api', + ]); + Passport::tokensExpireIn(now()->addMinutes(\config('auth.token_expiry_minutes'))); Passport::refreshTokensExpireIn(now()->addMinutes(\config('auth.refresh_token_expiry_minutes'))); Passport::personalAccessTokensExpireIn(now()->addMonths(6)); + + Passport::useClientModel(\App\PassportClient::class); + Passport::tokenModel()::observe(\App\Observers\TokenObserver::class); } } diff --git a/src/app/User.php b/src/app/User.php --- a/src/app/User.php +++ b/src/app/User.php @@ -686,6 +686,18 @@ return in_array(\App\Utils::countryForIP($ip), $countryCodes); } + /** + * Check if multi factor verification is enabled + * + * @return bool + */ + public function mfaEnabled(): bool + { + return \App\CompanionApp::where('user_id', $this->id) + ->where('mfa_enabled', true) + ->exists(); + } + /** * Retrieve and authenticate a user * @@ -695,7 +707,7 @@ * * @return array ['user', 'reason', 'errorMessage'] */ - public static function findAndAuthenticate($username, $password, $clientIP = null): array + public static function findAndAuthenticate($username, $password, $clientIP = null, $verifyMFA = true): array { $error = null; @@ -714,26 +726,28 @@ $error = AuthAttempt::REASON_PASSWORD; } - // Check user (request) location - if (!$error && !$user->validateLocation($clientIP)) { - $error = AuthAttempt::REASON_GEOLOCATION; - } + if ($verifyMFA) { + // Check user (request) location + if (!$error && !$user->validateLocation($clientIP)) { + $error = AuthAttempt::REASON_GEOLOCATION; + } - // Check 2FA - if (!$error) { - try { - (new \App\Auth\SecondFactor($user))->validate(request()->secondfactor); - } catch (\Exception $e) { - $error = AuthAttempt::REASON_2FA_GENERIC; - $message = $e->getMessage(); + // Check 2FA + if (!$error) { + try { + (new \App\Auth\SecondFactor($user))->validate(request()->secondfactor); + } catch (\Exception $e) { + $error = AuthAttempt::REASON_2FA_GENERIC; + $message = $e->getMessage(); + } } - } - // Check 2FA - Companion App - if (!$error && \App\CompanionApp::where('user_id', $user->id)->exists()) { - $attempt = \App\AuthAttempt::recordAuthAttempt($user, $clientIP); - if (!$attempt->waitFor2FA()) { - $error = AuthAttempt::REASON_2FA; + // Check 2FA - Companion App + if (!$error && $user->mfaEnabled()) { + $attempt = \App\AuthAttempt::recordAuthAttempt($user, $clientIP); + if (!$attempt->waitFor2FA()) { + $error = AuthAttempt::REASON_2FA; + } } } @@ -768,7 +782,15 @@ */ public static function findAndValidateForPassport($username, $password): User { - $result = self::findAndAuthenticate($username, $password); + \Log::info("findAndValidateForPassport {$username}: " . var_export(request(), true)); + $verifyMFA = true; + if (request()->scope == "mfa") { + \Log::info("Not validating MFA because this is a request for an mfa scope."); + // Don't verify MFA if this is only an mfa token. + // If we didn't do this, we couldn't pair backup devices. + $verifyMFA = false; + } + $result = self::findAndAuthenticate($username, $password, null, $verifyMFA); if (isset($result['reason'])) { if ($result['reason'] == AuthAttempt::REASON_2FA_GENERIC) { diff --git a/src/app/Utils.php b/src/app/Utils.php --- a/src/app/Utils.php +++ b/src/app/Utils.php @@ -505,6 +505,7 @@ 'app.webmail_url', 'app.support_email', 'app.company.copyright', + 'app.companion_download_link', 'mail.from.address' ]; diff --git a/src/config/app.php b/src/config/app.php --- a/src/config/app.php +++ b/src/config/app.php @@ -277,5 +277,9 @@ 'woat_ns1' => env('WOAT_NS1', 'ns01.' . env('APP_DOMAIN')), 'woat_ns2' => env('WOAT_NS2', 'ns02.' . env('APP_DOMAIN')), - 'ratelimit_whitelist' => explode(',', env('RATELIMIT_WHITELIST', '')) + 'ratelimit_whitelist' => explode(',', env('RATELIMIT_WHITELIST', '')), + 'companion_download_link' => env( + 'COMPANION_DOWNLOAD_LINK', + "https://mirror.apheleia-it.ch/pub/companion-app-beta.apk" + ) ]; diff --git a/src/database/migrations/2022_11_04_120000_companion_app_uuids_oauth_client.php b/src/database/migrations/2022_11_04_120000_companion_app_uuids_oauth_client.php new file mode 100644 --- /dev/null +++ b/src/database/migrations/2022_11_04_120000_companion_app_uuids_oauth_client.php @@ -0,0 +1,45 @@ +bigInteger('id')->change(); + $table->string('oauth_client', 36)->nullable(); + $table->foreign('oauth_client')->references('id')->on('oauth_clients')->onDelete('set null'); + } + ); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + \App\CompanionApp::truncate(); + Schema::table( + 'companion_apps', + function (Blueprint $table) { + $table->bigIncrements('id')->change(); + $table->dropForeign(['oauth_client']); + $table->dropColumn('oauth_client'); + } + ); + } +} diff --git a/src/database/migrations/2022_11_04_130000_oauth_client_scopes.php b/src/database/migrations/2022_11_04_130000_oauth_client_scopes.php new file mode 100644 --- /dev/null +++ b/src/database/migrations/2022_11_04_130000_oauth_client_scopes.php @@ -0,0 +1,39 @@ +string('allowed_scopes')->nullable(); + } + ); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table( + 'oauth_clients', + function (Blueprint $table) { + $table->dropColumn('allowed_scopes'); + } + ); + } +} diff --git a/src/resources/js/user/routes.js b/src/resources/js/user/routes.js --- a/src/resources/js/user/routes.js +++ b/src/resources/js/user/routes.js @@ -8,7 +8,8 @@ // Note: you can pack multiple components into the same chunk, webpackChunkName // is also used to get a sensible file name instead of numbers -const CompanionAppComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/CompanionApp') +const CompanionAppInfoComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/CompanionApp/Info') +const CompanionAppListComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/CompanionApp/List') const DashboardComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/Dashboard') const DistlistInfoComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/Distlist/Info') const DistlistListComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/Distlist/List') @@ -50,9 +51,15 @@ meta: { requiresAuth: true, perm: 'distlists' } }, { - path: '/companion', + path: '/companion/:companion', name: 'companion', - component: CompanionAppComponent, + component: CompanionAppInfoComponent, + meta: { requiresAuth: true, perm: 'companionapps' } + }, + { + path: '/companions', + name: 'companions', + component: CompanionAppListComponent, meta: { requiresAuth: true, perm: 'companionapps' } }, { diff --git a/src/resources/lang/en/app.php b/src/resources/lang/en/app.php --- a/src/resources/lang/en/app.php +++ b/src/resources/lang/en/app.php @@ -20,7 +20,8 @@ 'chart-payers' => 'Payers - last year', 'chart-users' => 'Users - last 8 weeks', - 'companion-deleteall-success' => 'All companion apps have been removed.', + 'companion-create-success' => 'Companion app has been created.', + 'companion-delete-success' => 'Companion app has been removed.', 'mandate-delete-success' => 'The auto-payment has been removed.', 'mandate-update-success' => 'The auto-payment has been updated.', diff --git a/src/resources/lang/en/ui.php b/src/resources/lang/en/ui.php --- a/src/resources/lang/en/ui.php +++ b/src/resources/lang/en/ui.php @@ -40,19 +40,39 @@ ], 'companion' => [ - 'title' => "Companion App", + 'title' => "Companion Apps", + 'companion' => "Companion App", 'name' => "Name", - 'description' => "Use the Companion App on your mobile phone for advanced two factor authentication.", + 'create' => "Pair new device", + 'create-recovery-device' => "Prepare recovery device", + 'description' => "Use the Companion App on your mobile phone for as multi factor authentication device.", + 'download-description' => "You may download the Companion App for Android here: " + . "Download", + 'description-detailed' => "Here is how this works: " . + "Pairing a device will automatically enable multi-factor autentication for all login attempts. " . + "This includes not only the Cockpit, but also logins via Webmail, IMAP, SMPT, DAV and ActiveSync. " . + "Any authentication attempt will result in a notification on your device, " . + "that you can use to confirm/deny if this was you. " . + "Once confirmed, the same username + IP combination will be whitelisted for 8 hours. " . + "Unpair all your active devices to disable multi-factor authentication again.", + 'description-warning' => "Warning: Loosing access to all your multi-factor authentication devices, " . + "will permanently lock you out of your account with no course for recovery. " . + "Always make sure you have a recovery QR-Code printed to pair a recovery device.", 'pair-new' => "Pair new device", + 'new' => "Pair new device", + 'recovery' => "Prepare recovery device", 'paired' => "Paired devices", - 'pairing-instructions' => "Pair a new device using the following QR-Code:", + 'print' => "Print for backup", + 'pairing-instructions' => "Pair your device using the following QR-Code.", + 'recovery-device' => "Recovery Device", 'deviceid' => "Device ID", 'list-empty' => "There are currently no devices", - 'delete' => "Remove devices", - 'remove-devices' => "Remove Devices", - 'remove-devices-text' => "Do you really want to remove all devices permanently?" - . " Please note that this action cannot be undone, and you can only remove all devices together." - . " You may pair devices you would like to keep individually again.", + 'delete' => "Delete/Unpair", + 'delete-companion' => "Delete/Unpair", + 'delete-text' => "You are about to delete this entry and unpair any paired companion app. " . + "This cannot be undone, but you can re-pair the device.", + 'pairing-successful' => "Your companion app is paired and ready to be used " . + "as multi-factor authentication device.", ], 'dashboard' => [ @@ -151,6 +171,7 @@ 'anyone' => "Anyone", 'code' => "Confirmation Code", 'config' => "Configuration", + 'companion' => "Companion App", 'date' => "Date", 'description' => "Description", 'details' => "Details", diff --git a/src/resources/vue/CompanionApp.vue b/src/resources/vue/CompanionApp.vue deleted file mode 100644 --- a/src/resources/vue/CompanionApp.vue +++ /dev/null @@ -1,59 +0,0 @@ - - - diff --git a/src/resources/vue/CompanionApp/Info.vue b/src/resources/vue/CompanionApp/Info.vue new file mode 100644 --- /dev/null +++ b/src/resources/vue/CompanionApp/Info.vue @@ -0,0 +1,126 @@ + + + diff --git a/src/resources/vue/CompanionApp/List.vue b/src/resources/vue/CompanionApp/List.vue new file mode 100644 --- /dev/null +++ b/src/resources/vue/CompanionApp/List.vue @@ -0,0 +1,65 @@ + + diff --git a/src/resources/vue/CompanionApp/ListWidget.vue b/src/resources/vue/CompanionApp/ListWidget.vue new file mode 100644 --- /dev/null +++ b/src/resources/vue/CompanionApp/ListWidget.vue @@ -0,0 +1,39 @@ + + + diff --git a/src/resources/vue/Dashboard.vue b/src/resources/vue/Dashboard.vue --- a/src/resources/vue/Dashboard.vue +++ b/src/resources/vue/Dashboard.vue @@ -42,7 +42,7 @@ {{ $t('dashboard.webmail') }} - + {{ $t('dashboard.companion') }} {{ $t('dashboard.beta') }} diff --git a/src/routes/api.php b/src/routes/api.php --- a/src/routes/api.php +++ b/src/routes/api.php @@ -60,21 +60,30 @@ Route::group( [ 'domain' => \config('app.website_domain'), - 'middleware' => 'auth:api', + 'middleware' => ['auth:api', 'scope:mfa,api'], 'prefix' => 'v4' ], function () { - Route::post('companion/register', [API\V4\CompanionAppsController::class, 'register']); - Route::post('auth-attempts/{id}/confirm', [API\V4\AuthAttemptsController::class, 'confirm']); Route::post('auth-attempts/{id}/deny', [API\V4\AuthAttemptsController::class, 'deny']); Route::get('auth-attempts/{id}/details', [API\V4\AuthAttemptsController::class, 'details']); Route::get('auth-attempts', [API\V4\AuthAttemptsController::class, 'index']); - Route::get('companion/pairing', [API\V4\CompanionAppsController::class, 'pairing']); - Route::apiResource('companion', API\V4\CompanionAppsController::class); Route::post('companion/register', [API\V4\CompanionAppsController::class, 'register']); - Route::post('companion/revoke', [API\V4\CompanionAppsController::class, 'revokeAll']); + } +); + +Route::group( + [ + 'domain' => \config('app.website_domain'), + 'middleware' => ['auth:api', 'scope:api'], + 'prefix' => 'v4' + ], + function () { + Route::apiResource('companions', API\V4\CompanionAppsController::class); + // This must not be accessible with the 2fa token, + // to prevent an attacker from pairing a new device with a stolen token. + Route::get('companions/{id}/pairing', [API\V4\CompanionAppsController::class, 'pairing']); Route::apiResource('domains', API\V4\DomainsController::class); Route::get('domains/{id}/confirm', [API\V4\DomainsController::class, 'confirm']); diff --git a/src/tests/Feature/Controller/CompanionAppsTest.php b/src/tests/Feature/Controller/CompanionAppsTest.php --- a/src/tests/Feature/Controller/CompanionAppsTest.php +++ b/src/tests/Feature/Controller/CompanionAppsTest.php @@ -5,6 +5,7 @@ use App\User; use App\CompanionApp; use Laravel\Passport\Token; +use Laravel\Passport\Passport; use Laravel\Passport\TokenRepository; use Tests\TestCase; @@ -35,60 +36,111 @@ } /** - * Test registering the app + * Test creating the app */ - public function testRegister(): void + public function testStore(): void { $user = $this->getTestUser('CompanionAppsTest1@userscontroller.com'); - $notificationToken = "notificationToken"; - $deviceId = "deviceId"; $name = "testname"; - $response = $this->actingAs($user)->post( - "api/v4/companion/register", - ['notificationToken' => $notificationToken, 'deviceId' => $deviceId, 'name' => $name] - ); - + $post = ['name' => $name]; + $response = $this->actingAs($user)->post("api/v4/companions", $post); $response->assertStatus(200); - $companionApp = \App\CompanionApp::where('device_id', $deviceId)->first(); + $json = $response->json(); + + $this->assertCount(2, $json); + $this->assertSame('success', $json['status']); + $this->assertSame("Companion app has been created.", $json['message']); + + $companionApp = \App\CompanionApp::where('name', $name)->first(); $this->assertTrue($companionApp != null); - $this->assertEquals($deviceId, $companionApp->device_id); $this->assertEquals($name, $companionApp->name); - $this->assertEquals($notificationToken, $companionApp->notification_token); + $this->assertFalse((bool)$companionApp->mfa_enabled); + } - // Test a token update - $notificationToken = "notificationToken2"; - $response = $this->actingAs($user)->post( - "api/v4/companion/register", - ['notificationToken' => $notificationToken, 'deviceId' => $deviceId, 'name' => $name] + /** + * Test destroying the app + */ + public function testDestroy(): void + { + $user = $this->getTestUser('CompanionAppsTest1@userscontroller.com'); + $user2 = $this->getTestUser('CompanionAppsTest2@userscontroller.com'); + + $response = $this->actingAs($user)->delete("api/v4/companions/foobar"); + $response->assertStatus(404); + + $companionApp = $this->getTestCompanionApp( + 'testdevice', + $user, + [ + 'notification_token' => 'notificationtoken', + 'mfa_enabled' => 1, + 'name' => 'testname', + ] ); - $response->assertStatus(200); + $client = Passport::client()->forceFill([ + 'user_id' => $user->id, + 'name' => "CompanionApp Password Grant Client", + 'secret' => "VerySecret", + 'provider' => 'users', + 'redirect' => 'https://' . \config('app.website_domain'), + 'personal_access_client' => 0, + 'password_client' => 1, + 'revoked' => false, + 'allowed_scopes' => ["mfa"] + ]); + print(var_export($client, true)); + $client->save(); + $companionApp->oauth_client = $client->id; + $companionApp->save(); - $companionApp->refresh(); - $this->assertEquals($notificationToken, $companionApp->notification_token); + $tokenRepository = app(TokenRepository::class); + $tokenRepository->create([ + 'id' => 'testtoken', + 'revoked' => false, + 'user_id' => $user->id, + 'client_id' => $client->id + ]); - // Failing input valdiation - $response = $this->actingAs($user)->post( - "api/v4/companion/register", - [] - ); - $response->assertStatus(422); + //Make sure we have a token to revoke + $tokenCount = Token::where('user_id', $user->id)->where('client_id', $client->id)->count(); + $this->assertTrue($tokenCount > 0); - // Other users device - $user2 = $this->getTestUser('CompanionAppsTest2@userscontroller.com'); - $response = $this->actingAs($user2)->post( - "api/v4/companion/register", - ['notificationToken' => $notificationToken, 'deviceId' => $deviceId, 'name' => $name] - ); + + $response = $this->actingAs($user2)->delete("api/v4/companions/{$companionApp->id}"); $response->assertStatus(403); + + $response = $this->actingAs($user)->delete("api/v4/companions/{$companionApp->id}"); + $response->assertStatus(200); + + $json = $response->json(); + + $this->assertCount(2, $json); + $this->assertSame('success', $json['status']); + $this->assertSame("Companion app has been removed.", $json['message']); + + $client->refresh(); + $this->assertSame((bool)$client->revoked, true); + + $companionApp = \App\CompanionApp::where('device_id', 'testdevice')->first(); + $this->assertTrue($companionApp == null); + + $tokenCount = Token::where('user_id', $user->id) + ->where('client_id', $client->id) + ->where('revoked', false)->count(); + $this->assertSame(0, $tokenCount); } + + /** + * Test listing apps + */ public function testIndex(): void { - $response = $this->get("api/v4/companion"); + $response = $this->get("api/v4/companions"); $response->assertStatus(401); $user = $this->getTestUser('CompanionAppsTest1@userscontroller.com'); @@ -103,7 +155,7 @@ ] ); - $response = $this->actingAs($user)->get("api/v4/companion"); + $response = $this->actingAs($user)->get("api/v4/companions"); $response->assertStatus(200); $json = $response->json(); @@ -117,7 +169,7 @@ $user2 = $this->getTestUser('CompanionAppsTest2@userscontroller.com'); $response = $this->actingAs($user2)->get( - "api/v4/companion" + "api/v4/companions" ); $response->assertStatus(200); @@ -126,75 +178,132 @@ $this->assertCount(0, $json['list']); } + + /** + * Test showing the app + */ public function testShow(): void { $user = $this->getTestUser('CompanionAppsTest1@userscontroller.com'); $companionApp = $this->getTestCompanionApp('testdevice', $user); - $response = $this->get("api/v4/companion/{$companionApp->id}"); + $response = $this->get("api/v4/companions/{$companionApp->id}"); $response->assertStatus(401); - $response = $this->actingAs($user)->get("api/v4/companion/aaa"); + $response = $this->actingAs($user)->get("api/v4/companions/aaa"); $response->assertStatus(404); - $response = $this->actingAs($user)->get("api/v4/companion/{$companionApp->id}"); + $response = $this->actingAs($user)->get("api/v4/companions/{$companionApp->id}"); $response->assertStatus(200); $json = $response->json(); $this->assertSame($companionApp->id, $json['id']); $user2 = $this->getTestUser('CompanionAppsTest2@userscontroller.com'); - $response = $this->actingAs($user2)->get("api/v4/companion/{$companionApp->id}"); + $response = $this->actingAs($user2)->get("api/v4/companions/{$companionApp->id}"); $response->assertStatus(403); } - public function testPairing(): void - { - $response = $this->get("api/v4/companion/pairing"); - $response->assertStatus(401); + /** + * Test registering the app + */ + public function testRegister(): void + { $user = $this->getTestUser('CompanionAppsTest1@userscontroller.com'); - $response = $this->actingAs($user)->get("api/v4/companion/pairing"); + + $companionApp = $this->getTestCompanionApp( + 'testdevice', + $user, + [ + 'notification_token' => 'notificationtoken', + 'mfa_enabled' => 0, + 'name' => 'testname', + ] + ); + + $notificationToken = "notificationToken"; + $deviceId = "deviceId"; + $name = "testname"; + + $response = $this->actingAs($user)->post( + "api/v4/companion/register", + ['notificationToken' => $notificationToken, 'deviceId' => $deviceId, 'name' => $name, 'companionId' => $companionApp->id] + ); + $response->assertStatus(200); - $json = $response->json(); - $this->assertArrayHasKey('qrcode', $json); - $this->assertSame('data:image/svg+xml;base64,', substr($json['qrcode'], 0, 26)); + $companionApp->refresh(); + $this->assertTrue($companionApp != null); + $this->assertEquals($deviceId, $companionApp->device_id); + $this->assertEquals($name, $companionApp->name); + $this->assertEquals($notificationToken, $companionApp->notification_token); + $this->assertTrue((bool)$companionApp->mfa_enabled); + + // Companion id required + $response = $this->actingAs($user)->post( + "api/v4/companion/register", + ['notificationToken' => $notificationToken, 'deviceId' => $deviceId, 'name' => $name] + ); + $response->assertStatus(422); + + // Test a token update + $notificationToken = "notificationToken2"; + $response = $this->actingAs($user)->post( + "api/v4/companion/register", + ['notificationToken' => $notificationToken, 'deviceId' => $deviceId, 'name' => $name, 'companionId' => $companionApp->id] + ); + + $response->assertStatus(200); + + $companionApp->refresh(); + $this->assertEquals($notificationToken, $companionApp->notification_token); + + // Failing input valdiation + $response = $this->actingAs($user)->post( + "api/v4/companion/register", + [] + ); + $response->assertStatus(422); + + // Other users device + $user2 = $this->getTestUser('CompanionAppsTest2@userscontroller.com'); + $response = $this->actingAs($user2)->post( + "api/v4/companion/register", + ['notificationToken' => $notificationToken, 'deviceId' => $deviceId, 'name' => $name, 'companionId' => $companionApp->id] + ); + $response->assertStatus(403); } - public function testRevoke(): void + + /** + * Test getting the pairing info + */ + public function testPairing(): void { $user = $this->getTestUser('CompanionAppsTest1@userscontroller.com'); - $companionApp = $this->getTestCompanionApp('testdevice', $user); - $clientIdentifier = \App\Tenant::getConfig($user->tenant_id, 'auth.companion_app.client_id'); - $tokenRepository = app(TokenRepository::class); - $tokenRepository->create([ - 'id' => 'testtoken', - 'revoked' => false, - 'user_id' => $user->id, - 'client_id' => $clientIdentifier - ]); - - //Make sure we have a token to revoke - $tokenCount = Token::where('user_id', $user->id)->where('client_id', $clientIdentifier)->count(); - $this->assertTrue($tokenCount > 0); + $companionApp = $this->getTestCompanionApp( + 'testdevice', + $user, + [ + 'notification_token' => 'notificationtoken', + 'mfa_enabled' => 0, + 'name' => 'testname', + ] + ); - $response = $this->post("api/v4/companion/revoke"); + $response = $this->get("api/v4/companions/{$companionApp->id}/pairing"); $response->assertStatus(401); - $response = $this->actingAs($user)->post("api/v4/companion/revoke"); + $response = $this->actingAs($user)->get("api/v4/companions/{$companionApp->id}/pairing"); $response->assertStatus(200); - $json = $response->json(); - $this->assertSame('success', $json['status']); - $this->assertArrayHasKey('message', $json); - $companionApp = \App\CompanionApp::where('device_id', 'testdevice')->first(); - $this->assertTrue($companionApp == null); + $companionApp->refresh(); + $this->assertTrue($companionApp->oauth_client != null); - $tokenCount = Token::where('user_id', $user->id) - ->where('client_id', $clientIdentifier) - ->where('revoked', false)->count(); - $this->assertSame(0, $tokenCount); + $json = $response->json(); + $this->assertArrayHasKey('qrcode', $json); + $this->assertSame('data:image/svg+xml;base64,', substr($json['qrcode'], 0, 26)); } } diff --git a/src/tests/TestCase.php b/src/tests/TestCase.php --- a/src/tests/TestCase.php +++ b/src/tests/TestCase.php @@ -4,6 +4,8 @@ use Illuminate\Foundation\Testing\TestCase as BaseTestCase; use Illuminate\Routing\Middleware\ThrottleRequests; +use Illuminate\Contracts\Auth\Authenticatable; +use Laravel\Passport\Passport; abstract class TestCase extends BaseTestCase { @@ -20,6 +22,18 @@ $this->withoutMiddleware(ThrottleRequests::class); } + /** + * Set the user as which we want to authenticate + */ + public function actingAs(Authenticatable $user, $guard = null) + { + Passport::actingAs( + $user, + ['api'] + ); + return parent::actingAs($user, $guard); + } + /** * Set baseURL to the regular UI location */