From 1e29a49d2891c788b38b359922f4405f1088b7d9 Mon Sep 17 00:00:00 2001 From: Dostkamp <4895210+Iso5786@users.noreply.github.com> Date: Wed, 13 Nov 2024 08:16:56 +0100 Subject: [PATCH] Revert "Feature/eppn email 956" --- apps/backpack/serializers_v1.py | 2 +- apps/badgeuser/models.py | 22 ++++---- apps/badgrsocialauth/providers/eduid/views.py | 29 +++++----- apps/directaward/api.py | 10 ++-- .../migrations/0018_auto_20241104_1117.py | 23 -------- apps/directaward/models.py | 53 +++++++++++-------- apps/directaward/permissions.py | 5 +- apps/directaward/schema.py | 5 +- apps/directaward/serializer.py | 13 ++--- apps/entity/utils.py | 10 ++-- apps/health/views.py | 2 + apps/issuer/helpers.py | 2 + apps/issuer/models.py | 11 ++++ apps/mainsite/pagination.py | 4 ++ apps/mainsite/renderers.py | 9 ++++ .../static/sample_direct_award_email_only.csv | 5 -- 16 files changed, 102 insertions(+), 103 deletions(-) delete mode 100644 apps/directaward/migrations/0018_auto_20241104_1117.py delete mode 100644 apps/mainsite/static/sample_direct_award_email_only.csv diff --git a/apps/backpack/serializers_v1.py b/apps/backpack/serializers_v1.py index ff6fed842..31365055a 100644 --- a/apps/backpack/serializers_v1.py +++ b/apps/backpack/serializers_v1.py @@ -290,7 +290,7 @@ class V1InstanceSerializer(serializers.Serializer): id = serializers.URLField(required=False) type = serializers.CharField() uid = BadgeStringField(required=False) - recipient = BadgeEmailField() + recipient = BadgeEmailField() # TODO: improve for richer types badge = V1BadgeClassSerializer() issuedOn = BadgeDateTimeField(required=False) # missing in some translated v0.5.0 expires = BadgeDateTimeField(required=False) diff --git a/apps/badgeuser/models.py b/apps/badgeuser/models.py index 4f71f1310..238b5009a 100644 --- a/apps/badgeuser/models.py +++ b/apps/badgeuser/models.py @@ -2,7 +2,7 @@ import datetime import re from itertools import chain -from django.db.models import Q + from allauth.account.models import EmailAddress, EmailConfirmation from django.conf import settings from django.contrib.auth.models import AbstractUser @@ -23,7 +23,7 @@ from badgeuser.utils import generate_badgr_username from cachemodel.decorators import cached_method from cachemodel.models import CacheModel -from directaward.models import DirectAward, DirectAwardBundle +from directaward.models import DirectAward from entity.models import BaseVersionedEntity from issuer.models import BadgeInstance from lti_edu.models import StudentsEnrolled @@ -618,10 +618,8 @@ def schac_homes(self): @property def direct_awards(self): - eppn_query = Q(eppn__in=self.eppns) - email_query = Q(recipient_email=self.email, bundle__identifier_type=DirectAwardBundle.IDENTIFIER_EMAIL) - unaccepted_direct_awards = DirectAward.objects.filter(eppn_query | email_query, status='Unaccepted') - return unaccepted_direct_awards + # TODO - add or query to use personal email address + return DirectAward.objects.filter(eppn__in=self.eppns, status='Unaccepted') def match_provisionments(self): """Used to match provisions on initial login""" @@ -634,9 +632,15 @@ def email_user(self, subject, html_message): """ Sends an email to this User. """ - # Allow sending, as this email is not blacklisted. - plain_text = strip_tags(html_message) - send_mail(subject, message=plain_text, html_message=html_message, recipient_list=[self.primary_email]) + try: + EmailBlacklist.objects.get(email=self.primary_email) + except EmailBlacklist.DoesNotExist: + # Allow sending, as this email is not blacklisted. + plain_text = strip_tags(html_message) + send_mail(subject, message=plain_text, html_message=html_message, recipient_list=[self.primary_email]) + else: + return + # TODO: Report email non-delivery somewhere. def publish(self): super(BadgeUser, self).publish() diff --git a/apps/badgrsocialauth/providers/eduid/views.py b/apps/badgrsocialauth/providers/eduid/views.py index e3808c167..dce745fce 100644 --- a/apps/badgrsocialauth/providers/eduid/views.py +++ b/apps/badgrsocialauth/providers/eduid/views.py @@ -216,18 +216,17 @@ def after_terms_agreement(request, **kwargs): eppn_json = response.json() request.user.clear_affiliations() for info in eppn_json: - if "eppn" in info and "schac_home_organization" in info: - request.user.add_affiliations( - [ - { - "eppn": info["eppn"].lower(), - "schac_home": info["schac_home_organization"], - } - ] - ) - logger.info( - f"Stored affiliations {info['eppn']} {info['schac_home_organization']}" - ) + request.user.add_affiliations( + [ + { + "eppn": info["eppn"].lower(), + "schac_home": info["schac_home_organization"], + } + ] + ) + logger.info( + f"Stored affiliations {info['eppn']} {info['schac_home_organization']}" + ) validated_names = [ info["validated_name"] for info in eppn_json if "validated_name" in info ] @@ -252,7 +251,7 @@ def after_terms_agreement(request, **kwargs): request.user.save() if not social_account: - # This fails if there is already a User account with that email, so we need to delete the old one + # This fails if there is already an User account with that email, so we need to delete the old one try: user = ( BadgeUser.objects.filter(is_teacher=False, email=payload["email"]) @@ -263,8 +262,8 @@ def after_terms_agreement(request, **kwargs): user.delete() except BadgeUser.DoesNotExist: pass - - # We don't create welcome badges anymore + # Create an eduIDBadge + create_edu_id_badge_instance(login) return ret diff --git a/apps/directaward/api.py b/apps/directaward/api.py index 96997117b..0ba2b5916 100644 --- a/apps/directaward/api.py +++ b/apps/directaward/api.py @@ -46,14 +46,13 @@ def convert_direct_award(direct_award): } def convert_badge_assertion(badge_instance): - user = badge_instance.user return { - "full_name": user.full_name, - "email": user.email, + "full_name": badge_instance.user.full_name, + "email": badge_instance.user.email, "public": badge_instance.public, "revoked": badge_instance.revoked, "entity_id": badge_instance.entity_id, - "eppn": user.eppns[0] if user.eppns else [] + "eppn": badge_instance.user.eppns[0] } results = { @@ -70,7 +69,8 @@ def convert_badge_assertion(badge_instance): convert_direct_award(da) for da in award_bundle.directaward_set.all() ], "badge_assertions": [ - convert_badge_assertion(ba) for ba in award_bundle.badgeinstance_set.all() + convert_badge_assertion(ba) + for ba in award_bundle.badgeinstance_set.all() ], } diff --git a/apps/directaward/migrations/0018_auto_20241104_1117.py b/apps/directaward/migrations/0018_auto_20241104_1117.py deleted file mode 100644 index 2d1bac80c..000000000 --- a/apps/directaward/migrations/0018_auto_20241104_1117.py +++ /dev/null @@ -1,23 +0,0 @@ -# Generated by Django 3.2.25 on 2024-11-04 10:17 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('directaward', '0017_directawardaudittrail'), - ] - - operations = [ - migrations.AddField( - model_name='directawardbundle', - name='identifier_type', - field=models.CharField(choices=[('eppn', 'eppn'), ('email', 'email')], default='eppn', max_length=254), - ), - migrations.AlterField( - model_name='directaward', - name='eppn', - field=models.CharField(blank=True, default=None, max_length=254, null=True), - ), - ] diff --git a/apps/directaward/models.py b/apps/directaward/models.py index ff879161d..f8706a1be 100644 --- a/apps/directaward/models.py +++ b/apps/directaward/models.py @@ -15,7 +15,7 @@ class DirectAward(BaseAuditedModel, BaseVersionedEntity, CacheModel): recipient_email = models.EmailField() - eppn = models.CharField(max_length=254, blank=True, null=True, default=None) + eppn = models.CharField(max_length=254) badgeclass = models.ForeignKey("issuer.BadgeClass", on_delete=models.CASCADE) bundle = models.ForeignKey( "directaward.DirectAwardBundle", null=True, on_delete=models.CASCADE @@ -56,12 +56,11 @@ class DirectAward(BaseAuditedModel, BaseVersionedEntity, CacheModel): def validate_unique(self, exclude=None): if ( - self.__class__.objects.filter( - eppn=self.eppn, badgeclass=self.badgeclass, status="Unaccepted", - bundle__identifier_type=DirectAwardBundle.IDENTIFIER_EPPN - ) - .exclude(pk=self.pk) - .exists() + self.__class__.objects.filter( + eppn=self.eppn, badgeclass=self.badgeclass, status="Unaccepted" + ) + .exclude(pk=self.pk) + .exists() ): raise IntegrityError( "DirectAward with this eppn and status Unaccepted already exists in the same badgeclass." @@ -85,12 +84,29 @@ def award(self, recipient): """Accept the direct award and make an assertion out of it""" from issuer.models import BadgeInstance - if self.eppn not in recipient.eppns and self.recipient_email != recipient.email and self.bundle.identifier_type != DirectAwardBundle.IDENTIFIER_EMAIL: - raise BadgrValidationError("Cannot award, eppn / email does not match", 999) - - if not recipient.validated_name: + if self.eppn not in recipient.eppns: + raise BadgrValidationError("Cannot award, eppn does not match", 999) + + institution = self.badgeclass.institution + identifiers = [ + inst.identifier for inst in self.badgeclass.award_allowed_institutions.all() + ] + [institution.identifier] + if institution.alternative_identifier: + identifiers.append(institution.alternative_identifier) + schac_homes = recipient.schac_homes + if self.badgeclass.formal: + allowed = ( + institution.identifier in schac_homes + or institution.alternative_identifier in schac_homes + ) + else: + allowed = ( + any(identifier in schac_homes for identifier in identifiers) + or recipient.validated_name + ) + if not allowed: raise BadgrValidationError( - "Cannot award, you do not have a validated name", + "Cannot award, you are not a member of the institution of the badgeclass", 999, ) evidence = None @@ -162,15 +178,6 @@ class DirectAwardBundle(BaseAuditedModel, BaseVersionedEntity, CacheModel): status = models.CharField( max_length=254, choices=STATUS_CHOICES, default=STATUS_ACTIVE ) - IDENTIFIER_EPPN = "eppn" - IDENTIFIER_EMAIL = "email" - IDENTIFIER_TYPES = ( - (IDENTIFIER_EPPN, "eppn"), - (IDENTIFIER_EMAIL, "email"), - ) - identifier_type = models.CharField( - max_length=254, choices=IDENTIFIER_TYPES, default=IDENTIFIER_EPPN - ) scheduled_at = models.DateTimeField(blank=True, null=True, default=None) @property @@ -205,8 +212,8 @@ def direct_award_revoked_count(self): direct_award_bundle=self, revoked=True ).count() return ( - revoked_count - + DirectAward.objects.filter(bundle=self, status="Revoked").count() + revoked_count + + DirectAward.objects.filter(bundle=self, status="Revoked").count() ) @property diff --git a/apps/directaward/permissions.py b/apps/directaward/permissions.py index b0dd04582..c778049df 100644 --- a/apps/directaward/permissions.py +++ b/apps/directaward/permissions.py @@ -4,7 +4,4 @@ class IsDirectAwardOwner(permissions.BasePermission): def has_object_permission(self, request, view, obj): - from directaward.models import DirectAwardBundle - user = request.user - return obj.eppn in user.eppns or ( - obj.recipient_email == user.email and obj.bundle.identifier_type == DirectAwardBundle.IDENTIFIER_EMAIL) + return obj.eppn in request.user.eppns diff --git a/apps/directaward/schema.py b/apps/directaward/schema.py index 89f93bed4..e2d202473 100644 --- a/apps/directaward/schema.py +++ b/apps/directaward/schema.py @@ -14,7 +14,7 @@ class Meta: class DirectAwardBundleType(DjangoObjectType): class Meta: model = DirectAwardBundle - fields = ('entity_id', 'badgeclass', 'created_at', 'updated_at', 'identifier_type', + fields = ('entity_id', 'badgeclass', 'created_at', 'updated_at', 'assertion_count', 'direct_award_count', 'direct_award_rejected_count', 'direct_award_scheduled_count', 'direct_award_revoked_count', 'initial_total') @@ -43,8 +43,7 @@ def resolve_direct_award(self, info, **kwargs): id = kwargs.get('id') if id is not None: da = DirectAward.objects.get(entity_id=id) - user = info.context.user - if da.eppn in user.eppns or (da.recipient_email == user.email and da.bundle.identifier_type == DirectAwardBundle.IDENTIFIER_EMAIL): + if da.eppn in info.context.user.eppns: return da def resolve_all_unclaimed_direct_awards(self, info, **kwargs): diff --git a/apps/directaward/serializer.py b/apps/directaward/serializer.py index b6638590d..300578b58 100644 --- a/apps/directaward/serializer.py +++ b/apps/directaward/serializer.py @@ -16,7 +16,7 @@ class Meta: model = DirectAward badgeclass = BadgeClassSlugRelatedField(slug_field="entity_id", required=False) - eppn = serializers.CharField(required=False, allow_blank=True, allow_null=True) + eppn = serializers.CharField(required=False) recipient_email = serializers.EmailField(required=False) status = serializers.CharField(required=False) evidence_url = serializers.URLField( @@ -33,9 +33,7 @@ class Meta: def validate_eppn(self, eppn): eppn_reg_exp_format = self.context['request'].user.institution.eppn_reg_exp_format - # For email identifier_type we don't validate eppn - eppn_required = self.root.initial_data["identifier_type"] == "eppn" - if eppn_reg_exp_format and eppn_required: + if eppn_reg_exp_format: eppn_re = re.compile(eppn_reg_exp_format, re.IGNORECASE) if not bool(eppn_re.match(eppn)): raise ValidationError(message="Incorrect eppn format", code="error") @@ -70,9 +68,6 @@ class Meta: status = serializers.CharField( write_only=True, default="Active", required=False, allow_null=True ) - identifier_type = serializers.CharField( - write_only=True, default="eppn", allow_null=False - ) scheduled_at = serializers.DateTimeField( write_only=True, required=False, allow_null=True ) @@ -101,10 +96,8 @@ def create(self, validated_data): direct_award_bundle = DirectAwardBundle.objects.create( initial_total=direct_awards.__len__(), **validated_data ) - eppn_required = validated_data.get("identifier_type", "eppn") == "eppn" for direct_award in direct_awards: - # Not required and already validated - direct_award["eppn"] = direct_award["eppn"].lower() if eppn_required else None + direct_award["eppn"] = direct_award["eppn"].lower() status = ( DirectAward.STATUS_SCHEDULED if scheduled_at diff --git a/apps/entity/utils.py b/apps/entity/utils.py index 748a502b7..45818f5ba 100644 --- a/apps/entity/utils.py +++ b/apps/entity/utils.py @@ -2,13 +2,13 @@ def get_form_error_code(error_type): - if error_type == 'null': + if error_type is 'null': return 901 - elif error_type == 'invalid': + elif error_type is 'invalid': return 902 - elif error_type == 'blank': + elif error_type is 'blank': return 903 - elif error_type == 'required': + elif error_type is 'required': return 904 elif isinstance(error_type, int): return error_type @@ -30,7 +30,7 @@ def validate_errors(serializer): 'error_code': get_form_error_code(vars(error)['code']), 'error_message': error }) - except TypeError: + except TypeError as e: # TODO: make this recursive for endless depth sub_fields = {} for sub_attr, sub_errors in error.items(): sub_fields[sub_attr] = [] diff --git a/apps/health/views.py b/apps/health/views.py index c47b136f9..78085b02a 100644 --- a/apps/health/views.py +++ b/apps/health/views.py @@ -5,6 +5,8 @@ """ from django.db import connection, DatabaseError from django.http import JsonResponse +# import logger # TODO integrate logging into results of health endpoint queries +# import requests # use for making requests to any dependency HTTP APIs. from rest_framework import status OK = 'OK' diff --git a/apps/issuer/helpers.py b/apps/issuer/helpers.py index 7ca4bf448..920b037d0 100644 --- a/apps/issuer/helpers.py +++ b/apps/issuer/helpers.py @@ -15,6 +15,7 @@ class DjangoCacheDict(MutableMapping): + """TODO: Fix this class, its broken!""" _keymap_cache_key = "DjangoCacheDict_keys" def __init__(self, namespace, id=None, timeout=None): @@ -123,6 +124,7 @@ def translate_errors(cls, badgecheck_messages): @classmethod def cache_instance(cls): if cls._cache_instance is None: + # TODO: note this class is broken and does not work correctly! cls._cache_instance = DjangoCacheRequestsCacheBackend(namespace='badgr_requests_cache') return cls._cache_instance diff --git a/apps/issuer/models.py b/apps/issuer/models.py index d3fdb8eb7..0e0156948 100644 --- a/apps/issuer/models.py +++ b/apps/issuer/models.py @@ -1085,6 +1085,17 @@ def save(self, *args, **kwargs): content=ContentFile(new_image.read()), save=False) + # TODO can this be permanently removed + # try: + # from badgeuser.models import CachedEmailAddress + # email_address = self.get_email_address() + # existing_email = CachedEmailAddress.cached.get_student_email(email_address) + # if email_address != existing_email.email and \ + # email_address not in [e.email for e in existing_email.cached_variants()]: + # existing_email.add_variant(email_address) + # except CachedEmailAddress.DoesNotExist: + # pass + if self.revoked is False: self.revocation_reason = None diff --git a/apps/mainsite/pagination.py b/apps/mainsite/pagination.py index f7b601845..f57ff4b26 100644 --- a/apps/mainsite/pagination.py +++ b/apps/mainsite/pagination.py @@ -124,6 +124,10 @@ def get_queryset(self) page_size = 100 # Model field to use for ordering. Must be unique, not null, and monotonically increasing. + # + # TODO: Add support for non-unique keys. This should be possible by adding support for ordering by multiple fields. + # For example, ordering = ('non_unique_field', 'pk') would fully disambiguate records. Would require compound index + # on those fields for efficient queries. ordering = 'pk' pagination_secret_key = getattr(settings, 'PAGINATION_SECRET_KEY', None) diff --git a/apps/mainsite/renderers.py b/apps/mainsite/renderers.py index 1b0aa51e3..3aee82c77 100644 --- a/apps/mainsite/renderers.py +++ b/apps/mainsite/renderers.py @@ -21,6 +21,15 @@ def render(self, data, media_type=None, renderer_context=None): if response is not None and response.exception: return None + + # TODO: Replace return None with commented lines below to get human-readable description in response + # + # Returning a non-None value in this function, as part of the exception handling code path in Django REST + # Framework, generates an HTTP response that Chrome rejects as invalid (ERR_INVALID_RESPONSE). Possible + # Django REST Framework bug? + # + # fieldnames = data.keys() + # rows = [data] else: fieldnames = data['fieldnames'] rows = data['rowdicts'] diff --git a/apps/mainsite/static/sample_direct_award_email_only.csv b/apps/mainsite/static/sample_direct_award_email_only.csv deleted file mode 100644 index 2f23313f2..000000000 --- a/apps/mainsite/static/sample_direct_award_email_only.csv +++ /dev/null @@ -1,5 +0,0 @@ -Recipient mailadres;Narrative (optional);Evidence URL (optional);Evidence NAME (optional);Evidence Description (optional);Grade (optional) -john@example.org;Lorum ipsum dolor set amet.;https://evicence.url/;Thesis of John;Description of the evidence;ECTS8 -andrew@example.org;;https://evicence.url/;;; -mary@shachome.org;;;;; -peter@shachome.org