fix: Request pre-signed url to upload picture via API

tests-qa/acceptance/040_Users/Avatar/070_S3Signature.robot

@@ -0,0 +1,138 @@
+*** Settings ***
+Library         RequestsLibrary
+Library         XML
+Resource        ../../ressources/vars/Urls.robot
+Resource        ../../ressources/Moves.robot
+Variables       ../../ressources/vars/users.yml
+Variables       ../../ressources/vars/moves.yml
+Variables       ../../ressources/vars/geokrety.yml
+Test Setup      Test Setup
+
+*** Variables ***
+
+@{ENDPOINTS_FOR_USER_1}    /api/v1/geokrety/${GEOKRETY_1.ref}/avatar/request-s3-file-signature
+...                        /api/v1/moves/${1}/avatar/request-s3-file-signature
+...                        /api/v1/users/${USER_1.id}/avatar/request-s3-file-signature
+...
+@{ENDPOINTS_FOR_USER_2}    /api/v1/geokrety/${GEOKRETY_2.ref}/avatar/request-s3-file-signature
+...                        /api/v1/moves/${2}/avatar/request-s3-file-signature
+...                        /api/v1/users/${USER_2.id}/avatar/request-s3-file-signature
+
+&{SECID_NOTHING}
+&{SECID_EMPTY}     secid=
+&{SECID_USER_1}    secid=${USER_1.secid}
+&{SECID_USER_2}    secid=${USER_2.secid}
+
+
+*** Test Cases ***
+
+Valid Parameters
+    [Template]    Post Request Valid
+    ${ENDPOINTS_FOR_USER_1}[0]    ${SECID_USER_1}
+    ${ENDPOINTS_FOR_USER_1}[1]    ${SECID_USER_1}
+    ${ENDPOINTS_FOR_USER_1}[2]    ${SECID_USER_1}
+
+    ${ENDPOINTS_FOR_USER_2}[0]    ${SECID_USER_2}
+    ${ENDPOINTS_FOR_USER_2}[1]    ${SECID_USER_2}
+    ${ENDPOINTS_FOR_USER_2}[2]    ${SECID_USER_2}
+
+
+Required Parameters
+    [Template]    Post Return Error
+    ${ENDPOINTS_FOR_USER_1}[0]    ${SECID_NOTHING}     200    Invalid "secid" length
+    ${ENDPOINTS_FOR_USER_1}[1]    ${SECID_NOTHING}     200    Invalid "secid" length
+    ${ENDPOINTS_FOR_USER_1}[2]    ${SECID_NOTHING}     200    Invalid "secid" length
+
+    ${ENDPOINTS_FOR_USER_1}[0]    ${SECID_EMPTY}       200    Invalid "secid" length
+    ${ENDPOINTS_FOR_USER_1}[1]    ${SECID_EMPTY}       200    Invalid "secid" length
+    ${ENDPOINTS_FOR_USER_1}[2]    ${SECID_EMPTY}       200    Invalid "secid" length
+
+    ${ENDPOINTS_FOR_USER_1}[0]    ${SECID_USER_2}      403    You are not the GeoKret owner
+    ${ENDPOINTS_FOR_USER_1}[1]    ${SECID_USER_2}      403    You have no write permission on this move
+    ${ENDPOINTS_FOR_USER_1}[2]    ${SECID_USER_2}      403    This is not your profile
+
+    ${ENDPOINTS_FOR_USER_2}[0]    ${SECID_USER_1}      403    You are not the GeoKret owner
+    ${ENDPOINTS_FOR_USER_2}[1]    ${SECID_USER_1}      403    You have no write permission on this move
+    ${ENDPOINTS_FOR_USER_2}[2]    ${SECID_USER_1}      403    This is not your profile
+
+*** Keywords ***
+
+Test Setup
+    Clear Database And Seed ${2} users
+    Seed ${1} geokrety owned by ${1}
+    Seed ${1} geokrety owned by ${2}
+    Post move    ${MOVE_1}
+    Post move    ${MOVE_41}
+
+
+Check Error In XML List
+  [Arguments]    ${root}   ${error}
+    ${value} =                            Get Elements                ${root}         xpath=.//error[.='${error}']
+    Length Should Be                      ${value}                    1
+
+Response Should be XML Error
+    [Arguments]    ${root}    @{errors}
+    Should Be Equal                       ${root.tag}                 gkxml
+
+    ${errors_count} =                     Get Length                  ${errors}
+    ${count} =                            XML.Get Element Count       ${root}         errors/error
+    Should Be Equal As Numbers            ${count}                    ${errors_count}
+
+    FOR     ${error}    IN    @{errors}
+      Check Error In XML List             ${root}                     ${error}
+    END
+
+Post Return Error
+    [Arguments]    ${url}    ${data}    ${code}    @{errors}
+    Create Session                          geokrety                    ${GK_URL}
+    ${auth} =     GET On Session            geokrety                    url=/
+
+    ${resp} =     POST On Session           geokrety                    url=${url}    data=${data}    expected_status=${code}
+    Should Not Be Empty    ${resp.content}
+
+    ${root} =     Parse XML                 ${resp.content}
+    Response Should be XML Error            ${root}                     @{errors}
+
+
+Response Should be XML Valid
+    [Arguments]    ${root}
+    Should Be Equal                       ${root.tag}                 gkxml
+
+
+    ${count} =                            XML.Get Element Count       ${root}         image-upload
+    Should Be Equal As Numbers            ${count}                    ${1}
+
+    ${count} =                            XML.Get Element Count       ${root}         image-upload/success
+    Should Be Equal As Numbers            ${count}                    ${1}
+    ${count} =                            XML.Get Element Count       ${root}         image-upload/uploadUrl
+    Should Be Equal As Numbers            ${count}                    ${1}
+    ${count} =                            XML.Get Element Count       ${root}         image-upload/s3Key
+    Should Be Equal As Numbers            ${count}                    ${1}
+    ${count} =                            XML.Get Element Count       ${root}         image-upload/headers
+    Should Be Equal As Numbers            ${count}                    ${1}
+    ${count} =                            XML.Get Element Count       ${root}         image-upload/headers/key
+    Should Be Equal As Numbers            ${count}                    ${1}
+    ${count} =                            XML.Get Element Count       ${root}         image-upload/headers/X-Amz-Credential
+    Should Be Equal As Numbers            ${count}                    ${1}
+    ${count} =                            XML.Get Element Count       ${root}         image-upload/headers/X-Amz-Algorithm
+    Should Be Equal As Numbers            ${count}                    ${1}
+    ${count} =                            XML.Get Element Count       ${root}         image-upload/headers/X-Amz-Date
+    Should Be Equal As Numbers            ${count}                    ${1}
+    ${count} =                            XML.Get Element Count       ${root}         image-upload/headers/Policy
+    Should Be Equal As Numbers            ${count}                    ${1}
+    ${count} =                            XML.Get Element Count       ${root}         image-upload/headers/X-Amz-Signature
+    Should Be Equal As Numbers            ${count}                    ${1}
+
+
+Post Request Valid
+    [Arguments]    ${url}    ${data}
+    Create Session                          geokrety                    ${GK_URL}
+    ${auth} =     GET On Session            geokrety                    url=/
+
+    ${resp} =     POST On Session           geokrety                    url=${url}    data=${data}
+    Should Not Be Empty    ${resp.content}
+
+    ${root} =     Parse XML                 ${resp.content}
+    Response Should be XML Valid            ${root}
+
+    Delete All Sessions

tests-qa/acceptance/ressources/vars/moves.yml

@@ -232,3 +232,16 @@ MOVE_36:
   comment: Hello
   app: robotframework
   app_ver: 3.2.1
+
+### Moves by Author 2 For GeoKret 2
+# DROP
+MOVE_41:
+  gkid: 2
+  move_type: 0
+  author: 2
+  waypoint: GC0001
+  lat: "43.60000"
+  lon: "7.00000"
+  comment: Hello
+  app: robotframework
+  app_ver: 3.2.1

website/app/GeoKrety/Controller/Pages/AbstractPictureUpload.php

@@ -4,14 +4,83 @@
 
 use Aws\S3\PostObjectV4;
 use GeoKrety\Model\Picture;
+use GeoKrety\Service\RateLimit;
 use GeoKrety\Service\S3Client;
+use GeoKrety\Service\Xml\Error;
+use GeoKrety\Service\Xml\Generic;
 use Sugar\Event;
 
+class UploadPermissionException extends \Exception {
+}
+
 abstract class AbstractPictureUpload extends Base {
     private string $imgKey;
 
-    public function request_s3_file_signature(\Base $f3) {
-        header('Content-Type: application/json; charset=utf-8');
+    public const CONTENT_TYPE_JSON = 'Content-Type: application/json; charset=utf-8';
+    public const CONTENT_TYPE_XML = 'Content-Type: application/xml; charset=utf-8';
+
+    private function wanted_response_content_type(\Base $f3) {
+        // Requests having secid are known to be XML
+        if ($f3->exists('REQUEST.secid')) {
+            return self::CONTENT_TYPE_XML;
+        }
+
+        // Default client is dropzone and is talking json
+        return self::CONTENT_TYPE_JSON;
+    }
+
+    private function set_response_content_type(\Base $f3) {
+        header($this->wanted_response_content_type($f3));
+    }
+
+    private function render_response(\Base $f3, array $response) {
+        if ($this->wanted_response_content_type($f3) === self::CONTENT_TYPE_XML) {
+            return Generic::buildGeneric(true, 'image-upload', $response);
+        }
+        // Default is Json
+        return json_encode($response);
+    }
+
+    /**
+     * Request a presigned url from the xml api
+     * Authentication is done by using a secid.
+     *
+     * @return void
+     */
+    public function request_s3_file_signature_api(\Base $f3) {
+        $this->set_response_content_type($f3);
+        $this->authenticate_via_secid($f3);
+        RateLimit::check_rate_limit_raw('API_V1_REQUEST_S3_FILE_SIGNATURE', $this->f3->get('REQUEST.secid'));
+        $data = $this->request_s3_file_signature($f3);
+        // Remove some dropzone internal headers on the public API
+        foreach (['success', 'uploadUrl', 's3Key'] as $key) {
+            $response[$key] = $data[$key];
+            unset($data[$key]);
+        }
+        $response = array_merge($response, ['headers' => $data]);
+        echo $this->render_response($f3, $response);
+    }
+
+    /**
+     * Request a presigned url from dropzone
+     * Authentication is done by the framework.
+     *
+     * @return void
+     */
+    public function request_s3_file_signature_ajax(\Base $f3) {
+        $this->set_response_content_type($f3);
+        $response = $this->request_s3_file_signature($f3);
+        echo $this->render_response($f3, $response);
+    }
+
+    private function request_s3_file_signature(\Base $f3) {
+        try {
+            $this->check_permission($f3);
+        } catch (UploadPermissionException $e) {
+            http_response_code(403);
+            Error::buildError(true, [$e->getMessage()]);
+            exit;
+        }
 
         $s3 = S3Client::instance()->getS3Public();
 
@@ -55,8 +124,8 @@ public function request_s3_file_signature(\Base $f3) {
                 'success' => 0,
                 'text' => $f3->get('validation.error'),
             ];
-            echo json_encode($response);
-            exit;
+
+            return $response;
         }
 
         try {
@@ -68,13 +137,13 @@ public function request_s3_file_signature(\Base $f3) {
                 'success' => 0,
                 'text' => 'Failed to store upload url into database.',
             ];
-            echo json_encode($response);
-            exit;
+
+            return $response;
         }
 
         Event::instance()->emit(sprintf('%s.presigned_request', $this->getEventNameBase()), $picture, $response);
 
-        echo json_encode($response);
+        return $response;
     }
 
     public function getImgKey() {
@@ -116,4 +185,13 @@ abstract public function getEventNameBase(): string;
     abstract public function getPictureType(): int;
 
     abstract public function setRelationships(Picture $picture): void;
+
+    /**
+     * Check if the current user has permission on this object.
+     *
+     * @return void
+     *
+     * @throws UploadPermissionException
+     */
+    abstract protected function check_permission(\Base $f3);
 }

website/app/GeoKrety/Controller/Pages/Base.php

@@ -63,6 +63,11 @@ public function loadCurrentUser() {
         }
     }
 
+    public function authenticate_via_secid(\Base $f3) {
+        $login = new Login();
+        $user = $login->secidAuth($this->f3, $this->f3->get('REQUEST.secid'));
+    }
+
     public function isLoggedIn(): bool {
         return !is_null($this->current_user);
     }

website/app/GeoKrety/Controller/Pages/GeokretAvatarUpload.php

@@ -32,4 +32,16 @@ public function getEventNameBase(): string {
     public function setRelationships(Picture $picture): void {
         $picture->geokret = $this->geokret->id;
     }
+
+    /**
+     * Check if the current user has permission on this object.
+     *
+     * @throws UploadPermissionException
+     */
+    protected function check_permission(\Base $f3): void {
+        if ($this->geokret->isOwner()) {
+            return;
+        }
+        throw new UploadPermissionException(_('You are not the GeoKret owner'));
+    }
 }

website/app/GeoKrety/Controller/Pages/MoveAvatarUpload.php

@@ -31,4 +31,16 @@ public function getEventNameBase(): string {
     public function setRelationships(Picture $picture): void {
         $picture->move = $this->move;
     }
+
+    /**
+     * Check if the current user has permission on this object.
+     *
+     * @throws UploadPermissionException
+     */
+    protected function check_permission(\Base $f3): void {
+        if ($this->hasWritePermission($this->move)) {
+            return;
+        }
+        throw new UploadPermissionException(_('You have no write permission on this move'));
+    }
 }

website/app/GeoKrety/Controller/Pages/MoveDelete.php

@@ -6,7 +6,14 @@
 use Sugar\Event;
 
 class MoveDelete extends Base {
-    use \MoveLoader;
+    use \MoveLoader {
+        beforeRoute as protected traitBeforeRoute;
+    }
+
+    public function beforeRoute(\Base $f3) {
+        $this->traitBeforeRoute($f3);
+        $this->checkAuthor($this->move);
+    }
 
     public function get(\Base $f3) {
         Smarty::render('extends:full_screen_modal.tpl|dialog/move_delete.tpl');

website/app/GeoKrety/Controller/Pages/Traits/MoveLoader.php

@@ -22,13 +22,15 @@ public function beforeRoute($f3) {
             $f3->error(404, _('This move does not exist.'));
         }
 
-        $this->checkAuthor($move);
-
         Smarty::assign('move', $this->move);
     }
 
+    protected function hasWritePermission(Move $move) {
+        return $move->isAuthor() || $move->geokret->isOwner();
+    }
+
     protected function checkAuthor(Move $move) {
-        if (!($move->isAuthor() || $move->geokret->isOwner())) {
+        if (!$this->hasWritePermission($move)) {
             \Base::instance()->error(403, _('You are not allowed to edit this move.'));
         }
     }

website/app/GeoKrety/Controller/Pages/UserAvatarUpload.php

@@ -31,4 +31,16 @@ public function getEventNameBase(): string {
     public function setRelationships(Picture $picture): void {
         $picture->user = $this->user;
     }
+
+    /**
+     * Check if the current user has permission on this object.
+     *
+     * @throws UploadPermissionException
+     */
+    protected function check_permission(\Base $f3): void {
+        if ($this->user->isCurrentUser()) {
+            return;
+        }
+        throw new UploadPermissionException(_('This is not your profile'));
+    }
 }

website/app/GeoKrety/Service/Config.php

@@ -374,6 +374,7 @@ public function __construct() {
             'API_V1_EXPORT2' => [1500, 60 * 60 * 24], // 1500/day
             'API_V1_EXPORT' => [12, 60], // 12/minute
             'API_V1_EXPORT_OC' => [12, 60], // 12/minute
+            'API_V1_REQUEST_S3_FILE_SIGNATURE' => [50, 60 * 60 * 24], // 50/day
             'API_GKT_V3_SEARCH' => [10000, 60 * 60 * 24], // 10000/day
             'API_GKT_V3_INVENTORY' => [1500, 60 * 60 * 24], // 1500/day
             'USERNAME_CHANGE' => [3, 60 * 60 * 24 * 28], // 3/month