From 477ff51d231a730b5640035e18f21fae05dd9dfd Mon Sep 17 00:00:00 2001 From: Yusaku Kitabatake Date: Wed, 22 Jul 2026 14:14:33 +0900 Subject: [PATCH] Fix redmine-61794 --- admin/loa/forms.py | 14 ++- admin/translations/django.pot | 6 + admin/translations/en/LC_MESSAGES/django.po | 6 + admin/translations/ja/LC_MESSAGES/django.po | 6 + admin_tests/loa/test_forms.py | 116 +++++++++++++----- api/institutions/authentication.py | 10 +- .../views/test_institution_auth_loa.py | 63 ++++++++-- scripts/populate_institutions.py | 1 + website/settings/defaults.py | 1 + 9 files changed, 166 insertions(+), 57 deletions(-) diff --git a/admin/loa/forms.py b/admin/loa/forms.py index 36d8c404c24..3ed1161c179 100644 --- a/admin/loa/forms.py +++ b/admin/loa/forms.py @@ -4,20 +4,22 @@ class LoAForm(forms.ModelForm): - CHOICES_AAL = [(0, _('NULL')), (1, _('AAL1')), (2, _('AAL2'))] CHOICES_IAL = [(0, _('NULL')), (1, _('IAL1')), (2, _('IAL2'))] + CHOICES_AAL = [(0, _('NULL')), (1, _('AAL1')), (2, _('AAL2'))] CHOICES_MFA = ( (False, _('Hide')), (True, _('Show')), ) - aal = forms.ChoiceField( - choices=CHOICES_AAL, - required=False, - ) ial = forms.ChoiceField( + label=_('Required IAL level'), choices=CHOICES_IAL, required=False, ) + aal = forms.ChoiceField( + label=_('Required AAL level'), + choices=CHOICES_AAL, + required=False, + ) is_mfa = forms.ChoiceField( label=_('Display MFA link button'), choices=CHOICES_MFA, @@ -33,7 +35,7 @@ def __init__(self, *args, **kwargs): class Meta: model = LoA fields = ( - 'aal', 'ial', + 'aal', 'is_mfa', ) diff --git a/admin/translations/django.pot b/admin/translations/django.pot index bf8d8dec6ad..a0b7cba9831 100644 --- a/admin/translations/django.pot +++ b/admin/translations/django.pot @@ -3960,5 +3960,11 @@ msgstr "" msgid "Level of Assurance" msgstr "" +msgid "Required IAL level" +msgstr "" + +msgid "Required AAL level" +msgstr "" + msgid "Display MFA link button" msgstr "" diff --git a/admin/translations/en/LC_MESSAGES/django.po b/admin/translations/en/LC_MESSAGES/django.po index 21c548340ee..4744e4a5e11 100644 --- a/admin/translations/en/LC_MESSAGES/django.po +++ b/admin/translations/en/LC_MESSAGES/django.po @@ -4012,5 +4012,11 @@ msgstr "" msgid "Level of Assurance" msgstr "" +msgid "Required IAL level" +msgstr "" + +msgid "Required AAL level" +msgstr "" + msgid "Display MFA link button" msgstr "" diff --git a/admin/translations/ja/LC_MESSAGES/django.po b/admin/translations/ja/LC_MESSAGES/django.po index 4e10fa749a8..1b89f4e078f 100644 --- a/admin/translations/ja/LC_MESSAGES/django.po +++ b/admin/translations/ja/LC_MESSAGES/django.po @@ -4404,5 +4404,11 @@ msgstr "表示しない" msgid "Level of Assurance" msgstr "要求する保証レベルの設定" +msgid "Required IAL level" +msgstr "要求するIALのレベル" + +msgid "Required AAL level" +msgstr "要求するAALのレベル" + msgid "Display MFA link button" msgstr "多要素認証実行ボタンの表示" diff --git a/admin_tests/loa/test_forms.py b/admin_tests/loa/test_forms.py index ac73e8cdc47..e0db5a6449e 100644 --- a/admin_tests/loa/test_forms.py +++ b/admin_tests/loa/test_forms.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- """Tests for admin.loa.forms.LoAForm.""" import pytest +from django.utils.translation import ugettext_lazy as _ from admin.loa.forms import LoAForm from osf.models.loa import LoA @@ -12,39 +13,58 @@ class TestLoAForm: """Tests for LoAForm.""" + # --------------------------------------------------------------- + # Fields and declaration order + # --------------------------------------------------------------- + def test_form_fields(self): form = LoAForm() - assert 'aal' in form.fields assert 'ial' in form.fields + assert 'aal' in form.fields assert 'is_mfa' in form.fields - def test_form_valid_with_all_fields(self): - form = LoAForm(data={'aal': '2', 'ial': '2', 'is_mfa': 'True'}) - assert form.is_valid(), form.errors + def test_form_field_order(self): + """Fields are declared in IAL -> AAL -> MFA order and rendered as such.""" + form = LoAForm() + assert list(form.fields.keys()) == ['ial', 'aal', 'is_mfa'] - def test_form_valid_with_zero_values(self): - """aal=0 and ial=0 represent the NULL choice.""" - form = LoAForm(data={'aal': '0', 'ial': '0', 'is_mfa': 'False'}) - assert form.is_valid(), form.errors + def test_form_meta_model(self): + assert LoAForm.Meta.model is LoA - def test_form_valid_with_empty_fields(self): - """All fields are not required, so empty strings should be accepted.""" - form = LoAForm(data={'aal': '', 'ial': '', 'is_mfa': ''}) - assert form.is_valid(), form.errors + def test_form_meta_fields(self): + assert LoAForm.Meta.fields == ('ial', 'aal', 'is_mfa') - def test_form_aal_choices(self): + # --------------------------------------------------------------- + # Labels + # --------------------------------------------------------------- + + def test_form_ial_label(self): + """ial has an explicit label instead of the auto-generated 'Ial'.""" form = LoAForm() - aal_values = [c[0] for c in form.fields['aal'].choices] - assert 0 in aal_values - assert 1 in aal_values - assert 2 in aal_values + assert str(form.fields['ial'].label) == str(_('Required IAL level')) + + def test_form_aal_label(self): + """aal has an explicit label instead of the auto-generated 'Aal'.""" + form = LoAForm() + assert str(form.fields['aal'].label) == str(_('Required AAL level')) + + def test_form_is_mfa_label(self): + form = LoAForm() + assert str(form.fields['is_mfa'].label) == str(_('Display MFA link button')) + + # --------------------------------------------------------------- + # Choices + # --------------------------------------------------------------- def test_form_ial_choices(self): form = LoAForm() ial_values = [c[0] for c in form.fields['ial'].choices] - assert 0 in ial_values - assert 1 in ial_values - assert 2 in ial_values + assert ial_values == [0, 1, 2] + + def test_form_aal_choices(self): + form = LoAForm() + aal_values = [c[0] for c in form.fields['aal'].choices] + assert aal_values == [0, 1, 2] def test_form_is_mfa_choices(self): form = LoAForm() @@ -52,6 +72,47 @@ def test_form_is_mfa_choices(self): assert False in mfa_values assert True in mfa_values + def test_form_is_mfa_initial_is_false(self): + form = LoAForm() + assert form.fields['is_mfa'].initial is False + + def test_form_fields_are_not_required(self): + form = LoAForm() + for name, field in form.fields.items(): + assert field.required is False, '{} should not be required'.format(name) + + # --------------------------------------------------------------- + # Validation + # --------------------------------------------------------------- + + def test_form_valid_with_all_fields(self): + form = LoAForm(data={'ial': '2', 'aal': '2', 'is_mfa': 'True'}) + assert form.is_valid(), form.errors + + def test_form_valid_with_zero_values(self): + """ial=0 and aal=0 represent the NULL choice.""" + form = LoAForm(data={'ial': '0', 'aal': '0', 'is_mfa': 'False'}) + assert form.is_valid(), form.errors + + def test_form_valid_with_empty_fields(self): + """All fields are not required, so empty strings should be accepted.""" + form = LoAForm(data={'ial': '', 'aal': '', 'is_mfa': ''}) + assert form.is_valid(), form.errors + + def test_form_invalid_ial_choice(self): + form = LoAForm(data={'ial': '99', 'aal': '1', 'is_mfa': 'False'}) + assert not form.is_valid() + assert 'ial' in form.errors + + def test_form_invalid_aal_choice(self): + form = LoAForm(data={'ial': '1', 'aal': '99', 'is_mfa': 'False'}) + assert not form.is_valid() + assert 'aal' in form.errors + + # --------------------------------------------------------------- + # Widget / instance + # --------------------------------------------------------------- + def test_form_widget_css_class(self): """All field widgets should have 'form-control form-control-sm' CSS class.""" form = LoAForm() @@ -63,20 +124,9 @@ def test_form_with_instance(self): institution = InstitutionFactory() modifier = AuthUserFactory() loa = LoA.objects.create( - institution=institution, aal=2, ial=1, is_mfa=True, modifier=modifier, + institution=institution, ial=1, aal=2, is_mfa=True, modifier=modifier, ) form = LoAForm(instance=loa) - assert form.initial['aal'] == 2 assert form.initial['ial'] == 1 + assert form.initial['aal'] == 2 assert form.initial['is_mfa'] is True - - def test_form_meta_model(self): - assert LoAForm.Meta.model is LoA - - def test_form_meta_fields(self): - assert LoAForm.Meta.fields == ('aal', 'ial', 'is_mfa') - - def test_form_invalid_aal_choice(self): - form = LoAForm(data={'aal': '99', 'ial': '1', 'is_mfa': 'False'}) - assert not form.is_valid() - assert 'aal' in form.errors diff --git a/api/institutions/authentication.py b/api/institutions/authentication.py index 6106901baa0..c681bebec9e 100644 --- a/api/institutions/authentication.py +++ b/api/institutions/authentication.py @@ -37,6 +37,7 @@ OSF_IAL2_STR, OSF_AAL1_STR, OSF_AAL2_STR, + OSF_IAL1_VAR, OSF_IAL2_VAR, OSF_AAL1_VAR, OSF_AAL2_VAR, @@ -232,6 +233,8 @@ def get_next(obj, *args): eduPersonAssurance = p_user.get('eduPersonAssurance') if re.search(OSF_IAL2_STR, str(eduPersonAssurance)): ial = OSF_IAL2_VAR + else: + ial = OSF_IAL1_VAR if re.search(OSF_AAL2_STR, str(eduPersonAssurance)): aal = OSF_AAL2_VAR elif re.search(OSF_AAL1_STR, str(eduPersonAssurance)): @@ -285,13 +288,6 @@ def get_next(obj, *args): ' institution.' ) loa_flag = False - elif loa.ial == 1: - if not ial: - message = ( - 'Institution login failed: Does not meet the required IAL.
Please check the IAL of your' - ' institution.' - ) - loa_flag = False if not loa_flag: message = 'Institution login failed: Does not meet the required AAL and IAL.' sentry.log_message(message) diff --git a/api_tests/institutions/views/test_institution_auth_loa.py b/api_tests/institutions/views/test_institution_auth_loa.py index f1cfea4310a..56829338767 100644 --- a/api_tests/institutions/views/test_institution_auth_loa.py +++ b/api_tests/institutions/views/test_institution_auth_loa.py @@ -3,7 +3,9 @@ Covers: - IAL / AAL extraction from eduPersonAssurance - - LoA validation logic (AAL2 required → MFA redirect, AAL1 required → ValidationError, IAL checks) + - IAL1 is the baseline: ial falls back to OSF_IAL1_VAR when IAL2 is absent + - LoA validation logic (AAL2 required → MFA redirect, AAL1 required → ValidationError, + IAL2 required → ValidationError, IAL1 required → always satisfied) - MFA URL construction with urlencode() - user.context containing mfa_url in response - ValidationError raised when LoA requirements are not met @@ -25,13 +27,10 @@ from website.settings import ( OSF_AAL2_VAR, OSF_AAL1_VAR, + OSF_IAL1_VAR, OSF_IAL2_VAR, ) -def make_user(username, fullname): - return UserFactory(username=username, fullname=fullname) - - def make_payload( institution, username, @@ -175,6 +174,37 @@ def test_ial2_extracted_from_edu_person_assurance( user = OSFUser.objects.get(username=username) assert user.ial == OSF_IAL2_VAR + def test_ial1_is_assigned_when_edu_person_assurance_is_empty( + self, app, institution, url_auth_institution, + ): + """IAL1 is the baseline: ial falls back to OSF_IAL1_VAR when the IdP + sends no eduPersonAssurance at all. + """ + username = 'user_ial1_default@inst.edu' + res = app.post( + url_auth_institution, + make_payload(institution, username), + ) + assert res.status_code == 200 + user = OSFUser.objects.get(username=username) + assert user.ial == OSF_IAL1_VAR + + def test_ial1_is_assigned_when_edu_person_assurance_has_no_ial2( + self, app, institution, url_auth_institution, + ): + """eduPersonAssurance carrying only AAL values still yields IAL1.""" + username = 'user_ial1_from_aal_only@inst.edu' + res = app.post( + url_auth_institution, + make_payload( + institution, username, + edu_person_assurance='https://www.gakunin.jp/profile/AAL2', + ), + ) + assert res.status_code == 200 + user = OSFUser.objects.get(username=username) + assert user.ial == OSF_IAL1_VAR + def test_aal_falls_back_to_shib_authn_context_class( self, app, institution, url_auth_institution, ): @@ -419,7 +449,10 @@ def test_ial2_required_user_has_ial2_passes( def test_ial2_required_user_has_no_ial_raises_error( self, app, institution, url_auth_institution, ): - """IAL2 required but IAL2 not provided → ValidationError (400).""" + """IAL2 required but not provided → ValidationError (400). + + The IAL1 fallback must NOT satisfy an IAL2 requirement. + """ modifier = UserFactory() LoA.objects.create( institution=institution, aal=0, ial=2, modifier=modifier, @@ -434,9 +467,13 @@ def test_ial2_required_user_has_no_ial_raises_error( # --------------------------------------------------------------- # LoA validation — IAL1 required + # + # IAL1 is the baseline assurance level: ial is always populated with + # either OSF_IAL2_VAR or OSF_IAL1_VAR, so an IAL1 requirement is + # satisfied by every authenticated user. # --------------------------------------------------------------- - def test_ial1_required_user_has_ial_passes( + def test_ial1_required_user_has_ial2_passes( self, app, institution, url_auth_institution, ): modifier = UserFactory() @@ -453,20 +490,24 @@ def test_ial1_required_user_has_ial_passes( ) assert res.status_code == 200 - def test_ial1_required_user_has_no_ial_raises_error( + def test_ial1_required_user_without_ial2_passes( self, app, institution, url_auth_institution, ): + """A user with no IAL attribute still meets an IAL1 requirement, + because ial falls back to OSF_IAL1_VAR. + """ modifier = UserFactory() LoA.objects.create( institution=institution, aal=0, ial=1, modifier=modifier, ) - username = 'user_ial1_fail@inst.edu' + username = 'user_ial1_baseline@inst.edu' res = app.post( url_auth_institution, make_payload(institution, username), - expect_errors=True, ) - assert res.status_code == 400 + assert res.status_code == 200 + user = OSFUser.objects.get(username=username) + assert user.ial == OSF_IAL1_VAR # --------------------------------------------------------------- # Combined AAL + IAL requirements diff --git a/scripts/populate_institutions.py b/scripts/populate_institutions.py index c9fb3df8792..bb235308331 100644 --- a/scripts/populate_institutions.py +++ b/scripts/populate_institutions.py @@ -1874,6 +1874,7 @@ def main(default_args=False): 'is_mfa':True, }, ], + 'test': [], } if __name__ == '__main__': diff --git a/website/settings/defaults.py b/website/settings/defaults.py index 5c58c4951c6..644f84ba689 100644 --- a/website/settings/defaults.py +++ b/website/settings/defaults.py @@ -2104,6 +2104,7 @@ class CeleryConfig: OSF_IAL2_STR = 'https://www\.gakunin\.jp/profile/IAL2' OSF_AAL1_STR = 'https://www\.gakunin\.jp/profile/AAL1' OSF_AAL2_STR = 'https://www\.gakunin\.jp/profile/AAL2' +OSF_IAL1_VAR = 'https://www.gakunin.jp/profile/IAL1' OSF_IAL2_VAR = 'https://www.gakunin.jp/profile/IAL2' OSF_AAL1_VAR = 'https://www.gakunin.jp/profile/AAL1' OSF_AAL2_VAR = 'https://www.gakunin.jp/profile/AAL2'