From 0d4851a2e55edf9af2de1bb50d33325df6356141 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Tue, 4 Feb 2025 09:16:01 +0100
Subject: [PATCH 01/25] Add s3 config validation

---
 .../Controllers/Api/UserDiskController.php    | 79 ++++++++++++++++++-
 1 file changed, 77 insertions(+), 2 deletions(-)

diff --git a/src/Http/Controllers/Api/UserDiskController.php b/src/Http/Controllers/Api/UserDiskController.php
index 2afdf37..afe347a 100644
--- a/src/Http/Controllers/Api/UserDiskController.php
+++ b/src/Http/Controllers/Api/UserDiskController.php
@@ -2,11 +2,15 @@
 
 namespace Biigle\Modules\UserDisks\Http\Controllers\Api;
 
+use Exception;
+use Illuminate\Support\Arr;
+use Illuminate\Support\Str;
+use Biigle\Modules\UserDisks\UserDisk;
+use Illuminate\Support\Facades\Storage;
 use Biigle\Http\Controllers\Api\Controller;
 use Biigle\Modules\UserDisks\Http\Requests\StoreUserDisk;
-use Biigle\Modules\UserDisks\Http\Requests\UpdateUserDisk;
 use Biigle\Modules\UserDisks\Http\Requests\ExtendUserDisk;
-use Biigle\Modules\UserDisks\UserDisk;
+use Biigle\Modules\UserDisks\Http\Requests\UpdateUserDisk;
 
 class UserDiskController extends Controller
 {
@@ -47,6 +51,14 @@ public function store(StoreUserDisk $request)
             return $disk;
         }
 
+        $errors = $this->validateS3Config($disk);
+        if ($errors) {
+            $disk->delete();
+            return $this->fuzzyRedirect()
+                ->withErrors($errors)
+                ->withInput();
+        }
+
         return $this->fuzzyRedirect('storage-disks')
             ->with('message', 'Storage disk created')
             ->with('messageType', 'success');
@@ -83,6 +95,13 @@ public function update(UpdateUserDisk $request)
             $request->getDiskOptions()
         );
 
+        $errors = $this->validateS3Config($request->disk);
+        if ($errors) {
+            return $this->fuzzyRedirect()
+                ->withErrors($errors)
+                ->withInput();
+        }
+
         $request->disk->save();
 
         if (!$this->isAutomatedRequest()) {
@@ -142,4 +161,60 @@ public function destroy($id)
                 ->with('messageType', 'success');
         }
     }
+    protected function validateS3Config(UserDisk $disk)
+    {
+        if ($disk->type != 's3') {
+            return [];
+        }
+
+        $errors = [];
+        $options = $disk->options;
+        $use_path_style_endpoint = $options['use_path_style_endpoint'];
+        $endpoint = $options['endpoint'];
+        $bucket = $options['bucket'];
+
+        // Check if endpoint contains bucket name
+        if (preg_match("/(\b\/" . $bucket . "\.|\b" . $bucket . "\b)/", $endpoint)) {
+            // Remove bucket name from end of url, because Storage::build() adds bucket name if url has path style
+            if ($use_path_style_endpoint && Str::endsWith($endpoint, $bucket)) {
+                $endpoint = Str::chopEnd($endpoint, $bucket);
+            }
+        } else {
+            $errors['endpoint'] = 'Missing or incorrect bucket name';
+        }
+
+        try {
+            // Use build() instead of disk(), because it is easier to configure and revert erroneous update requests
+            $disk = Storage::build([
+                'driver' => 's3',
+                'key' => $options['key'],
+                'secret' => $options['secret'],
+                'region' => Arr::has($options, 'region') ? $options['region'] : '',
+                'bucket' => $bucket,
+                'endpoint' => $endpoint,
+                'use_path_style_endpoint' => $use_path_style_endpoint,
+                'http' => ['connect_timeout' => 5],
+            ]);
+            $disk->allFiles();
+        } catch (Exception $e) {
+            $msg = $e->getMessage();
+            if (Str::contains($msg, "region", true)) {
+                $errors['region'] = 'Region required';
+            } else if (Str::contains($msg, 'timeout', true)) {
+                $errors['endpoint'] = 'Endpoint url incorrect or currently not available';
+            } else if(Str::contains($msg, 'cURL error', true)){
+                $errors['endpoint'] = 'Endpoint url incorrect';
+            } else if (
+                !Arr::has($errors, 'endpoint')
+                && Str::contains($msg, ["AccessDenied", "NoSuchBucket", "NoSuchKey"], true)
+            ) {
+                $errors['error'] = 'Access denied. Please check if your input is correct';
+            }
+
+            if (empty($errors)) {
+                $errors['error'] = 'An error occurred. Please check if your input is correct';
+            }
+        }
+        return $errors;
+    }
 }

From d1c3dc6456e7fb31613dfdb9453c2df861a8a32e Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Tue, 4 Feb 2025 09:16:23 +0100
Subject: [PATCH 02/25] Show error message if s3 config is invalid

---
 src/resources/views/store.blade.php  | 9 +++++++++
 src/resources/views/update.blade.php | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/src/resources/views/store.blade.php b/src/resources/views/store.blade.php
index c27a3e6..2e1cf09 100644
--- a/src/resources/views/store.blade.php
+++ b/src/resources/views/store.blade.php
@@ -44,6 +44,15 @@
                     </fieldset>
                 @endif
                 <div class="row">
+                    <div class="col-xs-12" @error('error') has-error @enderror>
+                        @error('error')
+                            <div class="panel panel-danger">
+                                <div class="panel-body text-danger">
+                                    {{$message}}
+                                </div>
+                            </div>
+                        @enderror
+                    </div>
                     <div class="col-xs-12">
                         @csrf
                         <div class="form-group clearfix">
diff --git a/src/resources/views/update.blade.php b/src/resources/views/update.blade.php
index 1e1a09d..b1d4d25 100644
--- a/src/resources/views/update.blade.php
+++ b/src/resources/views/update.blade.php
@@ -44,6 +44,15 @@
                     @include("user-disks::update.{$disk->type}")
                 </div>
                 <div class="row">
+                    <div class="col-xs-12" @error('error') has-error @enderror>
+                        @error('error')
+                            <div class="panel panel-danger">
+                                <div class="panel-body text-danger">
+                                    {{$message}}
+                                </div>
+                            </div>
+                        @enderror
+                    </div>
                     <div class="col-xs-12">
                         @csrf
                         @method('PUT')

From 7857719f67321aada709e06c2bdc189143a905ca Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Thu, 6 Feb 2025 09:05:17 +0100
Subject: [PATCH 03/25] Move help texts to only highlight errors

---
 src/resources/views/store/s3.blade.php  | 10 ++++++----
 src/resources/views/update/s3.blade.php |  6 +++---
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/resources/views/store/s3.blade.php b/src/resources/views/store/s3.blade.php
index 57c4400..afd9dd9 100644
--- a/src/resources/views/store/s3.blade.php
+++ b/src/resources/views/store/s3.blade.php
@@ -14,8 +14,10 @@
         @error('region')
             <p class="help-block">{{$message}}</p>
         @enderror
-        <p class="help-block">Leave empty if your cloud storage service does not support regions.</p>
     </div>
+    @if (!$errors->has('region'))
+        <p class="help-block">Leave empty if your cloud storage service does not support regions.</p>
+    @endif
 </div>
 <div class="col-xs-12">
     <div class="form-group @error('endpoint') has-error @enderror">
@@ -24,10 +26,10 @@
         @error('endpoint')
             <p class="help-block">{{$message}}</p>
         @enderror
-        <p class="help-block">
-            This must be the full URL including the bucket name, region etc.
-        </p>
     </div>
+    <p class="help-block">
+        This must be the full URL including the bucket name, region etc.
+    </p>
 </div>
 <div class="col-sm-6">
     <div class="form-group @error('key') has-error @enderror">
diff --git a/src/resources/views/update/s3.blade.php b/src/resources/views/update/s3.blade.php
index 37c4bca..03c9733 100644
--- a/src/resources/views/update/s3.blade.php
+++ b/src/resources/views/update/s3.blade.php
@@ -23,10 +23,10 @@
         @error('endpoint')
             <p class="help-block">{{$message}}</p>
         @enderror
-        <p class="help-block">
-            This must be the full URL including the bucket name, region etc.
-        </p>
     </div>
+    <p class="help-block">
+        This must be the full URL including the bucket name, region etc.
+    </p>
 </div>
 <div class="col-xs-12">
     <div class="panel panel-warning">

From 705a721999d72cb26036dfad939c234961ff1d97 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Tue, 11 Feb 2025 08:25:17 +0100
Subject: [PATCH 04/25] Rewrite code

Make code more elegant by using Storage::disk and DB::transaction.
---
 .../Controllers/Api/UserDiskController.php    | 60 ++++++++-----------
 1 file changed, 26 insertions(+), 34 deletions(-)

diff --git a/src/Http/Controllers/Api/UserDiskController.php b/src/Http/Controllers/Api/UserDiskController.php
index afe347a..c45ab8d 100644
--- a/src/Http/Controllers/Api/UserDiskController.php
+++ b/src/Http/Controllers/Api/UserDiskController.php
@@ -3,8 +3,8 @@
 namespace Biigle\Modules\UserDisks\Http\Controllers\Api;
 
 use Exception;
-use Illuminate\Support\Arr;
 use Illuminate\Support\Str;
+use Illuminate\Support\Facades\DB;
 use Biigle\Modules\UserDisks\UserDisk;
 use Illuminate\Support\Facades\Storage;
 use Biigle\Http\Controllers\Api\Controller;
@@ -89,21 +89,28 @@ public function store(StoreUserDisk $request)
      */
     public function update(UpdateUserDisk $request)
     {
-        $request->disk->name = $request->input('name', $request->disk->name);
-        $request->disk->options = array_merge(
-            $request->disk->options,
-            $request->getDiskOptions()
-        );
+        $errors = DB::transaction(function () use ($request) {
+            $request->disk->name = $request->input('name', $request->disk->name);
+            $request->disk->options = array_merge(
+                $request->disk->options,
+                $request->getDiskOptions()
+            );
+
+            $request->disk->save();
+
+            $errors = $this->validateS3Config($request->disk);
+            if ($errors) {
+                DB::rollBack();
+            }
+            return $errors;
+        });
 
-        $errors = $this->validateS3Config($request->disk);
         if ($errors) {
             return $this->fuzzyRedirect()
                 ->withErrors($errors)
                 ->withInput();
         }
 
-        $request->disk->save();
-
         if (!$this->isAutomatedRequest()) {
             return $this->fuzzyRedirect()
                 ->with('message', 'Storage disk updated')
@@ -169,45 +176,30 @@ protected function validateS3Config(UserDisk $disk)
 
         $errors = [];
         $options = $disk->options;
-        $use_path_style_endpoint = $options['use_path_style_endpoint'];
         $endpoint = $options['endpoint'];
         $bucket = $options['bucket'];
 
         // Check if endpoint contains bucket name
-        if (preg_match("/(\b\/" . $bucket . "\.|\b" . $bucket . "\b)/", $endpoint)) {
-            // Remove bucket name from end of url, because Storage::build() adds bucket name if url has path style
-            if ($use_path_style_endpoint && Str::endsWith($endpoint, $bucket)) {
-                $endpoint = Str::chopEnd($endpoint, $bucket);
-            }
-        } else {
-            $errors['endpoint'] = 'Missing or incorrect bucket name';
+        if (!preg_match("/(\b\/" . $bucket . "\.|\b" . $bucket . "\b)/", $endpoint)) {
+            $errors['endpoint'] = 'Endpoint url requires bucket name';
+            return $errors;
         }
 
         try {
-            // Use build() instead of disk(), because it is easier to configure and revert erroneous update requests
-            $disk = Storage::build([
-                'driver' => 's3',
-                'key' => $options['key'],
-                'secret' => $options['secret'],
-                'region' => Arr::has($options, 'region') ? $options['region'] : '',
-                'bucket' => $bucket,
-                'endpoint' => $endpoint,
-                'use_path_style_endpoint' => $use_path_style_endpoint,
-                'http' => ['connect_timeout' => 5],
-            ]);
-            $disk->allFiles();
+            $disk = Storage::disk("disk-{$disk->id}");
+            $files = $disk->getAdapter()->listContents('/', false);
+            // Need to access an element to check if endpoint url is valid
+            $files->current();
         } catch (Exception $e) {
             $msg = $e->getMessage();
+
             if (Str::contains($msg, "region", true)) {
                 $errors['region'] = 'Region required';
             } else if (Str::contains($msg, 'timeout', true)) {
                 $errors['endpoint'] = 'Endpoint url incorrect or currently not available';
-            } else if(Str::contains($msg, 'cURL error', true)){
+            } else if (Str::contains($msg, ['cURL error', 'Error parsing XML'], true)) {
                 $errors['endpoint'] = 'Endpoint url incorrect';
-            } else if (
-                !Arr::has($errors, 'endpoint')
-                && Str::contains($msg, ["AccessDenied", "NoSuchBucket", "NoSuchKey"], true)
-            ) {
+            } else if (Str::contains($msg, ["AccessDenied", "NoSuchBucket", "NoSuchKey"], true)) {
                 $errors['error'] = 'Access denied. Please check if your input is correct';
             }
 

From 7bf5a704db2f71594d4c29eb18b0d436d2b3deab Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Wed, 12 Feb 2025 10:19:31 +0100
Subject: [PATCH 05/25] Change error messages

---
 src/Http/Controllers/Api/UserDiskController.php | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/Http/Controllers/Api/UserDiskController.php b/src/Http/Controllers/Api/UserDiskController.php
index c45ab8d..89e5efe 100644
--- a/src/Http/Controllers/Api/UserDiskController.php
+++ b/src/Http/Controllers/Api/UserDiskController.php
@@ -181,30 +181,28 @@ protected function validateS3Config(UserDisk $disk)
 
         // Check if endpoint contains bucket name
         if (!preg_match("/(\b\/" . $bucket . "\.|\b" . $bucket . "\b)/", $endpoint)) {
-            $errors['endpoint'] = 'Endpoint url requires bucket name';
+            $errors['endpoint'] = 'Missing bucket name. Please check if the bucket name is present and spelled correctly.';
             return $errors;
         }
 
         try {
             $disk = Storage::disk("disk-{$disk->id}");
-            $files = $disk->getAdapter()->listContents('/', false);
+            $files = $disk->getAdapter()->listContents('', false);
             // Need to access an element to check if endpoint url is valid
             $files->current();
         } catch (Exception $e) {
             $msg = $e->getMessage();
 
-            if (Str::contains($msg, "region", true)) {
-                $errors['region'] = 'Region required';
-            } else if (Str::contains($msg, 'timeout', true)) {
-                $errors['endpoint'] = 'Endpoint url incorrect or currently not available';
+            if (Str::contains($msg, 'timeout', true)) {
+                $errors['endpoint'] = 'The endpoint URL could not be accessed. Does it exist?';
             } else if (Str::contains($msg, ['cURL error', 'Error parsing XML'], true)) {
-                $errors['endpoint'] = 'Endpoint url incorrect';
+                $errors['endpoint'] = 'This does not seem to be a valid S3 endpoint.';
             } else if (Str::contains($msg, ["AccessDenied", "NoSuchBucket", "NoSuchKey"], true)) {
-                $errors['error'] = 'Access denied. Please check if your input is correct';
+                $errors['error'] = 'The bucket could not be accessed. Please check for typos or missing access permissions.';
             }
 
             if (empty($errors)) {
-                $errors['error'] = 'An error occurred. Please check if your input is correct';
+                $errors['error'] = 'An error occurred. Please check if your input is correct.';
             }
         }
         return $errors;

From 8ece2ff24febffe8065f0e90d492dc3730a63b68 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Wed, 12 Feb 2025 10:49:09 +0100
Subject: [PATCH 06/25] Throw exception if s3 disk config is invalid

---
 .../Controllers/Api/UserDiskController.php    | 103 +++++++++---------
 1 file changed, 49 insertions(+), 54 deletions(-)

diff --git a/src/Http/Controllers/Api/UserDiskController.php b/src/Http/Controllers/Api/UserDiskController.php
index 89e5efe..1be442f 100644
--- a/src/Http/Controllers/Api/UserDiskController.php
+++ b/src/Http/Controllers/Api/UserDiskController.php
@@ -7,6 +7,7 @@
 use Illuminate\Support\Facades\DB;
 use Biigle\Modules\UserDisks\UserDisk;
 use Illuminate\Support\Facades\Storage;
+use Dotenv\Exception\ValidationException;
 use Biigle\Http\Controllers\Api\Controller;
 use Biigle\Modules\UserDisks\Http\Requests\StoreUserDisk;
 use Biigle\Modules\UserDisks\Http\Requests\ExtendUserDisk;
@@ -39,29 +40,31 @@ class UserDiskController extends Controller
      */
     public function store(StoreUserDisk $request)
     {
-        $disk = UserDisk::create([
-            'name' => $request->input('name'),
-            'type' => $request->input('type'),
-            'user_id' => $request->user()->id,
-            'expires_at' => now()->addMonths(config('user_disks.expires_months')),
-            'options' => $request->getDiskOptions(),
-        ]);
-
-        if ($this->isAutomatedRequest()) {
-            return $disk;
-        }
+        try {
+            DB::transaction(function () use ($request) {
+                $disk = UserDisk::create([
+                    'name' => $request->input('name'),
+                    'type' => $request->input('type'),
+                    'user_id' => $request->user()->id,
+                    'expires_at' => now()->addMonths(config('user_disks.expires_months')),
+                    'options' => $request->getDiskOptions(),
+                ]);
+
+                $this->validateS3Config($disk);
+
+                if ($this->isAutomatedRequest()) {
+                    return $disk;
+                }
+            });
 
-        $errors = $this->validateS3Config($disk);
-        if ($errors) {
-            $disk->delete();
+            return $this->fuzzyRedirect('storage-disks')
+                ->with('message', 'Storage disk created')
+                ->with('messageType', 'success');
+        } catch (ValidationException $e) {
             return $this->fuzzyRedirect()
-                ->withErrors($errors)
+                ->withErrors(['error' => $e->getMessage()])
                 ->withInput();
         }
-
-        return $this->fuzzyRedirect('storage-disks')
-            ->with('message', 'Storage disk created')
-            ->with('messageType', 'success');
     }
 
     /**
@@ -89,33 +92,29 @@ public function store(StoreUserDisk $request)
      */
     public function update(UpdateUserDisk $request)
     {
-        $errors = DB::transaction(function () use ($request) {
-            $request->disk->name = $request->input('name', $request->disk->name);
-            $request->disk->options = array_merge(
-                $request->disk->options,
-                $request->getDiskOptions()
-            );
-
-            $request->disk->save();
-
-            $errors = $this->validateS3Config($request->disk);
-            if ($errors) {
-                DB::rollBack();
+        try {
+            DB::transaction(function () use ($request) {
+                $request->disk->name = $request->input('name', $request->disk->name);
+                $request->disk->options = array_merge(
+                    $request->disk->options,
+                    $request->getDiskOptions()
+                );
+
+                $request->disk->save();
+
+                $this->validateS3Config($request->disk);
+            });
+
+            if (!$this->isAutomatedRequest()) {
+                return $this->fuzzyRedirect()
+                    ->with('message', 'Storage disk updated')
+                    ->with('messageType', 'success');
             }
-            return $errors;
-        });
-
-        if ($errors) {
+        } catch (ValidationException $e) {
             return $this->fuzzyRedirect()
-                ->withErrors($errors)
+                ->withErrors(['error' => $e->getMessage()])
                 ->withInput();
         }
-
-        if (!$this->isAutomatedRequest()) {
-            return $this->fuzzyRedirect()
-                ->with('message', 'Storage disk updated')
-                ->with('messageType', 'success');
-        }
     }
 
     /**
@@ -171,18 +170,16 @@ public function destroy($id)
     protected function validateS3Config(UserDisk $disk)
     {
         if ($disk->type != 's3') {
-            return [];
+            return;
         }
 
-        $errors = [];
         $options = $disk->options;
         $endpoint = $options['endpoint'];
         $bucket = $options['bucket'];
 
         // Check if endpoint contains bucket name
         if (!preg_match("/(\b\/" . $bucket . "\.|\b" . $bucket . "\b)/", $endpoint)) {
-            $errors['endpoint'] = 'Missing bucket name. Please check if the bucket name is present and spelled correctly.';
-            return $errors;
+            throw new ValidationException('Endpoint url must contain bucket name. Please check if bucket name is present and spelled correctly.');
         }
 
         try {
@@ -194,17 +191,15 @@ protected function validateS3Config(UserDisk $disk)
             $msg = $e->getMessage();
 
             if (Str::contains($msg, 'timeout', true)) {
-                $errors['endpoint'] = 'The endpoint URL could not be accessed. Does it exist?';
+                throw new ValidationException('The endpoint URL could not be accessed. Does it exist?');
             } else if (Str::contains($msg, ['cURL error', 'Error parsing XML'], true)) {
-                $errors['endpoint'] = 'This does not seem to be a valid S3 endpoint.';
+                throw new ValidationException('This does not seem to be a valid S3 endpoint.');
             } else if (Str::contains($msg, ["AccessDenied", "NoSuchBucket", "NoSuchKey"], true)) {
-                $errors['error'] = 'The bucket could not be accessed. Please check for typos or missing access permissions.';
-            }
-
-            if (empty($errors)) {
-                $errors['error'] = 'An error occurred. Please check if your input is correct.';
+                throw new ValidationException('The bucket could not be accessed. Please check for typos or missing access permissions.');
+            } else {
+                throw new ValidationException('An error occurred. Please check if your input is correct.');
             }
         }
-        return $errors;
+        return;
     }
 }

From 8a0f2f33367757732298a80f2e98ce52dff4e33d Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Thu, 20 Feb 2025 07:58:10 +0100
Subject: [PATCH 07/25] Change code structure to increase code readability

---
 src/resources/views/store/s3.blade.php | 10 ++++------
 src/resources/views/update.blade.php   | 12 ++++++------
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/src/resources/views/store/s3.blade.php b/src/resources/views/store/s3.blade.php
index afd9dd9..57c4400 100644
--- a/src/resources/views/store/s3.blade.php
+++ b/src/resources/views/store/s3.blade.php
@@ -14,10 +14,8 @@
         @error('region')
             <p class="help-block">{{$message}}</p>
         @enderror
-    </div>
-    @if (!$errors->has('region'))
         <p class="help-block">Leave empty if your cloud storage service does not support regions.</p>
-    @endif
+    </div>
 </div>
 <div class="col-xs-12">
     <div class="form-group @error('endpoint') has-error @enderror">
@@ -26,10 +24,10 @@
         @error('endpoint')
             <p class="help-block">{{$message}}</p>
         @enderror
+        <p class="help-block">
+            This must be the full URL including the bucket name, region etc.
+        </p>
     </div>
-    <p class="help-block">
-        This must be the full URL including the bucket name, region etc.
-    </p>
 </div>
 <div class="col-sm-6">
     <div class="form-group @error('key') has-error @enderror">
diff --git a/src/resources/views/update.blade.php b/src/resources/views/update.blade.php
index b1d4d25..9aa9544 100644
--- a/src/resources/views/update.blade.php
+++ b/src/resources/views/update.blade.php
@@ -44,15 +44,15 @@
                     @include("user-disks::update.{$disk->type}")
                 </div>
                 <div class="row">
-                    <div class="col-xs-12" @error('error') has-error @enderror>
                         @error('error')
-                            <div class="panel panel-danger">
-                                <div class="panel-body text-danger">
-                                    {{$message}}
+                        <div class="col-xs-12">
+                                <div class="panel panel-danger">
+                                    <div class="panel-body text-danger">
+                                        {{$message}}
+                                    </div>
                                 </div>
-                            </div>
+                        </div>
                         @enderror
-                    </div>
                     <div class="col-xs-12">
                         @csrf
                         @method('PUT')

From 846cf597be9c9fe3508169556e0c6a6d92df6178 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Fri, 21 Feb 2025 07:45:07 +0100
Subject: [PATCH 08/25] Throw validation exeptions that uses array messages

---
 .../Controllers/Api/UserDiskController.php    | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/Http/Controllers/Api/UserDiskController.php b/src/Http/Controllers/Api/UserDiskController.php
index 1be442f..657128b 100644
--- a/src/Http/Controllers/Api/UserDiskController.php
+++ b/src/Http/Controllers/Api/UserDiskController.php
@@ -7,11 +7,12 @@
 use Illuminate\Support\Facades\DB;
 use Biigle\Modules\UserDisks\UserDisk;
 use Illuminate\Support\Facades\Storage;
-use Dotenv\Exception\ValidationException;
 use Biigle\Http\Controllers\Api\Controller;
+use Illuminate\Validation\ValidationException;
 use Biigle\Modules\UserDisks\Http\Requests\StoreUserDisk;
 use Biigle\Modules\UserDisks\Http\Requests\ExtendUserDisk;
 use Biigle\Modules\UserDisks\Http\Requests\UpdateUserDisk;
+use Biigle\Modules\UserDisks\Exceptions\DiskValidationException;
 
 class UserDiskController extends Controller
 {
@@ -60,9 +61,9 @@ public function store(StoreUserDisk $request)
             return $this->fuzzyRedirect('storage-disks')
                 ->with('message', 'Storage disk created')
                 ->with('messageType', 'success');
-        } catch (ValidationException $e) {
+        } catch (DiskValidationException $e) {
             return $this->fuzzyRedirect()
-                ->withErrors(['error' => $e->getMessage()])
+                ->withErrors($e->getValidationError())
                 ->withInput();
         }
     }
@@ -101,7 +102,6 @@ public function update(UpdateUserDisk $request)
                 );
 
                 $request->disk->save();
-
                 $this->validateS3Config($request->disk);
             });
 
@@ -110,9 +110,9 @@ public function update(UpdateUserDisk $request)
                     ->with('message', 'Storage disk updated')
                     ->with('messageType', 'success');
             }
-        } catch (ValidationException $e) {
+        } catch (DiskValidationException $e) {
             return $this->fuzzyRedirect()
-                ->withErrors(['error' => $e->getMessage()])
+                ->withErrors($e->getValidationError())
                 ->withInput();
         }
     }
@@ -179,7 +179,7 @@ protected function validateS3Config(UserDisk $disk)
 
         // Check if endpoint contains bucket name
         if (!preg_match("/(\b\/" . $bucket . "\.|\b" . $bucket . "\b)/", $endpoint)) {
-            throw new ValidationException('Endpoint url must contain bucket name. Please check if bucket name is present and spelled correctly.');
+            throw ValidationException::withMessages(['endpoint' => 'Endpoint url must contain bucket name. Please check if name is present and spelled correctly.']);
         }
 
         try {
@@ -191,13 +191,13 @@ protected function validateS3Config(UserDisk $disk)
             $msg = $e->getMessage();
 
             if (Str::contains($msg, 'timeout', true)) {
-                throw new ValidationException('The endpoint URL could not be accessed. Does it exist?');
-            } else if (Str::contains($msg, ['cURL error', 'Error parsing XML'], true)) {
-                throw new ValidationException('This does not seem to be a valid S3 endpoint.');
-            } else if (Str::contains($msg, ["AccessDenied", "NoSuchBucket", "NoSuchKey"], true)) {
-                throw new ValidationException('The bucket could not be accessed. Please check for typos or missing access permissions.');
+                throw  ValidationException::withMessages(['error' => 'The endpoint URL could not be accessed. Does it exist?']);
+            } elseif (Str::contains($msg, ['cURL error', 'Error parsing XML'], true)) {
+                throw  ValidationException::withMessages(['endpoint' => 'This does not seem to be a valid S3 endpoint.']);
+            } elseif (Str::contains($msg, ["AccessDenied", "NoSuchBucket", "NoSuchKey"], true)) {
+                throw  ValidationException::withMessages(['error' => 'The bucket could not be accessed. Please check for typos or missing access permissions.']);
             } else {
-                throw new ValidationException('An error occurred. Please check if your input is correct.');
+                throw  ValidationException::withMessages(['error' => 'An error occurred. Please check if your input is correct.']);
             }
         }
         return;

From 2deebc05ad1ebf53bc61478a44afe71e6f0976a7 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Fri, 21 Feb 2025 10:07:25 +0100
Subject: [PATCH 09/25] Fix transaction return statement

---
 src/Http/Controllers/Api/UserDiskController.php | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/Http/Controllers/Api/UserDiskController.php b/src/Http/Controllers/Api/UserDiskController.php
index 657128b..c5d38a4 100644
--- a/src/Http/Controllers/Api/UserDiskController.php
+++ b/src/Http/Controllers/Api/UserDiskController.php
@@ -42,7 +42,7 @@ class UserDiskController extends Controller
     public function store(StoreUserDisk $request)
     {
         try {
-            DB::transaction(function () use ($request) {
+            $disk = DB::transaction(function () use ($request) {
                 $disk = UserDisk::create([
                     'name' => $request->input('name'),
                     'type' => $request->input('type'),
@@ -53,11 +53,13 @@ public function store(StoreUserDisk $request)
 
                 $this->validateS3Config($disk);
 
-                if ($this->isAutomatedRequest()) {
-                    return $disk;
-                }
+                return $disk;
             });
 
+            if ($this->isAutomatedRequest()) {
+                return $disk;
+            }
+
             return $this->fuzzyRedirect('storage-disks')
                 ->with('message', 'Storage disk created')
                 ->with('messageType', 'success');

From 4e1bc98ee79265c03a6d539414b19692b2583f4d Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Fri, 21 Feb 2025 10:07:59 +0100
Subject: [PATCH 10/25] Refactor code to enable testing

---
 src/Http/Controllers/Api/UserDiskController.php | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/Http/Controllers/Api/UserDiskController.php b/src/Http/Controllers/Api/UserDiskController.php
index c5d38a4..c6a41d6 100644
--- a/src/Http/Controllers/Api/UserDiskController.php
+++ b/src/Http/Controllers/Api/UserDiskController.php
@@ -185,10 +185,7 @@ protected function validateS3Config(UserDisk $disk)
         }
 
         try {
-            $disk = Storage::disk("disk-{$disk->id}");
-            $files = $disk->getAdapter()->listContents('', false);
-            // Need to access an element to check if endpoint url is valid
-            $files->current();
+            $this->canAccessDisk($disk);
         } catch (Exception $e) {
             $msg = $e->getMessage();
 
@@ -204,4 +201,12 @@ protected function validateS3Config(UserDisk $disk)
         }
         return;
     }
+
+    protected function canAccessDisk($disk)
+    {
+        $disk = Storage::disk("disk-{$disk->id}");
+        $files = $disk->getAdapter()->listContents('', false);
+        // Need to access an element to check if endpoint url is valid
+        $files->current();
+    }
 }

From a1e01e63fc16269cb54fd8e6745e2bce26bc9313 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Fri, 21 Feb 2025 10:11:08 +0100
Subject: [PATCH 11/25] Add and update tests

---
 .../Api/UserDiskControllerTest.php            | 188 ++++++++++++++++++
 1 file changed, 188 insertions(+)

diff --git a/tests/Http/Controllers/Api/UserDiskControllerTest.php b/tests/Http/Controllers/Api/UserDiskControllerTest.php
index 5ad2d51..aa2b1b4 100644
--- a/tests/Http/Controllers/Api/UserDiskControllerTest.php
+++ b/tests/Http/Controllers/Api/UserDiskControllerTest.php
@@ -2,11 +2,21 @@
 
 namespace Biigle\Tests\Modules\UserDisks\Http\Controllers\Api;
 
+use Mockery;
 use ApiTestCase;
 use Biigle\Modules\UserDisks\UserDisk;
+use Biigle\Modules\UserDisks\Http\Controllers\Api\UserDiskController;
 
 class UserDiskControllerTest extends ApiTestCase
 {
+    private $mockS3 = null;
+    public function setUp(): void
+    {
+        parent::setUp();
+        $this->mockS3 = Mockery::mock(UserDiskController::class)->shouldAllowmockingProtectedMethods()->makePartial();
+        $this->app->instance(UserDiskController::class, $this->mockS3);
+    }
+
     public function testStore()
     {
         $this->doTestApiRoute('POST', "/api/v1/user-disks");
@@ -37,12 +47,15 @@ public function testStore()
     public function testStoreS3()
     {
         $this->beUser();
+
+        $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
             ])
             ->assertStatus(422);
 
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
@@ -73,6 +86,8 @@ public function testStoreS3()
 
     public function testDuplicateNames(){
         $this->beUser();
+
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
             'type' => 's3',
@@ -97,6 +112,7 @@ public function testDuplicateNames(){
         ->assertStatus(422);
 
         $this->beEditor();
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
             'type' => 's3',
@@ -112,6 +128,8 @@ public function testDuplicateNames(){
     public function testStoreS3RegionEmpty()
     {
         $this->beUser();
+
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
@@ -130,12 +148,15 @@ public function testStoreS3RegionEmpty()
     public function testStoreS3PathStyle()
     {
         $this->beUser();
+
+        $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
             ])
             ->assertStatus(422);
 
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
@@ -190,6 +211,7 @@ public function testUpdateS3()
         ]);
 
         $this->be($disk->user);
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
                 'type' => 'unknown',
                 'name' => 'cba',
@@ -230,6 +252,7 @@ public function testUpdateS3PathStyle()
         ]);
 
         $this->be($disk->user);
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
                 'endpoint' => 'https://example.com/jkl',
             ])
@@ -245,6 +268,7 @@ public function testUpdateS3PathStyle()
         ];
         $this->assertEquals($expect, $disk->options);
 
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
                 'endpoint' => 'https://jkl.example.com/',
             ])
@@ -334,4 +358,168 @@ public function testDestroy()
         $this->deleteJson("/api/v1/user-disks/{$disk->id}")->assertStatus(200);
         $this->assertNull($disk->fresh());
     }
+    public function testStoreInvalidS3Config()
+    {
+        $this->beUser();
+
+        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'ucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])
+            ->assertUnprocessable();
+
+        $disk = UserDisk::where('user_id', $this->user()->id)->first();
+        $this->assertEmpty($disk);
+    }
+
+    public function testUpdateInvalidS3Config()
+    {
+        $disk = UserDisk::factory()->create([
+            'type' => 's3',
+            'name' => 'abc',
+            'options' => [
+                'key' => 'def',
+                'secret' => 'ghi',
+                'bucket' => 'jkl',
+                'region' => 'us-east-1',
+                'endpoint' => 'https://jkl.example.com',
+                'use_path_style_endpoint' => false,
+            ],
+        ]);
+
+        $this->be($disk->user);
+        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'endpoint' => 'https://bucket.example.com',
+        ])
+            ->assertUnprocessable();
+
+        $disk = $disk->fresh();
+        $this->assertEquals('https://jkl.example.com', $disk->options['endpoint']);
+    }
+
+    public function testStoreIncorrectBucketName()
+    {
+        $this->beUser();
+
+        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'ucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])
+            ->assertUnprocessable();
+
+        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://ucket.example.com',
+        ])
+            ->assertUnprocessable();
+
+        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'ucket',
+            'region' => '',
+            'endpoint' => 'http://example.com/bucket',
+        ])
+            ->assertUnprocessable();
+
+        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://example.com/ucket',
+        ])
+            ->assertUnprocessable();
+    }
+
+    public function testUpdateIncorrectBucketName()
+    {
+        $disk = UserDisk::factory()->create([
+            'type' => 's3',
+            'name' => 'abc',
+            'options' => [
+                'key' => 'def',
+                'secret' => 'ghi',
+                'bucket' => 'jkl',
+                'region' => 'us-east-1',
+                'endpoint' => 'https://jkl.example.com',
+                'use_path_style_endpoint' => false,
+            ],
+        ]);
+
+        $this->be($disk->user);
+
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])
+            ->assertUnprocessable();
+
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://m.example.com',
+        ])
+            ->assertUnprocessable();
+
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://example.com/onm',
+        ])
+            ->assertUnprocessable();
+
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://example.com/m',
+        ])
+            ->assertUnprocessable();
+    }
 }

From 9b9bea230e284268d22525882bd6135995574fdd Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Fri, 21 Feb 2025 10:16:14 +0100
Subject: [PATCH 12/25] Refactor code

---
 src/resources/views/update/s3.blade.php | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/resources/views/update/s3.blade.php b/src/resources/views/update/s3.blade.php
index 03c9733..1c794c9 100644
--- a/src/resources/views/update/s3.blade.php
+++ b/src/resources/views/update/s3.blade.php
@@ -23,10 +23,10 @@
         @error('endpoint')
             <p class="help-block">{{$message}}</p>
         @enderror
+         <p class="help-block">
+            This must be the full URL including the bucket name, region etc.
+        </p>
     </div>
-    <p class="help-block">
-        This must be the full URL including the bucket name, region etc.
-    </p>
 </div>
 <div class="col-xs-12">
     <div class="panel panel-warning">

From 9c3afcf0ef33087a36cd83c8285bfc4b38270124 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Mon, 24 Feb 2025 08:06:22 +0100
Subject: [PATCH 13/25] Fix failing tests

---
 .../Api/UserDiskControllerTest.php            | 23 +++++++++++++++----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/tests/Http/Controllers/Api/UserDiskControllerTest.php b/tests/Http/Controllers/Api/UserDiskControllerTest.php
index aa2b1b4..a6fd1f2 100644
--- a/tests/Http/Controllers/Api/UserDiskControllerTest.php
+++ b/tests/Http/Controllers/Api/UserDiskControllerTest.php
@@ -185,13 +185,25 @@ public function testStoreS3PathStyle()
 
     public function testUpdate()
     {
-        $disk = UserDisk::factory()->create();
+        $disk = UserDisk::factory()->create([
+            'type' => 's3',
+            'name' => 'abc',
+            'options' => [
+                'key' => 'def',
+                'secret' => 'ghi',
+                'bucket' => 'jkl',
+                'region' => 'us-east-1',
+                'endpoint' => 'https://jkl.example.com',
+                'use_path_style_endpoint' => false,
+            ],
+        ]);
         $this->doTestApiRoute('PUT', "/api/v1/user-disks/{$disk->id}");
 
         $this->beUser();
         $this->putJson("/api/v1/user-disks/{$disk->id}")->assertStatus(403);
 
         $this->be($disk->user);
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
         $this->putJson("/api/v1/user-disks/{$disk->id}")->assertStatus(200);
     }
 
@@ -300,6 +312,7 @@ public function testUpdateEmpty()
         ]);
         $this->be($disk->user);
 
+        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
                 'name' => 'cba',
                 'key' => '0',
@@ -474,7 +487,7 @@ public function testUpdateIncorrectBucketName()
 
         $this->be($disk->user);
 
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
             'type' => 'unknown',
             'name' => 'cba',
@@ -486,7 +499,7 @@ public function testUpdateIncorrectBucketName()
         ])
             ->assertUnprocessable();
 
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
             'type' => 'unknown',
             'name' => 'cba',
@@ -498,7 +511,7 @@ public function testUpdateIncorrectBucketName()
         ])
             ->assertUnprocessable();
 
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
             'type' => 'unknown',
             'name' => 'cba',
@@ -510,7 +523,7 @@ public function testUpdateIncorrectBucketName()
         ])
             ->assertUnprocessable();
 
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
             'type' => 'unknown',
             'name' => 'cba',

From 16ee9be94b637b68a4f36ddfa861d92ff05f492c Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Mon, 24 Feb 2025 08:20:35 +0100
Subject: [PATCH 14/25] Minor changes

---
 src/resources/views/update/s3.blade.php       |  2 +-
 .../Api/UserDiskControllerTest.php            | 39 ++++++-------------
 2 files changed, 13 insertions(+), 28 deletions(-)

diff --git a/src/resources/views/update/s3.blade.php b/src/resources/views/update/s3.blade.php
index 1c794c9..37c4bca 100644
--- a/src/resources/views/update/s3.blade.php
+++ b/src/resources/views/update/s3.blade.php
@@ -23,7 +23,7 @@
         @error('endpoint')
             <p class="help-block">{{$message}}</p>
         @enderror
-         <p class="help-block">
+        <p class="help-block">
             This must be the full URL including the bucket name, region etc.
         </p>
     </div>
diff --git a/tests/Http/Controllers/Api/UserDiskControllerTest.php b/tests/Http/Controllers/Api/UserDiskControllerTest.php
index a6fd1f2..1e502e6 100644
--- a/tests/Http/Controllers/Api/UserDiskControllerTest.php
+++ b/tests/Http/Controllers/Api/UserDiskControllerTest.php
@@ -9,7 +9,8 @@
 
 class UserDiskControllerTest extends ApiTestCase
 {
-    private $mockS3 = null;
+    private $mockS3;
+    
     public function setUp(): void
     {
         parent::setUp();
@@ -47,7 +48,6 @@ public function testStore()
     public function testStoreS3()
     {
         $this->beUser();
-
         $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
@@ -86,7 +86,6 @@ public function testStoreS3()
 
     public function testDuplicateNames(){
         $this->beUser();
-
         $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
@@ -128,7 +127,6 @@ public function testDuplicateNames(){
     public function testStoreS3RegionEmpty()
     {
         $this->beUser();
-
         $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
@@ -148,7 +146,6 @@ public function testStoreS3RegionEmpty()
     public function testStoreS3PathStyle()
     {
         $this->beUser();
-
         $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
@@ -374,7 +371,6 @@ public function testDestroy()
     public function testStoreInvalidS3Config()
     {
         $this->beUser();
-
         $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
@@ -384,8 +380,7 @@ public function testStoreInvalidS3Config()
             'bucket' => 'ucket',
             'region' => '',
             'endpoint' => 'http://bucket.example.com',
-        ])
-            ->assertUnprocessable();
+        ])->assertUnprocessable();
 
         $disk = UserDisk::where('user_id', $this->user()->id)->first();
         $this->assertEmpty($disk);
@@ -410,8 +405,7 @@ public function testUpdateInvalidS3Config()
         $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
             'endpoint' => 'https://bucket.example.com',
-        ])
-            ->assertUnprocessable();
+        ])->assertUnprocessable();
 
         $disk = $disk->fresh();
         $this->assertEquals('https://jkl.example.com', $disk->options['endpoint']);
@@ -420,7 +414,6 @@ public function testUpdateInvalidS3Config()
     public function testStoreIncorrectBucketName()
     {
         $this->beUser();
-
         $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
@@ -430,8 +423,7 @@ public function testStoreIncorrectBucketName()
             'bucket' => 'ucket',
             'region' => '',
             'endpoint' => 'http://bucket.example.com',
-        ])
-            ->assertUnprocessable();
+        ])->assertUnprocessable();
 
         $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->postJson("/api/v1/user-disks", [
@@ -442,8 +434,7 @@ public function testStoreIncorrectBucketName()
             'bucket' => 'bucket',
             'region' => '',
             'endpoint' => 'http://ucket.example.com',
-        ])
-            ->assertUnprocessable();
+        ])->assertUnprocessable();
 
         $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->postJson("/api/v1/user-disks", [
@@ -454,8 +445,7 @@ public function testStoreIncorrectBucketName()
             'bucket' => 'ucket',
             'region' => '',
             'endpoint' => 'http://example.com/bucket',
-        ])
-            ->assertUnprocessable();
+        ])->assertUnprocessable();
 
         $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->postJson("/api/v1/user-disks", [
@@ -466,8 +456,7 @@ public function testStoreIncorrectBucketName()
             'bucket' => 'bucket',
             'region' => '',
             'endpoint' => 'http://example.com/ucket',
-        ])
-            ->assertUnprocessable();
+        ])->assertUnprocessable();
     }
 
     public function testUpdateIncorrectBucketName()
@@ -496,8 +485,7 @@ public function testUpdateIncorrectBucketName()
             'bucket' => 'm',
             'region' => 'us-east-2',
             'endpoint' => 'https://onm.example.com',
-        ])
-            ->assertUnprocessable();
+        ])->assertUnprocessable();
 
         $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
@@ -508,8 +496,7 @@ public function testUpdateIncorrectBucketName()
             'bucket' => 'onm',
             'region' => 'us-east-2',
             'endpoint' => 'https://m.example.com',
-        ])
-            ->assertUnprocessable();
+        ])->assertUnprocessable();
 
         $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
@@ -520,8 +507,7 @@ public function testUpdateIncorrectBucketName()
             'bucket' => 'm',
             'region' => 'us-east-2',
             'endpoint' => 'https://example.com/onm',
-        ])
-            ->assertUnprocessable();
+        ])->assertUnprocessable();
 
         $this->mockS3->shouldReceive('canAccessDisk')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
@@ -532,7 +518,6 @@ public function testUpdateIncorrectBucketName()
             'bucket' => 'onm',
             'region' => 'us-east-2',
             'endpoint' => 'https://example.com/m',
-        ])
-            ->assertUnprocessable();
+        ])->assertUnprocessable();
     }
 }

From 2f4c758e92de48234433db6addac08e1f72e45a9 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Tue, 25 Feb 2025 08:04:01 +0100
Subject: [PATCH 15/25] Update test.yml

---
 .github/workflows/test.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index e2c3253..62d3e0f 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -57,9 +57,9 @@ jobs:
         docker pull ghcr.io/biigle/worker:latest
 
     - name: Start test database
-      run: docker-compose up -d --no-build database_testing && sleep 5
+      run: docker compose up -d --no-build database_testing && sleep 5
       working-directory: ../core
 
     - name: Run tests
-      run: docker-compose run --rm -u 1001 worker php -d memory_limit=1G vendor/bin/phpunit --random-order --filter 'Biigle\\Tests\\Modules\\'${MODULE_NAME}
+      run: docker compose run --rm -u 1001 worker php -d memory_limit=1G vendor/bin/phpunit --random-order --filter 'Biigle\\Tests\\Modules\\'${MODULE_NAME}
       working-directory: ../core

From 5ed1d2547d6d57b23cd2ac72c1dff2e96b4b54cb Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Wed, 9 Apr 2025 08:31:22 +0200
Subject: [PATCH 16/25] Change error message

---
 src/Http/Controllers/Api/UserDiskController.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Http/Controllers/Api/UserDiskController.php b/src/Http/Controllers/Api/UserDiskController.php
index c6a41d6..a756d16 100644
--- a/src/Http/Controllers/Api/UserDiskController.php
+++ b/src/Http/Controllers/Api/UserDiskController.php
@@ -181,7 +181,7 @@ protected function validateS3Config(UserDisk $disk)
 
         // Check if endpoint contains bucket name
         if (!preg_match("/(\b\/" . $bucket . "\.|\b" . $bucket . "\b)/", $endpoint)) {
-            throw ValidationException::withMessages(['endpoint' => 'Endpoint url must contain bucket name. Please check if name is present and spelled correctly.']);
+            throw ValidationException::withMessages(['endpoint' => 'The endpoint URL must contain the bucket name. Please check if the name is present and spelled correctly.']);
         }
 
         try {

From bbcd9bbc86b111b6067fe12e474c429f93ca3d5c Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Wed, 9 Apr 2025 08:42:03 +0200
Subject: [PATCH 17/25] Rename method and add php doc

---
 src/Http/Controllers/Api/UserDiskController.php | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/Http/Controllers/Api/UserDiskController.php b/src/Http/Controllers/Api/UserDiskController.php
index a756d16..3796310 100644
--- a/src/Http/Controllers/Api/UserDiskController.php
+++ b/src/Http/Controllers/Api/UserDiskController.php
@@ -185,7 +185,7 @@ protected function validateS3Config(UserDisk $disk)
         }
 
         try {
-            $this->canAccessDisk($disk);
+            $this->validateDiskAccess($disk);
         } catch (Exception $e) {
             $msg = $e->getMessage();
 
@@ -202,7 +202,14 @@ protected function validateS3Config(UserDisk $disk)
         return;
     }
 
-    protected function canAccessDisk($disk)
+    /**
+     * Checks if the endpoint url is valid and the disk can be accessed
+     * 
+     * @param mixed $disk Disk configured by the user that should be tested
+     * @return void
+     * @throws Exception If the disk cannot be accessed
+     */
+    protected function validateDiskAccess($disk)
     {
         $disk = Storage::disk("disk-{$disk->id}");
         $files = $disk->getAdapter()->listContents('', false);

From af08c9a4468a5b9c6c96c25ef09acbf01f7003a0 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Wed, 9 Apr 2025 08:42:30 +0200
Subject: [PATCH 18/25] Update tests

---
 .../Api/UserDiskControllerTest.php            | 44 +++++++++----------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/tests/Http/Controllers/Api/UserDiskControllerTest.php b/tests/Http/Controllers/Api/UserDiskControllerTest.php
index 1e502e6..772e227 100644
--- a/tests/Http/Controllers/Api/UserDiskControllerTest.php
+++ b/tests/Http/Controllers/Api/UserDiskControllerTest.php
@@ -48,14 +48,14 @@ public function testStore()
     public function testStoreS3()
     {
         $this->beUser();
-        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
             ])
             ->assertStatus(422);
 
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
@@ -86,7 +86,7 @@ public function testStoreS3()
 
     public function testDuplicateNames(){
         $this->beUser();
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
             'type' => 's3',
@@ -111,7 +111,7 @@ public function testDuplicateNames(){
         ->assertStatus(422);
 
         $this->beEditor();
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
             'type' => 's3',
@@ -127,7 +127,7 @@ public function testDuplicateNames(){
     public function testStoreS3RegionEmpty()
     {
         $this->beUser();
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
@@ -146,14 +146,14 @@ public function testStoreS3RegionEmpty()
     public function testStoreS3PathStyle()
     {
         $this->beUser();
-        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
             ])
             ->assertStatus(422);
 
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
@@ -200,7 +200,7 @@ public function testUpdate()
         $this->putJson("/api/v1/user-disks/{$disk->id}")->assertStatus(403);
 
         $this->be($disk->user);
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
         $this->putJson("/api/v1/user-disks/{$disk->id}")->assertStatus(200);
     }
 
@@ -220,7 +220,7 @@ public function testUpdateS3()
         ]);
 
         $this->be($disk->user);
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
                 'type' => 'unknown',
                 'name' => 'cba',
@@ -261,7 +261,7 @@ public function testUpdateS3PathStyle()
         ]);
 
         $this->be($disk->user);
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
                 'endpoint' => 'https://example.com/jkl',
             ])
@@ -277,7 +277,7 @@ public function testUpdateS3PathStyle()
         ];
         $this->assertEquals($expect, $disk->options);
 
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
                 'endpoint' => 'https://jkl.example.com/',
             ])
@@ -309,7 +309,7 @@ public function testUpdateEmpty()
         ]);
         $this->be($disk->user);
 
-        $this->mockS3->shouldReceive('canAccessDisk')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
                 'name' => 'cba',
                 'key' => '0',
@@ -371,7 +371,7 @@ public function testDestroy()
     public function testStoreInvalidS3Config()
     {
         $this->beUser();
-        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
             'type' => 's3',
@@ -402,7 +402,7 @@ public function testUpdateInvalidS3Config()
         ]);
 
         $this->be($disk->user);
-        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
             'endpoint' => 'https://bucket.example.com',
         ])->assertUnprocessable();
@@ -414,7 +414,7 @@ public function testUpdateInvalidS3Config()
     public function testStoreIncorrectBucketName()
     {
         $this->beUser();
-        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
             'type' => 's3',
@@ -425,7 +425,7 @@ public function testStoreIncorrectBucketName()
             'endpoint' => 'http://bucket.example.com',
         ])->assertUnprocessable();
 
-        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
             'type' => 's3',
@@ -436,7 +436,7 @@ public function testStoreIncorrectBucketName()
             'endpoint' => 'http://ucket.example.com',
         ])->assertUnprocessable();
 
-        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
             'type' => 's3',
@@ -447,7 +447,7 @@ public function testStoreIncorrectBucketName()
             'endpoint' => 'http://example.com/bucket',
         ])->assertUnprocessable();
 
-        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
             'type' => 's3',
@@ -476,7 +476,7 @@ public function testUpdateIncorrectBucketName()
 
         $this->be($disk->user);
 
-        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
             'type' => 'unknown',
             'name' => 'cba',
@@ -487,7 +487,7 @@ public function testUpdateIncorrectBucketName()
             'endpoint' => 'https://onm.example.com',
         ])->assertUnprocessable();
 
-        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
             'type' => 'unknown',
             'name' => 'cba',
@@ -498,7 +498,7 @@ public function testUpdateIncorrectBucketName()
             'endpoint' => 'https://m.example.com',
         ])->assertUnprocessable();
 
-        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
             'type' => 'unknown',
             'name' => 'cba',
@@ -509,7 +509,7 @@ public function testUpdateIncorrectBucketName()
             'endpoint' => 'https://example.com/onm',
         ])->assertUnprocessable();
 
-        $this->mockS3->shouldReceive('canAccessDisk')->never();
+        $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
             'type' => 'unknown',
             'name' => 'cba',

From 05094c85f1e282aa423f5147d39e7d3e7d23f61c Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Wed, 9 Apr 2025 08:52:14 +0200
Subject: [PATCH 19/25] Remove redundant error handling

---
 src/resources/views/store.blade.php | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/resources/views/store.blade.php b/src/resources/views/store.blade.php
index 2e1cf09..b8130f4 100644
--- a/src/resources/views/store.blade.php
+++ b/src/resources/views/store.blade.php
@@ -44,14 +44,15 @@
                     </fieldset>
                 @endif
                 <div class="row">
-                    <div class="col-xs-12" @error('error') has-error @enderror>
-                        @error('error')
+                    @error('error')
+                        <div class="col-xs-12">
                             <div class="panel panel-danger">
                                 <div class="panel-body text-danger">
                                     {{$message}}
                                 </div>
                             </div>
-                        @enderror
+                        </div>
+                    @enderror
                     </div>
                     <div class="col-xs-12">
                         @csrf

From 22a8865770ec9ff34f6afba24b98340f00daba3b Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Wed, 9 Apr 2025 08:52:55 +0200
Subject: [PATCH 20/25] Fix indentation

---
 src/resources/views/update.blade.php | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/resources/views/update.blade.php b/src/resources/views/update.blade.php
index 9aa9544..cedd032 100644
--- a/src/resources/views/update.blade.php
+++ b/src/resources/views/update.blade.php
@@ -44,15 +44,15 @@
                     @include("user-disks::update.{$disk->type}")
                 </div>
                 <div class="row">
-                        @error('error')
+                    @error('error')
                         <div class="col-xs-12">
-                                <div class="panel panel-danger">
-                                    <div class="panel-body text-danger">
-                                        {{$message}}
-                                    </div>
+                            <div class="panel panel-danger">
+                                <div class="panel-body text-danger">
+                                    {{$message}}
                                 </div>
+                            </div>
                         </div>
-                        @enderror
+                    @enderror
                     <div class="col-xs-12">
                         @csrf
                         @method('PUT')

From 3b56180b138ff1af408b3e6211b0bd13f5472a24 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Wed, 9 Apr 2025 09:27:16 +0200
Subject: [PATCH 21/25] Fix exception handling

---
 src/Http/Controllers/Api/UserDiskController.php | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/Http/Controllers/Api/UserDiskController.php b/src/Http/Controllers/Api/UserDiskController.php
index 3796310..076d782 100644
--- a/src/Http/Controllers/Api/UserDiskController.php
+++ b/src/Http/Controllers/Api/UserDiskController.php
@@ -12,7 +12,6 @@
 use Biigle\Modules\UserDisks\Http\Requests\StoreUserDisk;
 use Biigle\Modules\UserDisks\Http\Requests\ExtendUserDisk;
 use Biigle\Modules\UserDisks\Http\Requests\UpdateUserDisk;
-use Biigle\Modules\UserDisks\Exceptions\DiskValidationException;
 
 class UserDiskController extends Controller
 {
@@ -63,9 +62,9 @@ public function store(StoreUserDisk $request)
             return $this->fuzzyRedirect('storage-disks')
                 ->with('message', 'Storage disk created')
                 ->with('messageType', 'success');
-        } catch (DiskValidationException $e) {
+        } catch (ValidationException $e) {
             return $this->fuzzyRedirect()
-                ->withErrors($e->getValidationError())
+                ->withErrors($e->errors())
                 ->withInput();
         }
     }
@@ -112,9 +111,9 @@ public function update(UpdateUserDisk $request)
                     ->with('message', 'Storage disk updated')
                     ->with('messageType', 'success');
             }
-        } catch (DiskValidationException $e) {
+        } catch (ValidationException $e) {
             return $this->fuzzyRedirect()
-                ->withErrors($e->getValidationError())
+                ->withErrors($e->errors())
                 ->withInput();
         }
     }

From f587e3ad102b1f6c12b1cd0cf5ecb6d3f8645b53 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Wed, 9 Apr 2025 09:29:02 +0200
Subject: [PATCH 22/25] Update tests

---
 .../Api/UserDiskControllerTest.php            | 20 +++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/Http/Controllers/Api/UserDiskControllerTest.php b/tests/Http/Controllers/Api/UserDiskControllerTest.php
index 772e227..79d657f 100644
--- a/tests/Http/Controllers/Api/UserDiskControllerTest.php
+++ b/tests/Http/Controllers/Api/UserDiskControllerTest.php
@@ -380,7 +380,7 @@ public function testStoreInvalidS3Config()
             'bucket' => 'ucket',
             'region' => '',
             'endpoint' => 'http://bucket.example.com',
-        ])->assertUnprocessable();
+        ])->assertFound();
 
         $disk = UserDisk::where('user_id', $this->user()->id)->first();
         $this->assertEmpty($disk);
@@ -405,7 +405,7 @@ public function testUpdateInvalidS3Config()
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
             'endpoint' => 'https://bucket.example.com',
-        ])->assertUnprocessable();
+        ])->assertFound();
 
         $disk = $disk->fresh();
         $this->assertEquals('https://jkl.example.com', $disk->options['endpoint']);
@@ -423,7 +423,7 @@ public function testStoreIncorrectBucketName()
             'bucket' => 'ucket',
             'region' => '',
             'endpoint' => 'http://bucket.example.com',
-        ])->assertUnprocessable();
+        ])->assertFound();
 
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
@@ -434,7 +434,7 @@ public function testStoreIncorrectBucketName()
             'bucket' => 'bucket',
             'region' => '',
             'endpoint' => 'http://ucket.example.com',
-        ])->assertUnprocessable();
+        ])->assertFound();
 
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
@@ -445,7 +445,7 @@ public function testStoreIncorrectBucketName()
             'bucket' => 'ucket',
             'region' => '',
             'endpoint' => 'http://example.com/bucket',
-        ])->assertUnprocessable();
+        ])->assertFound();
 
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
@@ -456,7 +456,7 @@ public function testStoreIncorrectBucketName()
             'bucket' => 'bucket',
             'region' => '',
             'endpoint' => 'http://example.com/ucket',
-        ])->assertUnprocessable();
+        ])->assertFound();
     }
 
     public function testUpdateIncorrectBucketName()
@@ -485,7 +485,7 @@ public function testUpdateIncorrectBucketName()
             'bucket' => 'm',
             'region' => 'us-east-2',
             'endpoint' => 'https://onm.example.com',
-        ])->assertUnprocessable();
+        ])->assertFound();
 
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
@@ -496,7 +496,7 @@ public function testUpdateIncorrectBucketName()
             'bucket' => 'onm',
             'region' => 'us-east-2',
             'endpoint' => 'https://m.example.com',
-        ])->assertUnprocessable();
+        ])->assertFound();
 
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
@@ -507,7 +507,7 @@ public function testUpdateIncorrectBucketName()
             'bucket' => 'm',
             'region' => 'us-east-2',
             'endpoint' => 'https://example.com/onm',
-        ])->assertUnprocessable();
+        ])->assertFound();
 
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
@@ -518,6 +518,6 @@ public function testUpdateIncorrectBucketName()
             'bucket' => 'onm',
             'region' => 'us-east-2',
             'endpoint' => 'https://example.com/m',
-        ])->assertUnprocessable();
+        ])->assertFound();
     }
 }

From 0da95d3314d9e9d8c0727bab8c0c1734dae5b168 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Wed, 9 Apr 2025 09:41:43 +0200
Subject: [PATCH 23/25] Remove old return value

---
 .../Api/UserDiskControllerTest.php            | 20 +++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/Http/Controllers/Api/UserDiskControllerTest.php b/tests/Http/Controllers/Api/UserDiskControllerTest.php
index 79d657f..d91fab0 100644
--- a/tests/Http/Controllers/Api/UserDiskControllerTest.php
+++ b/tests/Http/Controllers/Api/UserDiskControllerTest.php
@@ -55,7 +55,7 @@ public function testStoreS3()
             ])
             ->assertStatus(422);
 
-        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once();
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
@@ -86,7 +86,7 @@ public function testStoreS3()
 
     public function testDuplicateNames(){
         $this->beUser();
-        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once();
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
             'type' => 's3',
@@ -111,7 +111,7 @@ public function testDuplicateNames(){
         ->assertStatus(422);
 
         $this->beEditor();
-        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once();
         $this->postJson("/api/v1/user-disks", [
             'name' => 'my disk',
             'type' => 's3',
@@ -127,7 +127,7 @@ public function testDuplicateNames(){
     public function testStoreS3RegionEmpty()
     {
         $this->beUser();
-        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once();
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
@@ -153,7 +153,7 @@ public function testStoreS3PathStyle()
             ])
             ->assertStatus(422);
 
-        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once();
         $this->postJson("/api/v1/user-disks", [
                 'name' => 'my disk',
                 'type' => 's3',
@@ -200,7 +200,7 @@ public function testUpdate()
         $this->putJson("/api/v1/user-disks/{$disk->id}")->assertStatus(403);
 
         $this->be($disk->user);
-        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once();
         $this->putJson("/api/v1/user-disks/{$disk->id}")->assertStatus(200);
     }
 
@@ -220,7 +220,7 @@ public function testUpdateS3()
         ]);
 
         $this->be($disk->user);
-        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
                 'type' => 'unknown',
                 'name' => 'cba',
@@ -261,7 +261,7 @@ public function testUpdateS3PathStyle()
         ]);
 
         $this->be($disk->user);
-        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
                 'endpoint' => 'https://example.com/jkl',
             ])
@@ -277,7 +277,7 @@ public function testUpdateS3PathStyle()
         ];
         $this->assertEquals($expect, $disk->options);
 
-        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
                 'endpoint' => 'https://jkl.example.com/',
             ])
@@ -309,7 +309,7 @@ public function testUpdateEmpty()
         ]);
         $this->be($disk->user);
 
-        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andReturn([]);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
                 'name' => 'cba',
                 'key' => '0',

From 788cf753a232afb7f6f70a0218c74e0cdb0f6546 Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Wed, 9 Apr 2025 10:00:05 +0200
Subject: [PATCH 24/25] Add tests to check validation error message

---
 .../Api/UserDiskControllerTest.php            | 30 ++++++++++++-------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/tests/Http/Controllers/Api/UserDiskControllerTest.php b/tests/Http/Controllers/Api/UserDiskControllerTest.php
index d91fab0..9dfc428 100644
--- a/tests/Http/Controllers/Api/UserDiskControllerTest.php
+++ b/tests/Http/Controllers/Api/UserDiskControllerTest.php
@@ -380,7 +380,8 @@ public function testStoreInvalidS3Config()
             'bucket' => 'ucket',
             'region' => '',
             'endpoint' => 'http://bucket.example.com',
-        ])->assertFound();
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
 
         $disk = UserDisk::where('user_id', $this->user()->id)->first();
         $this->assertEmpty($disk);
@@ -405,7 +406,8 @@ public function testUpdateInvalidS3Config()
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
             'endpoint' => 'https://bucket.example.com',
-        ])->assertFound();
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
 
         $disk = $disk->fresh();
         $this->assertEquals('https://jkl.example.com', $disk->options['endpoint']);
@@ -423,7 +425,8 @@ public function testStoreIncorrectBucketName()
             'bucket' => 'ucket',
             'region' => '',
             'endpoint' => 'http://bucket.example.com',
-        ])->assertFound();
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
 
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
@@ -434,7 +437,8 @@ public function testStoreIncorrectBucketName()
             'bucket' => 'bucket',
             'region' => '',
             'endpoint' => 'http://ucket.example.com',
-        ])->assertFound();
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
 
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
@@ -445,7 +449,8 @@ public function testStoreIncorrectBucketName()
             'bucket' => 'ucket',
             'region' => '',
             'endpoint' => 'http://example.com/bucket',
-        ])->assertFound();
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
 
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->postJson("/api/v1/user-disks", [
@@ -456,7 +461,8 @@ public function testStoreIncorrectBucketName()
             'bucket' => 'bucket',
             'region' => '',
             'endpoint' => 'http://example.com/ucket',
-        ])->assertFound();
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
     }
 
     public function testUpdateIncorrectBucketName()
@@ -485,7 +491,8 @@ public function testUpdateIncorrectBucketName()
             'bucket' => 'm',
             'region' => 'us-east-2',
             'endpoint' => 'https://onm.example.com',
-        ])->assertFound();
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
 
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
@@ -496,7 +503,8 @@ public function testUpdateIncorrectBucketName()
             'bucket' => 'onm',
             'region' => 'us-east-2',
             'endpoint' => 'https://m.example.com',
-        ])->assertFound();
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
 
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
@@ -507,7 +515,8 @@ public function testUpdateIncorrectBucketName()
             'bucket' => 'm',
             'region' => 'us-east-2',
             'endpoint' => 'https://example.com/onm',
-        ])->assertFound();
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
 
         $this->mockS3->shouldReceive('validateDiskAccess')->never();
         $this->putJson("/api/v1/user-disks/{$disk->id}", [
@@ -518,6 +527,7 @@ public function testUpdateIncorrectBucketName()
             'bucket' => 'onm',
             'region' => 'us-east-2',
             'endpoint' => 'https://example.com/m',
-        ])->assertFound();
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
     }
 }

From 380c3e85e990524837b0e431aa7f69dfdc61c3ab Mon Sep 17 00:00:00 2001
From: Leane Schlundt <lhecht@techfak.de>
Date: Wed, 9 Apr 2025 10:26:49 +0200
Subject: [PATCH 25/25] Add tests to check invalid bucket

---
 .../Api/UserDiskControllerTest.php            | 397 ++++++++++++++++++
 1 file changed, 397 insertions(+)

diff --git a/tests/Http/Controllers/Api/UserDiskControllerTest.php b/tests/Http/Controllers/Api/UserDiskControllerTest.php
index 9dfc428..2d3a8df 100644
--- a/tests/Http/Controllers/Api/UserDiskControllerTest.php
+++ b/tests/Http/Controllers/Api/UserDiskControllerTest.php
@@ -2,6 +2,7 @@
 
 namespace Biigle\Tests\Modules\UserDisks\Http\Controllers\Api;
 
+use Exception;
 use Mockery;
 use ApiTestCase;
 use Biigle\Modules\UserDisks\UserDisk;
@@ -530,4 +531,400 @@ public function testUpdateIncorrectBucketName()
         ])->assertFound()
             ->assertSessionHasErrors(['endpoint']);
     }
+
+    public function testStoreDiskAccessTimeout()
+    {
+        $this->beUser();
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some Timeout error'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some timeout error'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+    }
+
+    public function testStoreInvalidEndpoint()
+    {
+        $this->beUser();
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some cURL error'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some curl error'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some Error parsing XML'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some error parsing xml'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
+    }
+
+    public function testStoreInvalidBucket()
+    {
+        $this->beUser();
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('error AccessDenied'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('error accessDenied'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('error NoSuchBucket'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('error noSuchBucket'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('error NoSuchKey'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('error noSuchKey'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+    }
+
+    public function testStoreInvalidDiskConfig()
+    {
+        $this->beUser();
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some other error'));
+        $this->postJson("/api/v1/user-disks", [
+            'name' => 'my disk',
+            'type' => 's3',
+            'key' => 'abc',
+            'secret' => 'abc',
+            'bucket' => 'bucket',
+            'region' => '',
+            'endpoint' => 'http://bucket.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+    }
+
+    public function testUpdateDiskAccessTimeout()
+    {
+        $disk = UserDisk::factory()->create([
+            'type' => 's3',
+            'name' => 'abc',
+            'options' => [
+                'key' => 'def',
+                'secret' => 'ghi',
+                'bucket' => 'jkl',
+                'region' => 'us-east-1',
+                'endpoint' => 'https://jkl.example.com',
+                'use_path_style_endpoint' => false,
+            ],
+        ]);
+
+        $this->be($disk->user);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some Timeout error'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some timeout error'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+    }
+
+    public function testUpdateInvalidEndpoint()
+    {
+        $disk = UserDisk::factory()->create([
+            'type' => 's3',
+            'name' => 'abc',
+            'options' => [
+                'key' => 'def',
+                'secret' => 'ghi',
+                'bucket' => 'jkl',
+                'region' => 'us-east-1',
+                'endpoint' => 'https://jkl.example.com',
+                'use_path_style_endpoint' => false,
+            ],
+        ]);
+
+        $this->be($disk->user);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some cURL error'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some curl error'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some Error parsing XML'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some error parsing xml'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['endpoint']);
+    }
+
+    public function testUpdateInvalidBucket()
+    {
+        $disk = UserDisk::factory()->create([
+            'type' => 's3',
+            'name' => 'abc',
+            'options' => [
+                'key' => 'def',
+                'secret' => 'ghi',
+                'bucket' => 'jkl',
+                'region' => 'us-east-1',
+                'endpoint' => 'https://jkl.example.com',
+                'use_path_style_endpoint' => false,
+            ],
+        ]);
+
+        $this->be($disk->user);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('error AccessDenied'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('error accessDenied'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('error NoSuchBucket'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('error noSuchBucket'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('error NoSuchKey'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('error noSuchKey'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+    }
+
+    public function testUpdateInvalidDiskConfig()
+    {
+        $disk = UserDisk::factory()->create([
+            'type' => 's3',
+            'name' => 'abc',
+            'options' => [
+                'key' => 'def',
+                'secret' => 'ghi',
+                'bucket' => 'jkl',
+                'region' => 'us-east-1',
+                'endpoint' => 'https://jkl.example.com',
+                'use_path_style_endpoint' => false,
+            ],
+        ]);
+
+        $this->be($disk->user);
+        $this->mockS3->shouldReceive('validateDiskAccess')->once()->andThrow(new Exception('some other error'));
+        $this->putJson("/api/v1/user-disks/{$disk->id}", [
+            'type' => 'unknown',
+            'name' => 'cba',
+            'key' => 'fed',
+            'secret' => 'ihg',
+            'bucket' => 'onm',
+            'region' => 'us-east-2',
+            'endpoint' => 'https://onm.example.com',
+        ])->assertFound()
+            ->assertSessionHasErrors(['error']);
+    }
 }