Merge pull request #639 from cdosborn/fix-account-creation

Place upper limit on tas user validation, remove selected identity notion

CHANGELOG.md

@@ -73,6 +73,16 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
     ([#631](https://github.com/cyverse/atmosphere/pull/631))
   - Projects can be deleted if they only contain links/applications
     ([#640](https://github.com/cyverse/atmosphere/pull/640))
+  - Added timeout of 5 sec to tas api for user validation, and refactored to
+    make validation more explicit in the absence of the old selected_identity
+    notion ([#639](https://github.com/cyverse/atmosphere/pull/639))
+
+### Removed
+  - In the general feedback email we no longer include the users selected
+    provider, as it's no longer relevant
+    ([#603](https://github.com/cyverse/atmosphere/pull/603))
+  - Removed references to selected_identity
+    ([#639](https://github.com/cyverse/atmosphere/pull/639))
 
 ### Fixed
   - Deleting a project via api/v2/projects no longer deletes enddated
@@ -94,11 +104,6 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
   - Fix incorrect fetching of instances, upgrade to rtwo version 0.5.22
     ([#641](https://github.com/cyverse/atmosphere/pull/641))
 
-### Removed
-  - In the general feedback email we no longer include the users selected
-    provider, as it's no longer relevant
-    ([#603](https://github.com/cyverse/atmosphere/pull/603))
-
 ## [v32-2](https://github.com/cyverse/atmosphere/compare/v32-1...v32-2) - 2018-04-26
 ### Fixed
   - Quota updates concerning volumes would silently fail

api/tests/factories/allocation_source_factory.py

@@ -1,13 +1,23 @@
+from django.utils import timezone
 import factory
+from api.tests.factories.user_factory import UserFactory
 
 from core.models import AllocationSource, UserAllocationSource
 
 
 class AllocationSourceFactory(factory.DjangoModelFactory):
+    name = factory.fuzzy.FuzzyText()
+    compute_allowed = 168
+    start_date = factory.fuzzy.FuzzyDateTime(timezone.now())
+    end_date = None
+    renewal_strategy = "default"
+
     class Meta:
         model = AllocationSource
 
 
 class UserAllocationSourceFactory(factory.DjangoModelFactory):
+    allocation_source = factory.SubFactory(AllocationSourceFactory)
+    user = factory.SubFactory(UserFactory)
     class Meta:
         model = UserAllocationSource

api/tests/v1/test_profile.py

@@ -0,0 +1,44 @@
+from django.test import TestCase, override_settings
+from django.urls import reverse
+from rest_framework.test import APIRequestFactory, force_authenticate
+import mock
+
+from api.v1.views import Profile
+from api.tests.factories import UserFactory
+from core.models import AtmosphereUser
+
+class ProfileTests(TestCase):
+    @override_settings(AUTO_CREATE_NEW_ACCOUNTS=True)
+    def test_external_accounts_are_created(self):
+        """
+        Sanity check that configuration results in call to create_new_accounts.
+        """
+        user = UserFactory()
+        url = reverse('api:v1:profile')
+        view = Profile.as_view()
+        factory = APIRequestFactory()
+        request = factory.get(url)
+        force_authenticate(request, user=user)
+
+        with mock.patch("api.v1.views.profile.create_new_accounts") as mock_create_new_accounts:
+            response = view(request)
+            mock_create_new_accounts.assert_called_once()
+
+    @override_settings(AUTO_CREATE_NEW_ACCOUNTS=True)
+    def test_external_accounts_are_not_created_for_invalid_user(self):
+        """
+        Accounts are NOT created when when the user is invalid
+        """
+        user = UserFactory()
+        url = reverse('api:v1:profile')
+        view = Profile.as_view()
+        factory = APIRequestFactory()
+        request = factory.get(url)
+        force_authenticate(request, user=user)
+
+        # Patch the user so that they are invalid
+        with mock.patch.object(AtmosphereUser, 'is_valid', return_value=False), \
+                mock.patch("api.v1.views.profile.create_new_accounts") \
+                as mock_create_new_accounts:
+            response = view(request)
+            mock_create_new_accounts.assert_not_called()

api/v1/serializers/__init__.py

@@ -23,7 +23,6 @@
 from .machine_request_serializer import MachineRequestSerializer
 from .maintenance_record_serializer import MaintenanceRecordSerializer
 from .identity_detail_serializer import IdentityDetailSerializer
-from .atmo_user_serializer import AtmoUserSerializer
 from .cloud_admin_serializer import (
     CloudAdminSerializer, CloudAdminActionListSerializer)
 from .profile_serializer import ProfileSerializer
@@ -53,7 +52,7 @@
     InstanceSerializer, InstanceActionSerializer, InstanceHistorySerializer,
     ExportRequestSerializer, LicenseSerializer, POST_LicenseSerializer,
     MachineRequestSerializer, MaintenanceRecordSerializer,
-    IdentityDetailSerializer, AtmoUserSerializer, CloudAdminSerializer,
+    IdentityDetailSerializer, CloudAdminSerializer,
     CloudAdminActionListSerializer, ProfileSerializer,
     ProviderMachineSerializer, GroupSerializer,
     VolumeSerializer, NoProjectSerializer,

api/v1/serializers/profile_serializer.py

@@ -15,9 +15,6 @@ class ProfileSerializer(serializers.ModelSerializer):
     is_expired = serializers.BooleanField(source='user.is_expired')
     is_staff = serializers.BooleanField(source='user.is_staff')
     is_superuser = serializers.BooleanField(source='user.is_superuser')
-    selected_identity = IdentityRelatedField(
-        source='user.select_identity',
-        queryset=Identity.objects.all())
 
     class Meta:
         model = UserProfile

api/v1/views/profile.py

@@ -4,11 +4,13 @@
 """
 from rest_framework.response import Response
 from rest_framework import status
-
+from django.conf import settings
 from django.utils import timezone
+
 from core.exceptions import InvalidUser
 from api.v1.views.base import AuthAPIView
-from api.v1.serializers import ProfileSerializer, AtmoUserSerializer
+from api.v1.serializers import ProfileSerializer
+from core.models.user import create_new_accounts
 
 
 class Profile(AuthAPIView):
@@ -28,17 +30,27 @@ def get(self, request, provider_uuid=None, identity_uuid=None):
         # THIS IS A HACK! -- This check covered by permissions in v2
         if not user.is_enabled:
             return Response(
-                "The account <%s> has been disabled by an Administrator. "
+                "The account <{}> has been disabled by an Administrator. "
                 "Please contact your Cloud Administrator for more information."
-                % (user.username,), status=403)  # Forbidden
+                .format(user.username), status=403)
 
         profile = user.userprofile
         try:
             serialized_data = ProfileSerializer(profile).data
         except InvalidUser as exc:
             return Response(exc.message,
                             status=status.HTTP_403_FORBIDDEN)
-        user.select_identity()
+
+        if not user.is_valid():
+            return Response(
+                "This account <{}> is invalid. "
+                "Please contact your Cloud Administrator for more information."
+                .format(user.username), status=403)
+
+        # User is valid, build out any new accounts
+        if settings.AUTO_CREATE_NEW_ACCOUNTS:
+            new_identities = create_new_accounts(user.username)
+
         response = Response(serialized_data)
         return response
 
@@ -51,14 +63,6 @@ def patch(self, request, provider_uuid=None, identity_uuid=None):
         user = request.user
         profile = user.userprofile
         mutable_data = request.data.copy()
-        if "selected_identity" in mutable_data:
-            user_data = {"selected_identity":
-                         mutable_data.pop("selected_identity")}
-            serializer = AtmoUserSerializer(user,
-                                            data=user_data,
-                                            partial=True)
-            if serializer.is_valid():
-                serializer.save()
         serializer = ProfileSerializer(profile,
                                        data=mutable_data,
                                        partial=True)

api/v2/views/email.py

@@ -64,7 +64,7 @@ def _email(self, user, data):
             "ui": data.get("user-interface", ""),
             "server": settings.SERVER_URL,
             "message": data["message"],
-            "provider": user.selected_identity.provider,
+            "provider": volume.instance_source.provider,
             "volume": volume,
         }
 

atmosphere/settings/local.py.j2

@@ -467,6 +467,7 @@ INSTALLED_APPS += (
 TACC_API_USER = '{{ TACC_API_USER }}'
 TACC_API_PASS = '{{ TACC_API_PASS }}'
 TACC_API_URL = '{{ TACC_API_URL }}'
+TACC_API_TIMEOUT = '{{ TACC_API_TIMEOUT | default(5) }}'
 # Uncomment this DEFAULT_QUOTA_PLUGINS line to use the JetstreamSpecialAllocationQuota plugin
 DEFAULT_QUOTA_PLUGINS = [
     'jetstream.plugins.quota.default_quota.JetstreamSpecialAllocationQuota',
@@ -495,6 +496,7 @@ SPECIAL_ALLOCATION_SOURCES = {
 TACC_API_USER = ''
 TACC_API_PASS = ''
 TACC_API_URL = ''
+TACC_API_TIMEOUT = ''
 {% endif %}
 
 {% if ALLOCATION_OVERRIDES_NEVER_ENFORCE %}

core/admin.py

@@ -433,9 +433,6 @@ class UserProfileInline(admin.StackedInline):
 @admin.register(models.AtmosphereUser)
 class UserAdmin(AuthUserAdmin):
     inlines = [UserProfileInline]
-    fieldsets = AuthUserAdmin.fieldsets + (
-        (None, {'fields': ('selected_identity',)}),
-    )
 
 
 @admin.register(models.IdentityMembership)

core/migrations/remove_atmosphereuser_selected_identity.py

@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.4 on 2018-09-06 19:43
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('core', 'remove-old-allocation-models'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='atmosphereuser',
+            name='selected_identity',
+        ),
+    ]

core/models/profile.py

@@ -2,7 +2,7 @@
 
 from threepio import logger
 
-from core.models.user import AtmosphereUser, get_default_identity
+from core.models.user import AtmosphereUser
 from core.models.identity import Identity
 
 from hashlib import md5
@@ -20,11 +20,6 @@ class UserProfile(models.Model):
     icon_set = models.CharField(max_length=255, default='default')
     guacamole_color = models.CharField(max_length=15, default='default')
 
-    def user_quota(self):
-        identity = get_default_identity(self.user.username)
-        identity_member = identity.identity_memberships.all()[0]
-        return identity_member.get_quota_dict()
-
     def email_hash(self):
         m = md5()
         m.update(self.user.email)