From 7f5f67b6fd5036ea6103ff9f77629b584d4832f9 Mon Sep 17 00:00:00 2001 From: tishin-endou Date: Thu, 28 May 2026 08:57:42 +0800 Subject: [PATCH] feat(s3): support SigV4 in addon Co-Authored-By: An Qiuyu --- Dockerfile | 1 - addons/s3/README.md | 20 ++ addons/s3/apps.py | 1 + addons/s3/models.py | 85 ++++--- addons/s3/provider.py | 4 +- addons/s3/requirements.txt | 1 - addons/s3/settings/__init__.py | 2 +- addons/s3/settings/defaults.py | 2 +- addons/s3/static/node-cfg.js | 2 +- addons/s3/static/s3-rubeus-cfg.js | 83 ++++--- .../s3/static/s3AnonymousLogActionList.json | 20 +- addons/s3/static/s3LogActionList.json | 20 +- addons/s3/static/s3NodeConfig.js | 112 +++++----- addons/s3/static/s3UserConfig.js | 46 ++-- addons/s3/static/settings.json | 2 +- addons/s3/static/user-cfg.js | 2 +- addons/s3/templates/s3_credentials_modal.mako | 12 +- addons/s3/templates/s3_node_settings.mako | 26 +-- addons/s3/templates/s3_user_settings.mako | 14 +- addons/s3/tests/factories.py | 7 +- addons/s3/tests/test_model.py | 37 +-- addons/s3/tests/test_serializer.py | 7 +- addons/s3/tests/test_view.py | 211 +++++++++--------- addons/s3/tests/utils.py | 1 - addons/s3/utils.py | 130 +++++++---- addons/s3/views.py | 21 +- requirements.txt | 2 + 27 files changed, 478 insertions(+), 393 deletions(-) delete mode 100644 addons/s3/requirements.txt diff --git a/Dockerfile b/Dockerfile index f23a48e31cb..08e0fc07d22 100644 --- a/Dockerfile +++ b/Dockerfile @@ -53,7 +53,6 @@ COPY ./addons/onedrive/requirements.txt /code/addons/onedrive/ COPY ./addons/onedrivebusiness/requirements.txt /code/addons/onedrivebusiness/ #COPY ./addons/osfstorage/requirements.txt ./addons/osfstorage/ COPY ./addons/owncloud/requirements.txt ./addons/owncloud/ -COPY ./addons/s3/requirements.txt ./addons/s3/ COPY ./addons/twofactor/requirements.txt ./addons/twofactor/ #COPY ./addons/wiki/requirements.txt ./addons/wiki/ COPY ./addons/zotero/requirements.txt ./addons/zotero/ diff --git a/addons/s3/README.md b/addons/s3/README.md index 6b4261bd77a..d3347506daa 100644 --- a/addons/s3/README.md +++ b/addons/s3/README.md @@ -16,3 +16,23 @@ If you already have an access key and ID, skip this step 2. Scroll down to Configure Add-ons 3. Connect your account and enter your ID and secret 4. Select a bucket to work from, or create a new one. + + +## Creating a restricted-access AWS user for S3 connection + +1. Login to AWS +2. Open Identity and Access Management +3. Create a user, assign name, set "Access Type" to "Programmatic Access" +4. Permissions (simple): + 1. Add "AmazonS3FullAccess" policy +5. Permissions (minimal): + 1. Click "Create Policy" => opens a new window + 2. Make a new policy with the following Actions enabled: `["s3:DeleteObject", "s3:GetObject", "s3:ListBucket", "s3:PutObject", "s3:ReplicateObject", "s3:RestoreObject", "s3:ListAllMyBuckets", "s3:GetBucketLocation", "s3:CreateBucket"]` + 3. Resources: *I just do "All Resources", but an AWS-educated person would know better about how to narrow it down. + 4. Click "Create" + 5. Return to "Create User" window + 6. Reload policy list + 7. Search for your new policy and select. +6. Tags: *eh* +7. Click "Review", review, then click "Create user" +8. Note "Access key ID" and "Secret access key". These are the inputs to the OSF S3 addon. \ No newline at end of file diff --git a/addons/s3/apps.py b/addons/s3/apps.py index 48943b4d87d..ef520daeed1 100644 --- a/addons/s3/apps.py +++ b/addons/s3/apps.py @@ -12,6 +12,7 @@ class S3AddonAppConfig(BaseAddonAppConfig): + default = True name = 'addons.s3' label = 'addons_s3' full_name = 'Amazon S3' diff --git a/addons/s3/models.py b/addons/s3/models.py index b42a1552696..226467776bb 100644 --- a/addons/s3/models.py +++ b/addons/s3/models.py @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- - from addons.base.models import (BaseOAuthNodeSettings, BaseOAuthUserSettings, BaseStorageAddon) from django.db import models @@ -8,11 +6,18 @@ from addons.base import exceptions from addons.s3.provider import S3Provider from addons.s3.serializer import S3Serializer -from addons.s3.settings import (BUCKET_LOCATIONS, - ENCRYPT_UPLOADS_DEFAULT) -from addons.s3.utils import (bucket_exists, - get_bucket_location_or_error, - get_bucket_names) +from addons.s3.settings import ( + BUCKET_LOCATIONS, + ENCRYPT_UPLOADS_DEFAULT +) +from addons.s3.utils import ( + bucket_exists, + get_bucket_location_or_error, + get_bucket_names, + get_bucket_prefixes +) +from website.util import api_v2_url + class S3FileNode(BaseFileNode): _provider = 's3' @@ -53,10 +58,12 @@ def folder_path(self): @property def display_name(self): - return u'{0}: {1}'.format(self.config.full_name, self.folder_id) + return f'{self.config.full_name}: {self.folder_id}' def set_folder(self, folder_id, auth): - if not bucket_exists(self.external_account.oauth_key, self.external_account.oauth_secret, folder_id): + bucket_name = folder_id.split(':')[0] + + if not bucket_exists(self.external_account.oauth_key, self.external_account.oauth_secret, bucket_name): error_message = ('We are having trouble connecting to that bucket. ' 'Try a different one.') raise exceptions.InvalidFolderError(error_message) @@ -66,7 +73,7 @@ def set_folder(self, folder_id, auth): bucket_location = get_bucket_location_or_error( self.external_account.oauth_key, self.external_account.oauth_secret, - folder_id + bucket_name ) try: bucket_location = BUCKET_LOCATIONS[bucket_location] @@ -75,32 +82,46 @@ def set_folder(self, folder_id, auth): # Default to the key. When hit, add mapping to settings pass - self.folder_name = '{} ({})'.format(folder_id, bucket_location) + self.folder_name = f'{folder_id} ({bucket_location})' self.save() - self.nodelogger.log(action='bucket_linked', extra={'bucket': str(folder_id)}, save=True) + self.nodelogger.log(action='bucket_linked', extra={'bucket': bucket_name, 'path': self.folder_id}, save=True) - def get_folders(self, **kwargs): - # This really gets only buckets, not subfolders, - # as that's all we want to be linkable on a node. - try: + def get_folders(self, path, folder_id): + """ + Our S3 implementation allows for folder_id to be a bucket's root, or a subfolder in that bucket. + """ + # This is the root, so list all buckets. + if not folder_id: buckets = get_bucket_names(self) - except Exception: - raise exceptions.InvalidAuthError() - return [ - { + return [{ 'addon': 's3', 'kind': 'folder', - 'id': bucket, + 'id': f'{bucket}:/', 'name': bucket, - 'path': bucket, + 'bucket_name': bucket, + 'path': '/', 'urls': { - 'folders': '' + 'folders': api_v2_url( + f'nodes/{self.owner._id}/addons/s3/folders/', + params={ + 'id': bucket, + 'bucket_name': bucket, + } + ), } - } - for bucket in buckets - ] + } for bucket in buckets] + # This is for a directory for a specific bucket, folders (Prefixes), but not files (Keys) returned, because + # these we can only set folders as our base folder_id + else: + bucket_name, _, path = folder_id.partition(':/') + return get_bucket_prefixes( + self.external_account.oauth_key, + self.external_account.oauth_secret, + prefix=path, + bucket_name=bucket_name + ) @property def complete(self): @@ -124,7 +145,7 @@ def deauthorize(self, auth=None, log=True): def delete(self, save=True): self.deauthorize(log=False) - super(NodeSettings, self).delete(save=save) + super().delete(save=save) def serialize_waterbutler_credentials(self): if not self.has_auth: @@ -135,10 +156,16 @@ def serialize_waterbutler_credentials(self): } def serialize_waterbutler_settings(self): + """ + We use the folder id to hold the bucket location + """ if not self.folder_id: raise exceptions.AddonError('Cannot serialize settings for S3 addon') + + bucket_name = self.folder_id.split(':')[0] return { - 'bucket': self.folder_id, + 'bucket': bucket_name, + 'id': self.folder_id, 'encrypt_uploads': self.encrypt_uploads } @@ -146,7 +173,7 @@ def create_waterbutler_log(self, auth, action, metadata): url = self.owner.web_url_for('addon_view_or_download_file', path=metadata['path'], provider='s3') self.owner.add_log( - 's3_{0}'.format(action), + f's3_{action}', auth=auth, params={ 'project': self.owner.parent_id, diff --git a/addons/s3/provider.py b/addons/s3/provider.py index ba28d065d54..0321a4a7d4e 100644 --- a/addons/s3/provider.py +++ b/addons/s3/provider.py @@ -1,6 +1,6 @@ from addons.s3.serializer import S3Serializer -class S3Provider(object): +class S3Provider: """An alternative to `ExternalProvider` not tied to OAuth""" name = 'Amazon S3' @@ -8,7 +8,7 @@ class S3Provider(object): serializer = S3Serializer def __init__(self, account=None): - super(S3Provider, self).__init__() + super().__init__() # provide an unauthenticated session by default self.account = account diff --git a/addons/s3/requirements.txt b/addons/s3/requirements.txt deleted file mode 100644 index 8e6ed87adf9..00000000000 --- a/addons/s3/requirements.txt +++ /dev/null @@ -1 +0,0 @@ -boto==2.38.0 diff --git a/addons/s3/settings/__init__.py b/addons/s3/settings/__init__.py index 2b2f98881f6..40f955c5a78 100644 --- a/addons/s3/settings/__init__.py +++ b/addons/s3/settings/__init__.py @@ -7,4 +7,4 @@ try: from .local import * # noqa except ImportError: - logger.warn('No local.py settings file found') + logger.warning('No local.py settings file found') diff --git a/addons/s3/settings/defaults.py b/addons/s3/settings/defaults.py index 0b5cf009214..fbad8ff00a9 100644 --- a/addons/s3/settings/defaults.py +++ b/addons/s3/settings/defaults.py @@ -10,7 +10,7 @@ MAX_RENDER_SIZE = (1024 ** 2) * 3 # Max file size permitted by frontend in megabytes -MAX_UPLOAD_SIZE = 50 * 1024 # 50 GB +MAX_UPLOAD_SIZE = 5 * 1024 # 5 GB ALLOWED_ORIGIN = '*' diff --git a/addons/s3/static/node-cfg.js b/addons/s3/static/node-cfg.js index c7e33ca410a..ccef5f3e251 100644 --- a/addons/s3/static/node-cfg.js +++ b/addons/s3/static/node-cfg.js @@ -4,4 +4,4 @@ var s3NodeConfig = require('./s3NodeConfig.js').s3NodeConfig; var url = window.contextVars.node.urls.api + 's3/settings/'; -new s3NodeConfig('Amazon S3', '#s3Scope', url, '#s3Grid'); +new s3NodeConfig('Amazon S3', '#s3Scope', url, '#s3Grid'); \ No newline at end of file diff --git a/addons/s3/static/s3-rubeus-cfg.js b/addons/s3/static/s3-rubeus-cfg.js index 8493351f9ee..d76dcc8f744 100644 --- a/addons/s3/static/s3-rubeus-cfg.js +++ b/addons/s3/static/s3-rubeus-cfg.js @@ -1,47 +1,46 @@ - var Rubeus = require('rubeus'); +var Rubeus = require('rubeus'); - Rubeus.cfg.s3 = { +Rubeus.cfg.s3 = { - uploadMethod: 'PUT', - uploadUrl: null, - uploadAdded: function(file, item) { - var self = this; - var parent = self.getByID(item.parentID); - var name = file.name; - // Make it possible to upload into subfolders - while (parent.depth > 1 && !parent.isAddonRoot) { - name = parent.name + '/' + name; - parent = self.getByID(parent.parentID); - } - file.destination = name; - file.signedUrlFrom = parent.urls.upload; - }, + uploadMethod: 'PUT', + uploadUrl: null, + uploadAdded: function (file, item) { + var self = this; + var parent = self.getByID(item.parentID); + var name = file.name; + // Make it possible to upload into subfolders + while (parent.depth > 1 && !parent.isAddonRoot) { + name = parent.name + '/' + name; + parent = self.getByID(parent.parentID); + } + file.destination = name; + file.signedUrlFrom = parent.urls.upload; + }, - uploadSending: function(file, formData, xhr) { - xhr.setRequestHeader('Content-Type', file.type || 'application/octet-stream'); - xhr.setRequestHeader('x-amz-acl', 'private'); - }, + uploadSending: function (file, formData, xhr) { + xhr.setRequestHeader('Content-Type', file.type || 'application/octet-stream'); + xhr.setRequestHeader('x-amz-acl', 'private'); + }, - uploadSuccess: function(file, row) { - var self = this; - var parent = this.getByID(row.parentID); - row.urls = { - 'delete': parent.nodeApiUrl + 's3/' + file.destination + '/', - 'download': parent.nodeUrl + 's3/' + file.destination + '/download/', - 'view': parent.nodeUrl + 's3/' + file.destination + '/' - }; - row.permissions = parent.permissions; - this.updateItem(row); - var updated = Rubeus.Utils.itemUpdated(row, parent); - if (updated) { - self.changeStatus(row, Rubeus.Status.UPDATED); - self.delayRemoveRow(row); - } else { - self.changeStatus(row, Rubeus.Status.UPLOAD_SUCCESS, null, 2000, - function(row) { - self.showButtons(row); - }); - } + uploadSuccess: function (file, row) { + var self = this; + var parent = this.getByID(row.parentID); + row.urls = { + 'delete': parent.nodeApiUrl + 's3/' + file.destination + '/', + 'download': parent.nodeUrl + 's3/' + file.destination + '/download/', + 'view': parent.nodeUrl + 's3/' + file.destination + '/' + }; + row.permissions = parent.permissions; + this.updateItem(row); + var updated = Rubeus.Utils.itemUpdated(row, parent); + if (updated) { + self.changeStatus(row, Rubeus.Status.UPDATED); + self.delayRemoveRow(row); + } else { + self.changeStatus(row, Rubeus.Status.UPLOAD_SUCCESS, null, 2000, + function (row) { + self.showButtons(row); + }); } - }; - + } +}; diff --git a/addons/s3/static/s3AnonymousLogActionList.json b/addons/s3/static/s3AnonymousLogActionList.json index 788ddcfa5fd..9f470e89f03 100644 --- a/addons/s3/static/s3AnonymousLogActionList.json +++ b/addons/s3/static/s3AnonymousLogActionList.json @@ -1,11 +1,11 @@ { - "s3_bucket_linked" : "A user linked an Amazon S3 bucket to a project", - "s3_bucket_unlinked" : "A user unselected an Amazon S3 bucket in a project", - "s3_file_added" : "A user added a file to an Amazon S3 bucket in a project", - "s3_file_removed" : "A user removed a file in an Amazon S3 bucket in a project", - "s3_file_updated" : "A user updated a file in an Amazon S3 bucket in a project", - "s3_folder_created" : "A user created a folder in an Amazon S3 in a project", - "s3_node_authorized" : "A user authorized the Amazon S3 addon for a project", - "s3_node_deauthorized" : "A user deauthorized the Amazon S3 addon for a project", - "s3_node_deauthorized_no_user" : "Amazon S3 addon for a project deauthorized" -} + "s3_bucket_linked": "A user linked an Amazon S3 bucket to a project", + "s3_bucket_unlinked": "A user unselected an Amazon S3 bucket in a project", + "s3_file_added": "A user added a file to an Amazon S3 bucket in a project", + "s3_file_removed": "A user removed a file in an Amazon S3 bucket in a project", + "s3_file_updated": "A user updated a file in an Amazon S3 bucket in a project", + "s3_folder_created": "A user created a folder in an Amazon S3 in a project", + "s3_node_authorized": "A user authorized the Amazon S3 addon for a project", + "s3_node_deauthorized": "A user deauthorized the Amazon S3 addon for a project", + "s3_node_deauthorized_no_user": "Amazon S3 addon for a project deauthorized" +} \ No newline at end of file diff --git a/addons/s3/static/s3LogActionList.json b/addons/s3/static/s3LogActionList.json index f2dc7ceef05..63d88ca3ba7 100644 --- a/addons/s3/static/s3LogActionList.json +++ b/addons/s3/static/s3LogActionList.json @@ -1,11 +1,11 @@ { - "s3_bucket_linked" : "${user} linked the Amazon S3 bucket ${bucket} to ${node}", - "s3_bucket_unlinked" : "${user} unselected the Amazon S3 bucket ${bucket} in ${node}", - "s3_file_added" : "${user} added file ${path} to Amazon S3 bucket ${bucket} in ${node}", - "s3_file_removed" : "${user} removed ${path} in Amazon S3 bucket ${bucket} in ${node}", - "s3_file_updated" : "${user} updated file ${path} in Amazon S3 bucket ${bucket} in ${node}", - "s3_folder_created" : "${user} created folder ${path} in Amazon S3 bucket ${bucket} in ${node}", - "s3_node_authorized" : "${user} authorized the Amazon S3 addon for ${node}", - "s3_node_deauthorized" : "${user} deauthorized the Amazon S3 addon for ${node}", - "s3_node_deauthorized_no_user" : "Amazon S3 addon for ${node} deauthorized" -} + "s3_bucket_linked": "${user} linked the Amazon S3 bucket ${bucket} to ${node}", + "s3_bucket_unlinked": "${user} unselected the Amazon S3 bucket ${bucket} in ${node}", + "s3_file_added": "${user} added file ${path} to Amazon S3 bucket ${bucket} in ${node}", + "s3_file_removed": "${user} removed ${path} in Amazon S3 bucket ${bucket} in ${node}", + "s3_file_updated": "${user} updated file ${path} in Amazon S3 bucket ${bucket} in ${node}", + "s3_folder_created": "${user} created folder ${path} in Amazon S3 bucket ${bucket} in ${node}", + "s3_node_authorized": "${user} authorized the Amazon S3 addon for ${node}", + "s3_node_deauthorized": "${user} deauthorized the Amazon S3 addon for ${node}", + "s3_node_deauthorized_no_user": "Amazon S3 addon for ${node} deauthorized" +} \ No newline at end of file diff --git a/addons/s3/static/s3NodeConfig.js b/addons/s3/static/s3NodeConfig.js index f5369691dfb..071deaed584 100644 --- a/addons/s3/static/s3NodeConfig.js +++ b/addons/s3/static/s3NodeConfig.js @@ -15,7 +15,7 @@ var OauthAddonFolderPicker = require('js/oauthAddonNodeConfig')._OauthAddonNodeC var s3FolderPickerViewModel = oop.extend(OauthAddonFolderPicker, { bucketLocations: s3Settings.bucketLocations, - constructor: function(addonName, url, selector, folderPicker, opts, tbOpts) { + constructor: function (addonName, url, selector, folderPicker, opts, tbOpts) { var self = this; self.super.constructor(addonName, url, selector, folderPicker, tbOpts); // Non-OAuth fields @@ -26,9 +26,9 @@ var s3FolderPickerViewModel = oop.extend(OauthAddonFolderPicker, { {}, OauthAddonFolderPicker.prototype.treebeardOptions, { // TreeBeard Options - columnTitles: function() { + columnTitles: function () { return [{ - title: 'Buckets', + title: 'Buckets/Folders', width: '75%', sort: false }, { @@ -37,30 +37,24 @@ var s3FolderPickerViewModel = oop.extend(OauthAddonFolderPicker, { sort: false }]; }, - resolveToggle: function(item) { - return ''; - }, - resolveIcon: function(item) { - return m('i.fa.fa-folder-o', ' '); - }, }, tbOpts ); }, - connectAccount: function() { + connectAccount: function () { var self = this; - if( !self.accessKey() && !self.secretKey() ){ + if (!self.accessKey() && !self.secretKey()) { self.changeMessage('Please enter both an API access key and secret key.', 'text-danger'); return; } - if (!self.accessKey() ){ + if (!self.accessKey()) { self.changeMessage('Please enter an API access key.', 'text-danger'); return; } - if (!self.secretKey() ){ + if (!self.secretKey()) { self.changeMessage('Please enter an API secret key.', 'text-danger'); return; } @@ -68,17 +62,17 @@ var s3FolderPickerViewModel = oop.extend(OauthAddonFolderPicker, { return $osf.postJSON( self.urls().create, { - secret_key: self.secretKey(), - access_key: self.accessKey() - } - ).done(function(response) { + secret_key: self.secretKey(), + access_key: self.accessKey() + } + ).done(function (response) { $osf.unblock(); self.clearModal(); $('#s3InputCredentials').modal('hide'); self.changeMessage('Successfully added S3 credentials.', 'text-success', null, true); self.updateFromData(response); self.importAuth(); - }).fail(function(xhr, status, error) { + }).fail(function (xhr, status, error) { $osf.unblock(); var message = ''; var response = JSON.parse(xhr.responseText); @@ -110,7 +104,7 @@ var s3FolderPickerViewModel = oop.extend(OauthAddonFolderPicker, { * @param {String} bucketName user-provided name of bucket to validate * @param {Boolean} laxChecking whether to use the more permissive validation */ - isValidBucketName: function(bucketName, laxChecking) { + isValidBucketName: function (bucketName, laxChecking) { if (laxChecking === true) { return /^[a-zA-Z0-9.\-_]{1,255}$/.test(bucketName); } @@ -122,7 +116,7 @@ var s3FolderPickerViewModel = oop.extend(OauthAddonFolderPicker, { }, /** Reset all fields from S3 credentials input modal */ - clearModal: function() { + clearModal: function () { var self = this; self.message(''); self.messageClass('text-info'); @@ -130,22 +124,22 @@ var s3FolderPickerViewModel = oop.extend(OauthAddonFolderPicker, { self.accessKey(null); }, - createBucket: function(self, bucketName, bucketLocation) { + createBucket: function (self, bucketName, bucketLocation) { $osf.block(); bucketName = bucketName.toLowerCase(); return $osf.postJSON( self.urls().createBucket, { - bucket_name: bucketName, - bucket_location: bucketLocation - } - ).done(function(response) { + bucket_name: bucketName, + bucket_location: bucketLocation + } + ).done(function (response) { $osf.unblock(); self.loadedFolders(false); self.activatePicker(); var msg = 'Successfully created bucket "' + $osf.htmlEscape(bucketName) + '". You can now select it from the list.'; var msgType = 'text-success'; self.changeMessage(msg, msgType, null, true); - }).fail(function(xhr) { + }).fail(function (xhr) { var resp = JSON.parse(xhr.responseText); var message = resp.message; var title = resp.title || 'Problem creating bucket'; @@ -156,21 +150,21 @@ var s3FolderPickerViewModel = oop.extend(OauthAddonFolderPicker, { bootbox.confirm({ title: $osf.htmlEscape(title), message: $osf.htmlEscape(message), - callback: function(result) { + callback: function (result) { if (result) { self.openCreateBucket(); } }, - buttons:{ - confirm:{ - label:'Try again' + buttons: { + confirm: { + label: 'Try again' } } }); }); }, - openCreateBucket: function() { + openCreateBucket: function () { var self = this; // Generates html options for key-value pairs in BUCKET_LOCATION_MAP @@ -187,32 +181,32 @@ var s3FolderPickerViewModel = oop.extend(OauthAddonFolderPicker, { bootbox.dialog({ title: 'Create a new bucket', message: - '
' + - '
' + - '
' + - '
' + - ' ' + - '
' + - ' ' + - '
' + - '' + - '
'+ - '
' + - '
' + - '
' + - ' ' + - '
' + - '' + - '
' + - '
' + - '
' + - 'For more information on locations, click ' + - 'here' + - '' + - '
' + - '
', + '
' + + '
' + + '
' + + '
' + + ' ' + + '
' + + ' ' + + '
' + + '' + + '
' + + '
' + + '
' + + '
' + + ' ' + + '
' + + '' + + '
' + + '
' + + '
' + + 'See more ' + + 'information on locations' + + '' + + '
' + + '
', buttons: { cancel: { label: 'Cancel', @@ -234,7 +228,7 @@ var s3FolderPickerViewModel = oop.extend(OauthAddonFolderPicker, { bootbox.confirm({ title: 'Invalid bucket name', message: 'Amazon S3 buckets can contain lowercase letters, numbers, and hyphens separated by' + - ' periods. Please try another name.', + ' periods. Please try another name.', callback: function (result) { if (result) { self.openCreateBucket(); @@ -271,4 +265,4 @@ function s3NodeConfig(addonName, selector, url, folderPicker, opts, tbOpts) { module.exports = { s3NodeConfig: s3NodeConfig, _s3NodeConfigViewModel: s3FolderPickerViewModel -}; +}; \ No newline at end of file diff --git a/addons/s3/static/s3UserConfig.js b/addons/s3/static/s3UserConfig.js index 5489bc93ca8..ed9c4d433b2 100644 --- a/addons/s3/static/s3UserConfig.js +++ b/addons/s3/static/s3UserConfig.js @@ -32,26 +32,26 @@ function ViewModel(url) { ChangeMessageMixin.call(self); /** Reset all fields from S3 credentials input modal */ - self.clearModal = function() { + self.clearModal = function () { self.message(''); self.messageClass('text-info'); self.accessKey(null); self.secretKey(null); }; /** Send POST request to authorize S3 */ - self.connectAccount = function() { + self.connectAccount = function () { // Selection should not be empty - if( !self.accessKey() && !self.secretKey() ){ + if (!self.accessKey() && !self.secretKey()) { self.changeMessage('Please enter both an API access key and secret key.', 'text-danger'); return; } - if (!self.accessKey() ){ + if (!self.accessKey()) { self.changeMessage('Please enter an API access key.', 'text-danger'); return; } - if (!self.secretKey() ){ + if (!self.secretKey()) { self.changeMessage('Please enter an API secret key.', 'text-danger'); return; } @@ -61,12 +61,12 @@ function ViewModel(url) { access_key: self.accessKey, secret_key: self.secretKey, }) - ).done(function() { + ).done(function () { self.clearModal(); $modal.modal('hide'); self.updateAccounts(); - }).fail(function(xhr, textStatus, error) { + }).fail(function (xhr, textStatus, error) { var errorMessage = (xhr.status === 400 && xhr.responseJSON.message !== undefined) ? xhr.responseJSON.message : language.authError; self.changeMessage(errorMessage, 'text-danger'); Raven.captureMessage('Could not authenticate with S3', { @@ -79,20 +79,20 @@ function ViewModel(url) { }); }; - self.updateAccounts = function() { + self.updateAccounts = function () { return $.ajax({ url: url, type: 'GET', dataType: 'json' }).done(function (data) { - self.accounts($.map(data.accounts, function(account) { - var externalAccount = new ExternalAccount(account); + self.accounts($.map(data.accounts, function (account) { + var externalAccount = new ExternalAccount(account); externalAccount.accessKey = account.oauth_key; externalAccount.secretKey = account.oauth_secret; return externalAccount; })); - $('#s3-header').osfToggleHeight({height: 160}); - }).fail(function(xhr, status, error) { + $('#s3-header').osfToggleHeight({ height: 160 }); + }).fail(function (xhr, status, error) { self.changeMessage(language.userSettingsError, 'text-danger'); Raven.captureMessage('Error while updating addon account', { extra: { @@ -104,7 +104,7 @@ function ViewModel(url) { }); }; - self.askDisconnect = function(account) { + self.askDisconnect = function (account) { var self = this; bootbox.confirm({ title: 'Disconnect Amazon S3 Account?', @@ -117,26 +117,26 @@ function ViewModel(url) { self.disconnectAccount(account); } }, - buttons:{ - confirm:{ - label:'Disconnect', - className:'btn-danger' + buttons: { + confirm: { + label: 'Disconnect', + className: 'btn-danger' } } }); }; - self.disconnectAccount = function(account) { + self.disconnectAccount = function (account) { var self = this; var url = '/api/v1/oauth/accounts/' + account.id + '/'; var request = $.ajax({ url: url, type: 'DELETE' }); - request.done(function(data) { + request.done(function (data) { self.updateAccounts(); }); - request.fail(function(xhr, status, error) { + request.fail(function (xhr, status, error) { Raven.captureMessage('Error while removing addon authorization for ' + account.id, { extra: { url: url, @@ -148,8 +148,8 @@ function ViewModel(url) { return request; }; - self.selectionChanged = function() { - self.changeMessage('',''); + self.selectionChanged = function () { + self.changeMessage('', ''); }; self.updateAccounts(); @@ -170,4 +170,4 @@ function S3UserConfig(selector, url) { module.exports = { S3ViewModel: ViewModel, S3UserConfig: S3UserConfig -}; +}; \ No newline at end of file diff --git a/addons/s3/static/settings.json b/addons/s3/static/settings.json index 05ea9dc29c5..1a5060a1ae4 100644 --- a/addons/s3/static/settings.json +++ b/addons/s3/static/settings.json @@ -16,4 +16,4 @@ "sa-east-1": "Sao Paulo" }, "encryptUploads": true -} +} \ No newline at end of file diff --git a/addons/s3/static/user-cfg.js b/addons/s3/static/user-cfg.js index 48e6c1894e5..474aa68ca73 100644 --- a/addons/s3/static/user-cfg.js +++ b/addons/s3/static/user-cfg.js @@ -3,4 +3,4 @@ var S3UserConfig = require('./s3UserConfig.js').S3UserConfig; // Endpoint for S3 user settings var url = '/api/v1/settings/s3/accounts/'; -var s3UserConfig = new S3UserConfig('#s3AddonScope', url); +var s3UserConfig = new S3UserConfig('#s3AddonScope', url); \ No newline at end of file diff --git a/addons/s3/templates/s3_credentials_modal.mako b/addons/s3/templates/s3_credentials_modal.mako index de0092a9035..60dc9dc84e7 100644 --- a/addons/s3/templates/s3_credentials_modal.mako +++ b/addons/s3/templates/s3_credentials_modal.mako @@ -3,7 +3,7 @@
-

${ _("Connect an Amazon S3 Account")}

+

Connect an Amazon S3 Account

@@ -14,11 +14,11 @@
- +
- +
@@ -33,10 +33,10 @@
- ${ _("Cancel")} + Cancel - +
@@ -44,4 +44,4 @@
- + \ No newline at end of file diff --git a/addons/s3/templates/s3_node_settings.mako b/addons/s3/templates/s3_node_settings.mako index 809d86dbb2e..25a4eb3bf61 100644 --- a/addons/s3/templates/s3_node_settings.mako +++ b/addons/s3/templates/s3_node_settings.mako @@ -4,35 +4,35 @@ <%include file="s3_credentials_modal.mako"/>

- + ${addon_full_name} icon ${addon_full_name} authorized by % if not is_registration: ${_("Disconnect Account")} + class="text-danger pull-right addon-auth">Disconnect Account % endif - ${_("Import Account from Profile")} + Import Account from Profile

- ${_("Loading ...")} + Loading ...

- + - ${_("Connect Account")} + Connect Account @@ -47,7 +47,7 @@ - ${_("None")} + None

@@ -55,12 +55,12 @@ - +

- ${_("Loading buckets...")}

+ Loading buckets...

@@ -69,14 +69,14 @@
- ${_("Connect %(folderName)s?") % dict(folderName='“”') | n} + Connect “”?
- +
@@ -90,4 +90,4 @@

- + \ No newline at end of file diff --git a/addons/s3/templates/s3_user_settings.mako b/addons/s3/templates/s3_user_settings.mako index dc4274d9319..5d95c2df2c5 100644 --- a/addons/s3/templates/s3_user_settings.mako +++ b/addons/s3/templates/s3_user_settings.mako @@ -6,22 +6,22 @@ <%include file="s3_credentials_modal.mako"/>

- + ${addon_full_name} icon - ${_("Connect or Reauthorize Account")} + Connect or Reauthorize Account

- ${_("Disconnect Account")} + Disconnect Account
- + @@ -29,11 +29,11 @@ @@ -43,4 +43,4 @@ - + \ No newline at end of file diff --git a/addons/s3/tests/factories.py b/addons/s3/tests/factories.py index a8b49856d47..bafdbaf9c84 100644 --- a/addons/s3/tests/factories.py +++ b/addons/s3/tests/factories.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- """Factories for the S3 addon.""" import factory from factory.django import DjangoModelFactory @@ -11,9 +10,9 @@ class S3AccountFactory(ExternalAccountFactory): provider = 's3' - provider_id = factory.Sequence(lambda n: 'id-{0}'.format(n)) - oauth_key = factory.Sequence(lambda n: 'key-{0}'.format(n)) - oauth_secret = factory.Sequence(lambda n: 'secret-{0}'.format(n)) + provider_id = factory.Sequence(lambda n: f'id-{n}') + oauth_key = factory.Sequence(lambda n: f'key-{n}') + oauth_secret = factory.Sequence(lambda n: f'secret-{n}') display_name = 'S3 Fake User' diff --git a/addons/s3/tests/test_model.py b/addons/s3/tests/test_model.py index b616a66c652..af608f5cc8a 100644 --- a/addons/s3/tests/test_model.py +++ b/addons/s3/tests/test_model.py @@ -1,7 +1,4 @@ -# from nose.tools import * # noqa -import mock -from nose.tools import (assert_false, assert_true, - assert_equal, assert_is_none) +from unittest import mock import pytest import unittest @@ -38,26 +35,33 @@ class TestNodeSettings(OAuthAddonNodeSettingsTestSuiteMixin, unittest.TestCase): NodeSettingsClass = NodeSettings UserSettingsFactory = S3UserSettingsFactory + def _node_settings_class_kwargs(self, node, user_settings): + return { + 'user_settings': self.user_settings, + 'folder_id': 'bucket_name:/path_goes_here/with_folder_id', + 'owner': self.node + } + def test_registration_settings(self): registration = ProjectFactory() clone, message = self.node_settings.after_register( self.node, registration, self.user, ) - assert_is_none(clone) + assert clone is None def test_before_register_no_settings(self): self.node_settings.user_settings = None message = self.node_settings.before_register(self.node, self.user) - assert_false(message) + assert not message def test_before_register_no_auth(self): self.node_settings.external_account = None message = self.node_settings.before_register(self.node, self.user) - assert_false(message) + assert not message def test_before_register_settings_and_auth(self): message = self.node_settings.before_register(self.node, self.user) - assert_true(message) + assert message @mock.patch('website.archiver.tasks.archive') def test_does_not_get_copied_to_registrations(self, mock_archive): @@ -66,7 +70,7 @@ def test_does_not_get_copied_to_registrations(self, mock_archive): auth=Auth(user=self.user), draft_registration=DraftRegistrationFactory(branched_from=self.node), ) - assert_false(registration.has_addon('s3')) + assert not registration.has_addon('s3') ## Overrides ## @@ -78,7 +82,7 @@ def test_serialize_credentials(self): expected = {'access_key': self.node_settings.external_account.oauth_key, 'secret_key': self.node_settings.external_account.oauth_secret} - assert_equal(credentials, expected) + assert credentials == expected @mock.patch('addons.s3.models.bucket_exists') @mock.patch('addons.s3.models.get_bucket_location_or_error') @@ -89,13 +93,16 @@ def test_set_folder(self, mock_location, mock_exists): self.node_settings.set_folder(folder_id, auth=Auth(self.user)) self.node_settings.save() # Bucket was set - assert_equal(self.node_settings.folder_id, folder_id) + assert self.node_settings.folder_id == folder_id # Log was saved last_log = self.node.logs.latest() - assert_equal(last_log.action, '{0}_bucket_linked'.format(self.short_name)) + assert last_log.action == f'{self.short_name}_bucket_linked' def test_serialize_settings(self): settings = self.node_settings.serialize_waterbutler_settings() - expected = {'bucket': self.node_settings.folder_id, - 'encrypt_uploads': self.node_settings.encrypt_uploads} - assert_equal(settings, expected) + expected = { + 'bucket': 'bucket_name', + 'encrypt_uploads': self.node_settings.encrypt_uploads, + 'id': 'bucket_name:/path_goes_here/with_folder_id' + } + assert settings == expected diff --git a/addons/s3/tests/test_serializer.py b/addons/s3/tests/test_serializer.py index a1c637020c3..f1fa025d49c 100644 --- a/addons/s3/tests/test_serializer.py +++ b/addons/s3/tests/test_serializer.py @@ -1,6 +1,5 @@ -# -*- coding: utf-8 -*- """Serializer tests for the S3 addon.""" -import mock +from unittest import mock import pytest from addons.base.tests.serializers import StorageAddonSerializerTestSuiteMixin @@ -24,8 +23,8 @@ def setUp(self): self.mock_can_list = mock.patch('addons.s3.serializer.utils.can_list') self.mock_can_list.return_value = True self.mock_can_list.start() - super(TestS3Serializer, self).setUp() + super().setUp() def tearDown(self): self.mock_can_list.stop() - super(TestS3Serializer, self).tearDown() + super().tearDown() diff --git a/addons/s3/tests/test_view.py b/addons/s3/tests/test_view.py index 8db2b2a9091..47b0ad9435d 100644 --- a/addons/s3/tests/test_view.py +++ b/addons/s3/tests/test_view.py @@ -1,15 +1,12 @@ -# -*- coding: utf-8 -*- from rest_framework import status as http_status -from boto.exception import S3ResponseError -import mock -from nose.tools import (assert_equal, assert_equals, - assert_true, assert_in, assert_false) +from botocore.exceptions import NoCredentialsError +from unittest import mock import pytest from framework.auth import Auth from tests.base import OsfTestCase, get_default_metaschema -from osf_tests.factories import ProjectFactory, AuthUserFactory, DraftRegistrationFactory, InstitutionFactory +from osf_tests.factories import ProjectFactory, AuthUserFactory, DraftRegistrationFactory from addons.base.tests.views import ( OAuthAddonConfigViewsTestCaseMixin @@ -32,35 +29,35 @@ def setUp(self): self.mock_exists = mock.patch('addons.s3.views.utils.bucket_exists') self.mock_exists.return_value = True self.mock_exists.start() - super(TestS3Views, self).setUp() + super().setUp() def tearDown(self): self.mock_can_list.stop() self.mock_uid.stop() self.mock_exists.stop() - super(TestS3Views, self).tearDown() + super().tearDown() def test_s3_settings_input_empty_keys(self): url = self.project.api_url_for('s3_add_user_account') - rv = self.app.post_json(url, { + rv = self.app.post(url, json={ 'access_key': '', 'secret_key': '' - }, auth=self.user.auth, expect_errors=True) - assert_equals(rv.status_int, http_status.HTTP_400_BAD_REQUEST) - assert_in('All the fields above are required.', rv.body.decode()) + }, auth=self.user.auth, ) + assert rv.status_code == http_status.HTTP_400_BAD_REQUEST + assert 'All the fields above are required.' in rv.text def test_s3_settings_input_empty_access_key(self): url = self.project.api_url_for('s3_add_user_account') - rv = self.app.post_json(url, { + rv = self.app.post(url, json={ 'access_key': '', 'secret_key': 'Non-empty-secret-key' - }, auth=self.user.auth, expect_errors=True) - assert_equals(rv.status_int, http_status.HTTP_400_BAD_REQUEST) - assert_in('All the fields above are required.', rv.body.decode()) + }, auth=self.user.auth, ) + assert rv.status_code == http_status.HTTP_400_BAD_REQUEST + assert 'All the fields above are required.' in rv.text def test_s3_settings_input_empty_secret_key(self): url = self.project.api_url_for('s3_add_user_account') - rv = self.app.post_json(url, { + rv = self.app.post(url, json={ 'access_key': 'Non-empty-access-key', 'secret_key': '' }, auth=self.user.auth, expect_errors=True) @@ -86,11 +83,10 @@ def test_s3_set_bucket_no_settings(self): user = AuthUserFactory() self.project.add_contributor(user, save=True) url = self.project.api_url_for('s3_set_config') - res = self.app.put_json( - url, {'s3_bucket': 'hammertofall'}, auth=user.auth, - expect_errors=True + res = self.app.put( + url, json={'s3_bucket': 'hammertofall'}, auth=user.auth, ) - assert_equal(res.status_code, http_status.HTTP_400_BAD_REQUEST) + assert res.status_code == http_status.HTTP_400_BAD_REQUEST def test_s3_set_bucket_no_auth(self): @@ -98,11 +94,10 @@ def test_s3_set_bucket_no_auth(self): user.add_addon('s3') self.project.add_contributor(user, save=True) url = self.project.api_url_for('s3_set_config') - res = self.app.put_json( - url, {'s3_bucket': 'hammertofall'}, auth=user.auth, - expect_errors=True + res = self.app.put( + url, json={'s3_bucket': 'hammertofall'}, auth=user.auth, ) - assert_equal(res.status_code, http_status.HTTP_403_FORBIDDEN) + assert res.status_code == http_status.HTTP_403_FORBIDDEN def test_s3_set_bucket_registered(self): registration = self.project.register_node( @@ -110,35 +105,34 @@ def test_s3_set_bucket_registered(self): ) url = registration.api_url_for('s3_set_config') - res = self.app.put_json( - url, {'s3_bucket': 'hammertofall'}, auth=self.user.auth, - expect_errors=True, + res = self.app.put( + url, json={'s3_bucket': 'hammertofall'}, auth=self.user.auth, ) - assert_equal(res.status_code, http_status.HTTP_400_BAD_REQUEST) + assert res.status_code == http_status.HTTP_400_BAD_REQUEST @mock.patch('addons.s3.views.utils.can_list', return_value=False) def test_user_settings_cant_list(self, mock_can_list): url = api_url_for('s3_add_user_account') - rv = self.app.post_json(url, { + rv = self.app.post(url, json={ 'access_key': 'aldkjf', 'secret_key': 'las' - }, auth=self.user.auth, expect_errors=True) + }, auth=self.user.auth) - assert_in('Unable to list buckets.', rv.body.decode()) - assert_equals(rv.status_int, http_status.HTTP_400_BAD_REQUEST) + assert 'Unable to list buckets.' in rv.text + assert rv.status_code == http_status.HTTP_400_BAD_REQUEST def test_s3_remove_node_settings_owner(self): url = self.node_settings.owner.api_url_for('s3_deauthorize_node') self.app.delete(url, auth=self.user.auth) result = self.Serializer().serialize_settings(node_settings=self.node_settings, current_user=self.user) - assert_equal(result['nodeHasAuth'], False) + assert result['nodeHasAuth'] == False def test_s3_remove_node_settings_unauthorized(self): url = self.node_settings.owner.api_url_for('s3_deauthorize_node') - ret = self.app.delete(url, auth=None, expect_errors=True) + ret = self.app.delete(url, auth=None, ) - assert_equal(ret.status_code, 401) + assert ret.status_code == 401 def test_s3_get_node_settings_owner(self): self.node_settings.set_auth(self.external_account, self.user) @@ -148,23 +142,23 @@ def test_s3_get_node_settings_owner(self): res = self.app.get(url, auth=self.user.auth) result = res.json['result'] - assert_equal(result['nodeHasAuth'], True) - assert_equal(result['userIsOwner'], True) - assert_equal(result['folder']['path'], self.node_settings.folder_id) + assert result['nodeHasAuth'] == True + assert result['userIsOwner'] == True + assert result['folder']['path'] == self.node_settings.folder_id def test_s3_get_node_settings_unauthorized(self): url = self.node_settings.owner.api_url_for('s3_get_config') unauthorized = AuthUserFactory() - ret = self.app.get(url, auth=unauthorized.auth, expect_errors=True) + ret = self.app.get(url, auth=unauthorized.auth, ) - assert_equal(ret.status_code, 403) + assert ret.status_code == 403 ## Overrides ## @mock.patch('addons.s3.models.get_bucket_names') def test_folder_list(self, mock_names): mock_names.return_value = ['bucket1', 'bucket2'] - super(TestS3Views, self).test_folder_list() + super().test_folder_list() @mock.patch('addons.s3.models.bucket_exists') @mock.patch('addons.s3.models.get_bucket_location_or_error') @@ -172,25 +166,22 @@ def test_set_config(self, mock_location, mock_exists): mock_exists.return_value = True mock_location.return_value = '' self.node_settings.set_auth(self.external_account, self.user) - url = self.project.api_url_for('{0}_set_config'.format(self.ADDON_SHORT_NAME)) - res = self.app.put_json(url, { + url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_set_config') + res = self.app.put(url, json={ 'selected': self.folder }, auth=self.user.auth) - assert_equal(res.status_code, http_status.HTTP_200_OK) + assert res.status_code == http_status.HTTP_200_OK self.project.reload() self.node_settings.reload() - assert_equal( - self.project.logs.latest().action, - '{0}_bucket_linked'.format(self.ADDON_SHORT_NAME) - ) - assert_equal(res.json['result']['folder']['name'], self.node_settings.folder_name) + assert self.project.logs.latest().action == f'{self.ADDON_SHORT_NAME}_bucket_linked' + assert res.json['result']['folder']['name'] == self.node_settings.folder_name class TestCreateBucket(S3AddonTestCase, OsfTestCase): def setUp(self): - super(TestCreateBucket, self).setUp() + super().setUp() self.user = AuthUserFactory() self.consolidated_auth = Auth(user=self.user) @@ -212,57 +203,57 @@ def setUp(self): self.node_settings.save() def test_bad_names(self): - assert_false(validate_bucket_name('')) - assert_false(validate_bucket_name('no')) - assert_false(validate_bucket_name('a' * 64)) - assert_false(validate_bucket_name(' leadingspace')) - assert_false(validate_bucket_name('trailingspace ')) - assert_false(validate_bucket_name('bogus naMe')) - assert_false(validate_bucket_name('.cantstartwithp')) - assert_false(validate_bucket_name('or.endwith.')) - assert_false(validate_bucket_name('..nodoubles')) - assert_false(validate_bucket_name('no_unders_in')) - assert_false(validate_bucket_name('-leadinghyphen')) - assert_false(validate_bucket_name('trailinghyphen-')) - assert_false(validate_bucket_name('Mixedcase')) - assert_false(validate_bucket_name('empty..label')) - assert_false(validate_bucket_name('label-.trailinghyphen')) - assert_false(validate_bucket_name('label.-leadinghyphen')) - assert_false(validate_bucket_name('8.8.8.8')) - assert_false(validate_bucket_name('600.9000.0.28')) - assert_false(validate_bucket_name('no_underscore')) - assert_false(validate_bucket_name('_nounderscoreinfront')) - assert_false(validate_bucket_name('no-underscore-in-back_')) - assert_false(validate_bucket_name('no-underscore-in_the_middle_either')) + assert not validate_bucket_name('') + assert not validate_bucket_name('no') + assert not validate_bucket_name('a' * 64) + assert not validate_bucket_name(' leadingspace') + assert not validate_bucket_name('trailingspace ') + assert not validate_bucket_name('bogus naMe') + assert not validate_bucket_name('.cantstartwithp') + assert not validate_bucket_name('or.endwith.') + assert not validate_bucket_name('..nodoubles') + assert not validate_bucket_name('no_unders_in') + assert not validate_bucket_name('-leadinghyphen') + assert not validate_bucket_name('trailinghyphen-') + assert not validate_bucket_name('Mixedcase') + assert not validate_bucket_name('empty..label') + assert not validate_bucket_name('label-.trailinghyphen') + assert not validate_bucket_name('label.-leadinghyphen') + assert not validate_bucket_name('8.8.8.8') + assert not validate_bucket_name('600.9000.0.28') + assert not validate_bucket_name('no_underscore') + assert not validate_bucket_name('_nounderscoreinfront') + assert not validate_bucket_name('no-underscore-in-back_') + assert not validate_bucket_name('no-underscore-in_the_middle_either') def test_names(self): - assert_true(validate_bucket_name('imagoodname')) - assert_true(validate_bucket_name('still.passing')) - assert_true(validate_bucket_name('can-have-dashes')) - assert_true(validate_bucket_name('kinda.name.spaced')) - assert_true(validate_bucket_name('a-o.valid')) - assert_true(validate_bucket_name('11.12.m')) - assert_true(validate_bucket_name('a--------a')) - assert_true(validate_bucket_name('a' * 63)) + assert validate_bucket_name('imagoodname') + assert validate_bucket_name('still.passing') + assert validate_bucket_name('can-have-dashes') + assert validate_bucket_name('kinda.name.spaced') + assert validate_bucket_name('a-o.valid') + assert validate_bucket_name('11.12.m') + assert validate_bucket_name('a--------a') + assert validate_bucket_name('a' * 63) def test_bad_locations(self): - assert_false(validate_bucket_location('Venus')) - assert_false(validate_bucket_location('AlphaCentari')) - assert_false(validate_bucket_location('CostaRica')) + assert not validate_bucket_location('Venus') + assert not validate_bucket_location('AlphaCentauri') + assert not validate_bucket_location('CostaRica') def test_locations(self): - assert_true(validate_bucket_location('')) - assert_true(validate_bucket_location('eu-central-1')) - assert_true(validate_bucket_location('ca-central-1')) - assert_true(validate_bucket_location('us-west-1')) - assert_true(validate_bucket_location('us-west-2')) - assert_true(validate_bucket_location('ap-northeast-1')) - assert_true(validate_bucket_location('ap-northeast-2')) - assert_true(validate_bucket_location('ap-southeast-1')) - assert_true(validate_bucket_location('ap-southeast-2')) - assert_true(validate_bucket_location('sa-east-1')) - assert_true(validate_bucket_location('eu-west-1')) - assert_true(validate_bucket_location('eu-west-2')) + assert validate_bucket_location('') + assert validate_bucket_location('eu-central-1') + assert validate_bucket_location('ca-central-1') + assert validate_bucket_location('us-west-1') + assert validate_bucket_location('us-west-2') + assert validate_bucket_location('ap-northeast-1') + assert validate_bucket_location('ap-northeast-2') + assert validate_bucket_location('ap-southeast-1') + assert validate_bucket_location('ap-southeast-2') + assert validate_bucket_location('sa-east-1') + assert validate_bucket_location('eu-west-1') + assert validate_bucket_location('eu-west-2') @mock.patch('addons.s3.views.utils.create_bucket') @mock.patch('addons.s3.views.utils.get_bucket_names') @@ -274,39 +265,37 @@ def test_create_bucket_pass(self, mock_names, mock_make): 'doesntevenmatter' ] url = self.project.api_url_for('create_bucket') - ret = self.app.post_json( + ret = self.app.post( url, - { + json={ 'bucket_name': 'doesntevenmatter', 'bucket_location': '', }, auth=self.user.auth ) - assert_equal(ret.status_int, http_status.HTTP_200_OK) - assert_equal(ret.json, {}) + assert ret.status_code == http_status.HTTP_200_OK + assert ret.json == {} @mock.patch('addons.s3.views.utils.create_bucket') def test_create_bucket_fail(self, mock_make): - error = S3ResponseError(418, 'because Im a test') - error.message = 'This should work' + error = NoCredentialsError(operation_name='create_bucket') mock_make.side_effect = error - url = '/api/v1/project/{0}/s3/newbucket/'.format(self.project._id) - ret = self.app.post_json(url, {'bucket_name': 'doesntevenmatter'}, auth=self.user.auth, expect_errors=True) + url = f'/api/v1/project/{self.project._id}/s3/newbucket/' + ret = self.app.post(url, json={'bucket_name': 'doesntevenmatter'}, auth=self.user.auth) - assert_equals(ret.body.decode(), '{"message": "This should work", "title": "Problem connecting to S3"}') + assert ret.text == '{"message": "Unable to locate credentials", "title": "Problem connecting to S3"}' @mock.patch('addons.s3.views.utils.create_bucket') def test_bad_location_fails(self, mock_make): - url = '/api/v1/project/{0}/s3/newbucket/'.format(self.project._id) - ret = self.app.post_json( + url = f'/api/v1/project/{self.project._id}/s3/newbucket/' + ret = self.app.post( url, - { + json={ 'bucket_name': 'doesntevenmatter', 'bucket_location': 'not a real bucket location', }, auth=self.user.auth, - expect_errors=True) - - assert_equals(ret.body.decode(), '{"message": "That bucket location is not valid.", "title": "Invalid bucket location"}') + ) + assert ret.text == '{"message": "That bucket location is not valid.", "title": "Invalid bucket location"}' diff --git a/addons/s3/tests/utils.py b/addons/s3/tests/utils.py index 15b0153f23b..e83f4ebf489 100644 --- a/addons/s3/tests/utils.py +++ b/addons/s3/tests/utils.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- from addons.base.tests.base import OAuthAddonTestCaseMixin, AddonTestCase from addons.s3.tests.factories import S3AccountFactory from addons.s3.provider import S3Provider diff --git a/addons/s3/utils.py b/addons/s3/utils.py index 0738b5bf397..73d53ebf18e 100644 --- a/addons/s3/utils.py +++ b/addons/s3/utils.py @@ -1,14 +1,19 @@ import re +import logging +from dataclasses import dataclass +from typing import Optional # Python 3.6 does not support | operator for type hints + from rest_framework import status as http_status -from boto import exception -from boto.s3.connection import S3Connection -from boto.s3.connection import OrdinaryCallingFormat +import boto3 +from botocore import exceptions from framework.exceptions import HTTPError from addons.base.exceptions import InvalidAuthError, InvalidFolderError from addons.s3.settings import BUCKET_LOCATIONS +logger = logging.getLogger(__name__) + def connect_s3(access_key=None, secret_key=None, node_settings=None): """Helper to build an S3Connection object @@ -18,19 +23,27 @@ def connect_s3(access_key=None, secret_key=None, node_settings=None): if node_settings is not None: if node_settings.external_account is not None: access_key, secret_key = node_settings.external_account.oauth_key, node_settings.external_account.oauth_secret - connection = S3Connection(access_key, secret_key, calling_format=OrdinaryCallingFormat()) + connection = boto3.client( + 's3', + aws_access_key_id=access_key, + aws_secret_access_key=secret_key + ) return connection +def get_status_for_error(e: exceptions.ClientError) -> int: + return e.response['ResponseMetadata']['HTTPStatusCode'] + + def get_bucket_names(node_settings): try: - buckets = connect_s3(node_settings=node_settings).get_all_buckets() - except exception.NoAuthHandlerFound: + response = connect_s3(node_settings=node_settings).list_buckets() + except exceptions.NoCredentialsError: raise HTTPError(http_status.HTTP_403_FORBIDDEN) - except exception.BotoServerError as e: - raise HTTPError(e.status) + except exceptions.ClientError as e: + raise HTTPError(get_status_for_error(e)) - return [bucket.name for bucket in buckets] + return [bucket['Name'] for bucket in response['Buckets']] def validate_bucket_location(location): @@ -51,28 +64,34 @@ def validate_bucket_name(name): def create_bucket(node_settings, bucket_name, location=''): - return connect_s3(node_settings=node_settings).create_bucket(bucket_name, location=location) + client = connect_s3(node_settings=node_settings) + + # default bucket location won't work with location constraint + if not location or location == 'us-east-1': + return client.create_bucket(Bucket=bucket_name) + else: + return client.create_bucket( + Bucket=bucket_name, + CreateBucketConfiguration={ + 'LocationConstraint': location, + } + ) def bucket_exists(access_key, secret_key, bucket_name): - """Tests for the existance of a bucket and if the user + """Tests for the existence of a bucket and if the user can access it with the given keys """ if not bucket_name: return False - - connection = connect_s3(access_key, secret_key) - - if bucket_name != bucket_name.lower(): - # Must use ordinary calling format for mIxEdCaSe bucket names - # otherwise use the default as it handles bucket outside of the US - connection.calling_format = OrdinaryCallingFormat() + # bucket names are dns-compliant, therefore must be lowercase + bucket_name = bucket_name.lower() try: # Will raise an exception if bucket_name doesn't exist - connect_s3(access_key, secret_key).head_bucket(bucket_name) - except exception.S3ResponseError as e: - if e.status not in (301, 302): + connect_s3(access_key, secret_key).head_bucket(Bucket=bucket_name) + except exceptions.ClientError as e: + if get_status_for_error(e) not in (301, 302): return False return True @@ -87,38 +106,73 @@ def can_list(access_key, secret_key): return False try: - connect_s3(access_key, secret_key).get_all_buckets() - except exception.S3ResponseError: + connect_s3(access_key, secret_key).list_buckets() + except exceptions.ClientError: return False return True -def get_user_info(access_key, secret_key): + +@dataclass(frozen=True) # slots is not supported in python 3.6 +class Owner: + __slots__ = ['display_name', 'id'] + + display_name: str + id: str + + @classmethod + def from_dict(cls, data): + return cls(data['DisplayName'], data['ID']) + + +def get_user_info(access_key: str, secret_key: str) -> Optional[Owner]: """Returns an S3 User with .display_name and .id, or None """ if not (access_key and secret_key): return None try: - return connect_s3(access_key, secret_key).get_all_buckets().owner - except exception.S3ResponseError: + return Owner.from_dict(connect_s3(access_key, secret_key).list_buckets()['Owner']) + except exceptions.ClientError: return None - return None + def get_bucket_location_or_error(access_key, secret_key, bucket_name): """Returns the location of a bucket or raises AddonError """ - try: - connection = connect_s3(access_key, secret_key) - except Exception: - raise InvalidAuthError() - - if bucket_name != bucket_name.lower() or '.' in bucket_name: - # Must use ordinary calling format for mIxEdCaSe bucket names - # otherwise use the default as it handles bucket outside of the US - connection.calling_format = OrdinaryCallingFormat() + # bucket names are dns-compliant, therefore must be lowercase + bucket_name = bucket_name.lower() try: # Will raise an exception if bucket_name doesn't exist - return connect_s3(access_key, secret_key).get_bucket(bucket_name, validate=False).get_location() - except exception.S3ResponseError: + return connect_s3(access_key, secret_key).get_bucket_location(Bucket=bucket_name)['LocationConstraint'] + except exceptions.NoCredentialsError: + raise InvalidAuthError() + except exceptions.ClientError: raise InvalidFolderError() + + +def get_bucket_prefixes(access_key, secret_key, prefix, bucket_name): + s3 = boto3.client( + 's3', + aws_access_key_id=access_key, + aws_secret_access_key=secret_key + ) + + result = s3.list_objects(Bucket=bucket_name, Prefix=prefix, Delimiter='/') + folders = [] + for common_prefixes in result.get('CommonPrefixes', []): + key_name = common_prefixes.get('Prefix') + if key_name != prefix: + folders.append( + { + 'path': key_name, + 'id': f'{bucket_name}:/{key_name}', + 'folder_id': key_name, + 'kind': 'folder', + 'bucket_name': bucket_name, + 'name': key_name.split('/')[-2], + 'addon': 's3', + } + ) + + return folders diff --git a/addons/s3/views.py b/addons/s3/views.py index 672dd335305..5aef084cd90 100644 --- a/addons/s3/views.py +++ b/addons/s3/views.py @@ -1,6 +1,6 @@ from rest_framework import status as http_status -from boto import exception +from botocore import exceptions from django.core.exceptions import ValidationError from flask import request @@ -58,7 +58,9 @@ def _set_folder(node_addon, folder, auth): def s3_folder_list(node_addon, **kwargs): """ Returns all the subsequent folders under the folder id passed. """ - return node_addon.get_folders() + path = request.args.get('path', '') + folder_id = request.args.get('id', '') + return node_addon.get_folders(path=path, folder_id=folder_id) @must_be_logged_in @must_be_rdm_addons_allowed(SHORT_NAME) @@ -148,20 +150,15 @@ def create_bucket(auth, node_addon, **kwargs): try: utils.create_bucket(node_addon, bucket_name, bucket_location) - except exception.S3ResponseError as e: + except exceptions.NoCredentialsError as e: return { - 'message': e.message, + 'message': str(e), 'title': 'Problem connecting to S3', }, http_status.HTTP_400_BAD_REQUEST - except exception.S3CreateError as e: + except exceptions.ClientError as e: return { - 'message': e.message, - 'title': "Problem creating bucket '{0}'".format(bucket_name), - }, http_status.HTTP_400_BAD_REQUEST - except exception.BotoClientError as e: # Base class catchall - return { - 'message': e.message, - 'title': 'Error connecting to S3', + 'message': str(e), + 'title': f"Problem creating bucket '{bucket_name}'", }, http_status.HTTP_400_BAD_REQUEST return {} diff --git a/requirements.txt b/requirements.txt index c9ee19ae23b..f3574223512 100644 --- a/requirements.txt +++ b/requirements.txt @@ -133,3 +133,5 @@ Babel==2.5.1 Flask-Babel==1.0.0 rsa==3.1.4 flask-compress==1.15 + +dataclasses==0.8 \ No newline at end of file
- ${_("Private project")} + Private project - +