You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airavata.apache.org by ma...@apache.org on 2021/12/10 21:43:07 UTC
[airavata-django-portal] 14/24: AIRAVATA-3319 Alert admins if username isn't valid and provide a means to update it
This is an automated email from the ASF dual-hosted git repository.
machristie pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/airavata-django-portal.git
commit 0295ebafb99c646c7bd3667f8f78b349795752fb
Author: Marcus Christie <ma...@apache.org>
AuthorDate: Fri Dec 3 17:46:52 2021 -0500
AIRAVATA-3319 Alert admins if username isn't valid and provide a means to update it
---
.../IdentityServiceUserManagementContainer.vue | 3 +-
django_airavata/apps/api/serializers.py | 1 +
.../django_airavata_api/js/service_config.js | 2 +-
django_airavata/apps/api/views.py | 18 ++-
django_airavata/apps/auth/backends.py | 30 +++-
django_airavata/apps/auth/middleware.py | 8 +-
.../0010_userprofile_username_initialized.py | 18 +++
django_airavata/apps/auth/models.py | 1 +
django_airavata/apps/auth/tests/test_backends.py | 177 +++++++++++++++++++++
django_airavata/apps/auth/utils.py | 44 +++++
10 files changed, 289 insertions(+), 13 deletions(-)
diff --git a/django_airavata/apps/admin/static/django_airavata_admin/src/components/users/IdentityServiceUserManagementContainer.vue b/django_airavata/apps/admin/static/django_airavata_admin/src/components/users/IdentityServiceUserManagementContainer.vue
index 458d718..11a2e24 100644
--- a/django_airavata/apps/admin/static/django_airavata_admin/src/components/users/IdentityServiceUserManagementContainer.vue
+++ b/django_airavata/apps/admin/static/django_airavata_admin/src/components/users/IdentityServiceUserManagementContainer.vue
@@ -207,9 +207,8 @@ export default {
},
updateUsername(userProfile, username, newUsername) {
const updatedUserProfile = userProfile.clone();
- updatedUserProfile.userId = newUsername;
+ updatedUserProfile.newUsername = newUsername;
services.IAMUserProfileService.updateUsername({
- lookup: username,
data: updatedUserProfile,
}).finally(() => this.reloadUserProfiles());
},
diff --git a/django_airavata/apps/api/serializers.py b/django_airavata/apps/api/serializers.py
index 0d49368..fcceaa3 100644
--- a/django_airavata/apps/api/serializers.py
+++ b/django_airavata/apps/api/serializers.py
@@ -952,6 +952,7 @@ class IAMUserProfile(serializers.Serializer):
lookup_field='userId',
lookup_url_kwarg='user_id')
userHasWriteAccess = serializers.SerializerMethodField()
+ newUsername = serializers.CharField(write_only=True)
def update(self, instance, validated_data):
existing_group_ids = [group.id for group in instance['groups']]
diff --git a/django_airavata/apps/api/static/django_airavata_api/js/service_config.js b/django_airavata/apps/api/static/django_airavata_api/js/service_config.js
index d3c82b0..152a396 100644
--- a/django_airavata/apps/api/static/django_airavata_api/js/service_config.js
+++ b/django_airavata/apps/api/static/django_airavata_api/js/service_config.js
@@ -288,7 +288,7 @@ export default {
modelClass: IAMUserProfile,
},
updateUsername: {
- url: "/api/iam-user-profiles/<lookup>/update_username/",
+ url: "/api/iam-user-profiles/update_username/",
bodyParams: {
name: "data",
},
diff --git a/django_airavata/apps/api/views.py b/django_airavata/apps/api/views.py
index ded1201..b9b68fd 100644
--- a/django_airavata/apps/api/views.py
+++ b/django_airavata/apps/api/views.py
@@ -25,6 +25,7 @@ from airavata.model.group.ttypes import ResourcePermissionType
from airavata.model.user.ttypes import Status
from airavata_django_portal_sdk import experiment_util, user_storage
from django.conf import settings
+from django.contrib.auth import get_user_model
from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
from django.http import Http404, HttpResponse, JsonResponse
from django.shortcuts import redirect
@@ -1597,13 +1598,22 @@ class IAMUserViewSet(mixins.RetrieveModelMixin,
context={'request': request})
return Response(serializer.data)
- @action(methods=['put'], detail=True)
- def update_username(self, request, user_id=None):
+ @action(methods=['put'], detail=False)
+ def update_username(self, request):
serializer = self.get_serializer(data=request.data)
serializer.is_valid(raise_exception=True)
- old_username = user_id
- new_username = serializer.validated_data['userId']
+ old_username = serializer.validated_data['userId']
+ new_username = serializer.validated_data['newUsername']
iam_admin_client.update_username(old_username, new_username)
+ # set username_initialized to True so it is treated as valid.
+ django_user = get_user_model().objects.get(username=old_username)
+ django_user.user_profile.username_initialized = True
+ django_user.user_profile.save()
+ # Not strictly necessary since next time the user logs in, the Django
+ # user record for the user will get updated to have the new username.
+ # But this is done to keep it consistent.
+ django_user.username = new_username
+ django_user.save()
instance = self.get_instance(new_username)
serializer = self.serializer_class(instance=instance,
context={'request': request})
diff --git a/django_airavata/apps/auth/backends.py b/django_airavata/apps/auth/backends.py
index a60fd59..2c41bf5 100644
--- a/django_airavata/apps/auth/backends.py
+++ b/django_airavata/apps/auth/backends.py
@@ -78,7 +78,7 @@ class KeycloakBackend(object):
user = self._process_userinfo(request, userinfo)
if idp_alias is not None:
self._store_idp_userinfo(user, token, idp_alias)
- # TODO: if idp_alias, add idp userinfo too
+ self._check_username_initialization(request, user)
access_token = token['access_token']
# authz_token_middleware has already run, so must manually add
# the `request.authz_token` attribute
@@ -216,9 +216,9 @@ class KeycloakBackend(object):
logger.debug("userinfo: {}".format(userinfo))
sub = userinfo['sub']
username = userinfo['preferred_username']
- email = userinfo['email']
- first_name = userinfo['given_name']
- last_name = userinfo['family_name']
+ email = userinfo.get('email', '')
+ first_name = userinfo.get('given_name', None)
+ last_name = userinfo.get('family_name', None)
user = self._get_or_create_user(sub, username)
user_profile = user.user_profile
@@ -323,3 +323,25 @@ class KeycloakBackend(object):
user_profile.idp_userinfo.create(idp_alias=idp_alias, claim=claim, value=value)
except Exception:
logger.exception(f"Failed to store IDP userinfo for {user.username} from IDP {idp_alias}")
+
+ def _check_username_initialization(self, request, user):
+ # Check if the username assigned to the user was based on the user's
+ # email address or if it was assigned some random string (Keycloak's
+ # sub). If the latter, we'll want to alert the admins so that they can
+ # assign a proper username for the user.
+ user_profile = user.user_profile
+ if (not user_profile.username_initialized and
+ user_profile.userinfo_set.filter(claim='email').exists() and
+ user_profile.userinfo_set.filter(claim='preferred_username').exists() and
+ user_profile.userinfo_set.get(claim='email').value == user_profile.userinfo_set.get(claim='preferred_username').value):
+ user_profile.username_initialized = True
+ user_profile.save()
+
+ # TODO: also check idp_userinfo.preferred_username if it exists
+
+ if not user_profile.username_initialized:
+ try:
+ utils.send_admin_alert_about_uninitialized_username(
+ request, user.username, user.email, user.first_name, user.last_name)
+ except Exception:
+ logger.exception(f"Failed to send alert about username being uninitialized: {user.username}")
diff --git a/django_airavata/apps/auth/middleware.py b/django_airavata/apps/auth/middleware.py
index ba49ec3..503cec6 100644
--- a/django_airavata/apps/auth/middleware.py
+++ b/django_airavata/apps/auth/middleware.py
@@ -38,7 +38,10 @@ def gateway_groups_middleware(get_response):
request.is_gateway_admin = False
request.is_read_only_gateway_admin = False
- if not request.user.is_authenticated or not request.authz_token or not request.user.user_profile.is_complete:
+ if (not request.user.is_authenticated or
+ not request.authz_token or
+ (hasattr(request.user, "user_profile") and
+ not request.user.user_profile.is_complete)):
return get_response(request)
try:
@@ -87,7 +90,8 @@ def user_profile_completeness_check(get_response):
reverse('django_airavata_auth:user_profile'),
reverse('django_airavata_auth:logout'),
]
- if (not request.user.user_profile.is_complete and
+ if (hasattr(request.user, "user_profile") and
+ not request.user.user_profile.is_complete and
request.path not in allowed_paths and
'text/html' in request.META['HTTP_ACCEPT']):
return redirect('django_airavata_auth:user_profile')
diff --git a/django_airavata/apps/auth/migrations/0010_userprofile_username_initialized.py b/django_airavata/apps/auth/migrations/0010_userprofile_username_initialized.py
new file mode 100644
index 0000000..4b47459
--- /dev/null
+++ b/django_airavata/apps/auth/migrations/0010_userprofile_username_initialized.py
@@ -0,0 +1,18 @@
+# Generated by Django 3.2.8 on 2021-10-12 20:35
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('django_airavata_auth', '0009_auto_20210625_1725'),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name='userprofile',
+ name='username_initialized',
+ field=models.BooleanField(default=False),
+ ),
+ ]
diff --git a/django_airavata/apps/auth/models.py b/django_airavata/apps/auth/models.py
index 726efce..8296311 100644
--- a/django_airavata/apps/auth/models.py
+++ b/django_airavata/apps/auth/models.py
@@ -58,6 +58,7 @@ class UserProfile(models.Model):
# TODO: maybe this can be derived from whether there exists an Airavata
# User Profile for the user's username
username_locked = models.BooleanField(default=False)
+ username_initialized = models.BooleanField(default=False)
@property
def is_complete(self):
diff --git a/django_airavata/apps/auth/tests/test_backends.py b/django_airavata/apps/auth/tests/test_backends.py
new file mode 100644
index 0000000..bd696ac
--- /dev/null
+++ b/django_airavata/apps/auth/tests/test_backends.py
@@ -0,0 +1,177 @@
+from unittest.mock import MagicMock, patch
+
+from django.contrib.auth.models import AnonymousUser
+from django.core import mail
+from django.test import RequestFactory, TestCase, override_settings
+
+from django_airavata.apps.auth import backends
+
+KEYCLOAK_CLIENT_ID = "kc-client"
+KEYCLOAK_CLIENT_SECRET = "kc-secret"
+KEYCLOAK_TOKEN_URL = "https://example.org/auth"
+KEYCLOAK_USERINFO_URL = "https://example.org/userinfo"
+KEYCLOAK_VERIFY_SSL = True
+AUTHENTICATION_OPTIONS = {
+ 'external': [
+ {
+ 'idp_alias': 'oidc',
+ 'name': 'Some OIDC compliant IDP',
+ }
+ ]
+}
+GATEWAY_ID = "gateway-id"
+
+
+@override_settings(
+ KEYCLOAK_CLIENT_ID=KEYCLOAK_CLIENT_ID,
+ KEYCLOAK_CLIENT_SECRET=KEYCLOAK_CLIENT_SECRET,
+ KEYCLOAK_TOKEN_URL=KEYCLOAK_TOKEN_URL,
+ KEYCLOAK_USERINFO_URL=KEYCLOAK_USERINFO_URL,
+ KEYCLOAK_VERIFY_SSL=KEYCLOAK_VERIFY_SSL,
+ AUTHENTICATION_OPTIONS=AUTHENTICATION_OPTIONS,
+ GATEWAY_ID=GATEWAY_ID,
+)
+class KeycloakBackendTestCase(TestCase):
+
+ def setUp(self):
+ self.factory = RequestFactory()
+
+ @patch("django_airavata.apps.auth.backends.OAuth2Session")
+ def test_username_initialized_with_email(self, MockOAuth2Session):
+ """Test that username_initialized is set to True when username equals email address."""
+
+ # Tests scenario that new user logs in via external IDP and when they
+ # are assigned a username it is the same as their email address. This is
+ # normally what happens and is a good outcome for the username so
+ # username_initialized should be set to True and no alert email should
+ # be sent to admins.
+
+ # Mock out request for redirect flow, and OAuth2Session: token and userinfo
+ request = self.factory.get("/callback?code=abc123", secure=True)
+ request.user = AnonymousUser()
+ request.session = {
+ 'OAUTH2_STATE': 'state',
+ 'OAUTH2_REDIRECT_URI': 'redirect-uri',
+ }
+ mock_oauth2_session = MagicMock()
+ MockOAuth2Session.return_value = mock_oauth2_session
+ mock_oauth2_session.fetch_token.return_value = {
+ 'access_token': 'the-access-token',
+ 'expires_in': 900,
+ 'refresh_token': 'the-refresh-token',
+ 'refresh_expires_in': 86400,
+ }
+ mock_userinfo = MagicMock()
+ mock_oauth2_session.get.return_value = mock_userinfo
+ email = 'testuser@example.org'
+ mock_userinfo.json.return_value = {
+ 'sub': 'sub-123',
+ 'preferred_username': email,
+ 'email': email,
+ 'given_name': 'Test',
+ 'family_name': 'User',
+ }
+
+ # Mock out fetching IDP userinfo: AUTHENTICATION_OPTIONS: request to
+ # idp_token_url and userinfo_url
+
+ backend = backends.KeycloakBackend()
+ idp_alias = "oidc"
+ user = backend.authenticate(request, idp_alias=idp_alias)
+
+ self.assertTrue(user.user_profile.username_initialized)
+ self.assertEqual(0, len(mail.outbox))
+
+ @patch("django_airavata.apps.auth.backends.OAuth2Session")
+ def test_username_initialized_with_no_email(self, MockOAuth2Session):
+ """Test that username_initialized is set to False when there is no email address."""
+
+ # Tests scenario that new user logs in via external IDP and when they
+ # are assigned a username but don't have an email address. This usually
+ # means that they also have a randomly generated username and admins
+ # need to be alerted.
+
+ # Mock out request for redirect flow, and OAuth2Session: token and userinfo
+ request = self.factory.get("/callback?code=abc123", secure=True)
+ request.user = AnonymousUser()
+ request.session = {
+ 'OAUTH2_STATE': 'state',
+ 'OAUTH2_REDIRECT_URI': 'redirect-uri',
+ }
+ mock_oauth2_session = MagicMock()
+ MockOAuth2Session.return_value = mock_oauth2_session
+ mock_oauth2_session.fetch_token.return_value = {
+ 'access_token': 'the-access-token',
+ 'expires_in': 900,
+ 'refresh_token': 'the-refresh-token',
+ 'refresh_expires_in': 86400,
+ }
+ mock_userinfo = MagicMock()
+ mock_oauth2_session.get.return_value = mock_userinfo
+ # email = 'testuser@example.org'
+ mock_userinfo.json.return_value = {
+ 'sub': 'sub-123',
+ 'preferred_username': 'some-random-username',
+ # 'email': email,
+ 'given_name': 'Test',
+ 'family_name': 'User',
+ }
+
+ # Mock out fetching IDP userinfo: AUTHENTICATION_OPTIONS: request to
+ # idp_token_url and userinfo_url
+
+ backend = backends.KeycloakBackend()
+ idp_alias = "oidc"
+ user = backend.authenticate(request, idp_alias=idp_alias)
+
+ self.assertFalse(user.user_profile.userinfo_set.filter(claim='email').exists())
+ self.assertFalse(user.user_profile.username_initialized)
+ self.assertEqual(1, len(mail.outbox))
+ self.assertTrue(mail.outbox[0].subject.startswith("Please fix username"))
+
+ @patch("django_airavata.apps.auth.backends.OAuth2Session")
+ def test_username_initialized_with_email_not_username(self, MockOAuth2Session):
+ """Test that username_initialized is set to False when email address is different from username."""
+
+ # Tests scenario that new user logs in via external IDP and when they
+ # are assigned a username but it's different from their email address.
+ # This usually means that they also have a randomly generated username
+ # and admins need to be alerted.
+
+ # Mock out request for redirect flow, and OAuth2Session: token and userinfo
+ request = self.factory.get("/callback?code=abc123", secure=True)
+ request.user = AnonymousUser()
+ request.session = {
+ 'OAUTH2_STATE': 'state',
+ 'OAUTH2_REDIRECT_URI': 'redirect-uri',
+ }
+ mock_oauth2_session = MagicMock()
+ MockOAuth2Session.return_value = mock_oauth2_session
+ mock_oauth2_session.fetch_token.return_value = {
+ 'access_token': 'the-access-token',
+ 'expires_in': 900,
+ 'refresh_token': 'the-refresh-token',
+ 'refresh_expires_in': 86400,
+ }
+ mock_userinfo = MagicMock()
+ mock_oauth2_session.get.return_value = mock_userinfo
+ email = 'testuser@example.org'
+ mock_userinfo.json.return_value = {
+ 'sub': 'sub-123',
+ 'preferred_username': 'some-random-username',
+ 'email': email,
+ 'given_name': 'Test',
+ 'family_name': 'User',
+ }
+
+ # Mock out fetching IDP userinfo: AUTHENTICATION_OPTIONS: request to
+ # idp_token_url and userinfo_url
+
+ backend = backends.KeycloakBackend()
+ idp_alias = "oidc"
+ user = backend.authenticate(request, idp_alias=idp_alias)
+
+ self.assertTrue(user.user_profile.userinfo_set.filter(claim='email').exists())
+ self.assertFalse(user.user_profile.username_initialized)
+ self.assertEqual(1, len(mail.outbox))
+ self.assertTrue(mail.outbox[0].subject.startswith("Please fix username"))
diff --git a/django_airavata/apps/auth/utils.py b/django_airavata/apps/auth/utils.py
index 6da77ea..d7e714c 100644
--- a/django_airavata/apps/auth/utils.py
+++ b/django_airavata/apps/auth/utils.py
@@ -184,6 +184,50 @@ def send_admin_alert_about_invalid_username(request, username, email, first_name
msg.send()
+def send_admin_alert_about_uninitialized_username(request, username, email, first_name, last_name):
+ domain, port = split_domain_port(request.get_host())
+ context = Context({
+ "username": username,
+ "email": email,
+ "first_name": first_name,
+ "last_name": last_name,
+ "portal_title": settings.PORTAL_TITLE,
+ "gateway_id": settings.GATEWAY_ID,
+ "http_host": domain,
+ })
+ subject = Template("Please fix username: a user of {{portal_title}} ({{http_host}}) has been assigned a random username ({{username}})").render(context)
+ body = Template("""
+ <p>
+ Dear Admin,
+ </p>
+
+ <p>
+ The following user has a random username because the system could not
+ determine a proper username:
+ </p>
+
+ <p>Username: {{username}}</p>
+ <p>Name: {{first_name}} {{last_name}}</p>
+ <p>Email: {{email}}</p>
+
+ <p>
+ This likely happened because there was no appropriate user attribute to use
+ for the user's username when the user logged in through an external identity
+ provider. Please update the username to the user's email address or some
+ other appropriate value in the <a href="https://{{http_host}}/admin/users/">Manage
+ Users</a> view in the portal.
+ </p>
+ """.strip()).render(context)
+ msg = EmailMessage(subject=subject,
+ body=body,
+ from_email=f'"{settings.PORTAL_TITLE}" <{settings.SERVER_EMAIL}>',
+ to=[f'"{a[0]}" <{a[1]}>' for a in getattr(settings,
+ 'PORTAL_ADMINS',
+ settings.ADMINS)])
+ msg.content_subtype = 'html'
+ msg.send()
+
+
def send_email_to_user(template_id, context):
email_template = models.EmailTemplate.objects.get(pk=template_id)
subject = Template(email_template.subject).render(context)