feat: add user MFA factor management with sanitized test fixtures - #43
Conversation
Add methods to list enrolled and supported MFA factors, enroll a factor, activate a Google OTP factor, and delete factors, plus the related User factor entity classes. Tests record real Okta interactions via betamax; a pytest session hook sanitizes the recorded cassettes (redacting the TOTP shared secret and the real host, for both plaintext and gzipped bodies) so no secrets or host details are committed. Host parsing is centralized in small helpers. This squashes the branch's development history into a single commit so the earlier, un-sanitized cassette blobs are no longer part of the branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jorgemb
left a comment
jorgemb
left a comment
There was a problem hiding this comment.
Only changes, though not necessary as they are from internal implementation, are for the naming of the function _create_factor_from_data and the parameters it handles. I don't see a big issue in how it goes, just makes more explicit what you are expecting. Otherwise, looks good to me.
|
|
||
| """ | ||
| factor_type = factor_data.get('factorType', '') | ||
| provider = factor_data.get('provider', '') |
There was a problem hiding this comment.
Wouldn't it be better to receive these parameters already, instead of a dictionary to extract them from?
There was a problem hiding this comment.
If I know that all factors will have these fields, and will contain the same type as value, then yes maybe.
I like the current flexibility. For example, if a type doesn't have a field it currently doesn't hurt, and if its a different type it can be overridden.
| from oktalib.entities.users import UserFactorGoogleOTP | ||
|
|
||
|
|
||
| @pytest.fixture |
There was a problem hiding this comment.
Never seen a construct like this before. Really interesting.
| return self._user_assignment_data.get('profile', {}).get('samlRoles', []) | ||
|
|
||
|
|
||
| def _create_factor_from_data( |
There was a problem hiding this comment.
If you are extracting the values from the dictionary I would call this function: _create_factor_from_type_and_provider. But I know it looks more verbose.
There was a problem hiding this comment.
I am not creating the factor from a type and provider, I create it from data, while using the type and provider to decide which factor to create. And I think that the group name because quite long then, nor do I want to change the function name if the decision progress in the future needs more or less variables.
2 participants
Add methods to list enrolled and supported MFA factors, enroll a factor, activate a Google OTP factor, and delete factors, plus the related User factor entity classes.
Tests record real Okta interactions via betamax; a pytest session hook sanitizes the recorded cassettes (redacting the TOTP shared secret and the real host, for both plaintext and gzipped bodies) so no secrets or host details are committed. Host parsing is centralized in small helpers.
This squashes the branch's development history into a single commit so the earlier, un-sanitized cassette blobs are no longer part of the branch.