more test fixes

api/nodes/serializers.py

@@ -1096,39 +1096,40 @@ def update(self, instance, validated_data):
 
 
 class S3NodeAddonSettingsSerializer(NodeAddonSettingsSerializer):
- SHORT_NAME = 's3'
- FULL_NAME = 'Amazon S3'
 
  access_token = ser.CharField(required=False, allow_null=True, write_only=True)
  secret_token = ser.CharField(required=False, allow_null=True, write_only=True)
 
  def update(self, instance, validated_data):
+ """
+ Overrides NodeSettings behavior to allow S3 specific behavior.
+ """
  instance = super().update(instance, validated_data)
 
  access_token = validated_data.get('access_token')
  secret_token = validated_data.get('secret_token')
  if access_token and secret_token:
+ # Validate S3 credentials before creating account
  try:
  user_info = connect_s3(access_token, secret_token).get_all_buckets().owner
  except S3ResponseError:
- raise exceptions.PermissionDenied(f'The {self.FULL_NAME} credentials provided are incorrect.')
+ raise exceptions.PermissionDenied('The S3 credentials provided are incorrect.')
 
  account, created = ExternalAccount.objects.update_or_create(
- provider=self.SHORT_NAME,
+ provider='s3',
  provider_id=user_info.id,
  defaults={
  'display_name': user_info.display_name,
  'oauth_key': access_token,
  'oauth_secret': secret_token,
- 'provider_name': self.SHORT_NAME,
  },
  )
- account.save()
-
+ # If the user adds credentials via the v2 API add them to that user's user settings too.
  instance.external_account = account
  user = self.context['request'].user
- instance.user_settings = user.get_addon(self.SHORT_NAME)
+ instance.user_settings = user.get_or_add_addon('s3')
  instance.save()
+
  if not user.external_accounts.filter(id=account.id).exists():
  user.external_accounts.add(account)
  user.save()

api_tests/addons_tests/s3/test_configure_s3.py

@@ -6,67 +6,91 @@
 from addons.s3.tests.factories import S3UserSettingsFactory
 
 
+def _mock_s3_client():
+ """
+ Mock client for boto.s3.connection.S3Connection
+ """
+ _mock = lambda attributes: type('MockObject', (mock.Mock,), attributes)
+ return _mock({
+ 'get_all_buckets': _mock({
+ 'owner': _mock({
+ 'display_name': 'Jalen Hurts',
+ 'id': '#1',
+ }),
+ }),
+ 'head_bucket': _mock({}),
+ 'get_bucket': _mock({
+ 'get_location': lambda *args, **kwargs: 'us-west-1',
+ }),
+ })
+
+
 @pytest.mark.django_db
 class TestS3Config:
+ """
+ This tests features added as part of the the POSE grant, these features should allow our Amazon S3 addons to be
+ fully configured via osf.io's REST API, instead of relying on the legacy FE.
+ Features added:
+
+ 1. Ability to add credentials via API tested in `test_addon_credentials_PATCH`
+ 2. Ability to configure AWS Bucket and base folders entirely via API in tested in `test_addon_folders_PATCH`
+ """
 
  @pytest.fixture()
  def user(self):
  return AuthUserFactory()
 
  @pytest.fixture()
  def node(self, user):
- project = ProjectFactory(creator=user)
- project.get_or_add_addon('s3', auth=Auth(user))
- return project
+ return ProjectFactory(creator=user)
 
  @pytest.fixture()
- def node_with_authorized_addon(self, user, node):
- addon = node.get_addon('s3')
- external_account = ExternalAccountFactory()
- user.external_accounts.add(external_account)
- addon.external_account = external_account
+ def enabled_addon(self, node, user):
+ addon = node.get_or_add_addon('s3', auth=Auth(user))
  addon.user_settings = S3UserSettingsFactory(owner=user)
  addon.save()
- return node
+ return addon
 
- def setup_mock_s3(self, mock_s3):
- mock_return = lambda attributes: type('MockObject', (mock.Mock,), attributes)
- return_value = mock_return({
- 'get_all_buckets': mock_return({
- 'owner': mock_return({
- 'display_name': 'Jalen Hurts',
- 'id': '#1',
- }),
- }),
- 'head_bucket': mock_return({}),
- 'get_bucket': mock_return({
- 'get_location': lambda *args, **kwargs: 'us-west-1',
- }),
- })
- mock_s3.return_value = return_value
-
- @mock.patch('addons.s3.utils.connect_s3')
- def test_addon_credential_PATCH(self, mock_s3, app, node, user):
- self.setup_mock_s3(mock_s3)
-
- payload = {
- 'data': {'attributes': {'access_token': 'test_access_key', 'secret_token': 'test_secret_key'}},
- }
+ @pytest.fixture()
+ def node_with_authorized_addon(self, user, node, enabled_addon):
+ external_account = ExternalAccountFactory()
+ user.external_accounts.add(external_account)
+ enabled_addon.external_account = external_account
+ enabled_addon.save()
+ return node
 
- resp = app.patch_json_api(f'/{API_BASE}nodes/{node._id}/addons/s3/', payload, auth=user.auth)
+ @mock.patch('addons.s3.utils.S3Connection', return_value=_mock_s3_client())
+ def test_addon_credentials_PATCH(self, mock_s3, app, node, user, enabled_addon):
+ resp = app.patch_json_api(
+ f'/{API_BASE}nodes/{node._id}/addons/s3/',
+ {
+ 'data': {
+ 'attributes': {
+ 'access_token': 'test_access_key',
+ 'secret_token': 'test_secret_key'
+ }
+ },
+ },
+ auth=user.auth
+ )
  assert resp.status_code == 200
  assert resp.json['data']['attributes']['external_account_id']
  assert resp.json['data']['attributes']['node_has_auth']
 
- @mock.patch('addons.s3.utils.connect_s3')
+ @mock.patch('addons.s3.utils.S3Connection', return_value=_mock_s3_client())
  def test_addon_folders_PATCH(self, mock_s3, app, node_with_authorized_addon, user):
- self.setup_mock_s3(mock_s3)
-
- payload = {
- 'data': {'attributes': {'folder_id': 'test_folder_id', 'folder_path': 'test_folder_path:/'}},
- }
-
- resp = app.patch_json_api(f'/{API_BASE}nodes/{node_with_authorized_addon._id}/addons/s3/', payload, auth=user.auth)
+ resp = app.patch_json_api(
+ f'/{API_BASE}nodes/{node_with_authorized_addon._id}/addons/s3/',
+ {
+ 'data': {
+ 'attributes': {
+ 'folder_id': 'test_folder_id',
+ 'folder_path': 'test_folder_path:/'
+ }
+ },
+ },
+ auth=user.auth
+ )
  assert resp.status_code == 200
  assert resp.json['data']['attributes']['folder_id'] == 'test_folder_id'
  assert resp.json['data']['attributes']['folder_path'] == 'test_folder_id (California)'