Skip to content
This repository was archived by the owner on Mar 23, 2026. It is now read-only.

IAM: Migrate MFA Operations#13801

Merged
dfangl merged 5 commits into
iam/moto-migrationfrom
cris/unc-237
Feb 25, 2026
Merged

IAM: Migrate MFA Operations#13801
dfangl merged 5 commits into
iam/moto-migrationfrom
cris/unc-237

Conversation

@pinzon

@pinzon pinzon commented Feb 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Motivation

As per the IAM internalization project. This pr implements the operations and resources required for OpenID connector provider management in the IAM service.

Changes

localstack-core/localstack/services/iam/models.py

  • Added VirtualMFADevice dataclass with fields: serial_number, base32_string_seed, qr_code_png, path,
    name, user assignment info, tags
  • Added MFADevice dataclass for MFA devices assigned to users
  • Added storage attributes in IAMStore: virtual_mfa_devices and mfa_devices

localstack-core/localstack/services/iam/provider.py

  • Added imports for MFA types
  • Implemented helper methods:
    • _generate_mfa_serial_number() - generates ARN-style serial number
    • _generate_totp_secret() - generates Base32-encoded TOTP secret
    • _generate_qr_code_png() - generates placeholder QR code PNG
    • _validate_mfa_path() - validates path format and length
  • Implemented 6 MFA operations:
    • create_virtual_mfa_device - creates device with TOTP secret and QR code
    • delete_virtual_mfa_device - deletes device and removes user assignment
    • list_virtual_mfa_devices - lists with AssignmentStatus filtering and pagination
    • enable_mfa_device - assigns device to user (virtual and physical tokens)
    • deactivate_mfa_device - unassigns device from user
    • list_mfa_devices - lists MFA devices for a user

Tests

All MFA tests must pass.

TODO

Merge after #13800

@pinzon pinzon added semver: patch Non-breaking changes which can be included in patch releases docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes labels Feb 19, 2026
@github-actions

github-actions Bot commented Feb 19, 2026 •
edited
Loading

Copy link
Copy Markdown

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   2m 45s ⏱️ - 2h 9m 57s
519 tests  - 5 113  463 ✅  - 4 146  46 💤  - 370  10 ❌  - 577 
521 runs   - 5 113  463 ✅  - 4 146  48 💤  - 370  10 ❌  - 577 

For more details on these failures, see this check.

Results for commit 2cff512. ± Comparison against base commit c51a449.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Feb 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Test Results - Preflight, Unit

23 123 tests  +53   21 252 ✅ +73   6m 9s ⏱️ -32s
     1 suites ± 0    1 871 💤  - 20 
     1 files   ± 0        0 ❌ ± 0 

Results for commit 2cff512. ± Comparison against base commit c51a449.

♻️ This comment has been updated with latest results.

@pinzon pinzon added this to the 4.14 milestone Feb 20, 2026
@pinzon pinzon mentioned this pull request Feb 21, 2026
Merged
dfangl
dfangl suggested changes Feb 24, 2026
View reviewed changes

@dfangl dfangl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! My main issues are the pagination, the typing issues, and storing the user object in the MFA device again.

Maybe we can for now do a dynamic lookup instead of storing the object and relying on object identity for data consistency?

Also, we really should add locks here - but I am happy to tackle this in a follow up when iterating on the IAM provider!

Comment thread localstack-core/localstack/services/iam/models.py Outdated
Comment thread localstack-core/localstack/services/iam/provider.py Outdated
Comment thread localstack-core/localstack/services/iam/provider.py Outdated
)
store.MFA_DEVICES[serial_number] = device

user_entity = store.USERS[user_name]

@dfangl dfangl Feb 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use the user entity above here! You changed the _get_user_or_raise_error return value here - but this conflicts with other usages. Maybe we should rename that helper as well.

@dfangl dfangl Feb 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worst thing we could also dynamically look the user up based on the users MFA_DEVICES. That would avoid us tracking all this data, but the lookup complexity would be higher.

Comment thread localstack-core/localstack/services/iam/provider.py Outdated
dfangl
dfangl reviewed Feb 24, 2026
View reviewed changes
Comment thread localstack-core/localstack/services/iam/provider.py Outdated
# Virtual MFA device - update existing entry
device = store.MFA_DEVICES[serial_number]
device.device["EnableDate"] = enable_date
device.user = user_entity.user

@dfangl dfangl Feb 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens by the way if a device is already attached to another user?

@pinzon pinzon Feb 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a test and a fix to answer this.

@pinzon pinzon requested a review from dfangl February 24, 2026 21:04
@pinzon pinzon marked this pull request as ready for review February 24, 2026 21:04
@github-actions

github-actions Bot commented Feb 25, 2026

Copy link
Copy Markdown

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 5s ⏱️ +12s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 2cff512. ± Comparison against base commit c51a449.

@github-actions

github-actions Bot commented Feb 25, 2026

Copy link
Copy Markdown

Test Results (amd64) - Integration, Bootstrap

  5 files  ±    0    5 suites  ±0   11m 22s ⏱️ - 2h 30m 37s
543 tests  - 5 494  484 ✅  - 4 965  46 💤  - 526  13 ❌  - 3 
549 runs   - 5 494  484 ✅  - 4 965  52 💤  - 526  13 ❌  - 3 

For more details on these failures, see this check.

Results for commit 2cff512. ± Comparison against base commit c51a449.

dfangl
dfangl approved these changes Feb 25, 2026
View reviewed changes

@dfangl dfangl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for addressing my comments!

@dfangl dfangl merged commit 5086190 into iam/moto-migration Feb 25, 2026
27 of 33 checks passed
@dfangl dfangl deleted the cris/unc-237 branch February 25, 2026 09:17
dfangl pushed a commit that referenced this pull request Feb 26, 2026
@pinzon @dfangl
IAM: Migrate MFA Operations (#13801)
b3418b7
dfangl pushed a commit that referenced this pull request Feb 26, 2026
@pinzon @dfangl
IAM: Migrate MFA Operations (#13801)
07441d7
dfangl pushed a commit that referenced this pull request Mar 4, 2026
@pinzon @dfangl
IAM: Migrate MFA Operations (#13801)
88f6ac7
dfangl pushed a commit that referenced this pull request Mar 6, 2026
@pinzon @dfangl
IAM: Migrate MFA Operations (#13801)
178d1d0
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@dfangl dfangl dfangl approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Milestone

4.14

Development

Successfully merging this pull request may close these issues.

2 participants

@pinzon @dfangl