docs: verify enterprise IdP federation live, and trim the IdP client secret - #30
Merged
Conversation
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.
|
Commit: Security Scan Results
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.mdand 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):EntraIDas a supported providerAADSTSerrorclient_credentialsreturns a bearer token)EntraID_<id>,EXTERNAL_PROVIDER,email+namemapped from Entraid_token.identities[0].providerName = EntraID,iss= the Cognito pool200, the orchestrator answeredEverything 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.pygreen.The defect: the IdP client secret needs trimming
upsert_idp_secretstoredIDP_CLIENT_SECRETexactly as given. A secret copied from the Entra portal, or piped in fromaz 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 withinvalid_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
EntraID_…, notentraid_….az ad sp createstep 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 freshcreatedDateTime. Many guides tell you to create it by hand.client_idandredirect_urionly, not sign-in readiness. Reworded.EXTERNAL_PROVIDERaccounts 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/invokeis accepted — scope enforcement belongs in the agent; and Cognito answersinvalid_client_secretif you authenticate the app client with the M2M client's secret.Check
scripts/check-deploy-config.shgains check (k). It stubs the AWS CLI and asserts on the arguments the stub receives, becauseupsert_idp_secretdeliberately 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.mdis written from the run, not from the API reference — the ordering constraints and the error messages in it are ones that actually appeared.shellcheckfindings indeploy.share unchanged.