fivexl/terraform-aws-sso-elevator

Inconsistent Behavior with Self-Approve Permissions in Rules

EreminAnton opened this issue · 2 comments

If one rule explicitly states that resources require confirmation (allow self-approve: false), and another rule allows the user to self-approve for all accounts, the user still obtains the ability to self-approve. This behavior contradicts the settings where self-approve is explicitly denied. A setting of false should be prioritized. An explicit deny should always take precedence over an approve.

Have created a test case for the visualisation of this logic, will ensure that all corect and implement:

{
            "description": """If we have two statements, and one of them explicitly denies (with self_approval = False)
            while the other allows (with self_approval = True), then we should deny self_approval.""",
​
            "in": {
                "statements": frozenset(
​
                    # STATEMENTS IN USER CONFIGURATION
                    [
                        Statement.parse_obj(
                            {
                                "resource_type": "Account",
                                "resource": ["*"],
                                "permission_set": ["*"],
                                "approvers": [
                                    "CTO@test.com"
                                ],
                                # Allow self approval to all accounts
                                "allow_self_approval": True,
                                # If we have "allow_self_approval": false and * in resource
                                # Then self approval will be denied for everyone and everywhere because of explicit deny
                            }
                        ),
                        Statement.parse_obj(
                            {
                                "resource_type": "Account",
                                "resource": ["111111111111"],
                                "permission_set": ["AdministratorAccess"],
                                "approvers": [
                                    "Approver2@test.com"
                                ],
                                # Explicit deny self approval to Management account
                                # So even if CTO have access to Management account because of *
                                # he will not be able to self approve
                                "allow_self_approval": False,
                            }
                        ),
                    ]
                ),
​
                # USER REQUEST INPUT
                "account_id": "111111111111",
                "permission_set_name": "AdministratorAccess",
                "requester_email": "CTO@test.com",
            },
​
            # DECISSION: Requires approval because of explicit deny
            "out": AccessRequestDecision(
                grant=False,
                reason=DecisionReason.RequiresApproval,
                approvers=frozenset(["Approver2@test.com"]),
                based_on_statements=frozenset(
                    [
                        Statement.parse_obj(
                            {
                                "resource_type": "Account",
                                "resource": ["111111111111"],
                                "permission_set": ["AdministratorAccess"],
                                "approvers": [
                                    "Approver2@test.com",
                                ],
                                "allow_self_approval": False,
                            }
                        )
                    ]
                ),
            ),
        },