From a3325d0c5317f536e7eaf1204110e1b0cda3ce52 Mon Sep 17 00:00:00 2001
From: TOMONORI ENDOU <tishin.endou@gmail.com>
Date: Fri, 2 Oct 2026 15:26:40 +0900
Subject: [PATCH 1/9] feat(s3): sign S3 addon requests with SigV4

Replace the boto 2 client with boto3/botocore so that the S3 addon can talk
to regions that only accept Signature Version 4 (ap-northeast-1 and later).

- utils.py / views.py / models.py build the client through boto3 with a
  region-specific endpoint, and surface botocore exceptions as HTTPError
- folder_id becomes "bucket:/prefix/" so that a subfolder can be selected;
  the legacy "bucket" form is still accepted
- the node/user config JS, the Rubeus config and the log action lists follow
  the new folder shape
- requirements.txt gains boto3/botocore; addons/s3/requirements.txt is kept
  as an empty file because the Jenkins Dockerfile still does
  COPY ./addons/s3/requirements.txt and would fail the image build without it
- MAX_UPLOAD_SIZE stays at 50GB: multipart upload handles up to 5TB, so the
  lower limit the first draft introduced was unnecessary
- the ${ _("...") } gettext wrappers are kept in all three mako templates
  (20 translatable strings); only the aria-label/alt attributes and the th
  class change are new

Co-Authored-By: An Qiuyu <qiuyu.an@hotmail.com>
---
 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                    |   3 +-
 addons/s3/settings/__init__.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 |   2 +-
 addons/s3/templates/s3_node_settings.mako     |   6 +-
 addons/s3/templates/s3_user_settings.mako     |   6 +-
 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 +
 26 files changed, 460 insertions(+), 373 deletions(-)

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
index 8e6ed87adf9..a61cef6d77f 100644
--- a/addons/s3/requirements.txt
+++ b/addons/s3/requirements.txt
@@ -1 +1,2 @@
-boto==2.38.0
+# boto (v2) was removed as part of SigV4 migration.
+# boto3/botocore are declared in the root requirements.txt.
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/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:
-                    '<div class="row"> ' +
-                        '<div class="col-md-12"> ' +
-                            '<form class="form-horizontal" onsubmit="return false"> ' +
-                                '<div class="form-group"> ' +
-                                    '<label class="col-md-4 control-label" for="bucketName">Bucket Name</label> ' +
-                                    '<div class="col-md-8"> ' +
-                                        '<input id="bucketName" name="bucketName" type="text" placeholder="Enter bucket name" class="form-control" autofocus> ' +
-                                        '<div>' +
-                                            '<span id="bucketModalErrorMessage" ></span>' +
-                                        '</div>'+
-                                    '</div>' +
-                                '</div>' +
-                                '<div class="form-group"> ' +
-                                    '<label class="col-md-4 control-label" for="bucketLocation">Bucket Location</label> ' +
-                                    '<div class="col-md-8"> ' +
-                                        '<select id="bucketLocation" name="bucketLocation" class="form-control"> ' +
-                                            generateBucketOptions(self.bucketLocations) +
-                                        '</select>' +
-                                    '</div>' +
-                                '</div>' +
-                            '</form>' +
-                            '<span>For more information on locations, click ' +
-                                '<a href="http://help.osf.io/m/addons/l/863985">here</a>' +
-                            '</span>' +
-                        '</div>' +
-                    '</div>',
+                '<div class="row"> ' +
+                '<div class="col-md-12"> ' +
+                '<form class="form-horizontal" onsubmit="return false"> ' +
+                '<div class="form-group"> ' +
+                '<label class="col-md-4 control-label" for="bucketName">Bucket Name</label> ' +
+                '<div class="col-md-8"> ' +
+                '<input id="bucketName" name="bucketName" type="text" placeholder="Enter bucket name" class="form-control" autofocus> ' +
+                '<div>' +
+                '<span id="bucketModalErrorMessage" ></span>' +
+                '</div>' +
+                '</div>' +
+                '</div>' +
+                '<div class="form-group"> ' +
+                '<label class="col-md-4 control-label" for="bucketLocation">Bucket Location</label> ' +
+                '<div class="col-md-8"> ' +
+                '<select id="bucketLocation" name="bucketLocation" class="form-control"> ' +
+                generateBucketOptions(self.bucketLocations) +
+                '</select>' +
+                '</div>' +
+                '</div>' +
+                '</form>' +
+                '<span>See <a href="http://help.osf.io/m/addons/l/863985">more ' +
+                'information on locations</a>' +
+                '</span>' +
+                '</div>' +
+                '</div>',
             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..f097291f03a 100644
--- a/addons/s3/templates/s3_credentials_modal.mako
+++ b/addons/s3/templates/s3_credentials_modal.mako
@@ -44,4 +44,4 @@
 
         </div><!-- end modal-content -->
     </div>
-</div>
+</div>
\ 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..82a127a4e7c 100644
--- a/addons/s3/templates/s3_node_settings.mako
+++ b/addons/s3/templates/s3_node_settings.mako
@@ -4,7 +4,7 @@
     <%include file="s3_credentials_modal.mako"/>
 
     <h4 class="addon-title">
-        <img class="addon-icon" src=${addon_icon_url}>
+        <img class="addon-icon" src=${addon_icon_url} aria-label="${addon_full_name} icon" alt="${addon_full_name} icon">
         ${addon_full_name}
         <small class="authorized-by">
             <span data-bind="if: nodeHasAuth">
@@ -29,7 +29,7 @@
                 </p>
             </span>
 
-            <!-- Oauth Start Button -->
+            <!-- OAuth Start Button -->
             <span data-bind="if: showTokenCreateButton">
                 <a href="#s3InputCredentials" data-toggle="modal" class="pull-right text-primary addon-auth">
                     ${_("Connect  Account")}
@@ -90,4 +90,4 @@
     <div class="help-block">
         <p data-bind="html: message, attr: {class: messageClass}"></p>
     </div>
-</div>
+</div>
\ 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..83f7317d1ab 100644
--- a/addons/s3/templates/s3_user_settings.mako
+++ b/addons/s3/templates/s3_user_settings.mako
@@ -6,7 +6,7 @@
     <%include file="s3_credentials_modal.mako"/>
 
     <h4 class="addon-title">
-        <img class="addon-icon" src=${addon_icon_url}>
+        <img class="addon-icon" src=${addon_icon_url} aria-label="${addon_full_name} icon" alt="${addon_full_name} icon">
         <span data-bind="text: properName"></span>
         <small>
             <a href="#s3InputCredentials" data-toggle="modal" class="pull-right text-primary">${_("Connect or Reauthorize Account")}</a>
@@ -21,7 +21,7 @@
             <table class="table table-hover">
                 <thead>
                     <tr class="user-settings-addon-auth">
-                        <th class="text-muted default-authorized-by">${_("Authorized by ")}<em><span data-bind="text: name"></span></em></th><th></th>
+                        <th class="default-authorized-by">${_("Authorized by ")}<em><span data-bind="text: name"></span></em></th><th></th>
                     </tr>
                 </thead>
                 <!-- ko if: connectedNodes().length > 0 -->
@@ -43,4 +43,4 @@
         </div>
         <!-- /ko -->
     </div>
-</div>
+</div>
\ 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

From 2f451dc584a40daf8ebed39e826e90f37134d283 Mon Sep 17 00:00:00 2001
From: TOMONORI ENDOU <tishin.endou@gmail.com>
Date: Fri, 2 Oct 2026 15:27:41 +0900
Subject: [PATCH 2/9] fix(s3): reuse the existing external account when the
 access key is already registered
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Registering the same access key twice has to end up on the existing
ExternalAccount instead of creating a second row. The duplicate is caught by
the unique constraint on (provider, provider_id) — provider_id holds
"{aws_account_id}\t{access_key}" (GRDM-53044) — so the save raises
IntegrityError, not the ValidationError the handler was catching, and the
failed INSERT aborted the surrounding transaction.

Wrap the save in transaction.atomic() so the failure only rolls back to a
savepoint, and catch IntegrityError alongside ValidationError before falling
back to the account that is already stored.
---
 addons/s3/views.py | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/addons/s3/views.py b/addons/s3/views.py
index 5aef084cd90..1627bcf1a7f 100644
--- a/addons/s3/views.py
+++ b/addons/s3/views.py
@@ -2,6 +2,7 @@
 
 from botocore import exceptions
 from django.core.exceptions import ValidationError
+from django.db import IntegrityError, transaction
 from flask import request
 
 from framework.exceptions import HTTPError
@@ -97,16 +98,17 @@ def s3_add_user_account(auth, **kwargs):
     masked_access_key = f'****{access_key[-4:]}' if len(access_key) > 4 else '****'
     display_name = f'{user_info.display_name} ({masked_access_key})'
     try:
-        account = ExternalAccount(
-            provider=SHORT_NAME,
-            provider_name=FULL_NAME,
-            oauth_key=access_key,
-            oauth_secret=secret_key,
-            provider_id=provider_id,
-            display_name=display_name,
-        )
-        account.save()
-    except ValidationError:
+        with transaction.atomic():
+            account = ExternalAccount(
+                provider=SHORT_NAME,
+                provider_name=FULL_NAME,
+                oauth_key=access_key,
+                oauth_secret=secret_key,
+                provider_id=provider_id,
+                display_name=display_name,
+            )
+            account.save()
+    except (ValidationError, IntegrityError):
         # ... or get the old one
         account = ExternalAccount.objects.get(
             provider=SHORT_NAME,

From f70e7f0015e757588a614b63f1084709308a4cdd Mon Sep 17 00:00:00 2001
From: TOMONORI ENDOU <tishin.endou@gmail.com>
Date: Fri, 2 Oct 2026 15:28:16 +0900
Subject: [PATCH 3/9] test(s3): drop nose remnants and update the node addons
 mock to the SigV4 folder shape

- test_view.py still used assert_equals/assert_in and a missing
  InstitutionFactory import left over from the nose era; replace them with
  plain pytest asserts against rv.text
- _mock_folder_result still returned the pre-SigV4 get_folders payload, so the
  node settings tests asserted against a shape the view no longer produces
---
 addons/s3/tests/test_view.py              | 12 ++++++------
 api_tests/nodes/views/test_node_addons.py |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/addons/s3/tests/test_view.py b/addons/s3/tests/test_view.py
index 47b0ad9435d..56dd8c26f69 100644
--- a/addons/s3/tests/test_view.py
+++ b/addons/s3/tests/test_view.py
@@ -6,7 +6,7 @@
 
 from framework.auth import Auth
 from tests.base import OsfTestCase, get_default_metaschema
-from osf_tests.factories import ProjectFactory, AuthUserFactory, DraftRegistrationFactory
+from osf_tests.factories import ProjectFactory, AuthUserFactory, DraftRegistrationFactory, InstitutionFactory
 
 from addons.base.tests.views import (
     OAuthAddonConfigViewsTestCaseMixin
@@ -61,8 +61,8 @@ def test_s3_settings_input_empty_secret_key(self):
             'access_key': 'Non-empty-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())
+        assert rv.status_code == http_status.HTTP_400_BAD_REQUEST
+        assert 'All the fields above are required.' in rv.text
 
     def test_s3_settings_rdm_addons_denied(self):
         institution = InstitutionFactory()
@@ -72,12 +72,12 @@ def test_s3_settings_rdm_addons_denied(self):
         rdm_addon_option.is_allowed = False
         rdm_addon_option.save()
         url = self.project.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)
-        assert_equal(rv.status_int, http_status.HTTP_403_FORBIDDEN)
-        assert_in(b'You are prohibited from using this add-on.', rv.body)
+        assert rv.status_code == http_status.HTTP_403_FORBIDDEN
+        assert 'You are prohibited from using this add-on.' in rv.text
 
     def test_s3_set_bucket_no_settings(self):
         user = AuthUserFactory()
diff --git a/api_tests/nodes/views/test_node_addons.py b/api_tests/nodes/views/test_node_addons.py
index 459731af87c..660da131d84 100644
--- a/api_tests/nodes/views/test_node_addons.py
+++ b/api_tests/nodes/views/test_node_addons.py
@@ -1015,8 +1015,8 @@ def _settings_kwargs(self, node, user_settings):
     def _mock_folder_result(self):
         return {
             'name': 'a.bucket',
-            'path': 'a.bucket',
-            'id': 'a.bucket'
+            'path': '/',
+            'id': 'a.bucket:/'
         }
 
     @mock.patch('addons.s3.models.get_bucket_names')

From 578a9ebf3b4f0f63129f85b93d8779446c5beca5 Mon Sep 17 00:00:00 2001
From: TOMONORI ENDOU <tishin.endou@gmail.com>
Date: Fri, 2 Oct 2026 15:29:31 +0900
Subject: [PATCH 4/9] test(s3): cover folder_id formats, provider_id dedup and
 error paths

Extend addons/s3/tests/test_view.py so the SigV4 flow is exercised end to end:

- both folder_id forms ("bucket" and "bucket:/prefix/") and a non-existent
  bucket raising InvalidFolderError
- the provider_id format "{aws_account_id}\t{access_key}" and the dedup path
  when the same key is registered twice (GRDM-53044)
- the error branches: a missing key field, get_user_info returning None, an
  invalid bucket name, an empty bucket_exists result, can_list returning None,
  and the ClientError / InvalidAuthError conversions

Three test-harness fixes come with it:

- query ExternalAccount by provider_id, a plain CharField. oauth_key is an
  EncryptedTextField (JWE), so its ciphertext differs on every save and
  filtering on it never matches; the plaintext is checked by attribute access
  instead
- use post_json/put_json instead of the json= keyword, which the webtest_plus
  version used in CI does not accept, and pass expect_errors=True wherever a
  4xx/401/403 response is expected
- patch get_user_info with mock.patch(..., return_value=mock.Mock(...)); the
  .return_value attribute on the patcher object never reaches the started mock,
  so user_info.id / user_info.display_name were missing in the view
---
 addons/s3/tests/test_model.py |  16 ++++
 addons/s3/tests/test_view.py  | 159 +++++++++++++++++++++++++++++-----
 2 files changed, 151 insertions(+), 24 deletions(-)

diff --git a/addons/s3/tests/test_model.py b/addons/s3/tests/test_model.py
index af608f5cc8a..ac148c59812 100644
--- a/addons/s3/tests/test_model.py
+++ b/addons/s3/tests/test_model.py
@@ -98,6 +98,22 @@ def test_set_folder(self, mock_location, mock_exists):
         last_log = self.node.logs.latest()
         assert last_log.action == f'{self.short_name}_bucket_linked'
 
+    @mock.patch('addons.s3.models.bucket_exists')
+    def test_set_folder_nonexistent_bucket(self, mock_exists):
+        from addons.base import exceptions
+        mock_exists.return_value = False
+        with pytest.raises(exceptions.InvalidFolderError):
+            self.node_settings.set_folder('nonexistent-bucket:/', auth=Auth(self.user))
+
+    @mock.patch('addons.s3.models.bucket_exists')
+    @mock.patch('addons.s3.models.get_bucket_location_or_error')
+    def test_set_folder_invalid_auth(self, mock_location, mock_exists):
+        from addons.base.exceptions import InvalidAuthError
+        mock_exists.return_value = True
+        mock_location.side_effect = InvalidAuthError()
+        with pytest.raises(InvalidAuthError):
+            self.node_settings.set_folder('valid-bucket:/', auth=Auth(self.user))
+
     def test_serialize_settings(self):
         settings = self.node_settings.serialize_waterbutler_settings()
         expected = {
diff --git a/addons/s3/tests/test_view.py b/addons/s3/tests/test_view.py
index 56dd8c26f69..dd587cff2d2 100644
--- a/addons/s3/tests/test_view.py
+++ b/addons/s3/tests/test_view.py
@@ -15,6 +15,7 @@
 from addons.s3.utils import validate_bucket_name, validate_bucket_location
 from website.util import api_url_for
 from admin.rdm_addons.utils import get_rdm_addon_option
+from osf.models import ExternalAccount
 
 pytestmark = pytest.mark.django_db
 
@@ -23,8 +24,8 @@ def setUp(self):
         self.mock_can_list = mock.patch('addons.s3.views.utils.can_list')
         self.mock_can_list.return_value = True
         self.mock_can_list.start()
-        self.mock_uid = mock.patch('addons.s3.views.utils.get_user_info')
-        self.mock_uid.return_value = {'id': '1234567890', 'display_name': 's3.user'}
+        self.mock_uid = mock.patch('addons.s3.views.utils.get_user_info',
+                                   return_value=mock.Mock(id='1234567890', display_name='s3.user'))
         self.mock_uid.start()
         self.mock_exists = mock.patch('addons.s3.views.utils.bucket_exists')
         self.mock_exists.return_value = True
@@ -39,25 +40,25 @@ def tearDown(self):
 
     def test_s3_settings_input_empty_keys(self):
         url = self.project.api_url_for('s3_add_user_account')
-        rv = self.app.post(url, json={
+        rv = self.app.post_json(url, {
             'access_key': '',
             'secret_key': ''
-        }, auth=self.user.auth, )
+        }, auth=self.user.auth, expect_errors=True)
         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(url, json={
+        rv = self.app.post_json(url, {
             'access_key': '',
             'secret_key': 'Non-empty-secret-key'
-        }, auth=self.user.auth, )
+        }, auth=self.user.auth, expect_errors=True)
         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(url, json={
+        rv = self.app.post_json(url, {
             'access_key': 'Non-empty-access-key',
             'secret_key': ''
         }, auth=self.user.auth, expect_errors=True)
@@ -72,19 +73,77 @@ def test_s3_settings_rdm_addons_denied(self):
         rdm_addon_option.is_allowed = False
         rdm_addon_option.save()
         url = self.project.api_url_for('s3_add_user_account')
-        rv = self.app.post(url, json={
+        rv = self.app.post_json(url, {
             'access_key': 'aldkjf',
             'secret_key': 'las'
         }, auth=self.user.auth, expect_errors=True)
         assert rv.status_code == http_status.HTTP_403_FORBIDDEN
         assert 'You are prohibited from using this add-on.' in rv.text
 
+    # G5: GRDM-53044 provider_id = "{aws_account_id}\t{access_key}"
+    # setUp already patches get_user_info/can_list as truthy MagicMocks.
+    # Note: oauth_key/oauth_secret are EncryptedTextField (JWE, non-deterministic),
+    # so we query by provider_id (plain CharField) and verify via attribute access.
+    def test_provider_id_format(self):
+        url = self.project.api_url_for('s3_add_user_account')
+        access_key = 'AKIAIOSFODNN7EXAMPLE'
+        rv = self.app.post_json(url, {
+            'access_key': access_key,
+            'secret_key': 'wJalrXUtnFEMI/K7MDENG',
+        }, auth=self.user.auth)
+        assert rv.status_code == http_status.HTTP_200_OK
+        # provider_id is not encrypted — safe to query
+        provider_id = f'1234567890\t{access_key}'
+        account = ExternalAccount.objects.get(provider='s3', provider_id=provider_id)
+        # provider_id must be "{aws_account_id}\t{access_key}" (GRDM-53044)
+        assert account.provider_id.endswith(f'\t{access_key}')
+        assert '\t' in account.provider_id
+        assert account.oauth_key == access_key
+
+    def test_provider_id_dedup(self):
+        url = self.project.api_url_for('s3_add_user_account')
+        access_key = 'AKIAIOSFODNN7EXAMPLE'
+        provider_id = f'1234567890\t{access_key}'
+        rv1 = self.app.post_json(url, {
+            'access_key': access_key,
+            'secret_key': 'secret1',
+        }, auth=self.user.auth)
+        assert rv1.status_code == http_status.HTTP_200_OK
+        # Same AWS account (same provider_id) with new secret → update, no duplicate
+        rv2 = self.app.post_json(url, {
+            'access_key': access_key,
+            'secret_key': 'secret2',
+        }, auth=self.user.auth)
+        assert rv2.status_code == http_status.HTTP_200_OK
+        # Query by provider_id (plain CharField) — oauth_key is encrypted (JWE)
+        accounts = ExternalAccount.objects.filter(provider='s3', provider_id=provider_id)
+        assert accounts.count() == 1
+        assert accounts.first().oauth_secret == 'secret2'
+
+    # G1: missing key field → KeyError → 400
+    def test_s3_settings_missing_key_field(self):
+        url = self.project.api_url_for('s3_add_user_account')
+        rv = self.app.post_json(url, {}, auth=self.user.auth, expect_errors=True)
+        assert rv.status_code == http_status.HTTP_400_BAD_REQUEST
+
+    # G2: get_user_info returns None → 400 + "Unable to access account."
+    @mock.patch('addons.s3.views.utils.get_user_info', return_value=None)
+    def test_s3_settings_invalid_account(self, mock_get_user_info):
+        url = self.project.api_url_for('s3_add_user_account')
+        rv = self.app.post_json(url, {
+            'access_key': 'aldkjf',
+            'secret_key': 'las',
+        }, auth=self.user.auth, expect_errors=True)
+        assert rv.status_code == http_status.HTTP_400_BAD_REQUEST
+        assert 'Unable to access account.' in rv.text
+
     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(
-            url, json={'s3_bucket': 'hammertofall'}, auth=user.auth,
+        res = self.app.put_json(
+            url, {'s3_bucket': 'hammertofall'}, auth=user.auth,
+            expect_errors=True,
         )
         assert res.status_code == http_status.HTTP_400_BAD_REQUEST
 
@@ -94,8 +153,9 @@ 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(
-            url, json={'s3_bucket': 'hammertofall'}, auth=user.auth,
+        res = self.app.put_json(
+            url, {'s3_bucket': 'hammertofall'}, auth=user.auth,
+            expect_errors=True,
         )
         assert res.status_code == http_status.HTTP_403_FORBIDDEN
 
@@ -105,8 +165,9 @@ def test_s3_set_bucket_registered(self):
         )
 
         url = registration.api_url_for('s3_set_config')
-        res = self.app.put(
-            url, json={'s3_bucket': 'hammertofall'}, auth=self.user.auth,
+        res = self.app.put_json(
+            url, {'s3_bucket': 'hammertofall'}, auth=self.user.auth,
+            expect_errors=True,
         )
 
         assert res.status_code == http_status.HTTP_400_BAD_REQUEST
@@ -114,10 +175,10 @@ def test_s3_set_bucket_registered(self):
     @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(url, json={
+        rv = self.app.post_json(url, {
             'access_key': 'aldkjf',
             'secret_key': 'las'
-        }, auth=self.user.auth)
+        }, auth=self.user.auth, expect_errors=True)
 
         assert 'Unable to list buckets.' in rv.text
         assert rv.status_code == http_status.HTTP_400_BAD_REQUEST
@@ -130,7 +191,7 @@ def test_s3_remove_node_settings_owner(self):
 
     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, )
+        ret = self.app.delete(url, auth=None, expect_errors=True)
 
         assert ret.status_code == 401
 
@@ -149,7 +210,7 @@ def test_s3_get_node_settings_owner(self):
     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, )
+        ret = self.app.get(url, auth=unauthorized.auth, expect_errors=True)
 
         assert ret.status_code == 403
 
@@ -167,7 +228,7 @@ def test_set_config(self, mock_location, mock_exists):
         mock_location.return_value = ''
         self.node_settings.set_auth(self.external_account, self.user)
         url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_set_config')
-        res = self.app.put(url, json={
+        res = self.app.put_json(url, {
             'selected': self.folder
         }, auth=self.user.auth)
         assert res.status_code == http_status.HTTP_200_OK
@@ -265,9 +326,9 @@ def test_create_bucket_pass(self, mock_names, mock_make):
             'doesntevenmatter'
         ]
         url = self.project.api_url_for('create_bucket')
-        ret = self.app.post(
+        ret = self.app.post_json(
             url,
-            json={
+            {
                 'bucket_name': 'doesntevenmatter',
                 'bucket_location': '',
             },
@@ -283,19 +344,69 @@ def test_create_bucket_fail(self, mock_make):
         mock_make.side_effect = error
 
         url = f'/api/v1/project/{self.project._id}/s3/newbucket/'
-        ret = self.app.post(url, json={'bucket_name': 'doesntevenmatter'}, auth=self.user.auth)
+        ret = self.app.post_json(url, {'bucket_name': 'doesntevenmatter'}, auth=self.user.auth,
+                                 expect_errors=True)
 
         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 = f'/api/v1/project/{self.project._id}/s3/newbucket/'
-        ret = self.app.post(
+        ret = self.app.post_json(
             url,
-            json={
+            {
                 'bucket_name': 'doesntevenmatter',
                 'bucket_location': 'not a real bucket location',
             },
             auth=self.user.auth,
+            expect_errors=True,
         )
         assert ret.text == '{"message": "That bucket location is not valid.", "title": "Invalid bucket location"}'
+
+    # G4: invalid bucket name → 400 (name check fires before any S3 call)
+    def test_create_bucket_bad_name(self):
+        url = f'/api/v1/project/{self.project._id}/s3/newbucket/'
+        ret = self.app.post_json(
+            url,
+            {
+                'bucket_name': 'no',
+                'bucket_location': '',
+            },
+            auth=self.user.auth,
+            expect_errors=True,
+        )
+        assert ret.status_code == http_status.HTTP_400_BAD_REQUEST
+        assert 'That bucket name is not valid.' in ret.text
+
+    @mock.patch('addons.s3.views.utils.create_bucket')
+    def test_create_bucket_client_error(self, mock_make):
+        from botocore.exceptions import ClientError
+        error = ClientError(
+            {'Error': {'Code': 'BucketAlreadyExists', 'Message': 'The requested bucket name is not available.'}},
+            'CreateBucket'
+        )
+        mock_make.side_effect = error
+        url = f'/api/v1/project/{self.project._id}/s3/newbucket/'
+        ret = self.app.post_json(
+            url,
+            {'bucket_name': 'valid-bucket-name', 'bucket_location': ''},
+            auth=self.user.auth,
+            expect_errors=True,
+        )
+        assert ret.status_code == http_status.HTTP_400_BAD_REQUEST
+        assert ret.json['title'] == "Problem creating bucket 'valid-bucket-name'"
+        assert 'BucketAlreadyExists' in ret.json['message']
+
+
+class TestS3Utils:
+    """Pure unit tests for utility functions — no DB required."""
+
+    # G7: bucket_exists with empty name → False (early exit, no S3 call)
+    def test_bucket_exists_empty_name(self):
+        from addons.s3.utils import bucket_exists
+        assert bucket_exists('key', 'secret', '') is False
+
+    # G8: can_list(None, None) → False (early exit, no S3 call)
+    def test_can_list_no_keys(self):
+        from addons.s3.utils import can_list
+        assert can_list(None, None) is False

From b6843500490f301a847d72aa5e4ff9f26a54d22f Mon Sep 17 00:00:00 2001
From: TOMONORI ENDOU <tishin.endou@gmail.com>
Date: Fri, 2 Oct 2026 15:29:42 +0900
Subject: [PATCH 5/9] fix(s3): preserve bucket names as given and tolerate a
 missing owner display name

- stop lower-casing the bucket name in the settings check. S3 bucket names are
  case-sensitive in the legacy us-east-1 naming rules, so folding the case can
  point the addon at a different bucket; validate_bucket_name already rejects
  the names that are actually invalid
- fall back gracefully when the owner returned by GetUserInfo has no display
  name, instead of raising while building the account label
- keep the Owner entry of the admin storage-location response in its original
  shape so that the existing consumers keep working
---
 addons/s3/tests/test_model.py              | 16 ++++++++++++++++
 addons/s3/tests/test_view.py               |  6 ++++++
 addons/s3/utils.py                         |  8 ++------
 admin/rdm_custom_storage_location/utils.py |  5 ++++-
 4 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/addons/s3/tests/test_model.py b/addons/s3/tests/test_model.py
index ac148c59812..9e22b3f591f 100644
--- a/addons/s3/tests/test_model.py
+++ b/addons/s3/tests/test_model.py
@@ -122,3 +122,19 @@ def test_serialize_settings(self):
             'id': 'bucket_name:/path_goes_here/with_folder_id'
         }
         assert settings == expected
+
+    @pytest.mark.parametrize('folder_id, expected_bucket', [
+        ('bucket', 'bucket'),
+        ('bucket:/', 'bucket'),
+        ('bucket:/prefix/', 'bucket'),
+    ])
+    def test_serialize_settings_supports_legacy_and_prefixed_folder_ids(self, folder_id, expected_bucket):
+        self.node_settings.folder_id = folder_id
+
+        settings = self.node_settings.serialize_waterbutler_settings()
+
+        assert settings == {
+            'bucket': expected_bucket,
+            'encrypt_uploads': self.node_settings.encrypt_uploads,
+            'id': folder_id,
+        }
diff --git a/addons/s3/tests/test_view.py b/addons/s3/tests/test_view.py
index dd587cff2d2..8cc8a6a5452 100644
--- a/addons/s3/tests/test_view.py
+++ b/addons/s3/tests/test_view.py
@@ -401,6 +401,12 @@ def test_create_bucket_client_error(self, mock_make):
 class TestS3Utils:
     """Pure unit tests for utility functions — no DB required."""
 
+    def test_owner_from_dict_without_display_name(self):
+        from addons.s3.utils import Owner
+        owner = Owner.from_dict({'ID': 'owner-id'})
+        assert owner.id == 'owner-id'
+        assert owner.display_name == 'owner-id'
+
     # G7: bucket_exists with empty name → False (early exit, no S3 call)
     def test_bucket_exists_empty_name(self):
         from addons.s3.utils import bucket_exists
diff --git a/addons/s3/utils.py b/addons/s3/utils.py
index 73d53ebf18e..a3ed3ab8ad5 100644
--- a/addons/s3/utils.py
+++ b/addons/s3/utils.py
@@ -84,8 +84,6 @@ def bucket_exists(access_key, secret_key, bucket_name):
     """
     if not bucket_name:
         return False
-    # 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
@@ -121,7 +119,8 @@ class Owner:
 
     @classmethod
     def from_dict(cls, data):
-        return cls(data['DisplayName'], data['ID'])
+        owner_id = data['ID']
+        return cls(data.get('DisplayName') or owner_id, owner_id)
 
 
 def get_user_info(access_key: str, secret_key: str) -> Optional[Owner]:
@@ -139,9 +138,6 @@ def get_user_info(access_key: str, secret_key: str) -> Optional[Owner]:
 def get_bucket_location_or_error(access_key, secret_key, bucket_name):
     """Returns the location of a bucket or raises AddonError
     """
-    # 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_location(Bucket=bucket_name)['LocationConstraint']
diff --git a/admin/rdm_custom_storage_location/utils.py b/admin/rdm_custom_storage_location/utils.py
index df350ed2ae1..ae6ab5209d6 100644
--- a/admin/rdm_custom_storage_location/utils.py
+++ b/admin/rdm_custom_storage_location/utils.py
@@ -261,7 +261,10 @@ def test_s3_connection(access_key, secret_key, bucket):
     s3_response = {
         'id': user_info.id,
         'display_name': user_info.display_name,
-        'Owner': user_info.Owner,
+        'Owner': {
+            'ID': user_info.id,
+            'DisplayName': user_info.display_name,
+        },
     }
 
     return ({

From abb5a78de229f86f0daeb4e506590cf72ed4ef1c Mon Sep 17 00:00:00 2001
From: TOMONORI ENDOU <tishin.endou@gmail.com>
Date: Fri, 2 Oct 2026 15:29:53 +0900
Subject: [PATCH 6/9] test(s3): run the folder_id cases in a loop instead of
 pytest.mark.parametrize

pytest.mark.parametrize has no effect on a method of a unittest.TestCase: the
case is collected once and the extra arguments are never supplied, so
test_serialize_settings_supports_legacy_and_prefixed_folder_ids errored on a
missing fixture instead of checking the three folder_id forms. Iterate over
the cases inside the test body instead.
---
 addons/s3/tests/test_model.py | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/addons/s3/tests/test_model.py b/addons/s3/tests/test_model.py
index 9e22b3f591f..8e549c5d1ae 100644
--- a/addons/s3/tests/test_model.py
+++ b/addons/s3/tests/test_model.py
@@ -123,18 +123,20 @@ def test_serialize_settings(self):
         }
         assert settings == expected
 
-    @pytest.mark.parametrize('folder_id, expected_bucket', [
-        ('bucket', 'bucket'),
-        ('bucket:/', 'bucket'),
-        ('bucket:/prefix/', 'bucket'),
-    ])
-    def test_serialize_settings_supports_legacy_and_prefixed_folder_ids(self, folder_id, expected_bucket):
-        self.node_settings.folder_id = folder_id
-
-        settings = self.node_settings.serialize_waterbutler_settings()
-
-        assert settings == {
-            'bucket': expected_bucket,
-            'encrypt_uploads': self.node_settings.encrypt_uploads,
-            'id': folder_id,
-        }
+    def test_serialize_settings_supports_legacy_and_prefixed_folder_ids(self):
+        cases = [
+            ('bucket', 'bucket'),
+            ('bucket:/', 'bucket'),
+            ('bucket:/prefix/', 'bucket'),
+        ]
+
+        for folder_id, expected_bucket in cases:
+            self.node_settings.folder_id = folder_id
+
+            settings = self.node_settings.serialize_waterbutler_settings()
+
+            assert settings == {
+                'bucket': expected_bucket,
+                'encrypt_uploads': self.node_settings.encrypt_uploads,
+                'id': folder_id,
+            }

From 14a80274321b12263bc1f94d9528f8433526d712 Mon Sep 17 00:00:00 2001
From: TOMONORI ENDOU <tishin.endou@gmail.com>
Date: Thu, 1 Oct 2026 09:43:06 +0900
Subject: [PATCH 7/9] Report us-east-1 as the empty-string bucket location

get_bucket_location() returns a null LocationConstraint for us-east-1,
but BUCKET_LOCATIONS keys that region as '', so the null fell through to
set_folder and labelled the bucket '(None)' in the node settings UI.
---
 addons/s3/tests/test_view.py | 17 +++++++++++++++++
 addons/s3/utils.py           |  5 +++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/addons/s3/tests/test_view.py b/addons/s3/tests/test_view.py
index 8cc8a6a5452..61ed13c669b 100644
--- a/addons/s3/tests/test_view.py
+++ b/addons/s3/tests/test_view.py
@@ -416,3 +416,20 @@ def test_bucket_exists_empty_name(self):
     def test_can_list_no_keys(self):
         from addons.s3.utils import can_list
         assert can_list(None, None) is False
+
+    # G9: us-east-1 is reported as a null LocationConstraint, but BUCKET_LOCATIONS
+    # keys that region as ''.  Passing the null through labels the bucket '(None)'.
+    @mock.patch('addons.s3.utils.connect_s3')
+    def test_null_location_constraint_becomes_empty_string(self, mock_connect):
+        from addons.s3.utils import get_bucket_location_or_error
+        mock_connect.return_value.get_bucket_location.return_value = {'LocationConstraint': None}
+        assert get_bucket_location_or_error('key', 'secret', 'bucket') == ''
+
+    # G10: every other region reports its own name and must survive untouched
+    @mock.patch('addons.s3.utils.connect_s3')
+    def test_named_region_is_passed_through(self, mock_connect):
+        from addons.s3.utils import get_bucket_location_or_error
+        mock_connect.return_value.get_bucket_location.return_value = {
+            'LocationConstraint': 'ap-northeast-1'
+        }
+        assert get_bucket_location_or_error('key', 'secret', 'bucket') == 'ap-northeast-1'
diff --git a/addons/s3/utils.py b/addons/s3/utils.py
index a3ed3ab8ad5..8992ad9dd27 100644
--- a/addons/s3/utils.py
+++ b/addons/s3/utils.py
@@ -139,8 +139,9 @@ def get_bucket_location_or_error(access_key, secret_key, bucket_name):
     """Returns the location of a bucket or raises AddonError
     """
     try:
-        # Will raise an exception if bucket_name doesn't exist
-        return connect_s3(access_key, secret_key).get_bucket_location(Bucket=bucket_name)['LocationConstraint']
+        # Will raise an exception if bucket_name doesn't exist.
+        # us-east-1 is reported as a null LocationConstraint; BUCKET_LOCATIONS keys it as ''.
+        return connect_s3(access_key, secret_key).get_bucket_location(Bucket=bucket_name)['LocationConstraint'] or ''
     except exceptions.NoCredentialsError:
         raise InvalidAuthError()
     except exceptions.ClientError:

From 10873ee6c07272a391e6dfb6a97ccac285374778 Mon Sep 17 00:00:00 2001
From: TOMONORI ENDOU <tishin.endou@gmail.com>
Date: Thu, 1 Oct 2026 09:43:27 +0900
Subject: [PATCH 8/9] Translate list_objects failures into HTTPError in
 get_bucket_prefixes

The ListObjects call was unguarded, so expired or revoked keys and a
missing bucket escaped as a raw botocore exception and reached the user
as a 500 with no actionable message.  Map NoCredentialsError to 403 and
reuse get_status_for_error so a ClientError keeps the status S3 returned.
---
 addons/s3/tests/test_view.py | 57 ++++++++++++++++++++++++++++++++++++
 addons/s3/utils.py           |  8 ++++-
 2 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/addons/s3/tests/test_view.py b/addons/s3/tests/test_view.py
index 61ed13c669b..46e0aced70f 100644
--- a/addons/s3/tests/test_view.py
+++ b/addons/s3/tests/test_view.py
@@ -433,3 +433,60 @@ def test_named_region_is_passed_through(self, mock_connect):
             'LocationConstraint': 'ap-northeast-1'
         }
         assert get_bucket_location_or_error('key', 'secret', 'bucket') == 'ap-northeast-1'
+
+    # G11: the queried prefix comes back in CommonPrefixes and must not list itself
+    @mock.patch('addons.s3.utils.boto3.client')
+    def test_get_bucket_prefixes_skips_the_queried_prefix(self, mock_client):
+        from addons.s3.utils import get_bucket_prefixes
+        mock_client.return_value.list_objects.return_value = {
+            'CommonPrefixes': [
+                {'Prefix': 'parent/'},
+                {'Prefix': 'parent/child/'},
+            ]
+        }
+
+        folders = get_bucket_prefixes('key', 'secret', 'parent/', 'bucket')
+
+        assert len(folders) == 1
+        assert folders[0]['name'] == 'child'
+        assert folders[0]['id'] == 'bucket:/parent/child/'
+        assert folders[0]['path'] == 'parent/child/'
+        assert folders[0]['kind'] == 'folder'
+
+    # G12: a bucket with no sub-prefixes omits CommonPrefixes entirely
+    @mock.patch('addons.s3.utils.boto3.client')
+    def test_get_bucket_prefixes_no_common_prefixes(self, mock_client):
+        from addons.s3.utils import get_bucket_prefixes
+        mock_client.return_value.list_objects.return_value = {}
+        assert get_bucket_prefixes('key', 'secret', '', 'bucket') == []
+
+    # G13: boto failures must become HTTPError, not an uncaught 500
+    @mock.patch('addons.s3.utils.boto3.client')
+    def test_get_bucket_prefixes_missing_credentials_raises_403(self, mock_client):
+        from framework.exceptions import HTTPError
+        from addons.s3.utils import get_bucket_prefixes
+        mock_client.return_value.list_objects.side_effect = NoCredentialsError()
+
+        with pytest.raises(HTTPError) as exc_info:
+            get_bucket_prefixes('key', 'secret', '', 'bucket')
+
+        assert exc_info.value.code == http_status.HTTP_403_FORBIDDEN
+
+    # G14: a ClientError keeps whatever status S3 itself returned
+    @mock.patch('addons.s3.utils.boto3.client')
+    def test_get_bucket_prefixes_client_error_keeps_status(self, mock_client):
+        from botocore.exceptions import ClientError
+        from framework.exceptions import HTTPError
+        from addons.s3.utils import get_bucket_prefixes
+        mock_client.return_value.list_objects.side_effect = ClientError(
+            {
+                'Error': {'Code': 'NoSuchBucket', 'Message': 'denied'},
+                'ResponseMetadata': {'HTTPStatusCode': http_status.HTTP_404_NOT_FOUND},
+            },
+            'ListObjects',
+        )
+
+        with pytest.raises(HTTPError) as exc_info:
+            get_bucket_prefixes('key', 'secret', '', 'bucket')
+
+        assert exc_info.value.code == http_status.HTTP_404_NOT_FOUND
diff --git a/addons/s3/utils.py b/addons/s3/utils.py
index 8992ad9dd27..f0cb70da46c 100644
--- a/addons/s3/utils.py
+++ b/addons/s3/utils.py
@@ -155,7 +155,13 @@ def get_bucket_prefixes(access_key, secret_key, prefix, bucket_name):
         aws_secret_access_key=secret_key
     )
 
-    result = s3.list_objects(Bucket=bucket_name, Prefix=prefix, Delimiter='/')
+    try:
+        result = s3.list_objects(Bucket=bucket_name, Prefix=prefix, Delimiter='/')
+    except exceptions.NoCredentialsError:
+        raise HTTPError(http_status.HTTP_403_FORBIDDEN)
+    except exceptions.ClientError as e:
+        raise HTTPError(get_status_for_error(e))
+
     folders = []
     for common_prefixes in result.get('CommonPrefixes', []):
         key_name = common_prefixes.get('Prefix')

From 94d57ea2464e912e4be65d2705f7c22ba8543138 Mon Sep 17 00:00:00 2001
From: TOMONORI ENDOU <tishin.endou@gmail.com>
Date: Thu, 1 Oct 2026 09:43:44 +0900
Subject: [PATCH 9/9] Catch BotoCoreError when creating a bucket

create_bucket only handled NoCredentialsError and ClientError, so the
non-response failures - bad parameters, an unreachable endpoint - left
the user with a 500.  ClientError is not a BotoCoreError subclass and
NoCredentialsError is matched earlier, so the existing clauses keep
their behaviour.
---
 addons/s3/tests/test_view.py | 17 +++++++++++++++++
 addons/s3/views.py           |  7 +++++++
 2 files changed, 24 insertions(+)

diff --git a/addons/s3/tests/test_view.py b/addons/s3/tests/test_view.py
index 46e0aced70f..c60d796b91d 100644
--- a/addons/s3/tests/test_view.py
+++ b/addons/s3/tests/test_view.py
@@ -397,6 +397,23 @@ def test_create_bucket_client_error(self, mock_make):
         assert ret.json['title'] == "Problem creating bucket 'valid-bucket-name'"
         assert 'BucketAlreadyExists' in ret.json['message']
 
+    @mock.patch('addons.s3.views.utils.create_bucket')
+    def test_create_bucket_boto_core_error(self, mock_make):
+        # BotoCoreError covers the non-response failures (bad parameters, endpoint
+        # unreachable, ...).  Without the catchall these surface as a 500.
+        from botocore.exceptions import ParamValidationError
+        mock_make.side_effect = ParamValidationError(report='Invalid bucket configuration')
+        url = f'/api/v1/project/{self.project._id}/s3/newbucket/'
+        ret = self.app.post_json(
+            url,
+            {'bucket_name': 'valid-bucket-name', 'bucket_location': ''},
+            auth=self.user.auth,
+            expect_errors=True,
+        )
+        assert ret.status_code == http_status.HTTP_400_BAD_REQUEST
+        assert ret.json['title'] == 'Error connecting to S3'
+        assert 'Invalid bucket configuration' in ret.json['message']
+
 
 class TestS3Utils:
     """Pure unit tests for utility functions — no DB required."""
diff --git a/addons/s3/views.py b/addons/s3/views.py
index 1627bcf1a7f..5bccd9b3d3a 100644
--- a/addons/s3/views.py
+++ b/addons/s3/views.py
@@ -162,5 +162,12 @@ def create_bucket(auth, node_addon, **kwargs):
             'message': str(e),
             'title': f"Problem creating bucket '{bucket_name}'",
         }, http_status.HTTP_400_BAD_REQUEST
+    except exceptions.BotoCoreError as e:
+        # Base class catchall. ClientError is not a BotoCoreError, and NoCredentialsError
+        # is handled above, so neither of the clauses above is shadowed by this one.
+        return {
+            'message': str(e),
+            'title': 'Error connecting to S3',
+        }, http_status.HTTP_400_BAD_REQUEST
 
     return {}