Skip to content

Commit ea15560

Browse files
committed
Group permission sets by approval status
Display permission sets under the groups "Auto approved" and "Requires approval" so that users know what to expect before requesting the permission. This pushes users to request auto approved permissions, which is preferable since those are generally less permissive.
1 parent 6278d4d commit ea15560

5 files changed

Lines changed: 339 additions & 12 deletions

File tree

src/main.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,35 @@ def handle_duration_picker_action(ack): # noqa: ANN201, ANN001
739739
ack()
740740

741741

742+
def classify_auto_approved_permission_sets( # noqa: PLR0913
743+
statements: frozenset[statement.Statement],
744+
permission_sets: list[entities.aws.PermissionSet],
745+
account_ids: list[str],
746+
requester_email: str,
747+
user_group_ids: set[str],
748+
requester_slack_id: str,
749+
approver_group_resolver: Callable[[frozenset[str]], set[str]] | None = None,
750+
) -> set[str]:
751+
"""Return ARNs of permission sets that are auto-approved for all given accounts."""
752+
auto_approved = set()
753+
for ps in permission_sets:
754+
if all(
755+
access_control.make_decision_on_access_request(
756+
statements,
757+
account_id=aid,
758+
permission_set_name=ps.name,
759+
requester_email=requester_email,
760+
user_group_ids=user_group_ids,
761+
permission_set_arn=ps.arn,
762+
requester_slack_id=requester_slack_id,
763+
approver_group_resolver=approver_group_resolver,
764+
).grant
765+
for aid in account_ids
766+
):
767+
auto_approved.add(ps.arn)
768+
return auto_approved
769+
770+
742771
@app.action(slack_helpers.RequestForAccessView.ACCOUNT_ACTION_ID)
743772
def handle_account_selection(ack: Ack, body: dict, client: WebClient) -> SlackResponse:
744773
ack()
@@ -795,10 +824,37 @@ def handle_account_selection(ack: Ack, body: dict, client: WebClient) -> SlackRe
795824
updated_view = slack_helpers.RequestForAccessView.build_no_permission_sets_view(view_blocks=body["view"]["blocks"])
796825
return client.views_update(view_id=view_id, view=updated_view)
797826

827+
# Classify permission sets as auto-approved vs requires-approval
828+
user_email = user_view_map.get(f"{view_key}:user_email")
829+
auto_approved_arns: set[str] | None = None
830+
if user_email:
831+
resolver_cache: dict[frozenset[str], set[str]] = {}
832+
833+
def approver_group_resolver(group_ids: frozenset[str]) -> set[str]:
834+
if not group_ids:
835+
return set()
836+
if group_ids in resolver_cache:
837+
return resolver_cache[group_ids]
838+
group_users, _ = slack_helpers.resolve_approver_groups(client, group_ids)
839+
result = {u.id for u in group_users}
840+
resolver_cache[group_ids] = result
841+
return result
842+
843+
auto_approved_arns = classify_auto_approved_permission_sets(
844+
statements=cfg.statements,
845+
permission_sets=permission_sets,
846+
account_ids=account_ids,
847+
requester_email=user_email,
848+
user_group_ids=user_group_ids,
849+
requester_slack_id=user_id,
850+
approver_group_resolver=approver_group_resolver,
851+
)
852+
798853
updated_view = slack_helpers.RequestForAccessView.update_with_permission_sets(
799854
view_blocks=body["view"]["blocks"],
800855
permission_sets=permission_sets,
801856
display_names=cfg.permission_set_display_names,
857+
auto_approved_arns=auto_approved_arns,
802858
)
803859
return client.views_update(view_id=view_id, view=updated_view)
804860

src/slack_helpers.py

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
InputBlock,
1818
MarkdownTextObject,
1919
Option,
20+
OptionGroup,
2021
PlainTextInputElement,
2122
PlainTextObject,
2223
SectionBlock,
@@ -140,29 +141,56 @@ def _get_permission_set_display_name(
140141
name = ps.name
141142
return name[:75]
142143

144+
@classmethod
145+
def _build_permission_set_option(cls, ps: entities.aws.PermissionSet, display_names: dict[str, str] | None) -> Option:
146+
return Option(text=PlainTextObject(text=cls._get_permission_set_display_name(ps, display_names)), value=ps.arn)
147+
143148
@classmethod
144149
def build_select_permission_set_input_block(
145150
cls,
146151
permission_sets: list[entities.aws.PermissionSet],
147152
display_names: dict[str, str] | None = None,
153+
auto_approved_arns: set[str] | None = None,
148154
) -> InputBlock:
149-
sorted_permission_sets = sorted(
150-
permission_sets,
151-
key=lambda ps: cls._get_permission_set_display_name(ps, display_names).lower(),
152-
)
155+
sort_key = lambda ps: cls._get_permission_set_display_name(ps, display_names).lower() # noqa: E731
156+
157+
if auto_approved_arns is not None:
158+
auto_approved = sorted([ps for ps in permission_sets if ps.arn in auto_approved_arns], key=sort_key)
159+
requires_approval = sorted([ps for ps in permission_sets if ps.arn not in auto_approved_arns], key=sort_key)
160+
groups = []
161+
if auto_approved:
162+
groups.append(
163+
OptionGroup(
164+
label=PlainTextObject(text="Auto approved"),
165+
options=[cls._build_permission_set_option(ps, display_names) for ps in auto_approved],
166+
)
167+
)
168+
if requires_approval:
169+
groups.append(
170+
OptionGroup(
171+
label=PlainTextObject(text="Requires approval"),
172+
options=[cls._build_permission_set_option(ps, display_names) for ps in requires_approval],
173+
)
174+
)
175+
if groups:
176+
return InputBlock(
177+
block_id=cls.PERMISSION_SET_BLOCK_ID,
178+
label=PlainTextObject(text="Permission set"),
179+
element=StaticSelectElement(
180+
action_id=cls.PERMISSION_SET_ACTION_ID,
181+
placeholder=PlainTextObject(text="Select permission set"),
182+
option_groups=groups,
183+
),
184+
)
185+
186+
sorted_permission_sets = sorted(permission_sets, key=sort_key)
153187
return InputBlock(
154188
block_id=cls.PERMISSION_SET_BLOCK_ID,
155189
label=PlainTextObject(text="Permission set"),
156190
element=StaticSelectElement(
157191
action_id=cls.PERMISSION_SET_ACTION_ID,
158192
placeholder=PlainTextObject(text="Select permission set"),
159-
options=[
160-
Option(
161-
text=PlainTextObject(text=cls._get_permission_set_display_name(ps, display_names)),
162-
value=ps.arn,
163-
)
164-
for ps in sorted_permission_sets
165-
],
193+
options=[cls._build_permission_set_option(ps, display_names) for ps in sorted_permission_sets],
166194
),
167195
)
168196

@@ -198,6 +226,7 @@ def update_with_permission_sets(
198226
view_blocks: list,
199227
permission_sets: list[entities.aws.PermissionSet],
200228
display_names: dict[str, str] | None = None,
229+
auto_approved_arns: set[str] | None = None,
201230
) -> View:
202231
view = cls.build()
203232
view.submit_disabled = False # type: ignore[attr-defined]
@@ -206,7 +235,13 @@ def update_with_permission_sets(
206235
# Insert permission set dropdown after account dropdown
207236
blocks = insert_blocks(
208237
blocks=blocks,
209-
blocks_to_insert=[cls.build_select_permission_set_input_block(permission_sets, display_names=display_names)],
238+
blocks_to_insert=[
239+
cls.build_select_permission_set_input_block(
240+
permission_sets,
241+
display_names=display_names,
242+
auto_approved_arns=auto_approved_arns,
243+
)
244+
],
210245
after_block_id=cls.ACCOUNT_BLOCK_ID,
211246
)
212247
view.blocks = blocks

src/tests/test_config.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ def valid_config_dict(
134134
"max_permissions_duration_time": "24",
135135
"secondary_fallback_email_domains": secondary_fallback_email_domains,
136136
"permission_duration_list_override": permission_duration_list_override,
137+
"permission_set_display_names": json.dumps({}),
137138
}
138139

139140

@@ -418,3 +419,57 @@ def test_load_approval_config_returns_etag(mock_s3_client):
418419

419420
assert etag == '"test-etag"'
420421
assert data == {"statements": [], "group_statements": []}
422+
423+
424+
def test_config_parses_display_names_from_s3(mock_s3_client, monkeypatch):
425+
"""S3 config with permission_set_display_names is parsed into Config."""
426+
import boto3
427+
428+
s3_config = {
429+
"statements": [VALID_STATEMENT_DICT],
430+
"group_statements": [VALID_GROUP_STATEMENT_DICT],
431+
"permission_set_display_names": {"eks-dev": "EKS access"},
432+
}
433+
mock_s3_client.get_object.return_value = {"Body": MagicMock(read=lambda: json.dumps(s3_config).encode("utf-8"))}
434+
monkeypatch.setattr(boto3, "client", lambda service: mock_s3_client if service == "s3" else MagicMock())
435+
436+
config_dict = valid_config_dict(secondary_fallback_email_domains_as_json=False, permission_duration_list_override_as_json=False)
437+
config_dict["config_s3_key"] = "config/approval-config.json"
438+
del config_dict["statements"]
439+
del config_dict["group_statements"]
440+
del config_dict["permission_set_display_names"]
441+
442+
cfg = config.Config(**config_dict)
443+
assert cfg.permission_set_display_names == {"eks-dev": "EKS access"}
444+
445+
446+
def test_config_defaults_empty_display_names_from_s3(mock_s3_client, monkeypatch):
447+
"""S3 config without permission_set_display_names defaults to empty dict."""
448+
import boto3
449+
450+
s3_config = {
451+
"statements": [VALID_STATEMENT_DICT],
452+
"group_statements": [VALID_GROUP_STATEMENT_DICT],
453+
}
454+
mock_s3_client.get_object.return_value = {"Body": MagicMock(read=lambda: json.dumps(s3_config).encode("utf-8"))}
455+
monkeypatch.setattr(boto3, "client", lambda service: mock_s3_client if service == "s3" else MagicMock())
456+
457+
config_dict = valid_config_dict(secondary_fallback_email_domains_as_json=False, permission_duration_list_override_as_json=False)
458+
config_dict["config_s3_key"] = "config/approval-config.json"
459+
del config_dict["statements"]
460+
del config_dict["group_statements"]
461+
del config_dict["permission_set_display_names"]
462+
463+
cfg = config.Config(**config_dict)
464+
assert cfg.permission_set_display_names == {}
465+
466+
467+
def test_config_parses_display_names_from_env():
468+
"""Env var path: JSON string is parsed into dict."""
469+
config_dict = valid_config_dict(
470+
secondary_fallback_email_domains_as_json=False,
471+
permission_duration_list_override_as_json=False,
472+
)
473+
config_dict["permission_set_display_names"] = json.dumps({"admin": "Administrator"})
474+
cfg = config.Config(**config_dict)
475+
assert cfg.permission_set_display_names == {"admin": "Administrator"}

src/tests/test_main.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,3 +534,96 @@ def process_with_first_failure(**kwargs):
534534
finally:
535535
for p in patches:
536536
p.stop()
537+
538+
539+
class TestClassifyAutoApprovedPermissionSets:
540+
"""Tests for classify_auto_approved_permission_sets."""
541+
542+
def _ps(self, name: str, arn: str = "") -> entities.aws.PermissionSet:
543+
return entities.aws.PermissionSet(name=name, arn=arn or f"arn:sso:ps/{name}", description=None)
544+
545+
def _decision(self, grant: bool):
546+
import access_control
547+
548+
reason = access_control.DecisionReason.ApprovalNotRequired if grant else access_control.DecisionReason.RequiresApproval
549+
return access_control.AccessRequestDecision(
550+
grant=grant,
551+
reason=reason,
552+
based_on_statements=frozenset(),
553+
approvers=frozenset() if grant else frozenset(["approver@test.com"]),
554+
approver_groups=frozenset(),
555+
)
556+
557+
def test_auto_approved_for_all_accounts(self, import_main):
558+
main = import_main
559+
ps = self._ps("ReadOnly")
560+
561+
with patch.object(main.access_control, "make_decision_on_access_request", return_value=self._decision(grant=True)):
562+
result = main.classify_auto_approved_permission_sets(
563+
statements=frozenset(),
564+
permission_sets=[ps],
565+
account_ids=["111", "222"],
566+
requester_email="user@test.com",
567+
user_group_ids=set(),
568+
requester_slack_id="U123",
569+
)
570+
assert ps.arn in result
571+
572+
def test_requires_approval_for_all_accounts(self, import_main):
573+
main = import_main
574+
ps = self._ps("Admin")
575+
576+
with patch.object(main.access_control, "make_decision_on_access_request", return_value=self._decision(grant=False)):
577+
result = main.classify_auto_approved_permission_sets(
578+
statements=frozenset(),
579+
permission_sets=[ps],
580+
account_ids=["111", "222"],
581+
requester_email="user@test.com",
582+
user_group_ids=set(),
583+
requester_slack_id="U123",
584+
)
585+
assert ps.arn not in result
586+
587+
def test_mixed_accounts_requires_approval(self, import_main):
588+
"""PS auto-approved for account 111 but not 222 → not in auto_approved set."""
589+
main = import_main
590+
ps = self._ps("MixedRole")
591+
592+
def side_effect(**kwargs):
593+
if kwargs.get("account_id") == "111":
594+
return self._decision(grant=True)
595+
return self._decision(grant=False)
596+
597+
with patch.object(main.access_control, "make_decision_on_access_request", side_effect=side_effect):
598+
result = main.classify_auto_approved_permission_sets(
599+
statements=frozenset(),
600+
permission_sets=[ps],
601+
account_ids=["111", "222"],
602+
requester_email="user@test.com",
603+
user_group_ids=set(),
604+
requester_slack_id="U123",
605+
)
606+
assert ps.arn not in result
607+
608+
def test_multiple_permission_sets_classified_independently(self, import_main):
609+
"""Each PS is classified independently — one can be auto, another manual."""
610+
main = import_main
611+
ps_auto = self._ps("ReadOnly")
612+
ps_manual = self._ps("Admin")
613+
614+
def side_effect(**kwargs):
615+
if kwargs.get("permission_set_name") == "ReadOnly":
616+
return self._decision(grant=True)
617+
return self._decision(grant=False)
618+
619+
with patch.object(main.access_control, "make_decision_on_access_request", side_effect=side_effect):
620+
result = main.classify_auto_approved_permission_sets(
621+
statements=frozenset(),
622+
permission_sets=[ps_auto, ps_manual],
623+
account_ids=["111"],
624+
requester_email="user@test.com",
625+
user_group_ids=set(),
626+
requester_slack_id="U123",
627+
)
628+
assert ps_auto.arn in result
629+
assert ps_manual.arn not in result

0 commit comments

Comments
 (0)