Skip to content

docs: verify enterprise IdP federation live, and trim the IdP client secret - #30

Merged
Robobc merged 4 commits into
mainfrom
docs/entra-id-federation
Aug 19, 2026
Merged

docs: verify enterprise IdP federation live, and trim the IdP client secret#30
Robobc merged 4 commits into
mainfrom
docs/entra-id-federation

Conversation

@Robobc

@Robobc Robobc commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

What

Module 4 promises "federate your enterprise IdP into Cognito" but there was no walkthrough for it, and the path had never been run end to end. This adds docs/ENTERPRISE_IDP.md and fixes the defects the live run surfaced.

Verified live

Run against a real Microsoft Entra ID tenant, with the deployed platform pointed at it (IDP_TYPE=entra_id):

Check Result
Cognito identity provider created with the Entra OIDC endpoints pass
App clients list EntraID as a supported provider pass
Hosted UI redirects to the Entra authorize endpoint, no AADSTS error pass
The client secret Cognito holds is accepted by Entra (client_credentials returns a bearer token) pass
Interactive browser sign-in pass — code returned to the callback
Federated user created in the pool EntraID_<id>, EXTERNAL_PROVIDER, email + name mapped from Entra
Code exchanged for tokens Cognito-issued set; id_token.identities[0].providerName = EntraID, iss = the Cognito pool
Agent invoke with that user's token 200, the orchestrator answered

Everything created for the test was removed afterwards: the Entra app registration and the test secret are deleted, the federated user is deleted, and the environment is back on plain Cognito with scripts/invoke.py green.

The defect: the IdP client secret needs trimming

upsert_idp_secret stored IDP_CLIENT_SECRET exactly as given. A secret copied from the Entra portal, or piped in from az ad app credential reset -o tsv, carries a trailing newline. Cognito stores it verbatim and forwards it verbatim, so the token exchange fails at the IdP with invalid_client — which says nothing about whitespace, and the secret looks correct everywhere you can inspect it.

Fix: strip surrounding whitespace before storing, and reject a secret that is whitespace only rather than storing an empty one.

What the interactive run corrected in the doc

  • The federated username keeps the provider's casing: EntraID_…, not entraid_….
  • No az ad sp create step is needed. Entra creates the service principal itself during the first sign-in — confirmed by deleting it, signing in again, and finding it recreated with a fresh createdDateTime. Many guides tell you to create it by hand.
  • The no-browser check that fetches the Microsoft sign-in page proves less than it claimed: the page still renders with the service principal deleted, so it establishes the client_id and redirect_uri only, not sign-in readiness. Reworded.
  • Removing the provider leaves its users behind as orphaned EXTERNAL_PROVIDER accounts nobody can sign in as. Added the cleanup command.

Also documented, both observed: the runtime's JWT authorizer validates signature, issuer and client id but not scopes, so a user token without agentcore/invoke is accepted — scope enforcement belongs in the agent; and Cognito answers invalid_client_secret if you authenticate the app client with the M2M client's secret.

Check

scripts/check-deploy-config.sh gains check (k). It stubs the AWS CLI and asserts on the arguments the stub receives, because upsert_idp_secret deliberately unsets the plaintext after use so there is nothing to inspect afterwards. The check fails against the pre-fix function and passes after.

Review notes

  • docs/ENTERPRISE_IDP.md is written from the run, not from the API reference — the ordering constraints and the error messages in it are ones that actually appeared.
  • Okta and Ping share the same code path; the doc says which parts are Entra specific and which are generic OIDC.
  • The two pre-existing shellcheck findings in deploy.sh are unchanged.

Robobc added 4 commits August 19, 2026 11:52
The stack has federated Entra ID / Okta / Ping since the beginning and it had
never been run against a real tenant — the last capability claim in this repo
without evidence behind it. Verified end to end against a live Entra ID tenant,
and the run surfaced a defect worth fixing.

docs/ENTERPRISE_IDP.md: the module 4 walkthrough, written from the commands I
actually ran — computing the Cognito redirect URI before the IdP exists,
`az ad app create` with the flags that matter, storing the secret, deploying,
and four checks that isolate failures without a browser. Okta/Ping differ only
in taking a full issuer_url. Includes the verification table and a
symptom/cause table (redirect_mismatch, AADSTS50011, AADSTS700016,
invalid_client, missing id-token issuance, no button in the hosted UI).

deploy.sh + check (k): trim the IdP client secret before storing it.
A secret pasted from a console or piped from `az ad app credential reset -o tsv`
carries a trailing newline; Secrets Manager stores it verbatim, Cognito forwards
it to the IdP token endpoint, and the exchange fails with invalid_client saying
nothing about whitespace. Hit exactly this live. Whitespace-only now stops the
run instead of storing an empty secret. The check asserts on what the function
passed to the CLI, because it deliberately unsets the plaintext when done; it
fails against the untrimmed version.

Verified live (Entra tenant 697a5720…, 2026-08-19), all cleaned up after:
- App registration with the derived redirect URI, AzureADMyOrg, id-token
  issuance on.
- Cognito OIDC provider created: issuer https://login.microsoftonline.com/
  <tenant>/v2.0, scopes "openid email profile", email/name attribute mapping.
- App and web clients list EntraID alongside COGNITO.
- Cognito → Entra handoff: redirects with our client_id and Entra serves the
  sign-in page with NO AADSTS error, so client_id + redirect_uri + scopes are
  all accepted.
- The secret Cognito holds is genuinely valid at Entra: client_credentials
  against the tenant token endpoint returned a Bearer token. This is the check
  that separates "wrong secret" from "user cannot sign in", and it is in the doc.
- Switching IDP_TYPE cognito → entra_id → cognito removes and recreates the
  provider cleanly (also how the corrected secret was picked up).
- Not verified programmatically, and the doc says so: the interactive login
  itself, which needs a human at a browser.

Also documented, since it is a real property participants should know: the IdP
client secret is readable from Cognito via DescribeIdentityProvider by any
caller holding that permission. AWS behaviour, not something the stack can hide.

pytest 115 passed; check-deploy-config and check-workshop-flow pass; shellcheck
back at the pre-existing 2 findings (the new harness block needed disable
directives for variables read by eval'd functions).
MODULE_EXPLAIN[4] and the participant guide's module 4 row + docs index now
point at ENTERPRISE_IDP.md, so a participant who wants corporate logins finds
the walkthrough at the moment the wizard mentions federation.
Completes the one step of docs/ENTERPRISE_IDP.md that was documented as
unverified. Run against the same live tenant, with the platform deployed on
IDP_TYPE=entra_id.

Verified, in order:
  - browser sign-in returns to the callback with an authorization code
  - Cognito creates the user as EntraID_<id> / EXTERNAL_PROVIDER with email
    and name mapped from Entra
  - the code exchanges for a Cognito-issued token set; id_token carries
    identities[0].providerName = EntraID and iss = the Cognito pool
  - that user's access token invokes the orchestrator runtime: 200, answered
  - auth_time on a second, session-busted login lands within a second of the
    request, so the round trip really goes through Entra

Four things the run corrected in the doc:
  - the federated username keeps the provider's casing (EntraID_, not entraid_)
  - no `az ad sp create` step: Entra creates the service principal itself
    during the first sign-in (deleted it, signed in again, it came back with a
    fresh createdDateTime)
  - check (c) proves less than it claimed. The Microsoft sign-in page renders
    with the service principal deleted, so it establishes the client_id and
    redirect_uri only, not sign-in readiness
  - removing the provider leaves its users in the pool as orphaned
    EXTERNAL_PROVIDER accounts; added the cleanup command

Also documented: the runtime's JWT authorizer checks signature, issuer and
client id but not scopes, so a user token without agentcore/invoke is accepted
(observed) — scope enforcement belongs in the agent. And Cognito answers
invalid_client_secret if you authenticate the app client with the M2M client's
secret, which cost a confused minute.

Test environment restored: IDP_TYPE=cognito, provider removed, federated user
deleted, Entra app registration and Secrets Manager secret deleted.
scripts/invoke.py green afterwards.
run-workshop.sh writes the client secret to Secrets Manager itself instead of
going through deploy.sh, so the trailing-newline failure this branch fixes was
still live on that path — the same bug, one file over.

Trimming before the existing missing-vars check means a whitespace-only secret
becomes empty and is caught by the fail-fast that is already there, rather than
being stored and failing later at the IdP with invalid_client.

Verified: the trim expression turns $'  sekret\n' into 'sekret'; shellcheck
clean on the file.
@Robobc
Robobc merged commit 98d3f62 into main Aug 19, 2026
5 of 6 checks passed
@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown

Commit: f8b4557 | Updated: 2026-08-20 00:08:41 UTC

Security Scan Results

Scanner S C H M L I Time Action Result Thresh
bandit 0 0 0 0 0 0 883ms 0 PASSED MED (g)
cdk-nag 0 0 0 0 0 0 7.8s 0 PASSED MED (g)
cfn-nag 0 0 0 0 0 0 16ms 0 PASSED MED (g)
checkov 0 0 0 0 0 0 5.4s 0 PASSED MED (g)
detect-secrets 0 0 0 0 0 0 755ms 0 PASSED MED (g)
grype 0 0 0 0 0 0 1m 1s 0 PASSED MED (g)
npm-audit 0 0 0 0 0 0 164ms 0 PASSED MED (g)
opengrep 0 0 0 0 0 0 16.9s 0 PASSED MED (g)
semgrep 0 0 0 0 0 0 <1ms 0 MISSING MED (g)
syft 0 0 0 0 0 0 2.5s 0 PASSED MED (g)

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