From 4a99b71907b67167818024bec3ccd7a332070ab6 Mon Sep 17 00:00:00 2001 From: Roy Osherove <575051+royosherove@users.noreply.github.com> Date: Sun, 16 Aug 2026 14:15:01 +0000 Subject: [PATCH] fix(webui-auth): address P1/P2 review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1 fixes: - Domain uniqueness: append AWS::AccountId to Cognito hosted-UI domain prefix (was 'lowkey-${PackName}-${EnvironmentName}' — could collide globally per region). Now 'lowkey-${PackName}-${EnvironmentName}-${AWS::AccountId}'. - Rules assertion: reject stack create when EnableWebUIAuth=true but WebUIAdminEmail is empty. Fails at validation, not mid-deploy. P2 fixes: - Password → Secrets Manager: replaces CFN output WebUIAdminPassword with WebUIAdminSecretArn. New AWS::SecretsManager::Secret resource holds {email, password}. Lambda writes via PutSecretValue, IAM policy scoped to that specific secret. Aligns with 'ALL secrets in Secrets Manager' workspace rule. - Update event handling: Lambda now handles RequestType=Update. If AdminEmail changed on update, old user is deleted first. Otherwise password is regenerated and both Cognito + Secrets Manager are refreshed. - Existing user recovery: UsernameExistsException no longer silently succeeds with '(password unchanged)'. Falls through to admin_set_user_password so Secrets Manager stays authoritative. - Log safety: no password in CloudWatch. Event print excludes ResourceProperties; exception messages truncated to 180 chars. P3 cleanup: - Removed dead --webui-pool-id CLI flag (installer no longer uses existing pools — CFN always creates one). Installer: - Post-deploy fetches password from Secrets Manager via GetSecretValue instead of reading from CFN output. - Display box shows Secret ARN so operator can retrieve later. Validated: aws cloudformation validate-template passes (38 params, CAPABILITY_NAMED_IAM). bash -n install.sh passes. Zero 'aws cognito-idp' calls remaining in installer. --- deploy/cloudformation/template.yaml | 142 ++++++++++++++++++++-------- install.sh | 24 ++--- 2 files changed, 114 insertions(+), 52 deletions(-) diff --git a/deploy/cloudformation/template.yaml b/deploy/cloudformation/template.yaml index 5566c9b..d8b1303 100644 --- a/deploy/cloudformation/template.yaml +++ b/deploy/cloudformation/template.yaml @@ -396,6 +396,12 @@ Rules: - Assert: !Not [!Equals [!Ref ExistingSubnetId2, !Ref ExistingSubnetId]] AssertDescription: "ExistingSubnetId2 must be different from ExistingSubnetId (different AZs)." + WebUIAdminEmailRequired: + RuleCondition: !Equals [!Ref EnableWebUIAuth, 'true'] + Assertions: + - Assert: !Not [!Equals [!Ref WebUIAdminEmail, '']] + AssertDescription: "WebUIAdminEmail is required when EnableWebUIAuth is true." + # ============================================================================ # CONDITIONS # ============================================================================ @@ -1440,7 +1446,7 @@ Resources: Condition: EnableWebUI Properties: UserPoolId: !Ref WebUIUserPool - Domain: !Sub 'lowkey-${PackName}-${EnvironmentName}' + Domain: !Sub 'lowkey-${PackName}-${EnvironmentName}-${AWS::AccountId}' WebUIUserPoolClient: Type: AWS::Cognito::UserPoolClient @@ -1476,6 +1482,21 @@ Resources: IdToken: hours RefreshToken: days + WebUIAdminSecret: + Type: AWS::SecretsManager::Secret + Condition: EnableWebUI + Properties: + Name: !Sub '/lowkey/${EnvironmentName}/webui-admin' + Description: !Sub 'Initial WebUI admin credentials for ${EnvironmentName}' + SecretString: !Sub '{"email":"${WebUIAdminEmail}","password":"pending-lambda-write"}' + Tags: + - Key: loki:managed + Value: 'true' + - Key: loki:pack + Value: !Ref PackName + - Key: loki:env + Value: !Ref EnvironmentName + WebUIUserCreationRole: Type: AWS::IAM::Role Condition: EnableWebUI @@ -1499,7 +1520,13 @@ Resources: Action: - cognito-idp:AdminCreateUser - cognito-idp:AdminSetUserPassword + - cognito-idp:AdminDeleteUser Resource: !GetAtt WebUIUserPool.Arn + - Effect: Allow + Action: + - secretsmanager:PutSecretValue + - secretsmanager:UpdateSecret + Resource: !Ref WebUIAdminSecret WebUIUserCreationFunction: Type: AWS::Lambda::Function @@ -1510,14 +1537,18 @@ Resources: Handler: index.handler Timeout: 60 Role: !GetAtt WebUIUserCreationRole.Arn + Environment: + Variables: + SECRET_ARN: !Ref WebUIAdminSecret Code: ZipFile: | - import json, urllib.request, secrets, string, boto3 + import json, os, urllib.request, secrets, string, boto3 - def send_response(event, context, status, reason='', data={}): + def send_response(event, context, status, reason='', data=None): + data = data or {} reason_str = (reason or f'See CW: {context.log_stream_name}')[:256] - phys_id = (context.log_stream_name or 'custom-resource')[-64:] - safe_data = {k: str(v)[:128] for k, v in (data or {}).items()} + phys_id = (event.get('PhysicalResourceId') or context.log_stream_name or 'custom-resource')[-64:] + safe_data = {k: str(v)[:128] for k, v in data.items()} body = json.dumps({ 'Status': status, 'Reason': reason_str, 'PhysicalResourceId': phys_id, @@ -1541,24 +1572,13 @@ Resources: secrets.SystemRandom().shuffle(pwd) return ''.join(pwd) - def handler(event, context): - print(f'[INFO] Event: {json.dumps(event)}') - if event['RequestType'] == 'Delete': - send_response(event, context, 'SUCCESS', 'Delete is a no-op') - return - - props = event.get('ResourceProperties', {}) - pool_id = props.get('UserPoolId', '') - email = props.get('AdminEmail', '') - region = props.get('Region', 'us-east-1') - - if not pool_id or not email: - send_response(event, context, 'FAILED', 'Missing UserPoolId or AdminEmail') - return - - cognito = boto3.client('cognito-idp', region_name=region) - password = generate_password() + def write_secret(sm, secret_arn, email, password): + sm.put_secret_value( + SecretId=secret_arn, + SecretString=json.dumps({'email': email, 'password': password}) + ) + def create_or_reset_user(cognito, pool_id, email, password): try: cognito.admin_create_user( UserPoolId=pool_id, @@ -1570,26 +1590,68 @@ Resources: MessageAction='SUPPRESS' ) except cognito.exceptions.UsernameExistsException: - send_response(event, context, 'SUCCESS', 'User already exists', - {'Email': email, 'Password': '(existing user - password unchanged)'}) + # User exists (e.g. stack update or re-run); reset the password + # so the credentials in Secrets Manager remain valid. + pass + cognito.admin_set_user_password( + UserPoolId=pool_id, + Username=email, + Password=*** + Permanent=True + ) + + def delete_user_safely(cognito, pool_id, email): + if not email: return + try: + cognito.admin_delete_user(UserPoolId=pool_id, Username=email) + except cognito.exceptions.UserNotFoundException: + pass except Exception as e: - send_response(event, context, 'FAILED', f'User creation failed: {str(e)[:200]}') - return + print(f'[WARN] Could not delete old user {email}: {e}') + def handler(event, context): + # Log event WITHOUT ResourceProperties (which may contain email) + print(f"[INFO] RequestType={event.get('RequestType')} LogicalId={event.get('LogicalResourceId')}") try: - cognito.admin_set_user_password( - UserPoolId=pool_id, - Username=email, - Password=password, - Permanent=True - ) - except Exception as e: - send_response(event, context, 'FAILED', f'Password set failed: {str(e)[:200]}') - return + if event['RequestType'] == 'Delete': + # Secret and user pool are deleted by CFN — no manual cleanup needed + send_response(event, context, 'SUCCESS', 'Delete is a no-op') + return + + props = event.get('ResourceProperties', {}) + old_props = event.get('OldResourceProperties', {}) + pool_id = props.get('UserPoolId', '') + email = props.get('AdminEmail', '') + secret_arn = os.environ.get('SECRET_ARN', '') + region = props.get('Region', os.environ.get('AWS_REGION', 'us-east-1')) + + if not pool_id or not email or not secret_arn: + send_response(event, context, 'FAILED', + 'Missing UserPoolId, AdminEmail, or SECRET_ARN env') + return + + cognito = boto3.client('cognito-idp', region_name=region) + sm = boto3.client('secretsmanager', region_name=region) + + # On Update, if email changed, delete the old user first + if event['RequestType'] == 'Update': + old_email = old_props.get('AdminEmail', '') + if old_email and old_email != email: + delete_user_safely(cognito, pool_id, old_email) - send_response(event, context, 'SUCCESS', 'Initial user created', - {'Email': email, 'Password': password}) + password = ***) + create_or_reset_user(cognito, pool_id, email, password) + write_secret(sm, secret_arn, email, password) + + # Return only non-sensitive data; password is in Secrets Manager + send_response(event, context, 'SUCCESS', 'Admin user provisioned', + {'Email': email, 'SecretArn': secret_arn}) + except Exception as e: + # Never log the password; only the exception class + short message + err = f'{type(e).__name__}: {str(e)[:180]}' + print(f'[ERROR] {err}') + send_response(event, context, 'FAILED', err) WebUIUserCreationResource: Type: Custom::WebUIUserCreation @@ -1816,8 +1878,8 @@ Outputs: Description: Email of the initial WebUI admin user Value: !Ref WebUIAdminEmail - WebUIAdminPassword: + WebUIAdminSecretArn: Condition: EnableWebUI - Description: One-time initial password for the WebUI admin user — save immediately, not retrievable later - Value: !GetAtt WebUIUserCreationResource.Password + Description: ARN of Secrets Manager secret holding initial WebUI admin credentials (email + password). Rotate/delete after first use. + Value: !Ref WebUIAdminSecret diff --git a/install.sh b/install.sh index fcb7c82..7f0b4ae 100755 --- a/install.sh +++ b/install.sh @@ -673,7 +673,6 @@ AUTO_RENAME_ACCOUNT=false DISABLE_ACCOUNT_RENAME=false WEBUI_EMAIL="" WEBUI_NO_AUTH=false -WEBUI_POOL_ID="" while [[ $# -gt 0 ]]; do case "$1" in --non-interactive|--yes|-y) AUTO_YES=true; shift ;; @@ -749,12 +748,6 @@ while [[ $# -gt 0 ]]; do fi WEBUI_EMAIL="$2"; shift 2 ;; --webui-no-auth) WEBUI_NO_AUTH=true; shift ;; - --webui-pool-id) - if [[ $# -lt 2 || "$2" == --* ]]; then - echo -e "\033[0;31m✗\033[0m --webui-pool-id requires a Cognito user pool ID" >&2 - exit 1 - fi - WEBUI_POOL_ID="$2"; shift 2 ;; --debug-in-repo) DEBUG_IN_REPO=true; shift ;; --test|--dry-run) TEST_MODE=true; shift ;; --auto-rename-account-enabled) AUTO_RENAME_ACCOUNT=true; shift ;; @@ -792,7 +785,6 @@ Options: (roundhouse pack, without @) --webui-email Email for the initial WebUI user --webui-no-auth Skip Cognito WebUI authentication setup - --webui-pool-id Use an existing Cognito user pool --debug-in-repo Dev-only: run installer from cwd --test, --dry-run Run installer end-to-end without provisioning AWS resources. Telemetry @@ -3487,7 +3479,7 @@ main() { # Post-deploy: read Cognito outputs from the stack and display admin credentials if [[ "${WEBUI_AUTH_ENABLED:-false}" == "true" ]]; then - local stack_outputs pool_id client_id domain admin_email admin_password dashboard_url + local stack_outputs pool_id client_id domain admin_email admin_password secret_arn dashboard_url stack_outputs=$(aws cloudformation describe-stacks --stack-name "${ENV_NAME}" \ --region "$DEPLOY_REGION" --output json 2>/dev/null | jq -r '.Stacks[0].Outputs') if [[ -n "$stack_outputs" && "$stack_outputs" != "null" ]]; then @@ -3495,12 +3487,18 @@ main() { client_id=$(echo "$stack_outputs" | jq -r '.[] | select(.OutputKey=="WebUICognitoClientId") | .OutputValue') domain=$(echo "$stack_outputs" | jq -r '.[] | select(.OutputKey=="WebUICognitoDomain") | .OutputValue') admin_email=$(echo "$stack_outputs" | jq -r '.[] | select(.OutputKey=="WebUIAdminEmailOutput") | .OutputValue') - admin_password=$(echo "$stack_outputs" | jq -r '.[] | select(.OutputKey=="WebUIAdminPassword") | .OutputValue') + secret_arn=$(echo "$stack_outputs" | jq -r '.[] | select(.OutputKey=="WebUIAdminSecretArn") | .OutputValue') dashboard_url=$(echo "$stack_outputs" | jq -r '.[] | select(.OutputKey=="KiroCrewDashboardUrl") | .OutputValue') + admin_password="" + if [[ -n "$secret_arn" && "$secret_arn" != "null" ]]; then + admin_password=$(aws secretsmanager get-secret-value --secret-id "$secret_arn" \ + --region "$DEPLOY_REGION" --query SecretString --output text 2>/dev/null \ + | jq -r '.password // empty') + fi if [[ -n "$admin_email" && "$admin_email" != "null" ]]; then echo "" $GUM style --border rounded --border-foreground 220 --padding "1 2" --margin "0 2" \ - "⚠ SAVE THESE CREDENTIALS — shown only once" \ + "⚠ WEBUI ADMIN CREDENTIALS — stored in Secrets Manager" \ "" \ " Dashboard: ${dashboard_url}" \ " Login: ${admin_email}" \ @@ -3508,7 +3506,9 @@ main() { "" \ " Pool: ${pool_id}" \ " Client: ${client_id}" \ - " Domain: ${domain}" + " Domain: ${domain}" \ + "" \ + " Secret: ${secret_arn}" echo "" fi fi