ACL to private by default

[krzkrzkrz]

Is it possible to set the ACL for assets uploaded to private by default and let our bucket policy handle the permissions?

[sandstrom]

I really like asset sync, it's awesome! This adjustment would make it better though.

• First, it's good security practice to set things to private by default.
• Second, I think most people handle this using bucket policies or CloudFront Access Identities.

This is probably the line that should be switched, could introduce an option to modify it though.
https://github.com/rumblelabs/asset_sync/blob/master/lib/asset_sync/storage.rb#L135

[vdh]

Has anyone gotten anywhere with this?

I would monkey patch it for my own use, but the upload_file method doesn't look well suited to getting patched easily. I'd have to replicate the whole method just to change one default setting.

[wvidana]

This still open? here the line

asset_sync/lib/asset_sync/storage.rb

Line 151 in e006e5c

:public => true,

[PikachuEXE]

By reading the source I think custom_headers option can be used?

asset_sync/lib/asset_sync/storage.rb

Line 171 in e006e5c

elsif key = self.config.custom_headers.keys.detect {|k| f.match(Regexp.new(k))}

[pocari]

@PikachuEXE

I added following code to config/initializer/asset_sync.rb

    config.custom_headers = {
      '.*' => {
        'x-amz-acl' => 'private'
      }
    }

but Excon::Error::Forbidden: Expected(200) <=> Actual(403 Forbidden) has occured.(SignatureDoesNotMatch)

So I checked fog-aws gems codes, As @woqer have said, We need to be able to set public option.
@vdh 's #246 PR looks good to me.

[PikachuEXE]

The source code looks like this:

asset_sync/lib/asset_sync/storage.rb

Lines 168 to 178 in e006e5c

	if files_with_custom_headers.has_key? f
	file.merge! files_with_custom_headers[f]
	log "Overwriting #{f} with custom headers #{files_with_custom_headers[f].to_s}"
	elsif key = self.config.custom_headers.keys.detect {|k| f.match(Regexp.new(k))}
	headers = {}
	self.config.custom_headers[key].each do |k, value|
	headers[k.to_sym] = value
	end
	file.merge! headers
	log "Overwriting matching file #{f} with custom headers #{headers.to_s}"
	end

So I think it might work if you change 'x-amz-acl' => 'private' to public: false?

Since file is initialized as

asset_sync/lib/asset_sync/storage.rb

Lines 148 to 153 in e006e5c

	file = {
	:key => f,
	:body => file_handle,
	:public => true,
	:content_type => mime
	}

[pocari]

@PikachuEXE At first I thought so.

But If :public is true, acl set to public-read on fog-aws gem side.

https://github.com/fog/fog-aws/blob/master/lib/fog/aws/models/storage/file.rb#L152

[PikachuEXE]

OK I see what's the issue
Since public is a shortcut to set x-amz-acl to public-read / private
And you want that NOT to be set
Correct?

[pocari]

@PikachuEXE Since private is the default, you do not have to set it, but it is okay for me to explicitly do private.

[PikachuEXE]

So if I change the default to "x-amz-acl": "public-read"
Then you can override it without issue?

[pocari]

@PikachuEXE 👍

[PikachuEXE]

Oh sorry people want those to be handled by bucket policy
So If I make setting public: nil as removing public option, would that work for you guys?
@pocari @krzkrzkrz @sandstrom @vdh @woqer

The code change should be like:

elsif key = self.config.custom_headers.keys.detect {|k| f.match(Regexp.new(k))} 
   self.config.custom_headers[key].each do |k, value| 
     # Delete value on `nil`
     if value.nil?
       file.delete(k.to_sym)
     else
       file[k.to_sym] = value
     end
   end
   log "Overwriting matching file #{f} with custom headers #{headers.to_s}" 
 end 

[pocari]

@PikachuEXE
I think that it is better to control with the new property (like #249 ) than custom_headers.

Because I think that the setting will be as follows,

    config.custom_headers = {
      '.*' => {
        "public" => nil
      }
    }

but, In fact there is no http header called public.

[pocari]

This just came to my mind,

# configuration
# default value is true for compatibility.
config.config.fog_public = true | false, nil | :default

# in update_file method
...

case
when config.fog_public == :default
  file.delete(:public)
when config.fog_public
  file[:public] = true
else
  file[:public] = false
end

[PikachuEXE]

I think the default should still be true, but with nil
or some special value to indicate deletion of :public key from file

[pocari]

@PikachuEXE umm...

some special value to indicate deletion of :public

how about this?

# configuration
config.default_acl = true

# in update_file method
...

if config.default_acl
  file.delete(:public)
end

[PikachuEXE]

I think I will just add fog_public with special values
default_acl is not very understandable for people who don't know what ACL means (Is it AWS specific?)

[pocari]

@PikachuEXE

default_acl is not very understandable for people who don't know what ACL means (Is it AWS specific?)

Good point

I think I will just add fog_public with special values

👍

[wvidana]

I'd just try to respect the configs from provider instead of (re)setting them at the gem level. Is it possible to just get rid of the public option? If people need a different setup (public vs private) they should configure it somewhere else, or explicitly override it, but not set it by default.

PD: the concept of Access Control List (ACL) have been around in computing for some decades https://en.wikipedia.org/wiki/Access_control_list

[PikachuEXE]

@woqer
If this was written by me I will make it disabled by default so that you need to opt-in instead of opt-out
But for now since I don't want to bump major version yet
Let's add the option and see if that fits people's need first
If it does we can change the default value on next major version bump

[PikachuEXE]

Created #377, please let me know your opinion

[PikachuEXE]

I am releasing a new version with #377 if no one objects~

[sandstrom]

@PikachuEXE Awesome! 🎉

[aberenshtein]

@pocari not sure if this is still relevant, but this works for me
config.custom_headers = { '.*' => { 'acl' => 'bucket-owner-full-control' } }
https://www.rubydoc.info/gems/fog-aws/1.2.1/Fog/Storage/AWS/File#acl-instance_method