Skip to content

Commit

Permalink
trying to fix alternative auth methods support
Browse files Browse the repository at this point in the history
  • Loading branch information
@edulix
edulix committed Nov 12, 2023
1 parent fa654df commit 301f217
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 52 deletions.
9 changes: 3 additions & 6 deletions iam/api/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1580,9 +1580,7 @@ def post(request, pk=None):

alternative_auth_methods = req.get('alternative_auth_methods', None)
if alternative_auth_methods:
msg += check_alt_auth_methods(
alternative_auth_methods, extra_fields
)
msg += check_alt_auth_methods(req)
update_alt_methods_config(alternative_auth_methods)

scheduled_events = req.get('scheduled_events', None)
Expand Down Expand Up @@ -1820,9 +1818,8 @@ def post(request, pk=None):

alternative_auth_methods = req.get('alternative_auth_methods', None)
if alternative_auth_methods:
msg += check_alt_auth_methods(
alternative_auth_methods, extra_fields
)
msg += check_alt_auth_methods(req)
update_alt_methods_config(alternative_auth_methods)

if msg:
return json_response(status=400, message=msg)
Expand Down
6 changes: 6 additions & 0 deletions iam/authmethods/m_openidconnect.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ def validate_oidc_providers(self, data, request_data):
Validate that the provider ids are part of the oidc_providers in
`request_data`
'''
if "provider_ids" not in data:
raise MarshMallowValidationError(
message=(
f"provider_ids not found in `auth_method_config.config`"
)
)
for provider_id in data["provider_ids"]:
provider = next(
(
Expand Down
88 changes: 44 additions & 44 deletions iam/test_utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,18 @@ def test_empty(self):
'''
# None is valid
ret = check_alt_auth_methods(
alternative_auth_methods=None, extra_fields=[]
dict(alternative_auth_methods=None, extra_fields=[])
)
self.assertEqual(ret, '')

# Empty list is valid
ret = check_alt_auth_methods(
alternative_auth_methods=[], extra_fields=[]
dict(alternative_auth_methods=[], extra_fields=[])
)
self.assertEqual(ret, '')

# None is still valid independently of the extra_fields
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=None,
extra_fields=[
{
Expand All @@ -75,11 +75,11 @@ def test_empty(self):
"required_on_authentication": True
},
]
)
))
self.assertEqual(ret, '')

def test_basic(self):
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand Down Expand Up @@ -128,7 +128,7 @@ def test_basic(self):
"required_on_authentication": True
},
]
)
))
self.assertEqual(ret, '')

def test_invalid_types(self):
Expand All @@ -137,37 +137,37 @@ def test_invalid_types(self):
'''
# alternative_auth_methods must be a list or None, not a number
ret = check_alt_auth_methods(
alternative_auth_methods=33, extra_fields=[]
dict(alternative_auth_methods=33, extra_fields=[])
)
self.assertNotEqual(ret, '')

# alternative_auth_methods must be a list or None, not a dict
ret = check_alt_auth_methods(
alternative_auth_methods=dict(), extra_fields=[]
dict(alternative_auth_methods=dict(), extra_fields=[])
)
self.assertNotEqual(ret, '')

# Check the alternative auth method fails when it's not an object
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
"not-an-object"
],
extra_fields=[]
)
))
self.assertNotEqual(ret, '')

# Check the alternative auth method fails when it's not an object
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
123
],
extra_fields=[]
)
))
self.assertNotEqual(ret, '')

def test_id_field(self):
# Check the alternative auth method fails when it's missing "id" field
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id_": "email",
Expand All @@ -180,11 +180,11 @@ def test_id_field(self):
}
],
extra_fields=self.email_extra_fields()
)
))
self.assertNotEqual(ret, '')

# Check the alternative auth method fails when id field is not a string
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": dict(email="email"),
Expand All @@ -197,11 +197,11 @@ def test_id_field(self):
}
],
extra_fields=self.email_extra_fields()
)
))
self.assertNotEqual(ret, '')

# Check the alternative auth method works when id field is text
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand All @@ -214,12 +214,12 @@ def test_id_field(self):
}
],
extra_fields=self.email_extra_fields()
)
))
self.assertEqual(ret, '')

def test_id_field_duplicated(self):
# Check the alternative auth method fails when id field is not a string
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand All @@ -241,11 +241,11 @@ def test_id_field_duplicated(self):
},
],
extra_fields=self.email_extra_fields()
)
))
self.assertNotEqual(ret, '')

# Check the alternative auth method fails when id field is not a string
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand All @@ -267,15 +267,15 @@ def test_id_field_duplicated(self):
},
],
extra_fields=self.email_extra_fields()
)
))
self.assertEqual(ret, '')

def test_id_validate_other_fields(self):
'''
Validate other alt auth method fields
'''
# invalid auth_method_name type
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand All @@ -288,11 +288,11 @@ def test_id_validate_other_fields(self):
},
],
extra_fields=self.email_extra_fields()
)
))
self.assertNotEqual(ret, '')

# inexistent auth method name
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand All @@ -305,11 +305,11 @@ def test_id_validate_other_fields(self):
},
],
extra_fields=self.email_extra_fields()
)
))
self.assertNotEqual(ret, '')

# invalid auth_method_config
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand All @@ -322,11 +322,11 @@ def test_id_validate_other_fields(self):
},
],
extra_fields=self.email_extra_fields()
)
))
self.assertNotEqual(ret, '')

# invalid public_name
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand All @@ -339,12 +339,12 @@ def test_id_validate_other_fields(self):
},
],
extra_fields=self.email_extra_fields()
)
))
self.assertNotEqual(ret, '')

def test_id_validate_public_name_i18n(self):
# invalid public_name_i18n
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand All @@ -357,11 +357,11 @@ def test_id_validate_public_name_i18n(self):
},
],
extra_fields=self.email_extra_fields()
)
))
self.assertNotEqual(ret, '')

# invalid public_name_i18n 2
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand All @@ -374,11 +374,11 @@ def test_id_validate_public_name_i18n(self):
},
],
extra_fields=self.email_extra_fields()
)
))
self.assertNotEqual(ret, '')

# valid public_name_i18n
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand All @@ -391,15 +391,15 @@ def test_id_validate_public_name_i18n(self):
},
],
extra_fields=self.email_extra_fields()
)
))
self.assertEqual(ret, '')

def test_id_validate_extra_fields_equal_names(self):
'''
extra fields should be the same name in all alt auth methods
'''
# mismatched name
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand Down Expand Up @@ -430,11 +430,11 @@ def test_id_validate_extra_fields_equal_names(self):
"required_on_authentication": True
},
]
)
))
self.assertNotEqual(ret, '')

# extra field, called "name and surname"
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand Down Expand Up @@ -474,11 +474,11 @@ def test_id_validate_extra_fields_equal_names(self):
"required_on_authentication": True
},
]
)
))
self.assertNotEqual(ret, '')

# one alt auth method has different extra field name, the other is fine
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand Down Expand Up @@ -536,15 +536,15 @@ def test_id_validate_extra_fields_equal_names(self):
"required_on_authentication": True
},
]
)
))
self.assertNotEqual(ret, '')

def test_id_validate_extra_fields_equal_types(self):
'''
extra fields should be the same type
'''
# mismatched type
ret = check_alt_auth_methods(
ret = check_alt_auth_methods(dict(
alternative_auth_methods=[
{
"id": "email",
Expand Down Expand Up @@ -575,5 +575,5 @@ def test_id_validate_extra_fields_equal_types(self):
"required_on_authentication": True
},
]
)
))
self.assertNotEqual(ret, '')
10 changes: 8 additions & 2 deletions iam/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1368,7 +1368,7 @@ def update_alt_methods_config(alternative_auth_methods):
alt_auth_method['auth_method_config']['config'].update(base_config)

def check_alt_auth_methods(
alternative_auth_methods, extra_fields
auth_event_data
):
'''
Check that the alternative authentication methods conform with their
Expand Down Expand Up @@ -1400,6 +1400,11 @@ def check_alt_auth_methods(
'''
from authmethods import check_config, METHODS
from copy import deepcopy

alternative_auth_methods = auth_event_data.get(
'alternative_auth_methods', []
)
extra_fields = auth_event_data.get('extra_fields', [])

if alternative_auth_methods is None:
return ''
Expand All @@ -1412,7 +1417,8 @@ def check_and_update_config(auth_method):
auth_method['auth_method_config'] = updated_config
return check_config(
auth_method['auth_method_config'],
auth_method['auth_method_name']
auth_method['auth_method_name'],
auth_event_data
) == ''

def has_same_extra_fields(extra_fields1):
Expand Down

0 comments on commit 301f217

Please sign in to comment.