revert broken SSO policy changes (#2500)

timburke-hackit · web-flow · commit 3c3514faed3c · 2025-10-10T17:15:10.000+01:00
Reverts commits: - f8d1a5c Fix policy length (#2499) - e84e303 Fix glue database visibility (#2498) - 7996f9f add GetTags action (#2496) - ed4f4fe add GetPartition(s) actions (#2495) - 31d991a add required actions to additional databases (#2494) - 1dd0562 add GetDatabases to Glue Catalog permissions (#2493) - 26d3805 update glue policies (#2491)
diff --git a/terraform/core/05-departments.tf b/terraform/core/05-departments.tf
@@ -73,24 +73,6 @@ module "department_parking" {
   departmental_airflow_user       = true
   mwaa_etl_scripts_bucket_arn     = aws_s3_bucket.mwaa_etl_scripts_bucket.arn
   mwaa_key_arn                    = aws_kms_key.mwaa_key.arn
-  additional_glue_database_access = [
-    {
-      database_name = "${local.identifier_prefix}-liberator-raw-zone"
-      actions       = ["glue:GetTable", "glue:GetTables"]
-    },
-    {
-      database_name = "${local.identifier_prefix}-liberator-refined-zone"
-      actions       = ["glue:GetTable", "glue:GetTables"]
-    },
-    {
-      database_name = "${local.identifier_prefix}-liberator-trusted-zone"
-      actions       = ["glue:GetTable", "glue:GetTables"]
-    },
-    {
-      database_name = "${local.identifier_prefix}-raw-zone-unrestricted-address-api"
-      actions       = ["glue:GetTable", "glue:GetTables"]
-    },
-  ]
 }
 
 module "department_finance" {
diff --git a/terraform/modules/department/02-inputs-optional.tf b/terraform/modules/department/02-inputs-optional.tf
@@ -72,13 +72,6 @@ variable "additional_glue_database_access" {
   description = <<EOF
     Additional Glue database access to grant to the department.
     Allows specifying specific databases and the actions that can be performed on them.
-    
-    Note: The actions 'glue:GetDatabase', 'glue:GetDatabases', 'glue:GetPartition' and 'glue:GetPartitions' are automatically 
-    appended to the actions list to ensure databases appear in SQL editors and can be 
-    accessed. You only need to specify additional actions like table operations:
-    - glue:GetTable, glue:GetTables
-    - glue:CreateTable, glue:UpdateTable, glue:DeleteTable (for write access)
-    - glue:CreatePartition, glue:UpdatePartition, glue:DeletePartition (for write access)
   EOF
   type = list(object({
     database_name = string
diff --git a/terraform/modules/department/50-aws-iam-policies.tf b/terraform/modules/department/50-aws-iam-policies.tf
@@ -167,7 +167,7 @@ data "aws_iam_policy_document" "read_only_glue_access" {
 
   // Glue Access - Catalog level operations
   statement {
-    sid = "GlueCatalogReadOnlyAccess"
+    sid = "GlueCatalogAccess"
     actions = [
       "glue:GetCatalogImportStatus",
       "glue:GetDataCatalogEncryptionSettings",
@@ -179,10 +179,9 @@ data "aws_iam_policy_document" "read_only_glue_access" {
 
   // Glue Access - Department database and table operations
   statement {
-    sid = "GlueDepartmentDatabaseReadOnlyAccess"
+    sid = "GlueDepartmentDatabaseAccess"
     actions = [
       "glue:GetDatabase",
-      "glue:GetDatabases",
       "glue:GetTable",
       "glue:GetTables",
       "glue:GetTableVersion",
@@ -202,10 +201,6 @@ data "aws_iam_policy_document" "read_only_glue_access" {
       "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/${aws_glue_catalog_database.raw_zone_catalog_database.name}/*",
       "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/${aws_glue_catalog_database.refined_zone_catalog_database.name}/*",
       "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/${aws_glue_catalog_database.trusted_zone_catalog_database.name}/*",
-      "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/unrestricted-raw-zone/*",
-      "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/unrestricted-refined-zone/*",
-      "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/unrestricted-trusted-zone/*",
-
     ]
   }
 
@@ -227,7 +222,6 @@ data "aws_iam_policy_document" "read_only_glue_access" {
       "glue:GetWorkflowRun",
       "glue:GetWorkflowRuns",
       "glue:ListWorkflows",
-      "glue:GetTags",
       "glue:CheckSchemaVersionValidity",
     ]
     resources = ["*"]
@@ -237,18 +231,9 @@ data "aws_iam_policy_document" "read_only_glue_access" {
     for_each = var.additional_glue_database_access
     iterator = additional_db_access
     content {
-      sid    = "AdditionalGlueDatabaseAccess${replace(additional_db_access.value.database_name, "/[^a-zA-Z0-9]/", "")}"
-      effect = "Allow"
-      # Auto-append essential actions for database listing and access
-      actions = distinct(concat(
-        additional_db_access.value.actions,
-        [
-          "glue:GetDatabase",  # Required for specific database access
-          "glue:GetDatabases", # Required for SQL editor database listing
-          "glue:GetPartition",
-          "glue:GetPartitions",
-        ]
-      ))
+      sid     = "AdditionalGlueDatabaseAccess${replace(additional_db_access.value.database_name, "/[^a-zA-Z0-9]/", "")}"
+      effect  = "Allow"
+      actions = additional_db_access.value.actions
       resources = [
         "arn:aws:glue:eu-west-2:${data.aws_caller_identity.current.account_id}:catalog",
         "arn:aws:glue:eu-west-2:${data.aws_caller_identity.current.account_id}:database/${additional_db_access.value.database_name}",
@@ -273,7 +258,7 @@ data "aws_iam_policy_document" "s3_department_access" {
   ] : []
 
   statement {
-    sid    = "ListAllS3AndKmsKeysFullAccess"
+    sid    = "ListAllS3AndKmsKeys"
     effect = "Allow"
     actions = [
       "s3:ListAllMyBuckets",
@@ -283,7 +268,7 @@ data "aws_iam_policy_document" "s3_department_access" {
   }
 
   statement {
-    sid    = "KmsKeyFullAccessForS3"
+    sid    = "KmsKeyFullAccess"
     effect = "Allow"
     actions = [
       "kms:Encrypt",
@@ -545,7 +530,7 @@ data "aws_iam_policy_document" "glue_access" {
 
   // Glue Access - Catalog level operations
   statement {
-    sid = "GlueCatalogFullAccess"
+    sid = "GlueCatalogAccess"
     actions = [
       "glue:GetCatalogImportStatus",
       "glue:GetDataCatalogEncryptionSettings",
@@ -557,10 +542,9 @@ data "aws_iam_policy_document" "glue_access" {
 
   // Glue Access - Department database and table operations (read, write, delete)
   statement {
-    sid = "GlueDepartmentDatabaseFullAccess"
+    sid = "GlueDepartmentDatabaseAccess"
     actions = [
       "glue:GetDatabase",
-      "glue:GetDatabases",
       "glue:GetTable",
       "glue:GetTables",
       "glue:GetTableVersion",
@@ -589,9 +573,6 @@ data "aws_iam_policy_document" "glue_access" {
       "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/${aws_glue_catalog_database.raw_zone_catalog_database.name}/*",
       "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/${aws_glue_catalog_database.refined_zone_catalog_database.name}/*",
       "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/${aws_glue_catalog_database.trusted_zone_catalog_database.name}/*",
-      "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/unrestricted-raw-zone/*",
-      "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/unrestricted-refined-zone/*",
-      "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/unrestricted-trusted-zone/*",
     ]
   }
 
@@ -628,7 +609,6 @@ data "aws_iam_policy_document" "glue_access" {
       "glue:StopCrawlerSchedule",
       "glue:StopTrigger",
       "glue:StopWorkflowRun",
-      "glue:GetTags",
       "glue:TagResource",
       "glue:UpdateDevEndpoint",
       "glue:UpdateJob",
@@ -639,30 +619,6 @@ data "aws_iam_policy_document" "glue_access" {
     ]
     resources = ["*"]
   }
-
-  dynamic "statement" {
-    for_each = var.additional_glue_database_access
-    iterator = additional_db_access
-    content {
-      sid    = "AdditionalGlueDatabaseFullAccess${replace(additional_db_access.value.database_name, "/[^a-zA-Z0-9]/", "")}"
-      effect = "Allow"
-      # Auto-append essential actions for database listing and access
-      actions = distinct(concat(
-        additional_db_access.value.actions,
-        [
-          "glue:GetDatabase",  # Required for specific database access
-          "glue:GetDatabases", # Required for SQL editor database listing
-          "glue:GetPartition",
-          "glue:GetPartitions",
-        ]
-      ))
-      resources = [
-        "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:catalog",
-        "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:database/${additional_db_access.value.database_name}",
-        "arn:aws:glue:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:table/${additional_db_access.value.database_name}/*"
-      ]
-    }
-  }
 }
 
 resource "aws_iam_policy" "glue_access" {
diff --git a/terraform/modules/department/50-aws-iam-roles.tf b/terraform/modules/department/50-aws-iam-roles.tf
@@ -1,25 +1,27 @@
-data "aws_iam_policy_document" "sso_staging_additional_with_notebook" {
-  count = local.create_notebook ? 1 : 0
-
-  override_policy_documents = [
+// User Role for staging account - This role is a combination of policies ready to be applied to SSO
+data "aws_iam_policy_document" "sso_staging_user_policy" {
+  override_policy_documents = local.create_notebook ? [
+    data.aws_iam_policy_document.s3_department_access.json,
+    data.aws_iam_policy_document.glue_access.json,
+    data.aws_iam_policy_document.secrets_manager_read_only.json,
     data.aws_iam_policy_document.redshift_department_read_access.json,
     data.aws_iam_policy_document.notebook_access[0].json
-  ]
-}
-
-data "aws_iam_policy_document" "sso_staging_additional" {
-  count = local.create_notebook ? 0 : 1
-
-  override_policy_documents = [
+    ] : [
+    data.aws_iam_policy_document.s3_department_access.json,
+    data.aws_iam_policy_document.glue_access.json,
+    data.aws_iam_policy_document.secrets_manager_read_only.json,
     data.aws_iam_policy_document.redshift_department_read_access.json,
     data.aws_iam_policy_document.mwaa_department_web_server_access.json
   ]
 }
 
-data "aws_iam_policy_document" "sso_production_additional" {
+// User Role for production account - This role is a combination of policies ready to be applied to SSO
+data "aws_iam_policy_document" "sso_production_user_policy" {
   override_policy_documents = [
-    data.aws_iam_policy_document.athena_can_write_to_s3.json,
-    data.aws_iam_policy_document.redshift_department_read_access.json
+    data.aws_iam_policy_document.read_only_s3_department_access.json,
+    data.aws_iam_policy_document.read_only_glue_access.json,
+    data.aws_iam_policy_document.secrets_manager_read_only.json,
+    data.aws_iam_policy_document.athena_can_write_to_s3.json
   ]
 }
 
diff --git a/terraform/modules/department/60-aws-sso.tf b/terraform/modules/department/60-aws-sso.tf
@@ -15,59 +15,14 @@ resource "aws_ssoadmin_permission_set" "department" {
   tags             = var.tags
 }
 
-resource "aws_ssoadmin_managed_policy_attachment" "department_s3" {
+resource "aws_ssoadmin_permission_set_inline_policy" "department" {
   count = local.deploy_sso ? 1 : 0
 
   provider = aws.aws_hackit_account
 
+  inline_policy      = var.environment == "stg" ? data.aws_iam_policy_document.sso_staging_user_policy.json : data.aws_iam_policy_document.sso_production_user_policy.json
   instance_arn       = var.sso_instance_arn
   permission_set_arn = aws_ssoadmin_permission_set.department[0].arn
-  managed_policy_arn = var.environment == "stg" ? aws_iam_policy.s3_access.arn : aws_iam_policy.read_only_s3_access.arn
-}
-
-resource "aws_ssoadmin_managed_policy_attachment" "department_glue" {
-  count = local.deploy_sso ? 1 : 0
-
-  provider = aws.aws_hackit_account
-
-  instance_arn       = var.sso_instance_arn
-  permission_set_arn = aws_ssoadmin_permission_set.department[0].arn
-  managed_policy_arn = var.environment == "stg" ? aws_iam_policy.glue_access.arn : aws_iam_policy.read_only_glue_access.arn
-}
-
-resource "aws_ssoadmin_managed_policy_attachment" "department_secrets" {
-  count = local.deploy_sso ? 1 : 0
-
-  provider = aws.aws_hackit_account
-
-  instance_arn       = var.sso_instance_arn
-  permission_set_arn = aws_ssoadmin_permission_set.department[0].arn
-  managed_policy_arn = aws_iam_policy.secrets_manager_read_only.arn
-}
-
-resource "aws_iam_policy" "sso_department_additional_policy" {
-  count = local.deploy_sso ? 1 : 0
-
-  name        = lower("${var.identifier_prefix}-${local.department_identifier}-sso-additional-policy")
-  description = "Additional SSO policy for ${local.department_identifier} department in ${var.environment} environment"
-  policy = var.environment == "stg" ? (
-    local.create_notebook ?
-    data.aws_iam_policy_document.sso_staging_additional_with_notebook[0].json :
-    data.aws_iam_policy_document.sso_staging_additional[0].json
-    ) : (
-    data.aws_iam_policy_document.sso_production_additional.json
-  )
-  tags = var.tags
-}
-
-resource "aws_ssoadmin_managed_policy_attachment" "department_additional" {
-  count = local.deploy_sso ? 1 : 0
-
-  provider = aws.aws_hackit_account
-
-  instance_arn       = var.sso_instance_arn
-  permission_set_arn = aws_ssoadmin_permission_set.department[0].arn
-  managed_policy_arn = aws_iam_policy.sso_department_additional_policy[0].arn
 }
 
 data "aws_identitystore_group" "department" {
