Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a viewer error that occurs when AWS is not used. #133

Merged
merged 1 commit into from
Oct 10, 2022

Conversation

sutatoruta
Copy link
Contributor

@sutatoruta sutatoruta commented Oct 4, 2022

Even if AWS credentials config values are nil, $config.has_key? :aws returns true.
It might not be working as expected:

if $config.has_key? :aws
Aws.config.update({
credentials: Aws::Credentials.new(
$config[:aws][:access_key_id],
$config[:aws][:secret_access_key],
),
region: 'ap-northeast-1',
})
$signer = Aws::S3::Presigner.new
end

Since $singer is not nil, presigned_url is called without AWS credentials set and throws Aws::Sigv4::Errors::MissingCredentialsError:

def sign_file(file)
if file.has_key? 'url_private_download'
url = $signer.presigned_url(:get_object, {
bucket: 'tsgbot-slack-files',
key: file['id'],
expires_in: 180,
})
file['url_private_download'] = url
end
end
def sign_message_files(messages)
unless $signer.nil?
messages.each do |message|
if message.has_key? 'files'
message['files'].each do |file|
sign_file(file)
end
end
# Compatibility with old messages
if message.has_key? 'file'
file = message['file']
sign_file(file)
end
end
end
end

I fixed this problem by correcting $config.has_key? :aws to $config[:aws]&.values.all?.

Even if AWS credentials config values are nil, `$config.has_key? :aws` returns true.
@tyage
Copy link
Owner

tyage commented Oct 10, 2022

thank you!
looks fine!

@tyage tyage merged commit 1c53eb0 into tyage:master Oct 10, 2022
@tyage tyage mentioned this pull request Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants