Skip to content

feat: move Cognito WebUI auth into CFN stack - #82

Merged
royosherove merged 2 commits into
mainfrom
feat/cognito-in-cfn
Aug 16, 2026
Merged

feat: move Cognito WebUI auth into CFN stack#82
royosherove merged 2 commits into
mainfrom
feat/cognito-in-cfn

Conversation

@royosherove

Copy link
Copy Markdown
Member

Roy asked for Cognito to be initialized as part of the CFN stack instead of via direct API calls from the installer.

Changes

CFN template (deploy/cloudformation/template.yaml):

  • Params: EnableWebUIAuth, WebUIAdminEmail
  • Condition: EnableWebUI (IsKiroCrew AND EnableWebUIAuth==true)
  • Resources: WebUIUserPool, WebUIUserPoolDomain, WebUIUserPoolClient + Lambda-backed custom resource for initial user
  • Callback URLs reference !GetAtt KiroCrewDistribution.DomainName directly — no more post-deploy patching
  • Outputs: pool ID, client ID, domain, admin email, admin password (one-time)

Installer (install.sh):

  • configure_webui_auth() now only collects preferences (enable + email)
  • All aws cognito-idp calls removed (0 remaining)
  • Post-deploy reads outputs and displays credentials once
  • SSM writes removed (CFN outputs replace them)

Validation

  • bash -n install.sh
  • aws cloudformation validate-template ✓ (38 params)
  • PARAM_CFN_NAMES (26) == PARAM_VALUES (26) ✓

Deferred (per Roy)

  • Cognito cleanup on uninstall
  • Unattended (-y) support for KiroCrew

…sources)

- Parameters: EnableWebUIAuth, WebUIAdminEmail
- Condition: EnableWebUI (IsKiroCrew AND EnableWebUIAuth==true)
- Resources: WebUIUserPool, WebUIUserPoolClient, WebUIUserPoolDomain
- Custom Resource Lambda: WebUIUserCreationFunction for initial admin user

TODO:
- Fix backslash-n escape bug in Lambda Python (two spots)
- Add Cognito outputs (pool ID, client ID, domain, admin password)
- Update installer to pass parameters instead of direct Cognito API calls
- Validate CFN template
- Merge to main
Previously the installer made direct aws cognito-idp API calls to create
the user pool, client, domain, and initial user. This meant Cognito
resources lived outside CFN — not cleaned up by stack deletion, callback
URLs required post-deploy patching, and console-mode deploys couldn't
initialize auth at all.

Now:
- CFN parameters: EnableWebUIAuth (true/false), WebUIAdminEmail
- CFN resources: WebUIUserPool, WebUIUserPoolDomain, WebUIUserPoolClient
- Custom Resource Lambda (WebUIUserCreationFunction) generates a random
  password, creates the admin user, sets a permanent password, and
  returns the password as a CFN output (once, via GetAtt).
- Callback/logout URLs reference the CloudFront distribution directly
  via !GetAtt — no more post-deploy patching.
- Installer's configure_webui_auth() now only collects preferences
  (enable? admin email?) and sets vars for build_deploy_params.
- Stack outputs: WebUICognitoPoolId, WebUICognitoClientId,
  WebUICognitoDomain, WebUIAdminEmailOutput, WebUIAdminPassword.
- After deploy, installer reads outputs and displays credentials once
  in a gum-styled bordered box.

Cleaned up:
- Fixed literal \\n escape in Lambda Python (from WIP commit)
- Removed all aws cognito-idp calls from installer
- Removed SSM writes for Cognito config (CFN outputs replace them)
- Removed post-deploy callback URL patch block (CFN handles both URLs
  from the start via !GetAtt)
- Removed console-mode 'update callback URL manually' hint

Deferred (per Roy):
- Cognito cleanup on uninstall (future iteration)
- Unattended (-y) KiroCrew support (still falls back to openclaw)

Validated: aws cloudformation validate-template passes with 38 params.
PARAM_CFN_NAMES and PARAM_VALUES arrays are in sync (26 entries each).
@royosherove
royosherove merged commit 81e2bd8 into main Aug 16, 2026
4 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 725805c410

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1458 to +1460
AllowedOAuthFlows:
- code
AllowedOAuthFlowsUserPoolClient: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Enforce Cognito before reporting the dashboard protected

When WebUI auth is enabled, this only creates an OAuth client; it never attaches authentication to the request path or passes the pool/client/domain to KiroCrew. The existing KiroCrewHTTPListener forwards every request directly (template.yaml:616-625), and packs/kirocrew/resources/kirocrew-gateway.service:6-17 contains no Cognito configuration, despite the mandatory server-side enforcement described in docs/design/kirocrew-webui-auth.md:38-50. Consequently, selecting the installer’s “enterprise-grade” protection still leaves the public CloudFront dashboard unauthenticated; wire the client into an enforcing proxy/gateway or fail the auth option until that integration exists.

Useful? React with 👍 / 👎.

Comment on lines +1819 to +1822
WebUIAdminPassword:
Condition: EnableWebUI
Description: One-time initial password for the WebUI admin user — save immediately, not retrievable later
Value: !GetAtt WebUIUserCreationResource.Password

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep the permanent admin password out of stack outputs

With auth enabled, the Lambda sets this generated password as Permanent=True and this output stores it in CloudFormation indefinitely. It is therefore retrievable repeatedly by principals with routine DescribeStacks access rather than being “shown only once,” giving those principals credentials for the WebUI admin account. Deliver a temporary password through Cognito or store the secret in a separately permissioned secret instead of returning it as a stack output.

Useful? React with 👍 / 👎.

Comment on lines +1442 to +1443
UserPoolId: !Ref WebUIUserPool
Domain: !Sub 'lowkey-${PackName}-${EnvironmentName}'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Make the Cognito domain unique across StackSet accounts

For StackSet deployments, every target account using the same region, pack, and environment requests exactly the same Cognito managed-login prefix, but these prefixes must be unique across accounts within a region. Only the first stack can create the domain and the remaining stacks will roll back; the previous installer explicitly appended a random suffix and retried on collision. Include account/region-derived uniqueness or retain a randomized collision-safe suffix.

Useful? React with 👍 / 👎.

Comment on lines +1546 to +1548
if event['RequestType'] == 'Delete':
send_response(event, context, 'SUCCESS', 'Delete is a no-op')
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Remove the previous admin when its email changes

When an existing stack changes WebUIAdminEmail, the custom resource creates a new permanent-password user, receives a replacement physical ID because it is based on the invocation log stream, and then handles deletion of the old resource here as a no-op. The former email therefore retains valid dashboard access after an administrator believes it has been replaced; delete the old user during replacement or update the existing identity explicitly.

Useful? React with 👍 / 👎.

Comment on lines +369 to +373
WebUIAdminEmail:
Type: String
Default: ''
Description: "Email for the initial WebUI admin user. Required when EnableWebUIAuth is true."
AllowedPattern: '^([^@]+@[^@]+\.[^@]+)?$'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require an admin email whenever authentication is enabled

A direct CloudFormation or console deployment can set EnableWebUIAuth=true while leaving this parameter at its allowed empty default. CloudFormation then creates the preceding resources before WebUIUserCreationFunction rejects the missing email at template.yaml:1555-1557, causing a late full-stack rollback. Add a template rule tying a nonempty email to the enabled-auth setting so this invalid combination is rejected before deployment.

Useful? React with 👍 / 👎.

Comment thread install.sh
Comment on lines 2125 to +2127
configure_webui_auth() {
local pack_name="$1" webui_port="$2" callback_path="$3"
local pool_id="${WEBUI_POOL_ID:-}" client_id="" domain_prefix="" user_email="${WEBUI_EMAIL:-}"
local callback_url="http://localhost:${webui_port}${callback_path}"
local logout_url="http://localhost:${webui_port}/"
local pool_name choice pools pool_count suffix domain_json
local user_email="${WEBUI_EMAIL:-}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor or remove the existing-pool CLI option

When a user supplies the still-documented --webui-pool-id, parsing sets WEBUI_POOL_ID, but this rewritten function no longer reads it and the template unconditionally creates WebUIUserPool. The installation silently ignores the requested pool and provisions a separate identity store, potentially locking out users expected to authenticate through the existing pool; pass the pool choice into CloudFormation or reject/remove the option.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant