`verifyDirectFileUpload` always returns `false` even when upload succeeds

[lukehutch]

I get an upload description and my client uploads a file to GCP Storage. If I look in the dev console, I can see that the file correctly uploaded to the right bucket.

However, when I use the same upload description and call verifyDirectFileUpload, the response is always false. There is no error message or exception.

I don't want to just ignore the return value -- what could cause this problem?

The Serverpod code that checks if an upload succeeded is simply:

  @override
  Future<bool> fileExists({
    required Session session,
    required String path,
  }) async {
    var response = await _s3Client.headObject(path);
    return response.statusCode == 200;
  }

so I assume a false response means that GCP is returning a status code other than 200. However this is the production server, so I can't just set a breakpoint here to check it.

It is quite urgent that I get this fixed, since I'm trying to push out my 1.0 release -- would appreciate any help with this. Thanks!

[lukehutch]

I added a debug statement to Serverpod, and headObject is returning status code 400 (Bad Request).

I verified my HMACAccessKeyId and HMACSecretKey are correct. I even re-generated these, replaced them in passwords.yaml, updated the GitHub secrets, re-deployed my server, and yet this didn't fix the problem.

The path parameter should be correct -- I am using paths without an initial /, as the docs suggest for S3 compatibility (there is no mention of this being a problem on GCP, but I assume this should work, and at least not give a 400 error). (The Cloud Console shows the paths starting with a slash, however.)

I suspect that Serverpod is actually sending a badly-formed request to Google (and maybe they changed something recently that broke these requests...).

I tried just ignoring the response from verifyDirectFileUpload as a temporary workaround, but then getPublicUrl returns null, so basically I can't bypass this issue. It's a showstopper for my 1.0 release.

[vlidholt]

Have you tried this with a local server? That may be way easier to debug.

I ran through the upload flow a few months back, and everything was working correctly for me back then. Are you sure your server instance have access to the bucket? There may be some incorrect permissions that prevents Serverpod from accessing it.

[lukehutch]

Yes, it works with a local server.

I will check the permissions (no idea how, but I'll look into it). Whatever is there is just the Terraform default though.

I need to find a solution urgently, because I have to launch in the next 1-2 days. I may have to use another storage provider, or write my own code for GCP, if I can't figure this out. I just assumed this would work given that all the other GCP stuff is now working.

[lukehutch]

@vlidholt This is what I'm seeing under Permissions:

Looks reasonable...

[lukehutch]

Rather than using the weird S3 compatibility layer for GCP, why not switch to using the "official" (but not yet declared stable) API from labs.dart.dev:

https://pub.dev/packages/gcloud

and/or

https://pub.dev/packages/googleapis

I tried implementing a GoogleCloudStorageNative using these:

import 'dart:convert';
import 'dart:io';
import 'dart:typed_data';

import 'package:gcloud/storage.dart';
import 'package:googleapis/storage/v1.dart' as gcpStorage;
import 'package:googleapis_auth/auth_io.dart' as auth;
import 'package:serverpod/serverpod.dart';

const _googleClientSecretPath = 'config/google_client_secret.json';

/// Concrete implementation of Google Cloud Storage, using native GCP APIs,
/// for use with Serverpod.
class GoogleCloudStorageNative extends CloudStorage {
  final String project;
  final String bucket;
  late final String publicHost;

  Storage? _storage;

  Future<Storage> get storage async {
    if (_storage == null) {
      // Read the service account credentials from the file.
      var jsonCredentials =
          await new File(_googleClientSecretPath).readAsString();
      var credentials =
          new auth.ServiceAccountCredentials.fromJson(jsonCredentials);
      var client =
          await auth.clientViaServiceAccount(credentials, Storage.SCOPES);
      _storage = new Storage(client, project);
    }
    return _storage!;
  }

  /// Creates a new [GoogleCloudStorageNative] object.
  GoogleCloudStorageNative({
    required Serverpod serverpod,
    required String storageId,
    required this.project,
    required this.bucket,
    String? publicHost,
  }) : super(storageId) {
    this.publicHost = publicHost ?? 'storage.googleapis.com/$bucket';
  }

  @override
  Future<void> storeFile({
    required Session session,
    required String path,
    required ByteData byteData,
    DateTime? expiration,
    bool verified = true,
  }) async {
    await (await storage)
        .bucket(bucket)
        .writeBytes(path, byteData.buffer.asUint8List());
  }

  @override
  Future<ByteData?> retrieveFile({
    required Session session,
    required String path,
  }) async {
    var byteLists = await (await storage).bucket(bucket).read(path).toList();
    var totLength =
        byteLists.fold<int>(0, (prev, element) => prev + element.length);
    var bytes = Uint8List(totLength);
    var offset = 0;
    for (var byteList in byteLists) {
      bytes.setRange(offset, offset + byteList.length, byteList);
      offset += byteList.length;
    }
    return ByteData.view(bytes.buffer);
  }

  @override
  Future<Uri?> getPublicUrl({
    required Session session,
    required String path,
  }) async {
    if (await fileExists(session: session, path: path)) {
      return Uri.parse('https://$publicHost/$path');
    }
    return null;
  }

  @override
  Future<bool> fileExists({
    required Session session,
    required String path,
  }) async {
    try {
      await (await storage).bucket(bucket).info(path);
      return true;
    } catch (e) {
      print('Failed to check if file exists: $e'); // TODO
      return false;
    }
  }

  @override
  Future<void> deleteFile({
    required Session session,
    required String path,
  }) async {
    await (await storage).bucket(bucket).delete(path);
  }

  @override
  Future<String?> createDirectFileUploadDescription({
    required Session session,
    required String path,
    Duration expirationDuration = const Duration(minutes: 10),
  }) async {
    // TODO V4 signing
  }

  @override
  Future<bool> verifyDirectFileUpload({
    required Session session,
    required String path,
  }) async {
    return fileExists(session: session, path: path);
  }
}

However, I could not figure out createDirectFileUploadDescription.

Refs:

• https://cloud.google.com/storage/docs/access-control/signing-urls-with-helpers
• https://cloud.google.com/storage/docs/access-control/signing-urls-manually

[lukehutch]

OK, I have an update on the 400 error. Google Cloud Storage is returning the following response to _doSignedHeadRequest, called by headObject, called by GoogleCloudStorage.fileExists:

<?xml version='1.0' encoding='UTF-8'?>
<Error>
  <Code>MalformedSecurityHeader</Code>
  <Message>Invalid argument.</Message>
  <Details>Your request has a malformed header. Header was included in signedheaders, but not in the request.</Details>
  <ParameterName>x-amz-security-token</ParameterName>
</Error>

However, if I just do a regular GET or HEAD request on the URL, without these security headers, I get the correct file content and headers. So this request does not even need to be signed, given the default permissions set up by Terraform.

[lukehutch]

I removed the signing of GET and HEAD requests in #2523, and the problem is now resolved for (verifyDirectFileUpload returns true, and getPublicUrl now returns the correct URL).

However I still have the same error for DELETE requests. For now I'm going to just live with the fact that files can be uploaded but never deleted, but this needs to be fixed too.

Based on the error message in the previous comment, do all the security headers also have to be included as URL query parameters?

[vlidholt]

I think your solution may only work for public files though? If they are private, authentication will likely be needed.

[lukehutch]

Probably, but I never understood what the purpose of private files was. I don't recall seeing a good explanation of that in the docs.

As I mentioned, deletion will still require fixing the signing, so I guess once that's done, the other request types could also be signed again.

[olivoil]

I also get a 400 status code when calling verifyDirectFileUpload or retrieveFile. The body is empty on head requests, but get requests return the following:

<?xml version='1.0' encoding='UTF-8'?>
<Error>
  <Code>MalformedSecurityHeader</Code>
  <Message>Invalid argument.</Message>
  <Details>
    Your request has a malformed header. Header was included in signedheaders, but not in the request.
  </Details>
  <ParameterName>x-amz-security-token</ParameterName>
</Error>

I am able to generate an upload description, and upload a file with the FileUploader. My HMAC credentials are correct. It happens with both public and private storageId. I created the buckets manually following the docs and am running the server locally.

[lukehutch]

@olivoil yep, that's exactly what I was running into.

@vlidholt something badly needs fixing here, this was a showstopper for my release, and even with my workaround of disabling signing for GET requests, users can upload files but not delete them, so I need a solution fairly quickly or I'll be hosting a lot of orphaned files.

[olivoil]

@lukehutch @vlidholt In case it helps, I'm doing this in the meantime: https://gist.github.com/olivoil/36a69cf37f54d3a009540202cc413827

I haven't tested extensively yet, but createDirectFileUploadDescription and verifyDirectFileUpload both seem to work with this.

[lukehutch]

@lukehutch @vlidholt In case it helps, I'm doing this in the meantime: https://gist.github.com/olivoil/36a69cf37f54d3a009540202cc413827

I haven't tested extensively yet, but createDirectFileUploadDescription and verifyDirectFileUpload both seem to work with this.

Nice work, I started building exactly the same thing, but eventually gave up and decided to hack together a solution for the existing S3 compatibility API.

Ultimately native Google Cloud API support needs to exist (working with Google Cloud Storage through the S3 API is 👎), so you should put together a proper PR around this -- it looks great to me!

[olivoil]

I can test it a bit more and create a PR. Some considerations with this approach:

• The gcloud dart sdk does not implement pre-signed urls out of the box - see outstanding issue - so the signing logic would be part of serverpod
• It uses a service account rather than HMAC keys. If going this way, it may be good to update the site documentation
• It expands the definitions of UploadDescription by adding httpMethod and headers and using them in FileUploader. I think that makes sense since when using signed urls, the headers must match the headers the url was signed with
• It sets the x-goog-acl header to public-read if the storage is configured as public to make files accessible publicly. The header is not set on storages configured as private, so the files are not accessible publicly.

@vlidholt would you be open to a PR?