Skip to content

Commit

Permalink
Merge pull request #2763 from matt-domsch-sp/govcloud-fips-entrypoint
Browse files Browse the repository at this point in the history
The CarrierWave::Storage::File#public_url method returns the standard
  • Loading branch information
mshibuya authored Dec 1, 2024
2 parents a60d8d8 + 0d5a661 commit 054bd3d
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 8 deletions.
23 changes: 16 additions & 7 deletions lib/carrierwave/storage/fog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ module Storage
# [:fog_use_ssl_for_aws] (optional) #public_url will use https for the AWS generated URL]
# [:fog_aws_accelerate] (optional) #public_url will use s3-accelerate subdomain
# instead of s3, defaults to false
# [:fog_aws_fips] (optional) #public_url will use s3-fips subdomain
# instead of s3, defaults to false
#
#
# AWS credentials contain the following keys:
Expand Down Expand Up @@ -374,21 +376,28 @@ def public_url
when 'AWS'
# check if some endpoint is set in fog_credentials
if @uploader.fog_credentials.has_key?(:endpoint)
"#{@uploader.fog_credentials[:endpoint]}/#{@uploader.fog_directory}/#{encoded_path}"
if !@uploader.fog_aws_fips
"#{@uploader.fog_credentials[:endpoint]}/#{@uploader.fog_directory}/#{encoded_path}"
else
warn 'Use of options :endpoint and :fog_aws_fips=true together will fail, as FIPS endpoints do not support path-style URLs.'
nil
end
else
protocol = @uploader.fog_use_ssl_for_aws ? "https" : "http"

subdomain_regex = /^(?:[a-z]|\d(?!\d{0,2}(?:\d{1,3}){3}$))(?:[a-z0-9\.]|(?![\-])|\-(?![\.])){1,61}[a-z0-9]$/
# To use the virtual-hosted style, the bucket name needs to be representable as a subdomain
use_virtual_hosted_style = @uploader.fog_directory.to_s =~ subdomain_regex && !(protocol == 'https' && @uploader.fog_directory =~ /\./)

return nil if !use_virtual_hosted_style && @uploader.fog_aws_fips # FIPS Endpoints can only be used with Virtual Hosted-Style addressing.

region = @uploader.fog_credentials[:region].to_s
regional_host = case region
when DEFAULT_S3_REGION, ''
's3.amazonaws.com'
else
"s3.#{region}.amazonaws.com"
end
regional_host = 's3.amazonaws.com' # used for DEFAULT_S3_REGION or no region set
if @uploader.fog_aws_fips
regional_host = "s3-fips.#{region}.amazonaws.com" # https://aws.amazon.com/compliance/fips/
elsif ![DEFAULT_S3_REGION, ''].include?(region)
regional_host = "s3.#{region}.amazonaws.com"
end

if use_virtual_hosted_style
regional_host = 's3-accelerate.amazonaws.com' if @uploader.fog_aws_accelerate
Expand Down
2 changes: 2 additions & 0 deletions lib/carrierwave/uploader/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ module Configuration
add_config :fog_authenticated_url_expiration
add_config :fog_use_ssl_for_aws
add_config :fog_aws_accelerate
add_config :fog_aws_fips

# Mounting
add_config :ignore_integrity_errors
Expand Down Expand Up @@ -197,6 +198,7 @@ def reset_config
config.fog_authenticated_url_expiration = 600
config.fog_use_ssl_for_aws = true
config.fog_aws_accelerate = false
config.fog_aws_fips = false
config.store_dir = 'uploads'
config.cache_dir = 'uploads/tmp'
config.delete_tmp_file_after_storage = true
Expand Down
54 changes: 53 additions & 1 deletion spec/storage/fog_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,11 @@ def check_file
describe "CarrierWave::Storage::Fog::File" do
let(:store_path) { 'uploads/test.jpg' }
let(:fog_public) { true }
let(:endpoint) { nil }
before do
allow(@uploader).to receive(:store_path).and_return(store_path)
allow(@uploader).to receive(:fog_public).and_return(fog_public)
allow(@uploader).to receive(:endpoint).and_return(endpoint)
@fog_file = @storage.store!(CarrierWave::SanitizedFile.new(stub_file('test.jpg', 'image/jpeg')))
end

Expand Down Expand Up @@ -504,6 +506,20 @@ def check_file
expect(@fog_file.public_url).to include("https://#{CARRIERWAVE_DIRECTORY}.s3-accelerate.amazonaws.com")
end

it 'returns nil when both :endpoint and :fog_aws_fips=true' do
allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(endpoint: 'https://custom-endpoint.example.com'))
allow(@uploader).to receive(:fog_directory).and_return('SiteAssets')
allow(@uploader).to receive(:fog_aws_fips).and_return(true)
expect(@fog_file.url).to be nil
end

it 'returns endpoint+bucket when :endpoint and !:fog_aws_fips' do
allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(endpoint: 'https://custom-endpoint.example.com'))
allow(@uploader).to receive(:fog_directory).and_return('SiteAssets')
allow(@uploader).to receive(:fog_aws_fips).and_return(false)
expect(@fog_file.url).to include('https://custom-endpoint.example.com/SiteAssets')
end

context 'when the directory is not a valid subdomain' do
it "should not use a subdomain URL for AWS" do
allow(@uploader).to receive(:fog_directory).and_return('SiteAssets')
Expand All @@ -526,7 +542,8 @@ def check_file
nil => 's3.amazonaws.com',
'us-east-1' => 's3.amazonaws.com',
'us-east-2' => 's3.us-east-2.amazonaws.com',
'eu-central-1' => 's3.eu-central-1.amazonaws.com'
'eu-central-1' => 's3.eu-central-1.amazonaws.com',
'us-gov-west-1' => 's3.us-gov-west-1.amazonaws.com'
}.each do |region, expected_host|
it "should use a #{expected_host} hostname when using path style for access #{region} region" do
allow(@uploader).to receive(:fog_use_ssl_for_aws).and_return(true)
Expand All @@ -539,6 +556,23 @@ def check_file
end
end

context 'when the directory is not a valid subdomain and :fog_aws_fips' do
[
'us-east-1',
'us-east-2',
'us-gov-west-1'
].each do |region|
it "public_url should be nil" do
allow(@uploader).to receive(:fog_use_ssl_for_aws).and_return(true)
allow(@uploader).to receive(:fog_directory).and_return('foo.bar')
allow(@uploader).to receive(:fog_aws_fips).and_return(true)
allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(region: region))

expect(@fog_file.public_url).to be_nil
end
end
end

context 'when the directory is a valid subdomain' do
{
nil => 'foobar.s3.amazonaws.com',
Expand All @@ -557,6 +591,24 @@ def check_file
end
end

context 'when the directory is a valid subdomain and :fog_aws_fips' do
{
'us-east-1' => 'foobar.s3-fips.us-east-1.amazonaws.com',
'us-east-2' => 'foobar.s3-fips.us-east-2.amazonaws.com',
'eu-central-1' => 'foobar.s3-fips.eu-central-1.amazonaws.com', # Carrierwave shouldn't know which regions are FIPS-capable
'us-gov-west-1' => 'foobar.s3-fips.us-gov-west-1.amazonaws.com'
}.each do |region, expected_host|
it "should use a #{expected_host} hostname when using path style for access #{region} region" do
allow(@uploader).to receive(:fog_use_ssl_for_aws).and_return(true)
allow(@uploader).to receive(:fog_directory).and_return('foobar')
allow(@uploader).to receive(:fog_aws_fips).and_return(true)
allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(region: region))

expect(@fog_file.public_url).to include("https://#{expected_host}/")
end
end
end

it "should use https as a default protocol" do
expect(@uploader.fog_use_ssl_for_aws).to be true
expect(@fog_file.public_url).to start_with 'https://'
Expand Down

0 comments on commit 054bd3d

Please sign in to comment.