Skip to content
This repository has been archived by the owner on Sep 12, 2022. It is now read-only.

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

Merged
merged 19 commits into from
Sep 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 10 additions & 0 deletions api/tests/factories/allocation_source_factory.py
Original file line number Diff line number Diff line change
@@ -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
Empty file added api/tests/v1/__init__.py
Empty file.
44 changes: 44 additions & 0 deletions api/tests/v1/test_profile.py
Original file line number Diff line number Diff line change
@@ -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()
3 changes: 1 addition & 2 deletions api/v1/serializers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -53,7 +52,7 @@
InstanceSerializer, InstanceActionSerializer, InstanceHistorySerializer,
ExportRequestSerializer, LicenseSerializer, POST_LicenseSerializer,
MachineRequestSerializer, MaintenanceRecordSerializer,
IdentityDetailSerializer, AtmoUserSerializer, CloudAdminSerializer,
IdentityDetailSerializer, CloudAdminSerializer,
CloudAdminActionListSerializer, ProfileSerializer,
ProviderMachineSerializer, GroupSerializer,
VolumeSerializer, NoProjectSerializer,
Expand Down
35 changes: 0 additions & 35 deletions api/v1/serializers/atmo_user_serializer.py

This file was deleted.

3 changes: 0 additions & 3 deletions api/v1/serializers/profile_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 17 additions & 13 deletions api/v1/views/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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

Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion api/v2/views/email.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
2 changes: 2 additions & 0 deletions atmosphere/settings/local.py.j2
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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 %}
Expand Down
3 changes: 0 additions & 3 deletions core/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
19 changes: 19 additions & 0 deletions core/migrations/remove_atmosphereuser_selected_identity.py
Original file line number Diff line number Diff line change
@@ -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',
),
]
7 changes: 1 addition & 6 deletions core/models/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
Loading