Skip to content

Commit

Permalink
fix: fix import vcard stability (#5160)
Browse files Browse the repository at this point in the history
  • Loading branch information
asbiin authored May 3, 2021
1 parent 128e6e2 commit 3f2821d
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 47 deletions.
12 changes: 0 additions & 12 deletions app/Exceptions/AccountLimitException.php

This file was deleted.

1 change: 0 additions & 1 deletion app/Exceptions/Handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ class Handler extends ExceptionHandler
* @var array
*/
protected $dontReport = [
AccountLimitException::class,
OAuthServerException::class,
WrongIdException::class,
];
Expand Down
11 changes: 11 additions & 0 deletions app/Http/Controllers/Api/Contact/ApiDocumentController.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@

class ApiDocumentController extends ApiController
{
/**
* Instantiate a new controller instance.
*
* @return void
*/
public function __construct()
{
$this->middleware('limitations')->only('store');
parent::__construct();
}

/**
* Get the list of documents.
*
Expand Down
10 changes: 10 additions & 0 deletions app/Http/Controllers/Contacts/DocumentsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ class DocumentsController extends Controller
{
use JsonRespondController;

/**
* Instantiate a new controller instance.
*
* @return void
*/
public function __construct()
{
$this->middleware('limitations')->only('store');
}

/**
* Display the list of documents.
*
Expand Down
26 changes: 12 additions & 14 deletions app/Http/Controllers/SettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,24 @@
use LaravelWebauthn\Models\WebauthnKey;
use App\Http\Requests\InvitationRequest;
use App\Services\Contact\Tag\DestroyTag;
use App\Exceptions\AccountLimitException;
use App\Services\Account\Settings\ResetAccount;
use App\Services\Account\Settings\DestroyAccount;
use PragmaRX\Google2FALaravel\Facade as Google2FA;
use App\Http\Resources\Contact\ContactShort as ContactResource;
use App\Http\Resources\Settings\WebauthnKey\WebauthnKey as WebauthnKeyResource;

class SettingsController
class SettingsController extends Controller
{
/**
* Instantiate a new controller instance.
*
* @return void
*/
public function __construct()
{
$this->middleware('limitations')->only(['inviteUser', 'storeImport']);
}

/**
* Display a listing of the resource.
*
Expand Down Expand Up @@ -233,16 +242,9 @@ public function upload()

public function storeImport(ImportsRequest $request)
{
$account = auth()->user()->account;
if (AccountHelper::hasReachedContactLimit($account)
&& AccountHelper::hasLimitations($account)
&& ! $account->legacy_free_plan_unlimited_contacts) {
throw new AccountLimitException();
}

$filename = $request->file('vcard')->store('imports', config('filesystems.default'));

$importJob = $account->importjobs()->create([
$importJob = auth()->user()->account->importjobs()->create([
'user_id' => auth()->user()->id,
'type' => 'vcard',
'filename' => $filename,
Expand Down Expand Up @@ -308,10 +310,6 @@ public function addUser()
*/
public function inviteUser(InvitationRequest $request)
{
if (AccountHelper::hasLimitations(auth()->user()->account)) {
throw new AccountLimitException();
}

// Make sure the confirmation to invite has not been bypassed
if (! $request->input('confirmation')) {
return redirect()->back()->withErrors(trans('settings.users_error_please_confirm'))->withInput();
Expand Down
41 changes: 28 additions & 13 deletions app/Models/Account/ImportJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use App\Models\User\User;
use Sabre\VObject\Reader;
use Illuminate\Support\Arr;
use App\Helpers\AccountHelper;
use Sabre\VObject\Component\VCard;
use App\Services\VCard\ImportVCard;
use Illuminate\Database\Eloquent\Model;
Expand Down Expand Up @@ -49,7 +50,7 @@ class ImportJob extends Model
*
* @var VCardReader
*/
public $entries;
public $entries = null;

/**
* The attributes that aren't mass assignable.
Expand Down Expand Up @@ -104,15 +105,17 @@ public function process($behaviour = ImportVCard::BEHAVIOUR_ADD)
{
$this->initJob();

$this->getPhysicalFile();
if (! $this->failed && $this->getPhysicalFile()) {
$this->getEntries();

$this->getEntries();

$this->processEntries($behaviour);
$this->processEntries($behaviour);
}

$this->deletePhysicalFile();

$this->endJob();
if (! $this->failed) {
$this->endJob();
}
}

/**
Expand All @@ -122,6 +125,10 @@ public function process($behaviour = ImportVCard::BEHAVIOUR_ADD)
*/
private function initJob(): void
{
if (AccountHelper::hasLimitations($this->account)) {
$this->fail(trans('auth.not_authorized'));
}

$this->started_at = now();
$this->contacts_imported = 0;
$this->contacts_skipped = 0;
Expand Down Expand Up @@ -149,36 +156,44 @@ private function endJob(): void
private function fail(string $reason): void
{
$this->failed = true;
$this->failed_reason = $reason;
if (! $this->failed_reason) {
$this->failed_reason = $reason;
}
$this->endJob();
}

/**
* Get the physical file (the vCard file).
*
* @return self
* @return bool
*/
private function getPhysicalFile()
private function getPhysicalFile(): bool
{
try {
$this->physicalFile = Storage::disk(config('filesystems.default'))->readStream($this->filename);
} catch (FileNotFoundException $exception) {
$this->fail(trans('settings.import_vcard_file_not_found'));

return false;
}

return $this;
return true;
}

/**
* Delete the physical file from the disk.
*
* @return void
* @return bool
*/
private function deletePhysicalFile(): void
private function deletePhysicalFile(): bool
{
if (! Storage::disk(config('filesystems.default'))->delete($this->filename)) {
$this->fail(trans('settings.import_vcard_file_not_found'));

return false;
}

return true;
}

/**
Expand All @@ -202,7 +217,7 @@ private function processEntries($behaviour = ImportVCard::BEHAVIOUR_ADD)
while (true) {
try {
/** @var VCard|null */
$entry = $this->entries->getNext();
$entry = $this->entries !== null ? $this->entries->getNext() : null;
if (! $entry) {
// file end
break;
Expand Down
3 changes: 1 addition & 2 deletions app/Services/Contact/Contact/CreateContact.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use App\Models\Account\Account;
use App\Models\Contact\Contact;
use App\Jobs\AuditLog\LogAccountAudit;
use App\Exceptions\AccountLimitException;
use App\Jobs\Avatars\GenerateDefaultAvatar;
use App\Jobs\Avatars\GetAvatarsFromInternet;

Expand Down Expand Up @@ -64,7 +63,7 @@ public function execute(array $data): Contact
if (AccountHelper::hasReachedContactLimit($account)
&& AccountHelper::hasLimitations($account)
&& ! $account->legacy_free_plan_unlimited_contacts) {
throw new AccountLimitException();
abort(402);
}

// filter out the data that shall not be updated here
Expand Down
7 changes: 2 additions & 5 deletions app/Services/Contact/Document/UploadDocument.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use App\Models\Account\Account;
use App\Models\Contact\Contact;
use App\Models\Contact\Document;
use App\Exceptions\AccountLimitException;

class UploadDocument extends BaseService
{
Expand Down Expand Up @@ -36,10 +35,8 @@ public function execute(array $data): Document
$this->validate($data);

$account = Account::find($data['account_id']);
if (AccountHelper::hasReachedContactLimit($account)
&& AccountHelper::hasLimitations($account)
&& ! $account->legacy_free_plan_unlimited_contacts) {
throw new AccountLimitException();
if (AccountHelper::hasLimitations($account)) {
abort(402);
}

Contact::where('account_id', $data['account_id'])
Expand Down

0 comments on commit 3f2821d

Please sign in to comment.