From 1e115539c4bc8364e3bb7e8bf9730e7bcea7b6c0 Mon Sep 17 00:00:00 2001 From: Henrik Levkowetz Date: Mon, 1 Jul 2019 11:59:59 +0000 Subject: [PATCH] The previous nomcom.fields.EncryptedTextField relied on initial cleartext content having the same type as ciphertext. Under Python3, that's not the case (ciphertext has type bytes). Rewrote the nomcom app and tests to handle capture of comments and encryption to the Feedback.comments ciphertext differently. - Legacy-Id: 16350 --- ietf/nomcom/factories.py | 7 ++++- ietf/nomcom/forms.py | 21 ++++++++------ ietf/nomcom/models.py | 23 ++++++++++++--- ietf/nomcom/resources.py | 2 +- ietf/nomcom/tests.py | 60 +++++++++++++++++++--------------------- ietf/nomcom/utils.py | 12 +++++--- ietf/nomcom/views.py | 4 +-- 7 files changed, 77 insertions(+), 52 deletions(-) diff --git a/ietf/nomcom/factories.py b/ietf/nomcom/factories.py index 2509c1421..bffdd338b 100644 --- a/ietf/nomcom/factories.py +++ b/ietf/nomcom/factories.py @@ -1,3 +1,4 @@ +# Copyright The IETF Trust 2015-2019, All Rights Reserved import factory import random @@ -164,9 +165,13 @@ class FeedbackFactory(factory.DjangoModelFactory): nomcom = factory.SubFactory(NomComFactory) subject = factory.Faker('sentence') - comments = factory.Faker('paragraph') type_id = 'comment' + @factory.post_generation + def comments(obj, create, extracted, **kwargs): + comment_text = factory.Faker('paragraph').generate() + obj.comments = obj.nomcom.encrypt(comment_text) + class TopicFactory(factory.DjangoModelFactory): class Meta: model = Topic diff --git a/ietf/nomcom/forms.py b/ietf/nomcom/forms.py index c343c5d11..dd9c18686 100644 --- a/ietf/nomcom/forms.py +++ b/ietf/nomcom/forms.py @@ -293,7 +293,7 @@ class NominateForm(forms.ModelForm): # Complete nomination data feedback = Feedback.objects.create(nomcom=self.nomcom, - comments=qualifications, + comments=self.nomcom.encrypt(qualifications), type=FeedbackTypeName.objects.get(slug='nomina'), user=self.user) feedback.positions.add(position) @@ -408,7 +408,7 @@ class NominateNewPersonForm(forms.ModelForm): # Complete nomination data feedback = Feedback.objects.create(nomcom=self.nomcom, - comments=qualifications, + comments=self.nomcom.encrypt(qualifications), type=FeedbackTypeName.objects.get(slug='nomina'), user=self.user) feedback.positions.add(position) @@ -451,7 +451,7 @@ class NominateNewPersonForm(forms.ModelForm): class FeedbackForm(forms.ModelForm): nominator_email = forms.CharField(label='Commenter email',required=False) - comments = forms.CharField(label='Comments', widget=forms.Textarea(), strip=False) + comment_text = forms.CharField(label='Comments', widget=forms.Textarea(), strip=False) confirmation = forms.BooleanField(label='Email comments back to me as confirmation (if selected, your comments will be emailed to you in cleartext when you press Save).', required=False) @@ -484,13 +484,13 @@ class FeedbackForm(forms.ModelForm): if not NomineePosition.objects.accepted().filter(nominee=self.nominee, position=self.position): msg = "There isn't a accepted nomination for %s on the %s position" % (self.nominee, self.position) - self._errors["comments"] = self.error_class([msg]) + self._errors["comment_text"] = self.error_class([msg]) return self.cleaned_data def save(self, commit=True): feedback = super(FeedbackForm, self).save(commit=False) confirmation = self.cleaned_data['confirmation'] - comments = self.cleaned_data['comments'] + comment_text = self.cleaned_data['comment_text'] nomcom_template_path = '/nomcom/%s/' % self.nomcom.group.acronym author = None @@ -508,6 +508,7 @@ class FeedbackForm(forms.ModelForm): feedback.nomcom = self.nomcom feedback.user = self.user feedback.type = FeedbackTypeName.objects.get(slug='comment') + feedback.comments = self.nomcom.encrypt(comment_text) feedback.save() if self.nominee and self.position: feedback.positions.add(self.position) @@ -526,7 +527,7 @@ class FeedbackForm(forms.ModelForm): elif self.topic: about = self.topic.subject context = {'about': about, - 'comments': comments, + 'comments': comment_text, 'year': self.nomcom.year(), } path = nomcom_template_path + FEEDBACK_RECEIPT_TEMPLATE @@ -537,7 +538,6 @@ class FeedbackForm(forms.ModelForm): model = Feedback fields = ( 'nominator_email', - 'comments', 'confirmation', ) @@ -554,8 +554,9 @@ class FeedbackEmailForm(forms.Form): class QuestionnaireForm(forms.ModelForm): - comments = forms.CharField(label='Questionnaire response from this candidate', + comment_text = forms.CharField(label='Questionnaire response from this candidate', widget=forms.Textarea(), strip=False) + def __init__(self, *args, **kwargs): self.nomcom = kwargs.pop('nomcom', None) self.user = kwargs.pop('user', None) @@ -565,6 +566,7 @@ class QuestionnaireForm(forms.ModelForm): def save(self, commit=True): feedback = super(QuestionnaireForm, self).save(commit=False) + comment_text = self.cleaned_data['comment_text'] (position, nominee) = self.cleaned_data['nominee'] author = get_user_email(self.user) @@ -575,6 +577,7 @@ class QuestionnaireForm(forms.ModelForm): feedback.nomcom = self.nomcom feedback.user = self.user feedback.type = FeedbackTypeName.objects.get(slug='questio') + feedback.comments = self.nomcom.encrypt(comment_text) feedback.save() self.save_m2m() feedback.nominees.add(nominee) @@ -582,7 +585,7 @@ class QuestionnaireForm(forms.ModelForm): class Meta: model = Feedback - fields = ( 'comments', ) + fields = [] class NomComTemplateForm(DBTemplateForm): content = forms.CharField(label="Text", widget=forms.Textarea(attrs={'cols': '120', 'rows':'40', }), strip=False) diff --git a/ietf/nomcom/models.py b/ietf/nomcom/models.py index 92c1d1ed7..16d229ed7 100644 --- a/ietf/nomcom/models.py +++ b/ietf/nomcom/models.py @@ -11,20 +11,20 @@ from django.template.defaultfilters import linebreaks import debug # pyflakes:ignore -from ietf.nomcom.fields import EncryptedTextField from ietf.person.models import Person,Email from ietf.group.models import Group from ietf.name.models import NomineePositionStateName, FeedbackTypeName, TopicAudienceName from ietf.dbtemplate.models import DBTemplate -from ietf.nomcom.managers import NomineePositionManager, NomineeManager, \ - PositionManager, FeedbackManager +from ietf.nomcom.managers import (NomineePositionManager, NomineeManager, + PositionManager, FeedbackManager, ) from ietf.nomcom.utils import (initialize_templates_for_group, initialize_questionnaire_for_position, initialize_requirements_for_position, initialize_description_for_topic, delete_nomcom_templates) from ietf.utils.models import ForeignKey +from ietf.utils.pipe import pipe from ietf.utils.storage import NoLocationMigrationFileSystemStorage @@ -79,6 +79,21 @@ class NomCom(models.Model): def pending_email_count(self): return self.feedback_set.filter(type__isnull=True).count() + def encrypt(self, cleartext:str) -> bytes: + try: + cert_file = self.public_key.path + except ValueError as e: + raise ValueError("Trying to read the NomCom public key: " + str(e)) + + command = "%s smime -encrypt -in /dev/stdin %s" % (settings.OPENSSL_COMMAND, cert_file) + code, out, error = pipe(command, cleartext.encode()) + if code != 0: + log("openssl error: %s:\n Error %s: %s" %(command, code, error)) + if not error: + return out + else: + raise EncryptedException(error) + def delete_nomcom(sender, **kwargs): nomcom = kwargs.get('instance', None) @@ -250,7 +265,7 @@ class Feedback(models.Model): nominees = models.ManyToManyField('Nominee', blank=True) topics = models.ManyToManyField('Topic', blank=True) subject = models.TextField(verbose_name='Subject', blank=True) - comments = EncryptedTextField(verbose_name='Comments') + comments = models.BinaryField(verbose_name='Comments') type = ForeignKey(FeedbackTypeName, blank=True, null=True) user = ForeignKey(User, editable=False, blank=True, null=True, on_delete=models.SET_NULL) time = models.DateTimeField(auto_now_add=True) diff --git a/ietf/nomcom/resources.py b/ietf/nomcom/resources.py index 311485fb0..cc95d5ff7 100644 --- a/ietf/nomcom/resources.py +++ b/ietf/nomcom/resources.py @@ -129,7 +129,7 @@ class FeedbackResource(ModelResource): "id": ALL, "author": ALL, "subject": ALL, - "comments": ALL, + # "comments": ALL, "time": ALL, "nomcom": ALL_WITH_RELATIONS, "type": ALL_WITH_RELATIONS, diff --git a/ietf/nomcom/tests.py b/ietf/nomcom/tests.py index 6953026cc..262ae006d 100644 --- a/ietf/nomcom/tests.py +++ b/ietf/nomcom/tests.py @@ -422,18 +422,18 @@ class NomcomViewsTest(TestCase): nominee = Nominee.objects.get(email__person__user__username=COMMUNITY_USER) position = Position.objects.get(name='OAM') - comments = 'Plain text. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' + comment_text = 'Plain text. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' nomcom = get_nomcom_by_year(self.year) feedback = Feedback.objects.create(nomcom=nomcom, - comments=comments, + comments=nomcom.encrypt(comment_text), type=FeedbackTypeName.objects.get(slug='nomina')) feedback.positions.add(position) feedback.nominees.add(nominee) # to check feedback comments are saved like enrypted data - self.assertNotEqual(feedback.comments, comments) + self.assertNotEqual(feedback.comments, comment_text) - self.assertEqual(check_comments(feedback.comments, comments, self.privatekey_file), True) + self.assertEqual(check_comments(feedback.comments, comment_text, self.privatekey_file), True) # Check that the set reminder date is present reminder_dates = dict([ (d.id,str(d.date)) for d in nomcom.reminderdates_set.all() ]) @@ -629,13 +629,13 @@ class NomcomViewsTest(TestCase): self.assertEqual(len(q("#nominate-form")), 1) position = Position.objects.get(name=position_name) - comments = 'Test nominate view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' + comment_text = 'Test nominate view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' candidate_phone = '123456' test_data = {'searched_email': searched_email.pk, 'candidate_phone': candidate_phone, 'position': position.id, - 'qualifications': comments, + 'qualifications': comment_text, 'confirmation': confirmation} if not public: test_data['nominator_email'] = nominator_email @@ -655,9 +655,9 @@ class NomcomViewsTest(TestCase): self.assertEqual(feedback.author, nominator_email) # to check feedback comments are saved like enrypted data - self.assertNotEqual(feedback.comments, comments) + self.assertNotEqual(feedback.comments, comment_text) - self.assertEqual(check_comments(feedback.comments, comments, self.privatekey_file), True) + self.assertEqual(check_comments(feedback.comments, comment_text, self.privatekey_file), True) Nomination.objects.get(position=position, candidate_name=nominee.person.plain_name(), candidate_email=searched_email.address, @@ -697,14 +697,14 @@ class NomcomViewsTest(TestCase): position = Position.objects.get(name=position_name) candidate_email = nominee_email candidate_name = 'nominee' - comments = 'Test nominate view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' + comment_text = 'Test nominate view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' candidate_phone = '123456' test_data = {'candidate_name': candidate_name, 'candidate_email': candidate_email, 'candidate_phone': candidate_phone, 'position': position.id, - 'qualifications': comments, + 'qualifications': comment_text, 'confirmation': confirmation} if not public: test_data['nominator_email'] = nominator_email @@ -727,9 +727,9 @@ class NomcomViewsTest(TestCase): self.assertEqual(feedback.author, nominator_email) # to check feedback comments are saved like enrypted data - self.assertNotEqual(feedback.comments, comments) + self.assertNotEqual(feedback.comments, comment_text) - self.assertEqual(check_comments(feedback.comments, comments, self.privatekey_file), True) + self.assertEqual(check_comments(feedback.comments, comment_text, self.privatekey_file), True) Nomination.objects.get(position=position, candidate_name=candidate_name, candidate_email=candidate_email, @@ -772,14 +772,13 @@ class NomcomViewsTest(TestCase): position = Position.objects.get(name=position_name) nominee = Nominee.objects.get(email__address=nominee_email) - comments = 'Test add questionnaire view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' + comment_text = 'Test add questionnaire view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' - test_data = {'comments': comments, + test_data = {'comment_text': comment_text, 'nominee': '%s_%s' % (position.id, nominee.id)} response = self.client.post(self.add_questionnaire_url, test_data) - self.assertEqual(response.status_code, 200) self.assertContains(response, "alert-success") ## check objects @@ -788,9 +787,9 @@ class NomcomViewsTest(TestCase): type=FeedbackTypeName.objects.get(slug='questio')).latest('id') ## to check feedback comments are saved like enrypted data - self.assertNotEqual(feedback.comments, comments) + self.assertNotEqual(feedback.comments, comment_text) - self.assertEqual(check_comments(feedback.comments, comments, self.privatekey_file), True) + self.assertEqual(check_comments(feedback.comments, comment_text, self.privatekey_file), True) def test_public_feedback(self): login_testing_unauthorized(self, COMMUNITY_USER, self.public_feedback_url) @@ -859,7 +858,7 @@ class NomcomViewsTest(TestCase): comments = 'Test feedback view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' - test_data = {'comments': comments, + test_data = {'comment_text': comments, 'position_name': position.name, 'nominee_name': nominee.email.person.name, 'nominee_email': nominee.email.address, @@ -882,7 +881,6 @@ class NomcomViewsTest(TestCase): nominee_position.save() response = self.client.post(feedback_url, test_data) - self.assertEqual(response.status_code, 200) self.assertContains(response, "alert-success") self.assertNotContains(response, "feedbackform") @@ -961,7 +959,8 @@ class FeedbackTest(TestCase): #nomcom.public_key.storage.location = tempfile.gettempdir() nomcom.public_key.save('cert', File(open(self.cert_file.name, 'r'))) - comments = 'Plain text. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' + comment_text = 'Plain text. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' + comments = nomcom.encrypt(comment_text) feedback = Feedback.objects.create(nomcom=nomcom, comments=comments, type=FeedbackTypeName.objects.get(slug='nomina')) @@ -969,9 +968,8 @@ class FeedbackTest(TestCase): feedback.nominees.add(nominee) # to check feedback comments are saved like enrypted data - self.assertNotEqual(feedback.comments, comments) - - self.assertEqual(check_comments(feedback.comments, comments, self.privatekey_file), True) + self.assertNotEqual(feedback.comments, comment_text) + self.assertEqual(check_comments(feedback.comments, comment_text, self.privatekey_file), True) class ReminderTest(TestCase): @@ -1011,7 +1009,7 @@ class ReminderTest(TestCase): np.time = t_minus_4 np.save() feedback = Feedback.objects.create(nomcom=self.nomcom, - comments='some non-empty comments', + comments=self.nomcom.encrypt('some non-empty comments'), type=FeedbackTypeName.objects.get(slug='questio'), user=User.objects.get(username=CHAIR_USER)) feedback.positions.add(gen) @@ -1104,7 +1102,7 @@ class InactiveNomcomTests(TestCase): empty_outbox() fb_before = self.nc.feedback_set.count() - test_data = {'comments': 'Test feedback view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.', + test_data = {'comment_text': 'Test feedback view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.', 'nominator_email': self.plain_person.email_set.first().address, 'confirmation': True} response = self.client.post(url, test_data) @@ -1764,7 +1762,7 @@ Junk body for testing 'duplicate_persons':[nominee2.person.pk]}) self.assertEqual(response.status_code, 302) self.assertEqual(len(outbox),1) - self.assertTrue(all([str(x.person.pk) in outbox[0].get_payload(decode=True) for x in [nominee1,nominee2]])) + self.assertTrue(all([str(x.person.pk) in outbox[0].get_payload() for x in [nominee1,nominee2]])) def test_extract_email(self): url = reverse('ietf.nomcom.views.extract_email_lists',kwargs={'year':self.nc.year()}) @@ -1905,7 +1903,7 @@ class AcceptingTests(TestCase): response = self.client.get(posurl) self.assertIn('not currently accepting feedback', unicontent(response)) - test_data = {'comments': 'junk', + test_data = {'comment_text': 'junk', 'position_name': pos.name, 'nominee_name': pos.nominee_set.first().email.person.name, 'nominee_email': pos.nominee_set.first().email.address, @@ -1920,7 +1918,7 @@ class AcceptingTests(TestCase): response = self.client.get(topicurl) self.assertIn('not currently accepting feedback', unicontent(response)) - test_data = {'comments': 'junk', + test_data = {'comment_text': 'junk', 'confirmation': False, } response = self.client.post(topicurl, test_data) @@ -2046,7 +2044,7 @@ class TopicTests(TestCase): url = reverse('ietf.nomcom.views.public_feedback',kwargs={'year':self.nc.year() }) url += '?topic=%d'%topic.pk login_testing_unauthorized(self, self.plain_person.user.username, url) - response=self.client.post(url, {'comments':'junk', 'confirmation':False}) + response=self.client.post(url, {'comment_text':'junk', 'confirmation':False}) self.assertEqual(response.status_code, 200) self.assertContains(response, "alert-success") self.assertNotContains(response, "feedbackform") @@ -2062,7 +2060,7 @@ class TopicTests(TestCase): topic_url = feedback_url + '?topic=%d'%topic.pk r = self.client.get(topic_url) self.assertEqual(r.status_code,404) - r = self.client.post(topic_url, {'comments':'junk', 'confirmation':False}) + r = self.client.post(topic_url, {'comment_text':'junk', 'confirmation':False}) self.assertEqual(r.status_code,404) self.client.logout() @@ -2075,7 +2073,7 @@ class TopicTests(TestCase): self.assertContains(r, topic.subject) r = self.client.get(topic_url) self.assertEqual(r.status_code,200) - r = self.client.post(topic_url, {'comments':'junk', 'confirmation':False}) + r = self.client.post(topic_url, {'comment_text':'junk', 'confirmation':False}) self.assertEqual(r.status_code,200) self.assertEqual(topic.feedback_set.count(),1) self.client.logout() diff --git a/ietf/nomcom/utils.py b/ietf/nomcom/utils.py index 2146c262a..736acb5c5 100644 --- a/ietf/nomcom/utils.py +++ b/ietf/nomcom/utils.py @@ -61,7 +61,7 @@ def get_nomcom_by_year(year): def get_year_by_nomcom(nomcom): acronym = nomcom.group.acronym - m = re.search('(?P\d\d\d\d)', acronym) + m = re.search(r'(?P\d\d\d\d)', acronym) return m.group(0) @@ -172,7 +172,7 @@ def store_nomcom_private_key(request, year, private_key): else: command = "%s bf -e -in /dev/stdin -k \"%s\" -a" code, out, error = pipe(command % (settings.OPENSSL_COMMAND, - settings.SECRET_KEY), private_key) + settings.SECRET_KEY), private_key.encode()) if code != 0: log("openssl error: %s:\n Error %s: %s" %(command, code, error)) if error: @@ -182,7 +182,7 @@ def store_nomcom_private_key(request, year, private_key): def validate_private_key(key): key_file = tempfile.NamedTemporaryFile(delete=False) - key_file.write(key) + key_file.write(key.encode()) key_file.close() command = "%s rsa -in %s -check -noout" @@ -460,6 +460,10 @@ def create_feedback_email(nomcom, msg): feedback = Feedback(nomcom=nomcom, author=by, subject=subject or '', - comments=body) + comments=nomcom.encrypt(body)) feedback.save() return feedback + +class EncryptedException(Exception): + pass + \ No newline at end of file diff --git a/ietf/nomcom/views.py b/ietf/nomcom/views.py index 2da08cf68..4afaff6f3 100644 --- a/ietf/nomcom/views.py +++ b/ietf/nomcom/views.py @@ -640,7 +640,7 @@ def private_questionnaire(request, year): if form.is_valid(): form.save() messages.success(request, 'The questionnaire response has been registered.') - questionnaire_response = form.cleaned_data['comments'] + questionnaire_response = form.cleaned_data['comment_text'] form = QuestionnaireForm(nomcom=nomcom, user=request.user) else: form = QuestionnaireForm(nomcom=nomcom, user=request.user) @@ -688,7 +688,7 @@ def process_nomination_status(request, year, nominee_position_id, state, date, h f = Feedback.objects.create(nomcom = nomcom, author = nominee_position.nominee.email, subject = '%s nomination %s'%(nominee_position.nominee.name(),state), - comments = form.cleaned_data['comments'], + comments = nomcom.encrypt(form.cleaned_data['comments']), type_id = 'comment', user = who, )