direct uploads to S3 for attachments including IFC models

Co-authored-by: Oliver Günther <mail@oliverguenther.de>

Gemfile

@@ -169,6 +169,7 @@ gem 'puma', '~> 4.3.5' # used for development and optionally for production
 gem 'nokogiri', '~> 1.10.8'
 
 gem 'carrierwave', '~> 1.3.1'
+gem 'carrierwave_direct', '~> 2.1.0'
 gem 'fog-aws'
 
 gem 'aws-sdk-core', '~> 3.91.0'

Gemfile.lock

@@ -334,6 +334,9 @@ GEM
       activemodel (>= 4.0.0)
       activesupport (>= 4.0.0)
       mime-types (>= 1.16)
+    carrierwave_direct (2.1.0)
+      carrierwave (>= 1.0.0)
+      fog-aws
     cells (4.1.7)
       declarative-builder (< 0.2.0)
       declarative-option (< 0.2.0)
@@ -989,6 +992,7 @@ DEPENDENCIES
   capybara (~> 3.32.0)
   capybara-screenshot (~> 1.0.17)
   carrierwave (~> 1.3.1)
+  carrierwave_direct (~> 2.1.0)
   cells-erb (~> 0.1.0)
   cells-rails (~> 0.0.9)
   commonmarker (~> 0.21.0)

app/models/attachment.rb

@@ -251,6 +251,31 @@ class Attachment < ApplicationRecord
     end
   end
 
+  def self.pending_direct_uploads
+    where(digest: "", downloads: -1)
+  end
+
+  def self.create_pending_direct_upload(file_name:, author:, container: nil, content_type: nil, file_size: 0)
+    a = create(
+      container: container,
+      author: author,
+      content_type: content_type || "application/octet-stream",
+      filesize: file_size,
+      digest: "",
+      downloads: -1
+    )
+
+    # We need to use update_column because `file` is an uploader which expects a File (not a string)
+    # to upload usually. But in this case the data has already been uploaded and we just point to it.
+    a.update_column :file, file_name
+
+    a.reload
+  end
+
+  def pending_direct_upload?
+    digest == "" && downloads == -1
+  end
+
   private
 
   def schedule_cleanup_uncontainered_job

app/uploaders/direct_fog_uploader.rb

@@ -0,0 +1,40 @@
+require_relative 'fog_file_uploader'
+
+class DirectFogUploader < FogFileUploader
+  include CarrierWaveDirect::Uploader
+
+  def self.for_attachment(attachment)
+    for_uploader attachment.file
+  end
+
+  def self.for_uploader(fog_file_uploader)
+    raise ArgumentError, "FogFileUploader expected" unless fog_file_uploader.is_a? FogFileUploader
+
+    uploader = self.new
+
+    uploader.instance_variable_set "@file", fog_file_uploader.file
+    uploader.instance_variable_set "@key", fog_file_uploader.path
+
+    uploader
+  end
+
+  def self.direct_fog_hash(attachment:, success_action_redirect: nil, success_action_status: "201")
+    uploader = for_attachment attachment
+
+    if success_action_redirect.present?
+      uploader.success_action_redirect = success_action_redirect
+      uploader.use_action_status = false
+    else
+      uploader.success_action_status = success_action_status
+      uploader.use_action_status = true
+    end
+
+    hash = uploader.direct_fog_hash(enforce_utf8: false)
+
+    if success_action_redirect.present?
+      hash.merge(success_action_redirect: success_action_redirect)
+    else
+      hash.merge(success_action_status: success_action_status)
+    end
+  end
+end

app/workers/attachments/cleanup_uncontainered_job.rb

@@ -36,6 +36,11 @@ class Attachments::CleanupUncontaineredJob < ApplicationJob
       .where(container: nil)
       .where(too_old)
       .destroy_all
+
+    Attachment
+      .pending_direct_uploads
+      .where(too_old)
+      .destroy_all # prepared direct uploads that never finished
   end
 
   private

app/workers/attachments/finish_direct_upload_job.rb

@@ -0,0 +1,53 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2020 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+class Attachments::FinishDirectUploadJob < ApplicationJob
+  queue_with_priority :high
+
+  def perform(attachment_id)
+    attachment = Attachment.pending_direct_uploads.where(id: attachment_id).first
+    local_file = attachment && attachment.file.local_file
+
+    if local_file.nil?
+      return Rails.logger.error("File for attachment #{attachment_id} was not uploaded.")
+    end
+
+    begin
+      attachment.downloads = 0
+      attachment.set_file_size local_file unless attachment.filesize && attachment.filesize > 0
+      attachment.set_content_type local_file unless attachment.content_type.present?
+      attachment.set_digest local_file unless attachment.digest.present?
+
+      attachment.save! if attachment.changed?
+    ensure
+      File.unlink(local_file.path) if File.exist?(local_file.path)
+    end
+  end
+end

config/initializers/carrierwave.rb

@@ -47,6 +47,8 @@ module CarrierWave
         config.fog_credentials = { provider: provider }.merge(credentials)
         config.fog_directory   = directory
         config.fog_public      = public
+
+        config.use_action_status = true
       end
     end
   end

config/initializers/secure_headers.rb

@@ -21,7 +21,7 @@ SecureHeaders::Configuration.default do |config|
   frame_src << OpenProject::Configuration[:security_badge_url]
 
   # Default src
-  default_src = %w('self')
+  default_src = %w('self') + [OpenProject::Configuration.remote_storage_host].compact
 
   # Allow requests to CLI in dev mode
   connect_src = default_src
@@ -56,7 +56,7 @@ SecureHeaders::Configuration.default do |config|
     # Allow fonts from self, asset host, or DATA uri
     font_src: assets_src + %w(data:),
     # Form targets can only be self
-    form_action: %w('self'),
+    form_action: default_src,
     # Allow iframe from vimeo (welcome video)
     frame_src: frame_src + %w('self'),
     frame_ancestors: %w('self'),

docs/development/running-tests/README.md

@@ -72,6 +72,22 @@ Due to flaky test results on Travis (`No output has been received in the last 10
 
 Firefox tests through Selenium are run with Chrome as `--headless` by default. To override this and watch the Chrome instance set the ENV variable `OPENPROJECT_TESTING_NO_HEADLESS=1`.
 
+##### Troubleshooting
+
+```
+Failure/Error: raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}"
+
+     ActionController::RoutingError:
+       No route matches [GET] "/javascripts/locales/en.js"
+```
+
+If you get an error like this when running feature specs it means your assets have not been built.
+You can fix this either by accessing a page locally (if the rails server is running) once or by precompiling the assets like this:
+
+```
+bundle exec rake assets:precompile
+```
+
 ### Cucumber
 
 **Note:** *We do not write new cucumber features. The current plan is to move away from

docs/installation-and-operations/configuration/README.md

@@ -38,6 +38,7 @@ Configuring OpenProject through environment variables is detailed [in this separ
 * [`omniauth_direct_login_provider`](#omniauth-direct-login-provider) (default: nil)
 * [`disable_password_login`](#disable-password-login) (default: false)
 * [`attachments_storage`](#attachments-storage) (default: file)
+* [`direct_uploads`](#direct-uploads) (default: true)
 * [`hidden_menu_items`](#hidden-menu-items) (default: {})
 * [`disabled_modules`](#disabled-modules) (default: [])
 * [`blacklisted_routes`](#blacklisted-routes) (default: [])
@@ -172,6 +173,21 @@ In the case of fog you only have to configure everything under `fog`, however. D
 to `fog` just yet. Instead leave it as `file`. This is because the current attachments storage is used as the source
 for the migration.
 
+### direct uploads
+
+*default: true*
+
+When using fog attachments uploaded in the frontend will be posted directly
+to the cloud rather than going through the OpenProject servers. This allows large attachments to be uploaded
+without the need to increase the `client_max_body_size` for the proxy in front of OpenProject.
+Also it prevents web processes from being blocked through long uploads.
+
+If, for what ever reason, this is undesirable, you can disable this option.
+In that case attachments will be posted as usual to the OpenProject server which then uploads the file
+to the remote storage in an extra step.
+
+**Note**: This only works for S3 right now. When using fog with another provider this configuration will be `false`. The same goes for when no fog storage is configured.
+
 ### Overriding the help link
 
 You can override the default help menu of OpenProject by specifying a `force_help_link` option to

frontend/src/app/angular4-modules.ts

@@ -48,6 +48,7 @@ import {OpenprojectPluginsModule} from "core-app/modules/plugins/openproject-plu
 import {ConfirmFormSubmitController} from "core-components/modals/confirm-form-submit/confirm-form-submit.directive";
 import {ProjectMenuAutocompleteComponent} from "core-components/projects/project-menu-autocomplete/project-menu-autocomplete.component";
 import {OpenProjectFileUploadService} from "core-components/api/op-file-upload/op-file-upload.service";
+import {OpenProjectDirectFileUploadService} from './components/api/op-file-upload/op-direct-file-upload.service';
 import {LinkedPluginsModule} from "core-app/modules/plugins/linked-plugins.module";
 import {HookService} from "core-app/modules/plugins/hook-service";
 import {DynamicBootstrapper} from "core-app/globals/dynamic-bootstrapper";
@@ -143,6 +144,7 @@ import {RevitAddInSettingsButtonService} from "core-app/modules/bim/revit_add_in
     { provide: APP_INITIALIZER, useFactory: initializeServices, deps: [Injector], multi: true },
     PaginationService,
     OpenProjectFileUploadService,
+    OpenProjectDirectFileUploadService,
     // Split view
     CommentService,
     ConfirmDialogService,

frontend/src/app/components/api/op-file-upload/op-direct-file-upload.service.ts

@@ -0,0 +1,153 @@
+//-- copyright
+// OpenProject is an open source project management software.
+// Copyright (C) 2012-2020 the OpenProject GmbH
+//
+// This program is free software; you can redistribute it and/or
+// modify it under the terms of the GNU General Public License version 3.
+//
+// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+// Copyright (C) 2006-2013 Jean-Philippe Lang
+// Copyright (C) 2010-2013 the ChiliProject Team
+//
+// This program is free software; you can redistribute it and/or
+// modify it under the terms of the GNU General Public License
+// as published by the Free Software Foundation; either version 2
+// of the License, or (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+//
+// See docs/COPYRIGHT.rdoc for more details.
+//++
+
+import {Injectable} from "@angular/core";
+import {HttpEvent, HttpResponse} from "@angular/common/http";
+import {HalResource} from "core-app/modules/hal/resources/hal-resource";
+import {from, Observable, of} from "rxjs";
+import {share, switchMap} from "rxjs/operators";
+import {OpenProjectFileUploadService, UploadBlob, UploadFile, UploadInProgress} from './op-file-upload.service';
+
+interface PrepareUploadResult {
+  url:string;
+  form:FormData;
+  response:any;
+}
+
+@Injectable()
+export class OpenProjectDirectFileUploadService extends OpenProjectFileUploadService {
+  /**
+   * Upload a single file, get an UploadResult observable
+   * @param {string} url
+   * @param {UploadFile} file
+   * @param {string} method
+   */
+  public uploadSingle(url:string, file:UploadFile|UploadBlob, method:string = 'post', responseType:'text'|'json' = 'text') {
+    const observable = from(this.getDirectUploadFormFrom(url, file))
+      .pipe(
+        switchMap(this.uploadToExternal(file, method, responseType)),
+        share()
+      );
+
+    return [file, observable] as UploadInProgress;
+  }
+
+  private uploadToExternal(file:UploadFile|UploadBlob, method:string, responseType:string):(result:PrepareUploadResult) => Observable<HttpEvent<unknown>> {
+    return result => {
+      result.form.append('file', file, file.customName || file.name);
+
+      return this
+        .http
+        .request<HalResource>(
+          method,
+          result.url,
+          {
+            body: result.form,
+            // Observe the response, not the body
+            observe: 'events',
+            // This is important as the CORS policy for the bucket is * and you can't use credentals then,
+            // besides we don't need them here anyway.
+            withCredentials: false,
+            responseType: responseType as any,
+            // Subscribe to progress events. subscribe() will fire multiple times!
+            reportProgress: true
+          }
+        )
+        .pipe(switchMap(this.finishUpload(result)));
+    };
+  }
+
+  private finishUpload(result:PrepareUploadResult):(result:HttpEvent<unknown>) => Observable<HttpEvent<unknown>> {
+    return event => {
+      if (event instanceof HttpResponse) {
+        return this
+          .http
+          .get(
+            result.response._links.completeUpload.href,
+            {
+              observe: 'response'
+            }
+          );
+      }
+
+      // Return as new observable due to switchMap
+      return of(event);
+    };
+  }
+
+  public getDirectUploadFormFrom(url:string, file:UploadFile|UploadBlob):Promise<PrepareUploadResult> {
+    const formData = new FormData();
+    const metadata = {
+      description: file.description,
+      fileName: file.customName || file.name,
+      fileSize: file.size,
+      contentType: file.type
+    };
+
+    /*
+     * @TODO We could calculate the MD5 hash here too and pass that.
+     * The MD5 hash can be used as the `content-md5` option during the upload to S3 for instance.
+     * This way S3 can verify the integrity of the file which we currently don't do.
+     */
+
+    // add the metadata object
+    formData.append(
+      'metadata',
+      JSON.stringify(metadata),
+    );
+
+    const result = this
+      .http
+      .request<HalResource>(
+        "post",
+        url,
+        {
+          body: formData,
+          withCredentials: true,
+          responseType: "json" as any
+        }
+      )
+      .toPromise()
+      .then((res) => {
+        let form = new FormData();
+
+        _.each(res._links.addAttachment.form_fields, (value, key) => {
+          form.append(key, value);
+        });
+
+        return { url: res._links.addAttachment.href, form: form, response: res };
+      })
+      .catch((err) => {
+        console.log(err);
+
+        return new FormData();
+      });
+
+    return result;
+  }
+}

frontend/src/app/components/api/op-file-upload/op-file-upload.service.spec.ts

@@ -27,6 +27,7 @@
 //++
 
 import {OpenProjectFileUploadService, UploadFile, UploadResult} from './op-file-upload.service';
+import {OpenProjectDirectFileUploadService} from "core-components/api/op-file-upload/op-direct-file-upload.service";
 import {HttpClientTestingModule, HttpTestingController} from "@angular/common/http/testing";
 import {getTestBed, TestBed} from "@angular/core/testing";
 import {HalResourceService} from "core-app/modules/hal/services/hal-resource.service";
@@ -45,6 +46,7 @@ describe('opFileUpload service', () => {
         {provide: States, useValue: new States()},
         I18nService,
         OpenProjectFileUploadService,
+        OpenProjectDirectFileUploadService,
         HalResourceService
       ]
     });

frontend/src/app/components/wp-edit-form/work-package-filter-values.spec.ts

@@ -48,6 +48,7 @@ import {PathHelperService} from "core-app/modules/common/path-helper/path-helper
 import {UIRouterModule} from "@uirouter/angular";
 import {LoadingIndicatorService} from "core-app/modules/common/loading-indicator/loading-indicator.service";
 import {OpenProjectFileUploadService} from "core-components/api/op-file-upload/op-file-upload.service";
+import {OpenProjectDirectFileUploadService} from "core-components/api/op-file-upload/op-direct-file-upload.service";
 import {HookService} from "core-app/modules/plugins/hook-service";
 import {IsolatedQuerySpace} from "core-app/modules/work_packages/query-space/isolated-query-space";
 import {HalEventsService} from "core-app/modules/hal/services/hal-events.service";
@@ -83,6 +84,7 @@ describe('WorkPackageFilterValues', () => {
         CurrentUserService,
         HookService,
         OpenProjectFileUploadService,
+        OpenProjectDirectFileUploadService,
         LoadingIndicatorService,
         HalResourceService,
         NotificationsService,

frontend/src/app/modules/apiv3/endpoints/work_packages/work-package-cache.spec.ts

@@ -35,6 +35,7 @@ import {OpenprojectHalModule} from 'core-app/modules/hal/openproject-hal.module'
 import {WorkPackageResource} from 'core-app/modules/hal/resources/work-package-resource';
 import {HalResourceService} from 'core-app/modules/hal/services/hal-resource.service';
 import {OpenProjectFileUploadService} from 'core-components/api/op-file-upload/op-file-upload.service';
+import {OpenProjectDirectFileUploadService} from "core-components/api/op-file-upload/op-direct-file-upload.service";
 import {SchemaCacheService} from 'core-components/schemas/schema-cache.service';
 import {States} from 'core-components/states.service';
 import {HalResourceNotificationService} from "core-app/modules/hal/services/hal-resource-notification.service";
@@ -70,7 +71,8 @@ describe('WorkPackageCache', () => {
         {provide: NotificationsService, useValue: {}},
         {provide: HalResourceNotificationService, useValue: {handleRawError: () => false}},
         {provide: WorkPackageNotificationService, useValue: {}},
-        {provide: OpenProjectFileUploadService, useValue: {}}
+        {provide: OpenProjectFileUploadService, useValue: {}},
+        {provide: OpenProjectDirectFileUploadService, useValue: {}},
       ]
     });
 

frontend/src/app/modules/common/config/configuration.service.ts

@@ -65,6 +65,14 @@ export class ConfigurationService {
     return this.userPreference('timeZone');
   }
 
+  public isDirectUploads() {
+    return !!this.prepareAttachmentURL;
+  }
+
+  public get prepareAttachmentURL() {
+    return _.get(this.configuration, ['prepareAttachment', 'href']);
+  }
+
   public get maximumAttachmentFileSize() {
     return this.systemPreference('maximumAttachmentFileSize');
   }

frontend/src/app/modules/hal/resources/mixins/attachable-mixin.ts

@@ -32,8 +32,10 @@ import {OpenProjectFileUploadService, UploadFile} from 'core-components/api/op-f
 import {HalResourceNotificationService} from "core-app/modules/hal/services/hal-resource-notification.service";
 import {PathHelperService} from 'core-app/modules/common/path-helper/path-helper.service';
 import {NotificationsService} from 'core-app/modules/common/notifications/notifications.service';
+import {ConfigurationService} from 'core-app/modules/common/config/configuration.service';
 import {HttpErrorResponse} from "@angular/common/http";
 import {APIV3Service} from "core-app/modules/apiv3/api-v3.service";
+import { OpenProjectDirectFileUploadService } from 'core-app/components/api/op-file-upload/op-direct-file-upload.service';
 
 type Constructor<T = {}> = new (...args:any[]) => T;
 
@@ -44,8 +46,10 @@ export function Attachable<TBase extends Constructor<HalResource>>(Base:TBase) {
     private NotificationsService:NotificationsService;
     private halNotification:HalResourceNotificationService;
     private opFileUpload:OpenProjectFileUploadService;
+    private opDirectFileUpload:OpenProjectDirectFileUploadService;
     private pathHelper:PathHelperService;
     private apiV3Service:APIV3Service;
+    private config:ConfigurationService;
 
     /**
      * Can be used in the mixed in class to disable
@@ -168,9 +172,11 @@ export function Attachable<TBase extends Constructor<HalResource>>(Base:TBase) {
     }
 
     private performUpload(files:UploadFile[]) {
-      let href = '';
+      let href: string = this.directUploadURL || '';
 
-      if (this.isNew || !this.id || !this.attachmentsBackend) {
+      if (href) {
+        return this.opDirectFileUpload.uploadAndMapResponse(href, files);
+      } else if (this.isNew || !this.id || !this.attachmentsBackend) {
         href = this.apiV3Service.attachments.path;
       } else {
         href = this.addAttachment.$link.href;
@@ -179,6 +185,18 @@ export function Attachable<TBase extends Constructor<HalResource>>(Base:TBase) {
       return this.opFileUpload.uploadAndMapResponse(href, files);
     }
 
+    private get directUploadURL():string|null {
+      if (this.$links.prepareAttachment) {
+        return this.$links.prepareAttachment.href;
+      }
+
+      if (this.isNew) {
+        return this.config.prepareAttachmentURL
+      } else {
+        return null;
+      }
+    }
+
     private updateState() {
       if (this.state) {
         this.state.putValue(this as any);
@@ -195,7 +213,12 @@ export function Attachable<TBase extends Constructor<HalResource>>(Base:TBase) {
       if (!this.opFileUpload) {
         this.opFileUpload = this.injector.get(OpenProjectFileUploadService);
       }
-
+      if (!this.opDirectFileUpload) {
+        this.opDirectFileUpload = this.injector.get(OpenProjectDirectFileUploadService);
+      }
+      if (!this.config) {
+        this.config = this.injector.get(ConfigurationService);
+      }
       if (!this.pathHelper) {
         this.pathHelper = this.injector.get(PathHelperService);
       }

frontend/src/app/modules/hal/resources/work-package-resource.spec.ts

@@ -42,6 +42,7 @@ import {ConfigurationService} from 'core-app/modules/common/config/configuration
 import {I18nService} from "core-app/modules/common/i18n/i18n.service";
 import {StateService} from "@uirouter/core";
 import {OpenProjectFileUploadService} from "core-components/api/op-file-upload/op-file-upload.service";
+import {OpenProjectDirectFileUploadService} from "core-components/api/op-file-upload/op-direct-file-upload.service";
 import {WorkPackageCreateService} from 'core-app/components/wp-new/wp-create.service';
 import {WorkPackageNotificationService} from "core-app/modules/work_packages/notifications/work-package-notification.service";
 import {WorkPackagesActivityService} from "core-components/wp-single-view-tabs/activity-panel/wp-activity.service";
@@ -76,6 +77,7 @@ describe('WorkPackage', () => {
         NotificationsService,
         ConfigurationService,
         OpenProjectFileUploadService,
+        OpenProjectDirectFileUploadService,
         LoadingIndicatorService,
         PathHelperService,
         I18nService,

frontend/src/app/modules/hal/resources/work-package-resource.ts

@@ -178,18 +178,6 @@ export class WorkPackageBaseResource extends HalResource {
     return fieldName === 'description' ? 'full' : 'constrained';
   }
 
-  private performUpload(files:UploadFile[]) {
-    let href = '';
-
-    if (this.isNew) {
-      href = this.apiV3Service.attachments.path;
-    } else {
-      href = this.attachments.$href!;
-    }
-
-    return this.opFileUpload.uploadAndMapResponse(href, files);
-  }
-
   public isParentOf(otherWorkPackage:WorkPackageResource) {
     return otherWorkPackage.parent?.$links.self.$link.href === this.$links.self.$link.href;
   }

lib/api/errors/not_found.rb

@@ -33,8 +33,10 @@ module API
       identifier 'NotFound'
       code 404
 
-      def initialize(*)
-        super I18n.t('api_v3.errors.code_404')
+      def initialize(*args)
+        opts = args.last.is_a?(Hash) ? args.last : {}
+
+        super opts[:message] || I18n.t('api_v3.errors.code_404')
       end
     end
   end

lib/api/v3/attachments/attachable_representer_mixin.rb

@@ -43,6 +43,15 @@ module API
             }
           end
 
+          link :prepareAttachment do
+            next unless OpenProject::Configuration.direct_uploads?
+
+            {
+              href: attachments_by_resource + '/prepare',
+              method: :post
+            }
+          end
+
           link :addAttachment,
                cache_if: -> do
                  represented.attachments_addable?(current_user)

lib/api/v3/attachments/attachment_metadata_representer.rb

@@ -46,6 +46,10 @@ module API
                  },
                  setter: ->(fragment:, **) { self.description = fragment['raw'] },
                  render_nil: true
+
+        property :content_type, render_nil: false
+        property :file_size, render_nil: false
+        property :digest, render_nil: false
       end
     end
   end

lib/api/v3/attachments/attachment_upload_representer.rb

@@ -0,0 +1,158 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2020 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'roar/decorator'
+require 'roar/json/hal'
+
+module API
+  module V3
+    module Attachments
+      class AttachmentUploadRepresenter < ::API::Decorators::Single
+        include API::Decorators::DateProperty
+        include API::Decorators::FormattableProperty
+        include API::Decorators::LinkedResource
+
+        self_link title_getter: ->(*) { represented.filename }
+
+        associated_resource :author,
+                            v3_path: :user,
+                            representer: ::API::V3::Users::UserRepresenter
+
+        def self.associated_container_getter
+          ->(*) do
+            next unless embed_links && container_representer
+
+            container_representer
+              .new(represented.container, current_user: current_user)
+          end
+        end
+
+        def self.associated_container_link
+          ->(*) do
+            return nil unless v3_container_name == 'nil_class' || api_v3_paths.respond_to?(v3_container_name)
+
+            ::API::Decorators::LinkObject
+              .new(represented,
+                   path: v3_container_name,
+                   property_name: :container,
+                   title_attribute: container_title_attribute)
+              .to_hash
+          end
+        end
+
+        attr_reader :form_url
+        attr_reader :form_fields
+
+        attr_reader :attachment
+
+        def initialize(attachment, options = {})
+          super
+
+          fog_hash = DirectFogUploader.direct_fog_hash attachment: attachment
+
+          @form_url = fog_hash[:uri]
+          @form_fields = fog_hash.except :uri
+          @attachment = attachment
+        end
+
+        associated_resource :container,
+                            getter: associated_container_getter,
+                            link: associated_container_link
+
+        link :addAttachment do
+          {
+            href: form_url,
+            method: :post,
+            form_fields: form_fields
+          }
+        end
+
+        link :delete do
+          {
+            href: api_v3_paths.attachment_upload(represented.id),
+            method: :delete
+          }
+        end
+
+        link :staticDownloadLocation do
+          {
+            href: api_v3_paths.attachment_content(attachment.id)
+          }
+        end
+
+        link :downloadLocation do
+          location = if attachment.external_storage?
+                       attachment.external_url
+                     else
+                       api_v3_paths.attachment_content(attachment.id)
+                     end
+          {
+            href: location
+          }
+        end
+
+        link :completeUpload do
+          {
+            href: api_v3_paths.attachment_uploaded(attachment.id)
+          }
+        end
+
+        property :id
+        property :file_name,
+                 getter: ->(*) { filename }
+
+        formattable_property :description,
+                             plain: true
+
+        date_time_property :created_at
+
+        def _type
+          'AttachmentUpload'
+        end
+
+        def container_representer
+          name = v3_container_name.camelcase
+
+          "::API::V3::#{name.pluralize}::#{name}Representer".constantize
+        rescue NameError
+          nil
+        end
+
+        def v3_container_name
+          ::API::Utilities::PropertyNameConverter.from_ar_name(represented.container.class.name.underscore).underscore
+        end
+
+        def container_title_attribute
+          represented.container.respond_to?(:subject) ? :subject : :title
+        end
+      end
+    end
+  end
+end

lib/api/v3/attachments/attachments_api.rb

@@ -39,13 +39,25 @@ module API
             def container
               nil
             end
+
+            def check_attachments_addable
+              raise API::Errors::Unauthorized if Redmine::Acts::Attachable.attachables.none?(&:attachments_addable?)
+            end
           end
 
           post do
-            raise API::Errors::Unauthorized if Redmine::Acts::Attachable.attachables.none?(&:attachments_addable?)
+            check_attachments_addable
 
-            ::API::V3::Attachments::AttachmentRepresenter.new(parse_and_create,
-                                                              current_user: current_user)
+            ::API::V3::Attachments::AttachmentRepresenter.new(parse_and_create, current_user: current_user)
+          end
+
+          namespace :prepare do
+            post do
+              require_direct_uploads
+              check_attachments_addable
+
+              ::API::V3::Attachments::AttachmentUploadRepresenter.new(parse_and_prepare, current_user: current_user)
+            end
           end
 
           route_param :id, type: Integer, desc: 'Attachment ID' do
@@ -70,6 +82,18 @@ module API
                 respond_with_attachment @attachment, cache_seconds: 604799
               end
             end
+
+            namespace :uploaded do
+              get do
+                attachment = Attachment.pending_direct_uploads.where(id: params[:id]).first!
+
+                raise API::Errors::NotFound unless attachment.file.readable?
+
+                ::Attachments::FinishDirectUploadJob.perform_later attachment.id
+
+                ::API::V3::Attachments::AttachmentRepresenter.new(attachment, current_user: current_user)
+              end
+            end
           end
         end
       end

lib/api/v3/attachments/attachments_by_container_api.rb

@@ -56,13 +56,40 @@ module API
             unless metadata.file_name
               raise ::API::Errors::Validation.new(
                 :file_name,
-                "fileName #{I18n.t('activerecord.errors.messages.blank')}."
+                "fileName #{I18n.t('activerecord.errors.messages.blank')}"
               )
             end
 
             metadata
           end
 
+          def parse_and_prepare
+            metadata = parse_metadata params[:metadata]
+
+            unless metadata
+              raise ::API::Errors::InvalidRequestBody.new(I18n.t('api_v3.errors.multipart_body_error'))
+            end
+
+            unless metadata.file_size
+              raise ::API::Errors::Validation.new(
+                :file_size,
+                "fileSize #{I18n.t('activerecord.errors.messages.blank')}"
+              )
+            end
+
+            create_attachment metadata
+          end
+
+          def create_attachment(metadata)
+            Attachment.create_pending_direct_upload(
+              file_name: metadata.file_name,
+              container: container,
+              author: current_user,
+              content_type: metadata.content_type,
+              file_size: metadata.file_size
+            )
+          end
+
           def parse_and_create
             metadata = parse_metadata params[:metadata]
             file = params[:file]
@@ -87,6 +114,20 @@ module API
             end
           end
 
+          def check_permissions(permissions)
+            if permissions.empty?
+              raise API::Errors::Unauthorized unless container.attachments_addable?(current_user)
+            else
+              authorize_any(permissions, projects: container.project)
+            end
+          end
+
+          def require_direct_uploads
+            unless OpenProject::Configuration.direct_uploads?
+              raise API::Errors::NotFound, message: "Only available if direct uploads are enabled."
+            end
+          end
+
           def with_handled_create_errors
             yield
           rescue ActiveRecord::RecordInvalid => error
@@ -125,16 +166,21 @@ module API
 
         def self.create(permissions = [])
           -> do
-            if permissions.empty?
-              raise API::Errors::Unauthorized unless container.attachments_addable?(current_user)
-            else
-              authorize_any(permissions, projects: container.project)
-            end
+            check_permissions permissions
 
             ::API::V3::Attachments::AttachmentRepresenter.new(parse_and_create,
                                                               current_user: current_user)
           end
         end
+
+        def self.prepare(permissions = [])
+          -> do
+            require_direct_uploads
+            check_permissions permissions
+
+            ::API::V3::Attachments::AttachmentUploadRepresenter.new(parse_and_prepare, current_user: current_user)
+          end
+        end
       end
     end
   end

lib/api/v3/attachments/attachments_by_post_api.rb

@@ -45,6 +45,10 @@ module API
 
           get &API::V3::Attachments::AttachmentsByContainerAPI.read
           post &API::V3::Attachments::AttachmentsByContainerAPI.create([:edit_messages])
+
+          namespace :prepare do
+            post &API::V3::Attachments::AttachmentsByContainerAPI.prepare([:edit_messages])
+          end
         end
       end
     end

lib/api/v3/attachments/attachments_by_wiki_page_api.rb

@@ -45,6 +45,10 @@ module API
 
           get &API::V3::Attachments::AttachmentsByContainerAPI.read
           post &API::V3::Attachments::AttachmentsByContainerAPI.create
+
+          namespace :prepare do
+            post &API::V3::Attachments::AttachmentsByContainerAPI.prepare
+          end
         end
       end
     end

lib/api/v3/attachments/attachments_by_work_package_api.rb

@@ -54,6 +54,10 @@ module API
           # :edit_work_packages in this endpoint and require clients to upload uncontainered work packages
           # first and attach them on wp creation.
           post &API::V3::Attachments::AttachmentsByContainerAPI.create([:edit_work_packages])
+
+          namespace :prepare do
+            post &API::V3::Attachments::AttachmentsByContainerAPI.prepare([:edit_work_packages])
+          end
         end
       end
     end

lib/api/v3/configuration/configuration_representer.rb

@@ -46,6 +46,15 @@ module API
           }
         end
 
+        link :prepareAttachment do
+          next unless OpenProject::Configuration.direct_uploads?
+
+          {
+            href: api_v3_paths.prepare_new_attachment_upload,
+            method: :post
+          }
+        end
+
         property :maximum_attachment_file_size,
                  getter: ->(*) { attachment_max_size.to_i.kilobyte }
 

lib/api/v3/utilities/path_helper.rb

@@ -123,6 +123,17 @@ module API
             "#{wiki_page(id)}/attachments"
           end
 
+          def self.prepare_new_attachment_upload
+            "#{root}/attachments/prepare"
+          end
+
+          index :attachment_upload
+          show :attachment_upload
+
+          def self.attachment_uploaded(attachment_id)
+            "#{root}/attachments/#{attachment_id}/uploaded"
+          end
+
           def self.available_assignees(project_id)
             "#{project(project_id)}/available_assignees"
           end

lib/open_project/configuration.rb

@@ -46,6 +46,10 @@ module OpenProject
       'autologin_cookie_path'   => '/',
       'autologin_cookie_secure' => false,
       'database_cipher_key'     => nil,
+      # only applicable in conjunction with fog (effectively S3) attachments
+      # which will be uploaded directly to the cloud storage rather than via OpenProject's
+      # server process.
+      'direct_uploads'          => true,
       'show_community_links' => true,
       'log_level' => 'info',
       'scm_git_command'         => nil,

lib/open_project/configuration/helpers.rb

@@ -41,6 +41,20 @@ module OpenProject
         (self['attachments_storage'] || 'file').to_sym
       end
 
+      ##
+      # We only allow direct uploads to S3 as we are using the carrierwave_direct
+      # gem which only supports S3 for the time being.
+      def direct_uploads
+        return false unless remote_storage?
+        return false unless remote_storage_aws?
+
+        self['direct_uploads']
+      end
+
+      def direct_uploads?
+        direct_uploads
+      end
+
       # Augur connect host
       def enterprise_trial_creation_host
         if Rails.env.production?
@@ -54,6 +68,20 @@ module OpenProject
         attachments_storage == :file
       end
 
+      def remote_storage?
+        attachments_storage == :fog
+      end
+
+      def remote_storage_aws?
+        fog_credentials[:provider] == "AWS"
+      end
+
+      def remote_storage_host
+        if remote_storage_aws?
+          "#{fog_directory}.s3.amazonaws.com"
+        end
+      end
+
       def attachments_storage_path
         Rails.root.join(self['attachments_storage_path'] || 'files')
       end

modules/bim/app/contracts/bim/ifc_models/base_contract.rb

@@ -68,16 +68,20 @@ module Bim
       end
 
       def ifc_attachment_is_ifc
-        return unless model.ifc_attachment&.new_record?
+        return unless model.ifc_attachment&.new_record? || model.ifc_attachment&.pending_direct_upload?
 
-        firstline = File.open(model.ifc_attachment.file.file.path, &:readline)
+        file_path = model.ifc_attachment.file.local_file.path
 
         begin
+          firstline = File.open(file_path, &:readline)
+
           unless firstline.match?(/^ISO-10303-21;/)
             errors.add :base, :invalid_ifc_file
           end
         rescue ArgumentError
           errors.add :base, :invalid_ifc_file
+        ensure
+          FileUtils.rm file_path if File.exists? file_path
         end
       end
 

modules/bim/app/controllers/bim/ifc_models/ifc_models_controller.rb

@@ -33,11 +33,12 @@ module Bim
     class IfcModelsController < BaseController
       helper_method :gon
 
-      before_action :find_project_by_project_id, only: %i[index new create show defaults edit update destroy]
+      before_action :find_project_by_project_id, only: %i[index new create show defaults edit update destroy direct_upload_finished]
       before_action :find_ifc_model_object, only: %i[edit update destroy]
       before_action :find_all_ifc_models, only: %i[show defaults index]
 
-      before_action :authorize
+      before_action :authorize, except: [:set_direct_upload_file_name, :direct_upload_finished]
+      skip_before_action :verify_authenticity_token, only: [:set_direct_upload_file_name]
 
       menu_item :ifc_models
 
@@ -48,9 +49,25 @@ module Bim
 
       def new
         @ifc_model = @project.ifc_models.build
+
+        if OpenProject::Configuration.direct_uploads?
+          @pending_upload = Attachment.create_pending_direct_upload(file_name: "model.ifc", author: current_user)
+          @form = DirectFogUploader.direct_fog_hash(
+            attachment: @pending_upload,
+            success_action_redirect: direct_upload_finished_bcf_project_ifc_models_url
+          )
+        end
       end
 
-      def edit;
+      def edit
+        if OpenProject::Configuration.direct_uploads?
+          @pending_upload = Attachment.create_pending_direct_upload(file_name: "model.ifc", author: current_user)
+          @form = DirectFogUploader.direct_fog_hash(
+            attachment: @pending_upload,
+            success_action_redirect: direct_upload_finished_bcf_project_ifc_models_url
+          )
+          session[:pending_ifc_model_ifc_model_id] = @ifc_model.id
+        end
       end
 
       def show
@@ -61,6 +78,70 @@ module Bim
         frontend_redirect @ifc_models.defaults.pluck(:id).uniq
       end
 
+      def set_direct_upload_file_name
+        session[:pending_ifc_model_title] = params[:title]
+        session[:pending_ifc_model_is_default] = params[:isDefault]
+      end
+
+      def direct_upload_finished
+        id = request.params[:key].scan(/\/file\/(\d+)\//).flatten.first
+        attachment = Attachment.pending_direct_uploads.where(id: id).first
+
+        if attachment.nil? # this should not happen
+          flash[:error] = "Direct upload failed."
+
+          redirect_to action: :new
+        end
+
+        params = {
+          title: session[:pending_ifc_model_title],
+          project: @project,
+          ifc_attachment: attachment,
+          is_default: session[:pending_ifc_model_is_default]
+        }
+
+        new_model = true
+
+        if session[:pending_ifc_model_ifc_model_id]
+          ifc_model = Bim::IfcModels::IfcModel.find_by id: session[:pending_ifc_model_ifc_model_id]
+          new_model = false
+
+          call = ::Bim::IfcModels::UpdateService
+            .new(user: current_user, model: ifc_model)
+            .call(params.with_indifferent_access)
+
+          @ifc_model = call.result
+        else
+          call = ::Bim::IfcModels::CreateService
+            .new(user: current_user)
+            .call(params.with_indifferent_access)
+
+          @ifc_model = call.result
+        end
+
+        session.delete :pending_ifc_model_title
+        session.delete :pending_ifc_model_is_default
+        session.delete :pending_ifc_model_ifc_model_id
+
+        if call.success?
+          ::Attachments::FinishDirectUploadJob.perform_later attachment.id
+
+          if new_model
+            flash[:notice] = t('ifc_models.flash_messages.upload_successful')
+          else
+            flash[:notice] = t(:notice_successful_update)
+          end
+
+          redirect_to action: :index
+        else
+          attachment.destroy
+
+          flash[:error] = call.errors.full_messages.join(" ")
+
+          redirect_to action: :new
+        end
+      end
+
       def create
         combined_params = permitted_model_params
           .to_h

modules/bim/app/services/bim/ifc_models/set_attributes_service.rb

@@ -39,7 +39,7 @@ module Bim
         super
 
         change_by_system do
-          model.uploader = model.ifc_attachment&.author if model.ifc_attachment&.new_record?
+          model.uploader = model.ifc_attachment&.author if model.ifc_attachment&.new_record? || model.ifc_attachment&.pending_direct_upload?
         end
       end
 
@@ -61,14 +61,22 @@ module Bim
       end
 
       def set_title
-        model.title = model.ifc_attachment&.file&.filename&.gsub(/\.\w+$/, '')
+        model.title ||= model.ifc_attachment&.file&.filename&.gsub(/\.\w+$/, '')
       end
 
       def set_ifc_attachment(ifc_attachment)
         return unless ifc_attachment
 
         model.attachments.each(&:mark_for_destruction)
-        model.attach_files('first' => {'file' => ifc_attachment, 'description' => 'ifc'})
+
+        if ifc_attachment.is_a?(Attachment)
+          ifc_attachment.description = "ifc"
+          ifc_attachment.save! unless ifc_attachment.new_record?
+
+          model.attachments << ifc_attachment
+        else
+          model.attach_files('first' => {'file' => ifc_attachment, 'description' => 'ifc'})
+        end
       end
     end
   end

modules/bim/app/views/bim/ifc_models/ifc_models/_form.html.erb

@@ -35,9 +35,59 @@ See doc/COPYRIGHT.rdoc for more details.
     <%= f.text_field :title, required: true, container_class: '-wide' %>
   </div>
 <% end %>
+
+<% if OpenProject::Configuration.direct_uploads? %>
+  <% Hash(@form).each do |key, value| %>
+    <input type="hidden" name="<%= key %>" value="<%= value %>" />
+  <% end %>
+<% end %>
+
 <div class="form--field <%= @ifc_model.new_record? ? '-required': '' %>">
-  <%= f.file_field :ifc_attachment %>
+  <% if OpenProject::Configuration.direct_uploads? %>
+    <input class="form--file-field" type="file" name="file" />
+  <% else %>
+    <%= f.file_field :ifc_attachment %>
+  <% end %>
 </div>
+
 <div class="form--field">
   <%= f.check_box 'is_default' %>
 </div>
+
+<% if OpenProject::Configuration.direct_uploads? %>
+
+<%= nonced_javascript_tag do %>
+  jQuery(document).ready(function() {
+    jQuery("input[type=file]").change(function(e){
+      var fileName = e.target.files[0].name;
+
+      jQuery.post(
+        "<%= set_direct_upload_file_name_bcf_project_ifc_models_path %>",
+        {
+          title: fileName,
+          isDefault: jQuery("#bim_ifc_models_ifc_model_is_default").is(":checked") ? 1 : 0
+        }
+      );
+
+      // rebuild form to post to S3 directly
+      if (jQuery("input[name=utf8]").length == 1) {
+        jQuery("input[name=utf8]").remove();
+        jQuery("input[name=authenticity_token]").remove();
+        jQuery("input[name=_method]").remove();
+
+        var url = jQuery("input[name=uri]").val();
+
+        jQuery("form").attr("action", url);
+        jQuery("form").attr("enctype", "multipart/form-data");
+
+        jQuery("input[name=uri]").remove();
+
+        jQuery("form").submit(function() {
+          jQuery("#bim_ifc_models_ifc_model_title").prop("disabled", "disabled");
+        });
+      }
+    });
+  });
+<% end %>
+
+<% end %>

modules/bim/config/routes.rb

@@ -46,6 +46,8 @@ OpenProject::Application.routes.draw do
       resources :ifc_models, controller: 'bim/ifc_models/ifc_models' do
         collection do
           get :defaults
+          get :direct_upload_finished
+          post :set_direct_upload_file_name
         end
       end
     end

modules/bim/spec/features/bcf/direct_ifc_upload_spec.rb

@@ -0,0 +1,57 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2020 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+
+describe 'direct IFC upload', type: :feature, js: true, with_direct_uploads: :redirect, with_config: { edition: 'bim' } do
+  let(:user) { FactoryBot.create :admin }
+  let(:project) { FactoryBot.create :project, enabled_module_names: %i[bim] }
+  let(:ifc_fixture) { Rails.root.join('modules/bim/spec/fixtures/files/minimal.ifc') }
+
+  before do
+    login_as user
+
+    allow_any_instance_of(Bim::IfcModels::BaseContract).to receive(:ifc_attachment_is_ifc).and_return true
+  end
+
+  it 'should work' do
+    visit new_bcf_project_ifc_model_path(project_id: project.identifier)
+
+    page.attach_file("file", ifc_fixture, visible: :all)
+
+    click_on "Create"
+
+    expect(page).to have_content("Upload succeeded")
+
+    expect(Attachment.count).to eq 1
+    expect(Attachment.first[:file]).to eq 'model.ifc'
+
+    expect(Bim::IfcModels::IfcModel.count).to eq 1
+    expect(Bim::IfcModels::IfcModel.first.title).to eq "minimal.ifc"
+  end
+end

modules/grids/app/controllers/api/v3/attachments/attachments_by_grid_api.rb

@@ -45,6 +45,10 @@ module API
 
           get &API::V3::Attachments::AttachmentsByContainerAPI.read
           post &API::V3::Attachments::AttachmentsByContainerAPI.create
+
+          namespace :prepare do
+            post &API::V3::Attachments::AttachmentsByContainerAPI.prepare
+          end
         end
       end
     end

modules/meeting/lib/api/v3/attachments/attachments_by_meeting_content_api.rb

@@ -45,6 +45,10 @@ module API
 
           get &API::V3::Attachments::AttachmentsByContainerAPI.read
           post &API::V3::Attachments::AttachmentsByContainerAPI.create
+
+          namespace :prepare do
+            post &API::V3::Attachments::AttachmentsByContainerAPI.prepare
+          end
         end
       end
     end

spec/features/work_packages/attachments/attachment_upload_spec.rb

@@ -77,57 +77,86 @@ describe 'Upload attachment to work package', js: true do
     end
 
     context 'on a new page' do
-      let!(:new_page) { Pages::FullWorkPackageCreate.new }
-      let!(:type) { FactoryBot.create(:type_task) }
-      let!(:status) { FactoryBot.create(:status, is_default: true) }
-      let!(:priority) { FactoryBot.create(:priority, is_default: true) }
-      let!(:project) do
-        FactoryBot.create(:project, types: [type])
-      end
-
-      before do
-        visit new_project_work_packages_path(project.identifier, type: type.id)
-      end
-
-      it 'can upload an image via drag & drop (Regression #28189)' do
-        subject = new_page.edit_field :subject
-        subject.set_value 'My subject'
-
-        target = find('.ck-content')
-        attachments.drag_and_drop_file(target, image_fixture)
-
-        sleep 2
-        expect(page).not_to have_selector('notification-upload-progress')
-
-        editor.in_editor do |container, editable|
-          expect(editable).to have_selector('img[src*="/api/v3/attachments/"]', wait: 20)
+      shared_examples 'it supports image uploads via drag & drop' do
+        let!(:new_page) { Pages::FullWorkPackageCreate.new }
+        let!(:type) { FactoryBot.create(:type_task) }
+        let!(:status) { FactoryBot.create(:status, is_default: true) }
+        let!(:priority) { FactoryBot.create(:priority, is_default: true) }
+        let!(:project) do
+          FactoryBot.create(:project, types: [type])
         end
 
-        sleep 2
+        let(:post_conditions) { nil }
 
-        # Besides testing caption functionality this also slows down clicking on the submit button
-        # so that the image is properly embedded
-        caption = page.find('figure.image figcaption')
-        caption.click(x: 10, y: 10)
-        sleep 0.2
-        caption.base.send_keys('Some image caption')
+        before do
+          visit new_project_work_packages_path(project.identifier, type: type.id)
+        end
 
-        sleep 2
+        it 'can upload an image via drag & drop (Regression #28189)' do
+          subject = new_page.edit_field :subject
+          subject.set_value 'My subject'
 
-        scroll_to_and_click find('#work-packages--edit-actions-save')
+          target = find('.ck-content')
+          attachments.drag_and_drop_file(target, image_fixture)
 
-        wp_page.expect_notification(
-          message: 'Successful creation.'
-        )
+          sleep 2
+          expect(page).not_to have_selector('notification-upload-progress')
 
-        field = wp_page.edit_field :description
+          editor.in_editor do |container, editable|
+            expect(editable).to have_selector('img[src*="/api/v3/attachments/"]', wait: 20)
+          end
 
-        expect(field.display_element).to have_selector('img')
-        expect(field.display_element).to have_content('Some image caption')
+          sleep 2
 
-        wp = WorkPackage.last
-        expect(wp.subject).to eq('My subject')
-        expect(wp.attachments.count).to eq(1)
+          # Besides testing caption functionality this also slows down clicking on the submit button
+          # so that the image is properly embedded
+          caption = page.find('figure.image figcaption')
+          caption.click(x: 10, y: 10)
+          sleep 0.2
+          caption.base.send_keys('Some image caption')
+
+          scroll_to_and_click find('#work-packages--edit-actions-save')
+
+          wp_page.expect_notification(
+            message: 'Successful creation.'
+          )
+
+          field = wp_page.edit_field :description
+
+          expect(field.display_element).to have_selector('img')
+          expect(field.display_element).to have_content('Some image caption')
+
+          wp = WorkPackage.last
+          expect(wp.subject).to eq('My subject')
+          expect(wp.attachments.count).to eq(1)
+
+          post_conditions
+        end
+      end
+
+      it_behaves_like 'it supports image uploads via drag & drop'
+
+      # We do a complete integration test for direct uploads on this example.
+      # If this works all parts in the backend and frontend work properly together.
+      # Technically one could test this not only for new work packages, but also for existing
+      # ones, and for new and existing other attachable resources. But the code is the same
+      # everywhere so if this works it should work everywhere else too.
+      context 'with direct uploads', with_direct_uploads: true do
+        before do
+          allow_any_instance_of(Attachment).to receive(:diskfile).and_return Struct.new(:path).new(image_fixture.to_s)
+        end
+
+        it_behaves_like 'it supports image uploads via drag & drop' do
+          let(:post_conditions) do
+            # check the attachment was created successfully
+            expect(Attachment.count).to eq 1
+            a = Attachment.first
+            expect(a[:file]).to eq image_fixture.basename.to_s
+
+            # check /api/v3/attachments/:id/uploaded was called
+            expect(::Attachments::FinishDirectUploadJob).to have_been_enqueued
+          end
+        end
       end
     end
   end

spec/lib/api/v3/attachments/attachment_metadata_representer_spec.rb

@@ -31,14 +31,22 @@ require 'spec_helper'
 describe ::API::V3::Attachments::AttachmentMetadataRepresenter do
   include API::V3::Utilities::PathHelper
 
-  let(:metadata) {
+  let(:metadata) do
     data = Hashie::Mash.new
     data.file_name = original_file_name
     data.description = original_description
+    data.content_type = original_content_type
+    data.file_size = original_file_size
+    data.digest = original_digest
     data
-  }
+  end
+
   let(:original_file_name) { 'a file name' }
   let(:original_description) { 'a description' }
+  let(:original_content_type) { 'text/plain' }
+  let(:original_file_size) { 42 }
+  let(:original_digest) { "0xFF" }
+
   let(:representer) { ::API::V3::Attachments::AttachmentMetadataRepresenter.new(metadata) }
 
   describe 'generation' do
@@ -49,6 +57,9 @@ describe ::API::V3::Attachments::AttachmentMetadataRepresenter do
     end
 
     it { is_expected.to be_json_eql(original_file_name.to_json).at_path('fileName') }
+    it { is_expected.to be_json_eql(original_content_type.to_json).at_path('contentType') }
+    it { is_expected.to be_json_eql(original_file_size.to_json).at_path('fileSize') }
+    it { is_expected.to be_json_eql(original_digest.to_json).at_path('digest') }
 
     it_behaves_like 'API V3 formattable', 'description' do
       let(:format) { 'plain' }
@@ -60,7 +71,10 @@ describe ::API::V3::Attachments::AttachmentMetadataRepresenter do
     let(:parsed_hash) {
       {
         'fileName' => 'the parsed name',
-        'description' => { 'raw' => 'the parsed description' }
+        'description' => { 'raw' => 'the parsed description' },
+        'contentType' => 'text/html',
+        'fileSize' => 43,
+        'digest' => '0x00'
       }
     }
 
@@ -72,5 +86,8 @@ describe ::API::V3::Attachments::AttachmentMetadataRepresenter do
 
     it { expect(subject.file_name).to eql('the parsed name') }
     it { expect(subject.description).to eql('the parsed description') }
+    it { expect(subject.content_type).to eql('text/html') }
+    it { expect(subject.file_size).to eql(43) }
+    it { expect(subject.digest).to eql('0x00') }
   end
 end

spec/lib/open_project/configuration_spec.rb

@@ -480,4 +480,39 @@ describe OpenProject::Configuration do
       end
     end
   end
+
+  context 'helpers' do
+    describe '#direct_uploads?' do
+      let(:value) { OpenProject::Configuration.direct_uploads? }
+
+      it 'should be false by default' do
+        expect(value).to be false
+      end
+
+      context 'with remote storage' do
+        def self.storage(provider)
+          {
+            attachments_storage: :fog,
+            fog: {
+              credentials: {
+                provider: provider
+              }
+            }
+          }
+        end
+
+        context 'AWS', with_config: storage('AWS') do
+          it 'should be true' do
+            expect(value).to be true
+          end
+        end
+
+        context 'Azure', with_config: storage('azure') do
+          it 'should be false' do
+            expect(value).to be false
+          end
+        end
+      end
+    end
+  end
 end

spec/models/attachment_spec.rb

@@ -226,39 +226,18 @@ describe Attachment, type: :model do
     end
   end
 
-  describe "#external_url" do
+  # We just use with_direct_uploads here to make sure the
+  # FogAttachment class is defined and Fog is mocked.
+  describe "#external_url", with_direct_uploads: true do
     let(:author) { FactoryBot.create :user }
 
     let(:image_path) { Rails.root.join("spec/fixtures/files/image.png") }
     let(:text_path) { Rails.root.join("spec/fixtures/files/testfile.txt") }
     let(:binary_path) { Rails.root.join("spec/fixtures/files/textfile.txt.gz") }
 
-    let(:fog_attachment_class) do
-      class FogAttachment < Attachment
-        # Remounting the uploader overrides the original file setter taking care of setting,
-        # among other things, the content type. So we have to restore that original
-        # method this way.
-        # We do this in a new, separate class, as to not interfere with any other specs.
-        alias_method :set_file, :file=
-        mount_uploader :file, FogFileUploader
-        alias_method :file=, :set_file
-      end
-
-      FogAttachment
-    end
-
-    let(:image_attachment) { fog_attachment_class.new author: author, file: File.open(image_path) }
-    let(:text_attachment) { fog_attachment_class.new author: author, file: File.open(text_path) }
-    let(:binary_attachment) { fog_attachment_class.new author: author, file: File.open(binary_path) }
-
-    before do
-      Fog.mock!
-
-      connection = Fog::Storage.new provider: "AWS"
-      connection.directories.create key: "my-bucket"
-
-      CarrierWave::Configuration.configure_fog! credentials: {}, directory: "my-bucket", public: false
-    end
+    let(:image_attachment) { FogAttachment.new author: author, file: File.open(image_path) }
+    let(:text_attachment) { FogAttachment.new author: author, file: File.open(text_path) }
+    let(:binary_attachment) { FogAttachment.new author: author, file: File.open(binary_path) }
 
     shared_examples "it has a temporary download link" do
       let(:url_options) { {} }

spec/rails_helper.rb

@@ -47,6 +47,12 @@ require 'test_prof/recipes/rspec/before_all'
 # directory. Alternatively, in the individual `*_spec.rb` files, manually
 # require only the support files necessary.
 #
+# The files are sorted before requiring them to ensure the load order is the same
+# everywhere. There are certain helpers that depend on a expected order.
+# The CI may load the files in a different order than what you see locally which
+# may lead to broken specs on the CI, if we don't sort here
+# (example: with_config.rb has to precede with_direct_uploads.rb).
+#
 Dir[Rails.root.join('spec/support/**/*.rb')].each { |f| require f }
 Dir[Rails.root.join('spec/features/support/**/*.rb')].each { |f| require f }
 Dir[Rails.root.join('spec/lib/api/v3/support/**/*.rb')].each { |f| require f }

spec/requests/api/v3/attachments/attachment_resource_shared_examples.rb

@@ -29,6 +29,112 @@
 require 'spec_helper'
 require 'rack/test'
 
+shared_examples 'it supports direct uploads' do
+  include Rack::Test::Methods
+  include API::V3::Utilities::PathHelper
+  include FileHelpers
+
+  let(:container_href) { raise "define me!" }
+  let(:request_path) { raise "define me!" }
+
+  before do
+    allow(User).to receive(:current).and_return current_user
+  end
+
+  describe 'POST /prepare', with_settings: { attachment_max_size: 512 } do
+    let(:request_parts) { { metadata: metadata, file: file } }
+    let(:metadata) { { fileName: 'cat.png', fileSize: file.size }.to_json }
+    let(:file) { mock_uploaded_file(name: 'original-filename.txt') }
+
+    def request!
+      post request_path, request_parts
+    end
+
+    subject(:response) { last_response }
+
+    context 'with local storage' do
+      before do
+        request!
+      end
+
+      it 'should respond with HTTP Not Found' do
+        expect(subject.status).to eq(404)
+      end
+    end
+    
+    context 'with remote AWS storage', with_direct_uploads: true do
+      before do
+        request!
+      end
+
+      context 'with no filesize metadata' do
+        let(:metadata) { { fileName: 'cat.png' }.to_json }
+
+        it 'should respond with 422 due to missing file size metadata' do
+          expect(subject.status).to eq(422)
+          expect(subject.body).to include 'fileSize'
+        end
+      end
+
+      context 'with the correct parameters' do
+        let(:json) { JSON.parse subject.body }
+
+        it 'should prepare a direct upload' do
+          expect(subject.status).to eq 201
+
+          expect(json["_type"]).to eq "AttachmentUpload"
+          expect(json["fileName"]).to eq "cat.png"
+        end
+
+        describe 'response' do
+          describe '_links' do
+            describe 'container' do
+              let(:link) { json.dig "_links", "container" }
+
+              before do
+                expect(link).to be_present
+              end
+
+              it "it points to the expected container" do
+                expect(link["href"]).to eq container_href
+              end
+            end
+
+            describe 'addAttachment' do
+              let(:link) { json.dig "_links", "addAttachment" }
+
+              before do
+                expect(link).to be_present
+              end
+
+              it 'should point to AWS' do
+                expect(link["href"]).to eq "https://#{MockCarrierwave.bucket}.s3.amazonaws.com/"
+              end
+
+              it 'should have the method POST' do
+                expect(link["method"]).to eq "post"
+              end
+
+              it 'should include form fields' do
+                fields = link["form_fields"]
+
+                expect(fields).to be_present
+                expect(fields).to include(
+                  "key", "acl", "policy",
+                  "X-Amz-Signature", "X-Amz-Credential", "X-Amz-Algorithm", "X-Amz-Date",
+                  "success_action_status"
+                )
+
+                expect(fields["key"]).to end_with "cat.png"
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end
+
 shared_examples 'an APIv3 attachment resource', type: :request, content_type: :json do |include_by_container = true|
   include Rack::Test::Methods
   include API::V3::Utilities::PathHelper
@@ -366,6 +472,11 @@ shared_examples 'an APIv3 attachment resource', type: :request, content_type: :j
   end
 
   context 'by container', if: include_by_container do
+    it_behaves_like 'it supports direct uploads' do
+      let(:request_path) { "/api/v3/#{attachment_type}s/#{container.id}/attachments/prepare" }
+      let(:container_href) { "/api/v3/#{attachment_type}s/#{container.id}" }
+    end
+
     subject(:response) { last_response }
 
     describe '#get' do

spec/requests/api/v3/attachments_spec.rb

@@ -0,0 +1,103 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2020 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+require_relative 'attachments/attachment_resource_shared_examples'
+
+describe API::V3::Attachments::AttachmentsAPI, type: :request do
+  include Rack::Test::Methods
+  include API::V3::Utilities::PathHelper
+  include FileHelpers
+
+  let(:current_user) { FactoryBot.create(:user, member_in_project: project, member_through_role: role) }
+
+  let(:project) { FactoryBot.create(:project, public: false) }
+  let(:role) { FactoryBot.create(:role, permissions: permissions) }
+  let(:permissions) { [:add_work_packages] }
+
+  context(
+    'with missing permissions',
+    with_config: {
+      attachments_storage: :fog,
+      fog: { credentials: { provider: 'AWS' } }
+    }
+  ) do
+    let(:permissions) { [] }
+
+    let(:request_path) { api_v3_paths.prepare_new_attachment_upload }
+    let(:request_parts) { { metadata: metadata, file: file } }
+    let(:metadata) { { fileName: 'cat.png' }.to_json }
+    let(:file) { mock_uploaded_file(name: 'original-filename.txt') }
+
+    before do
+      post request_path, request_parts
+    end
+
+    it 'should forbid to prepare attachments' do
+      expect(last_response.status).to eq 403
+    end
+  end
+
+  it_behaves_like 'it supports direct uploads' do
+    let(:request_path) { api_v3_paths.prepare_new_attachment_upload }
+    let(:container_href) { nil }
+
+    describe 'GET /uploaded' do
+      let(:digest) { "" }
+      let(:attachment) { FactoryBot.create :attachment, digest: digest, author: current_user, container: nil, container_type: nil, downloads: -1 }
+
+      before do
+        get "/api/v3/attachments/#{attachment.id}/uploaded"
+      end
+
+      context 'with no pending attachments' do
+        let(:digest) { "0xFF" }
+
+        it 'should return 404' do
+          expect(last_response.status).to eq 404
+        end
+      end
+
+      context 'with a pending attachment' do
+        it 'should enqueue a FinishDirectUpload job' do
+          expect(::Attachments::FinishDirectUploadJob).to have_been_enqueued.at_least(1)
+        end
+
+        it 'should respond with HTTP OK' do
+          expect(last_response.status).to eq 200
+        end
+
+        it 'should return the attachment representation' do
+          json = JSON.parse last_response.body
+
+          expect(json["_type"]).to eq "Attachment"
+        end
+      end
+    end
+  end
+end

spec/support/carrierwave.rb

@@ -26,17 +26,31 @@
 # See docs/COPYRIGHT.rdoc for more details.
 #++
 
-mock_credentials = {
-  provider: 'AWS',
-  aws_access_key_id: 'someaccesskeyid',
-  aws_secret_access_key: 'someprivateaccesskey',
-  region: 'us-east-1'
-}
-mock_bucket = 'test-bucket'
+module MockCarrierwave
+  extend self
 
-Fog.mock!
-Fog.credentials = mock_credentials
-CarrierWave::Configuration.configure_fog! directory: mock_bucket, credentials: mock_credentials
+  def apply
+    Fog.mock!
+    Fog.credentials = credentials
 
-connection = Fog::Storage.new provider: mock_credentials[:provider]
-connection.directories.create key: mock_bucket
+    CarrierWave::Configuration.configure_fog! directory: bucket, credentials: credentials
+
+    connection = Fog::Storage.new provider: credentials[:provider]
+    connection.directories.create key: bucket
+  end
+
+  def bucket
+    'test-bucket'
+  end
+
+  def credentials
+    {
+      provider: 'AWS',
+      aws_access_key_id: 'someaccesskeyid',
+      aws_secret_access_key: 'someprivateaccesskey',
+      region: 'us-east-1'
+    }
+  end
+end
+
+MockCarrierwave.apply

spec/support/shared/with_config.rb

@@ -27,36 +27,64 @@
 # See docs/COPYRIGHT.rdoc for more details.
 #++
 
-def aggregate_mocked_configuration(example, config)
-  # We have to manually check parent groups for with_config:,
-  # since they are being ignored otherwise
-  example.example_group.module_parents.each do |parent|
-    if parent.respond_to?(:metadata) && parent.metadata[:with_config]
-      config.reverse_merge!(parent.metadata[:with_config])
+class WithConfig
+  attr_reader :context
+
+  def initialize(context)
+    @context = context
+  end
+
+  ##
+  # We need this so calls to rspec mocks (allow, expect etc.) will work here as expected.
+  def method_missing(method, *args, &block)
+    if context.respond_to?(method)
+      context.send method, *args, &block
+    else
+      super
     end
   end
 
-  config
+  ##
+  # Stubs the given configurations.
+  #
+  # @config [Hash] Hash containing the configurations with keys as seen in `configuration.rb`.
+  def before(example, config)
+    allow(OpenProject::Configuration).to receive(:[]).and_call_original
+
+    aggregate_mocked_configuration(example, config)
+      .with_indifferent_access
+      .each(&method(:stub_key))
+  end
+
+  def stub_key(key, value)
+    allow(OpenProject::Configuration)
+      .to receive(:[])
+      .with(key.to_s)
+      .and_return(value)
+
+    allow(OpenProject::Configuration)
+      .to receive(:[])
+      .with(key.to_sym)
+      .and_return(value)
+  end
+
+  def aggregate_mocked_configuration(example, config)
+    # We have to manually check parent groups for with_config:,
+    # since they are being ignored otherwise
+    example.example_group.module_parents.each do |parent|
+      if parent.respond_to?(:metadata) && parent.metadata[:with_config]
+        config.reverse_merge!(parent.metadata[:with_config])
+      end
+    end
+
+    config
+  end
 end
 
 RSpec.configure do |config|
   config.before(:each) do |example|
-    config = example.metadata[:with_config]
-    if config.present?
-      config = aggregate_mocked_configuration(example, config).with_indifferent_access
+    with_config = example.metadata[:with_config]
 
-      allow(OpenProject::Configuration).to receive(:[]).and_call_original
-      config.each do |k, v|
-        allow(OpenProject::Configuration)
-          .to receive(:[])
-          .with(k.to_s)
-          .and_return(v)
-
-        allow(OpenProject::Configuration)
-          .to receive(:[])
-          .with(k.to_sym)
-          .and_return(v)
-      end
-    end
+    WithConfig.new(self).before example, with_config if with_config.present?
   end
 end

spec/support/shared/with_direct_uploads.rb

@@ -0,0 +1,199 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2020 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+class WithDirectUploads
+  attr_reader :context
+
+  def initialize(context)
+    @context = context
+  end
+
+  ##
+  # We need this so calls to rspec mocks (allow, expect etc.) will work here as expected.
+  def method_missing(method, *args, &block)
+    if context.respond_to?(method)
+      context.send method, *args, &block
+    else
+      super
+    end
+  end
+
+  def before(example)
+    stub_config example
+
+    mock_attachment
+    stub_frontend redirect: redirect?(example) if stub_frontend?(example)
+
+    stub_uploader
+  end
+
+  def stub_frontend?(example)
+    example.metadata[:js]
+  end
+
+  def redirect?(example)
+    example.metadata[:with_direct_uploads] == :redirect
+  end
+
+  def around(example)
+    example.metadata[:driver] = :headless_firefox_billy
+
+    csp_config = SecureHeaders::Configuration.instance_variable_get("@default_config").csp
+    csp_config.connect_src = ["'self'", "my-bucket.s3.amazonaws.com"]
+
+    begin
+      example.run
+    ensure
+      csp_config.connect_src = %w('self')
+    end
+  end
+
+  def mock_attachment
+    allow(Attachment).to receive(:create) do |*args|
+      # We don't use create here because this would cause an infinite loop as FogAttachment's #create
+      # uses the base class's #create which is what we are mocking here. All this is necessary to begin
+      # with because the Attachment class is initialized with the LocalFileUploader before this test
+      # is ever run and we need remote attachments using the FogFileUploader in this scenario.
+      record = FogAttachment.new *args
+      record.save
+      record
+    end
+
+    # This is so the uploaded callback works. Since we can't actually substitute the Attachment class
+    # used there we get a LocalFileUploader file for the attachment which is not readable when
+    # everything else is mocked to be remote.
+    allow_any_instance_of(FileUploader).to receive(:readable?).and_return true
+  end
+
+  def stub_frontend(redirect: false)
+    proxy.stub("https://" + OpenProject::Configuration.remote_storage_host + ":443/", method: 'options').and_return(
+      headers: {
+        'Access-Control-Allow-Methods' => 'POST',
+        'Access-Control-Allow-Origin'  => '*'
+      },
+      code: 200
+    )
+
+    if redirect
+      stub_with_redirect
+    else # use status response instead of redirect by default
+      stub_with_status
+    end
+  end
+
+  def stub_with_redirect
+    proxy
+      .stub("https://" + OpenProject::Configuration.remote_storage_host + ":443/", method: 'post')
+      .and_return(Proc.new { |params, headers, body, url, method|
+        key = body.scan(/key"\s*([^\s]+)\s/m).flatten.first
+        redirect_url = body.scan(/success_action_redirect"\s*(http[^\s]+)\s/m).flatten.first
+        ok = body =~ /X-Amz-Signature/ # check that the expected post to AWS was made with the form fields
+
+        {
+          code: ok ? 302 : 403,
+          headers: {
+            'Location' => ok ? redirect_url + '?key=' + CGI.escape(key) : nil,
+            'Access-Control-Allow-Methods' => 'POST',
+            'Access-Control-Allow-Origin'  => '*'
+          }
+        }
+      })
+  end
+
+  def stub_with_status
+    proxy
+      .stub("https://" + OpenProject::Configuration.remote_storage_host + ":443/", method: 'post')
+      .and_return(Proc.new { |params, headers, body, url, method|
+        {
+          code: (body =~ /X-Amz-Signature/) ? 201 : 403, # check that the expected post to AWS was made with the form fields
+          headers: {
+            'Access-Control-Allow-Methods' => 'POST',
+            'Access-Control-Allow-Origin'  => '*'
+          }
+        }
+      })
+  end
+
+  def stub_uploader
+    creds = config[:fog][:credentials]
+
+    allow_any_instance_of(FogFileUploader).to receive(:fog_credentials).and_return creds
+
+    allow_any_instance_of(FogFileUploader).to receive(:aws_access_key_id).and_return creds[:aws_access_key_id]
+    allow_any_instance_of(FogFileUploader).to receive(:aws_secret_access_key).and_return creds[:aws_secret_access_key]
+    allow_any_instance_of(FogFileUploader).to receive(:provider).and_return creds[:provider]
+    allow_any_instance_of(FogFileUploader).to receive(:region).and_return creds[:region]
+    allow_any_instance_of(FogFileUploader).to receive(:directory).and_return config[:fog][:directory]
+
+    allow(OpenProject::Configuration).to receive(:direct_uploads?).and_return(true)
+  end
+
+  def stub_config(example)
+    WithConfig.new(context).before example, config
+  end
+
+  def config
+    {
+      attachments_storage: :fog,
+      fog: {
+        directory: MockCarrierwave.bucket,
+        credentials: MockCarrierwave.credentials
+      }
+    }
+  end
+end
+
+RSpec.configure do |config|
+  config.before(:each) do |example|
+    next unless example.metadata[:with_direct_uploads]
+
+    WithDirectUploads.new(self).before example
+
+    class FogAttachment < Attachment
+      # Remounting the uploader overrides the original file setter taking care of setting,
+      # among other things, the content type. So we have to restore that original
+      # method this way.
+      # We do this in a new, separate class, as to not interfere with any other specs.
+      alias_method :set_file, :file=
+      mount_uploader :file, FogFileUploader
+      alias_method :file=, :set_file
+    end
+  end
+
+  config.around(:each) do |example|
+    enabled = example.metadata[:with_direct_uploads]
+
+    if enabled
+      WithDirectUploads.new(self).around example
+    else
+      example.run
+    end
+  end
+end

spec/views/layouts/base.html.erb_spec.rb

@@ -267,4 +267,17 @@ describe 'layouts/base', type: :view do
       end
     end
   end
+
+  describe 'openproject_initializer meta tag' do
+    let(:current_user) { anonymous }
+    let(:base) { 'meta[name=openproject_initializer]' }
+
+    before do
+      render
+    end
+
+    it 'has the meta tag' do
+      expect(rendered).to have_selector(base, visible: false)
+    end
+  end
 end

spec/workers/attachments/cleanup_uncontainered_job_integration_spec.rb

@@ -32,6 +32,7 @@ require 'spec_helper'
 
 describe Attachments::CleanupUncontaineredJob, type: :job do
   let(:grace_period) { 120 }
+
   let!(:containered_attachment) { FactoryBot.create(:attachment) }
   let!(:old_uncontainered_attachment) do
     FactoryBot.create(:attachment, container: nil, created_at: Time.now - grace_period.minutes)
@@ -39,6 +40,17 @@ describe Attachments::CleanupUncontaineredJob, type: :job do
   let!(:new_uncontainered_attachment) do
     FactoryBot.create(:attachment, container: nil, created_at: Time.now - (grace_period - 1).minutes)
   end
+
+  let!(:finished_upload) do
+    FactoryBot.create(:attachment, created_at: Time.now - grace_period.minutes, digest: "0x42")
+  end
+  let!(:old_pending_upload) do
+    FactoryBot.create(:attachment, created_at: Time.now - grace_period.minutes, digest: "", downloads: -1)
+  end
+  let!(:new_pending_upload) do
+    FactoryBot.create(:attachment, created_at: Time.now - (grace_period - 1).minutes, digest: "", downloads: -1)
+  end
+
   let(:job) { described_class.new }
 
   before do
@@ -47,10 +59,10 @@ describe Attachments::CleanupUncontaineredJob, type: :job do
       .and_return(grace_period)
   end
 
-  it 'removes all uncontainered attachments that are older than the grace period' do
+  it 'removes all uncontainered attachments and pending uploads that are older than the grace period' do
     job.perform
 
     expect(Attachment.all)
-      .to match_array([containered_attachment, new_uncontainered_attachment])
+      .to match_array([containered_attachment, new_uncontainered_attachment, finished_upload, new_pending_upload])
   end
 end