Skip to content

Commit 4b4a4e9

Browse files
committed
Re-fetch info on access elevator cache miss
1 parent 9c72da7 commit 4b4a4e9

2 files changed

Lines changed: 140 additions & 11 deletions

File tree

src/main.py

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,34 @@ def classify_auto_approved_permission_sets( # noqa: PLR0913
768768
return auto_approved
769769

770770

771+
def _get_cached_user_info(view_key: str, user_id: str, client: "WebClient") -> tuple[set[str], str | None]:
772+
"""Get user group IDs and email from cache, re-fetching from Identity Center on miss."""
773+
user_group_ids = user_view_map.get(f"{view_key}:group_ids")
774+
user_email = user_view_map.get(f"{view_key}:user_email")
775+
if user_group_ids is not None:
776+
return user_group_ids, user_email
777+
778+
logger.info("User info cache miss (container recycled), re-fetching from Identity Center")
779+
identity_store_id = sso.get_identity_store_id(cfg, sso_client)
780+
user_email = slack_helpers.get_user(client, id=user_id).email
781+
user_principal_id, _ = sso.get_user_principal_id_by_email(
782+
identity_store_client=identity_store_client,
783+
identity_store_id=identity_store_id,
784+
email=user_email,
785+
cfg=cfg,
786+
)
787+
user_group_ids = sso.get_user_group_ids(
788+
identity_store_client=identity_store_client,
789+
identity_store_id=identity_store_id,
790+
user_principal_id=user_principal_id,
791+
)
792+
# Re-populate cache for subsequent calls in this container
793+
user_view_map[f"{view_key}:group_ids"] = user_group_ids
794+
user_view_map[f"{view_key}:user_principal_id"] = user_principal_id
795+
user_view_map[f"{view_key}:user_email"] = user_email
796+
return user_group_ids, user_email
797+
798+
771799
@app.action(slack_helpers.RequestForAccessView.ACCOUNT_ACTION_ID)
772800
def handle_account_selection(ack: Ack, body: dict, client: WebClient) -> SlackResponse:
773801
ack()
@@ -791,19 +819,12 @@ def handle_account_selection(ack: Ack, body: dict, client: WebClient) -> SlackRe
791819
updated_view = slack_helpers.RequestForAccessView.build_no_permission_sets_view(view_blocks=body["view"]["blocks"])
792820
return client.views_update(view_id=view_id, view=updated_view)
793821

794-
# Get cached user_group_ids
822+
# Get cached user info, re-fetching from Identity Center on cache miss
823+
# (e.g. when Lambda container was recycled between form load and account selection)
795824
user_id = body.get("user", {}).get("id")
796825
callback_id = slack_helpers.RequestForAccessView.CALLBACK_ID
797826
view_key = f"{user_id}:{callback_id}"
798-
group_ids_key = f"{view_key}:group_ids"
799-
user_group_ids = user_view_map.get(group_ids_key)
800-
if user_group_ids is None:
801-
logger.warning(
802-
f"User group IDs not found in cache for key: {group_ids_key}. "
803-
"This may happen if Lambda container was recycled between form load and account selection. "
804-
"Defaulting to empty set, which will restrict visibility to statements without required_group_membership."
805-
)
806-
user_group_ids = set()
827+
user_group_ids, user_email = _get_cached_user_info(view_key, user_id, client)
807828

808829
# Compute intersection of permission sets across all selected accounts
809830
valid_ps_names = statement.get_permission_sets_for_accounts_and_user(cfg.statements, account_ids, user_group_ids)
@@ -825,7 +846,6 @@ def handle_account_selection(ack: Ack, body: dict, client: WebClient) -> SlackRe
825846
return client.views_update(view_id=view_id, view=updated_view)
826847

827848
# Classify permission sets as auto-approved vs requires-approval
828-
user_email = user_view_map.get(f"{view_key}:user_email")
829849
auto_approved_arns: set[str] | None = None
830850
if user_email:
831851
resolver_cache: dict[frozenset[str], set[str]] = {}

src/tests/test_main.py

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,3 +627,112 @@ def side_effect(*_args, **kwargs):
627627
)
628628
assert ps_auto.arn in result
629629
assert ps_manual.arn not in result
630+
631+
632+
class TestGetCachedUserInfo:
633+
"""Tests for _get_cached_user_info re-fetching user info on cache miss."""
634+
635+
def test_cache_hit_returns_cached_values(self, import_main):
636+
"""When group_ids are cached, returns cached values without API calls."""
637+
main = import_main
638+
main.user_view_map.clear()
639+
640+
view_key = "U_CACHED:request_for__account_access_submitted"
641+
main.user_view_map[f"{view_key}:group_ids"] = {"group-1"}
642+
main.user_view_map[f"{view_key}:user_email"] = "cached@test.com"
643+
644+
mock_client = MagicMock()
645+
646+
with (
647+
patch.object(main.sso, "get_identity_store_id") as mock_get_id,
648+
patch.object(main.sso, "get_user_principal_id_by_email") as mock_get_principal,
649+
patch.object(main.sso, "get_user_group_ids") as mock_get_groups,
650+
):
651+
group_ids, email = main._get_cached_user_info(view_key, "U_CACHED", mock_client)
652+
653+
mock_get_id.assert_not_called()
654+
mock_get_principal.assert_not_called()
655+
mock_get_groups.assert_not_called()
656+
657+
assert group_ids == {"group-1"}
658+
assert email == "cached@test.com"
659+
660+
def test_cache_miss_refetches_from_identity_center(self, import_main):
661+
"""When group_ids are not cached, re-fetches from Identity Center."""
662+
main = import_main
663+
main.user_view_map.clear()
664+
665+
view_key = "U_COLD:request_for__account_access_submitted"
666+
refetched_groups = {"group-dev", "group-admin"}
667+
668+
mock_client = MagicMock()
669+
670+
with (
671+
patch.object(main.sso, "get_identity_store_id", return_value="d-123456"),
672+
patch.object(
673+
main.slack_helpers,
674+
"get_user",
675+
return_value=entities.slack.User(id="U_COLD", email="cold@test.com", real_name="Cold User"),
676+
),
677+
patch.object(main.sso, "get_user_principal_id_by_email", return_value=("principal-cold", None)),
678+
patch.object(main.sso, "get_user_group_ids", return_value=refetched_groups) as mock_get_groups,
679+
):
680+
group_ids, email = main._get_cached_user_info(view_key, "U_COLD", mock_client)
681+
682+
mock_get_groups.assert_called_once()
683+
684+
assert group_ids == refetched_groups
685+
assert email == "cold@test.com"
686+
687+
def test_cache_miss_repopulates_cache(self, import_main):
688+
"""After re-fetching, user info is stored back in the cache."""
689+
main = import_main
690+
main.user_view_map.clear()
691+
692+
view_key = "U_REPOP:request_for__account_access_submitted"
693+
mock_client = MagicMock()
694+
695+
with (
696+
patch.object(main.sso, "get_identity_store_id", return_value="d-123456"),
697+
patch.object(
698+
main.slack_helpers,
699+
"get_user",
700+
return_value=entities.slack.User(id="U_REPOP", email="repop@test.com", real_name="Repop User"),
701+
),
702+
patch.object(main.sso, "get_user_principal_id_by_email", return_value=("principal-repop", None)),
703+
patch.object(main.sso, "get_user_group_ids", return_value={"group-x"}),
704+
):
705+
main._get_cached_user_info(view_key, "U_REPOP", mock_client)
706+
707+
assert main.user_view_map[f"{view_key}:group_ids"] == {"group-x"}
708+
assert main.user_view_map[f"{view_key}:user_principal_id"] == "principal-repop"
709+
assert main.user_view_map[f"{view_key}:user_email"] == "repop@test.com"
710+
711+
def test_second_call_after_refetch_uses_cache(self, import_main):
712+
"""After a cache miss repopulates, the next call uses the cache."""
713+
main = import_main
714+
main.user_view_map.clear()
715+
716+
view_key = "U_TWICE:request_for__account_access_submitted"
717+
mock_client = MagicMock()
718+
719+
with (
720+
patch.object(main.sso, "get_identity_store_id", return_value="d-123456"),
721+
patch.object(
722+
main.slack_helpers,
723+
"get_user",
724+
return_value=entities.slack.User(id="U_TWICE", email="twice@test.com", real_name="Twice User"),
725+
),
726+
patch.object(main.sso, "get_user_principal_id_by_email", return_value=("principal-twice", None)),
727+
patch.object(main.sso, "get_user_group_ids", return_value={"group-y"}) as mock_get_groups,
728+
):
729+
# First call: cache miss, re-fetches
730+
main._get_cached_user_info(view_key, "U_TWICE", mock_client)
731+
assert mock_get_groups.call_count == 1
732+
733+
# Second call: cache hit, no re-fetch
734+
group_ids, email = main._get_cached_user_info(view_key, "U_TWICE", mock_client)
735+
assert mock_get_groups.call_count == 1 # still 1, not 2
736+
737+
assert group_ids == {"group-y"}
738+
assert email == "twice@test.com"

0 commit comments

Comments
 (0)