Merge pull request #11 from PostHog/tom/grousp

Add option to limit requests based on group membership

Author: Piccirello

README.md

@@ -716,6 +716,7 @@ settings:
 | <a name="module_audit_bucket"></a> [audit\_bucket](#module\_audit\_bucket) | fivexl/account-baseline/aws//modules/s3_baseline | 2.0.0 |
 | <a name="module_config_bucket"></a> [config\_bucket](#module\_config\_bucket) | fivexl/account-baseline/aws//modules/s3_baseline | 2.0.0 |
 | <a name="module_http_api"></a> [http\_api](#module\_http\_api) | terraform-aws-modules/apigateway-v2/aws | 5.0.0 |
+| <a name="module_slack_handler_alias"></a> [slack\_handler\_alias](#module\_slack\_handler\_alias) | terraform-aws-modules/lambda/aws//modules/alias | 8.1.2 |
 | <a name="module_sso_elevator_dependencies"></a> [sso\_elevator\_dependencies](#module\_sso\_elevator\_dependencies) | terraform-aws-modules/lambda/aws | 8.1.2 |
 
 ## Resources
@@ -731,7 +732,7 @@ settings:
 | [aws_iam_role.eventbridge_role](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role) | resource |
 | [aws_iam_role_policy.eventbridge_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy) | resource |
 | [aws_lambda_permission.eventbridge](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_permission) | resource |
-| [aws_lambda_permission.url](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_permission) | resource |
+| [aws_lambda_provisioned_concurrency_config.slack_handler](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lambda_provisioned_concurrency_config) | resource |
 | [aws_s3_object.approval_config](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_object) | resource |
 | [aws_scheduler_schedule_group.one_time_schedule_group](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/scheduler_schedule_group) | resource |
 | [aws_sns_topic.dlq](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/sns_topic) | resource |
@@ -749,10 +750,10 @@ settings:
 
 | Name | Description | Type | Default | Required |
 |------|-------------|------|---------|:--------:|
+| <a name="input_allow_anyone_to_end_session_early"></a> [allow\_anyone\_to\_end\_session\_early](#input\_allow\_anyone\_to\_end\_session\_early) | Controls who can click the "End session early" button to revoke access before the scheduled expiration.<br/>If false (default), only the requester and approvers listed in the matching statement can end the session.<br/>If true, anyone in the Slack channel can end any session early. | `bool` | `false` | no |
 | <a name="input_api_gateway_name"></a> [api\_gateway\_name](#input\_api\_gateway\_name) | The name of the API Gateway for SSO Elevator's access-requester Lambda | `string` | `"sso-elevator-access-requster"` | no |
 | <a name="input_api_gateway_throttling_burst_limit"></a> [api\_gateway\_throttling\_burst\_limit](#input\_api\_gateway\_throttling\_burst\_limit) | The maximum number of requests that API Gateway allows in a burst. | `number` | `5` | no |
 | <a name="input_api_gateway_throttling_rate_limit"></a> [api\_gateway\_throttling\_rate\_limit](#input\_api\_gateway\_throttling\_rate\_limit) | The maximum number of requests that API Gateway allows per second. | `number` | `1` | no |
-| <a name="input_allow_anyone_to_end_session_early"></a> [allow\_anyone\_to\_end\_session\_early](#input\_allow\_anyone\_to\_end\_session\_early) | Controls who can click the "End session early" button. If false (default), only the requester and approvers can end the session. If true, anyone in the channel can end any session. | `bool` | `false` | no |
 | <a name="input_approver_renotification_backoff_multiplier"></a> [approver\_renotification\_backoff\_multiplier](#input\_approver\_renotification\_backoff\_multiplier) | The multiplier applied to the wait time for each subsequent notification sent to the approver. Default is 2, which means the wait time will double for each attempt. | `number` | `2` | no |
 | <a name="input_approver_renotification_initial_wait_time"></a> [approver\_renotification\_initial\_wait\_time](#input\_approver\_renotification\_initial\_wait\_time) | The initial wait time before the first re-notification to the approver is sent. This is measured in minutes. If set to 0, no re-notifications will be sent. | `number` | `15` | no |
 | <a name="input_attribute_sync_enabled"></a> [attribute\_sync\_enabled](#input\_attribute\_sync\_enabled) | Enable attribute-based group sync feature. When enabled, users will be automatically added to groups based on their Identity Store attributes. | `bool` | `false` | no |
@@ -786,7 +787,6 @@ settings:
 | <a name="input_logs_retention_in_days"></a> [logs\_retention\_in\_days](#input\_logs\_retention\_in\_days) | The number of days you want to retain log events in the log group for both Lambda functions and API Gateway. | `number` | `365` | no |
 | <a name="input_max_permissions_duration_time"></a> [max\_permissions\_duration\_time](#input\_max\_permissions\_duration\_time) | Maximum duration (in hours) for permissions granted by Elevator. Max number - 48 hours.<br/>  Due to Slack's dropdown limit of 100 items, anything above 48 hours will cause issues when generating half-hour increments<br/>  and Elevator will not display more then 48 hours in the dropdown. | `number` | `24` | no |
 | <a name="input_permission_duration_list_override"></a> [permission\_duration\_list\_override](#input\_permission\_duration\_list\_override) | An explicit list of duration values to appear in the drop-down menu users use to select how long to request permissions for.<br/>  Each entry in the list should be formatted as "hh:mm", e.g. "01:30" for an hour and a half. Note that while the number of minutes<br/>  must be between 0-59, the number of hours can be any number.<br/>  If this variable is set, the max\_permission\_duration\_time is ignored. | `list(string)` | `[]` | no |
-| <a name="input_provisioned_concurrent_executions"></a> [provisioned\_concurrent\_executions](#input\_provisioned\_concurrent\_executions) | Provisioned concurrent executions for the Slack handler Lambda. Set to a positive number to reduce cold starts. | `number` | `-1` | no |
 | <a name="input_request_expiration_hours"></a> [request\_expiration\_hours](#input\_request\_expiration\_hours) | After how many hours should the request expire? If set to 0, the request will never expire. | `number` | `8` | no |
 | <a name="input_requester_lambda_name"></a> [requester\_lambda\_name](#input\_requester\_lambda\_name) | value for the requester lambda name | `string` | `"access-requester"` | no |
 | <a name="input_revoker_lambda_name"></a> [revoker\_lambda\_name](#input\_revoker\_lambda\_name) | value for the revoker lambda name | `string` | `"access-revoker"` | no |
@@ -806,6 +806,7 @@ settings:
 | <a name="input_send_dm_if_user_not_in_channel"></a> [send\_dm\_if\_user\_not\_in\_channel](#input\_send\_dm\_if\_user\_not\_in\_channel) | If the user is not in the SSO Elevator channel, Elevator will send them a direct message with the request status <br/>(waiting for approval, declined, approved, etc.) and the result of the request.<br/>Using this feature requires the following Slack app permissions: "channels:read", "groups:read", and "im:write". <br/>Please ensure these permissions are enabled in the Slack app configuration. | `bool` | `true` | no |
 | <a name="input_slack_bot_token"></a> [slack\_bot\_token](#input\_slack\_bot\_token) | value for the Slack bot token | `string` | n/a | yes |
 | <a name="input_slack_channel_id"></a> [slack\_channel\_id](#input\_slack\_channel\_id) | value for the Slack channel ID | `string` | n/a | yes |
+| <a name="input_slack_handler_provisioned_concurrent_executions"></a> [slack\_handler\_provisioned\_concurrent\_executions](#input\_slack\_handler\_provisioned\_concurrent\_executions) | Provisioned concurrent executions for the Slack handler Lambda. Set to a positive number to reduce cold starts. | `number` | `-1` | no |
 | <a name="input_slack_signing_secret"></a> [slack\_signing\_secret](#input\_slack\_signing\_secret) | value for the Slack signing secret | `string` | n/a | yes |
 | <a name="input_sso_instance_arn"></a> [sso\_instance\_arn](#input\_sso\_instance\_arn) | value for the SSO instance ARN | `string` | `""` | no |
 | <a name="input_tags"></a> [tags](#input\_tags) | A map of tags to assign to resources. | `map(string)` | `{}` | no |

src/access_control.py

@@ -10,7 +10,7 @@
 import schedule
 import sso
 from entities import BaseModel
-from statement import GroupStatement, Statement, get_affected_group_statements, get_affected_statements
+from statement import GroupStatement, Statement, get_affected_group_statements, get_affected_statements, get_eligible_statements_for_user
 
 logger = config.get_logger("access_control")
 cfg = config.get_config()
@@ -55,13 +55,31 @@ def determine_affected_statements(
     raise TypeError("Statements contain mixed or unsupported types.")
 
 
-def make_decision_on_access_request(  # noqa: PLR0911
+def make_decision_on_access_request(  # noqa: PLR0911, PLR0913
     statements: FrozenSet[Statement] | FrozenSet[GroupStatement],
     requester_email: str,
     permission_set_name: str | None = None,
     account_id: str | None = None,
     group_id: str | None = None,
+    user_group_ids: set[str] | None = None,
 ) -> AccessRequestDecision:
+    """Make a decision on an access request.
+
+    Args:
+        statements: The statements to evaluate.
+        requester_email: The email of the user requesting access.
+        permission_set_name: The name of the permission set being requested.
+        account_id: The AWS account ID being requested.
+        group_id: The group ID for group-based access requests.
+        user_group_ids: The set of SSO group IDs the user belongs to.
+            If None, group-based filtering is skipped (backwards compatible).
+            If an empty set, only statements without required_group_membership will match.
+    """
+    # Filter statements by user's group membership eligibility if user_group_ids provided
+    # This is only applicable to Statement (not GroupStatement)
+    if user_group_ids is not None and isinstance(statements, frozenset) and all(isinstance(s, Statement) for s in statements):
+        statements = get_eligible_statements_for_user(statements, user_group_ids)  # type: ignore # noqa: PGH003
+
     affected_statements = determine_affected_statements(statements, account_id, permission_set_name, group_id)
 
     decision_based_on_statements: set[Statement] | set[GroupStatement] = set()

src/config.py

@@ -82,6 +82,7 @@ def to_set_if_list_or_str(v: list | str) -> frozenset[str]:
             "resource_type": _dict.get("ResourceType"),
             "approval_is_not_required": _dict.get("ApprovalIsNotRequired"),
             "allow_self_approval": _dict.get("AllowSelfApproval"),
+            "required_group_membership": to_set_if_list_or_str(_dict.get("RequiredGroupMembership", set())),
         }
     )
 

src/group.py

@@ -29,7 +29,7 @@
 
 
 @handle_errors
-def handle_request_for_group_access_submittion(
+def handle_request_for_group_access_submittion(  # noqa: PLR0915
     body: dict,
     ack: Ack,  # noqa: ARG001
     client: WebClient,

src/main.py

@@ -145,13 +145,49 @@ def load_select_options_for_account_access_request(client: WebClient, body: dict
     logger.info("Loading select options for view (accounts only)")
     logger.debug("Request body", extra={"body": body})
 
-    accounts = organizations.get_accounts_from_config_with_cache(org_client=org_client, s3_client=s3_client, cfg=cfg)
-
     user_id = body.get("user", {}).get("id")
     callback_id = slack_helpers.RequestForAccessView.CALLBACK_ID
     view_key = f"{user_id}:{callback_id}"
 
+    # Get user's SSO info and group memberships for filtering
+    sso_instance = sso.describe_sso_instance(sso_client, cfg.sso_instance_arn)
+    user_email = slack_helpers.get_user(client, id=user_id).email
+    user_principal_id, _ = sso.get_user_principal_id_by_email(
+        identity_store_client=identity_store_client,
+        identity_store_id=sso_instance.identity_store_id,
+        email=user_email,
+        cfg=cfg,
+    )
+    user_group_ids = sso.get_user_group_ids(
+        identity_store_client=identity_store_client,
+        identity_store_id=sso_instance.identity_store_id,
+        user_principal_id=user_principal_id,
+    )
+
+    # Cache user_group_ids for use in handle_account_selection
+    user_view_map[f"{view_key}:group_ids"] = user_group_ids
+
+    # Filter accounts based on user's eligible statements
+    eligible_account_ids = statement.get_accounts_for_user(cfg.statements, user_group_ids)
+
     view_id = user_view_map.get(view_key)
+
+    # If no eligible accounts, show empty view
+    if not eligible_account_ids:
+        logger.info("User has no eligible accounts", extra={"user_id": user_id})
+        view = slack_helpers.RequestForAccessView.build_no_eligible_accounts_view()
+        if view_id:
+            return client.views_update(view_id=view_id, view=view)
+        trigger_id = body["trigger_id"]
+        return client.views_open(trigger_id=trigger_id, view=view)
+
+    # Get all accounts and filter to eligible ones
+    all_accounts = organizations.get_accounts_from_config_with_cache(org_client=org_client, s3_client=s3_client, cfg=cfg)
+    if "*" in eligible_account_ids:
+        accounts = all_accounts
+    else:
+        accounts = [a for a in all_accounts if a.id in eligible_account_ids]
+
     if not view_id:
         logger.warning(
             f"View ID not found for key: {view_key}. "
@@ -350,15 +386,35 @@ def handle_request_for_access_submittion(  # noqa: PLR0915, PLR0912
     request = slack_helpers.RequestForAccessView.parse(body)
     logger.info("View submitted", extra={"view": request})
     requester = slack_helpers.get_user(client, id=request.requester_slack_id)
+
+    # Get user's group memberships for eligibility check (defense in depth)
+    sso_instance = sso.describe_sso_instance(sso_client, cfg.sso_instance_arn)
+    user_principal_id, _ = sso.get_user_principal_id_by_email(
+        identity_store_client=identity_store_client,
+        identity_store_id=sso_instance.identity_store_id,
+        email=requester.email,
+        cfg=cfg,
+    )
+    user_group_ids = sso.get_user_group_ids(
+        identity_store_client=identity_store_client,
+        identity_store_id=sso_instance.identity_store_id,
+        user_principal_id=user_principal_id,
+    )
+
     decision = access_control.make_decision_on_access_request(
         cfg.statements,
         account_id=request.account_id,
         permission_set_name=request.permission_set_name,
         requester_email=requester.email,
+        user_group_ids=user_group_ids,
     )
     logger.info("Decision on request was made", extra={"decision": decision.dict()})
 
-    account = organizations.describe_account(org_client, request.account_id)
+    try:
+        account = organizations.describe_account(org_client, request.account_id)
+    except Exception:
+        logger.warning("Failed to describe account, using account ID as fallback", extra={"account_id": request.account_id})
+        account = entities.aws.Account(id=request.account_id, name=request.account_id)
 
     show_buttons = bool(decision.approvers)
     slack_response = client.chat_postMessage(
@@ -535,14 +591,27 @@ def handle_account_selection(ack: Ack, body: dict, client: WebClient) -> SlackRe
     )
     logger.info(f"Selected account: {account_id}")
 
-    valid_ps_names = statement.get_permission_sets_for_account(cfg.statements, account_id)
-    logger.info(f"Valid permission sets for account: {valid_ps_names}")
+    # Get cached user_group_ids
+    user_id = body.get("user", {}).get("id")
+    callback_id = slack_helpers.RequestForAccessView.CALLBACK_ID
+    view_key = f"{user_id}:{callback_id}"
+    group_ids_key = f"{view_key}:group_ids"
+    user_group_ids = user_view_map.get(group_ids_key)
+    if user_group_ids is None:
+        logger.warning(
+            f"User group IDs not found in cache for key: {group_ids_key}. "
+            "This may happen if Lambda container was recycled between form load and account selection. "
+            "Defaulting to empty set, which will restrict visibility to statements without required_group_membership."
+        )
+        user_group_ids = set()
+
+    # Filter permission sets based on user's eligible statements
+    valid_ps_names = statement.get_permission_sets_for_account_and_user(cfg.statements, account_id, user_group_ids)
+    logger.info(f"Valid permission sets for account and user: {valid_ps_names}")
 
     if not valid_ps_names:
         view_id = body["view"]["id"]
-        updated_view = slack_helpers.RequestForAccessView.build_no_permission_sets_view(
-            view_blocks=body["view"]["blocks"]
-        )
+        updated_view = slack_helpers.RequestForAccessView.build_no_permission_sets_view(view_blocks=body["view"]["blocks"])
         return client.views_update(view_id=view_id, view=updated_view)
 
     if "*" in valid_ps_names:
@@ -554,9 +623,7 @@ def handle_account_selection(ack: Ack, body: dict, client: WebClient) -> SlackRe
     # Handle case where filtered list is empty (configured names don't exist in SSO)
     if not permission_sets:
         view_id = body["view"]["id"]
-        updated_view = slack_helpers.RequestForAccessView.build_no_permission_sets_view(
-            view_blocks=body["view"]["blocks"]
-        )
+        updated_view = slack_helpers.RequestForAccessView.build_no_permission_sets_view(view_blocks=body["view"]["blocks"])
         return client.views_update(view_id=view_id, view=updated_view)
 
     view_id = body["view"]["id"]
@@ -654,11 +721,13 @@ def handle_early_revoke_button_click(body: dict, client: WebClient, context: Bol
         except Exception:
             account_name = button_payload.account_id
 
-        private_metadata = json.dumps({
-            "button_payload": button_payload.model_dump(mode="json"),
-            "channel_id": channel_id,
-            "thread_ts": thread_ts,
-        })
+        private_metadata = json.dumps(
+            {
+                "button_payload": button_payload.model_dump(mode="json"),
+                "channel_id": channel_id,
+                "thread_ts": thread_ts,
+            }
+        )
 
         modal = slack_helpers.EarlyRevokeModal.build(
             account_name=account_name,
@@ -668,11 +737,13 @@ def handle_early_revoke_button_click(body: dict, client: WebClient, context: Bol
         )
     elif button_payload.group_id and button_payload.group_name:
         # Group access
-        private_metadata = json.dumps({
-            "button_payload": button_payload.model_dump(mode="json"),
-            "channel_id": channel_id,
-            "thread_ts": thread_ts,
-        })
+        private_metadata = json.dumps(
+            {
+                "button_payload": button_payload.model_dump(mode="json"),
+                "channel_id": channel_id,
+                "thread_ts": thread_ts,
+            }
+        )
 
         modal = slack_helpers.EarlyRevokeModal.build(
             group_name=button_payload.group_name,