Support PIN code in OOB IdP authentication#23
Conversation
covers the PIN-required variant of OOB IdP authentication: prompts the user for the PIN shown in the browser after IdP login, posts it to AdvanceAuthentication with the OOBAUTHPIN mechanism and IdpLoginSessionId. adds regression tests for the existing polling (non-PIN) path.
some tenants are configured to display a PIN code in the browser after external IdP login. New-IDSession now detects the IdpOobAuthPinRequired flag on the Start-Authentication response, prompts the user for the PIN, and completes the session via AdvanceAuthentication using the OOBAUTHPIN mechanism and IdpLoginSessionId. previous tenants hung forever in the OobAuthStatus polling loop with no way to enter the PIN. reference: ark-sdk-python __perform_pin_code_idp_authentication
entry under unreleased/Fixed for the new PIN-confirmation variant of OOB IdP authentication in New-IDSession.
exercises the new branch's cleanup + response validation: API error during PIN submission calls Clear-AdvanceAuthentication and re-throws; a non-LoginSuccess summary or missing token is rejected with cleanup.
mirrors the error-handling contract of Start-AdvanceAuthentication: on any failure at the PIN submission stage, Clear-AdvanceAuthentication is invoked to clean up the in-progress session and the original error is re-thrown. a successful response is additionally validated to have Summary=LoginSuccess and a Token before flowing into the downstream session output path.
adds a description paragraph to both the markdown source and the generated help file covering the new PIN-required OOB IdP variant, so Get-Help output explains the behaviour users see at runtime.
There was a problem hiding this comment.
Pull request overview
Fixes New-IDSession hanging for tenants that require a post-IdP PIN entry during OOB IdP authentication by adding a dedicated PIN submission path and documenting the behavior.
Changes:
- Add
IdpOobAuthPinRequiredhandling inNew-IDSessionto prompt for a PIN (secure input) and submit it via/Security/AdvanceAuthenticationusingOOBAUTHPIN. - Add Pester coverage for PIN-required and polling (non-PIN) OOB IdP authentication paths, including cleanup-on-error behavior.
- Update command help/docs and changelog to describe the new PIN-required flow.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/New-IDSession.Tests.ps1 | Adds Pester tests covering OOB IdP PIN-required flow, error handling/cleanup, and polling regression protection. |
| IdentityCommand/Public/New-IDSession.ps1 | Implements PIN-required branch for OOB IdP authentication and keeps polling behavior for non-PIN tenants. |
| IdentityCommand/en-US/IdentityCommand-help.xml | Documents PIN prompt behavior for tenants requiring a PIN after external IdP authentication. |
| docs/collections/_commands/New-IDSession.md | Adds documentation note about PIN-required tenants and the interactive prompt. |
| CHANGELOG.md | Records the fix for PIN-required OOB IdP authentication hanging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Start-Sleep 2 | ||
| $OobPinAuthRequest = @{ } | ||
| $OobPinAuthRequest['Method'] = 'POST' | ||
| $OobPinAuthRequest['Uri'] = "$tenant_url/Security/AdvanceAuthentication" |
There was a problem hiding this comment.
In the OOB IdP PIN flow, the request URI is built from the local $tenant_url. If Start-Authentication returns PodFqdn, it updates $ISPSSSession.tenant_url and subsequent calls should target that redirected pod; using the original $tenant_url can break the flow. Build the AdvanceAuthentication (and other OOB IdP) URIs from $ISPSSSession.tenant_url instead.
| $OobPinAuthRequest['Uri'] = "$tenant_url/Security/AdvanceAuthentication" | |
| $OobPinAuthRequest['Uri'] = "$($ISPSSSession.tenant_url)/Security/AdvanceAuthentication" |
There was a problem hiding this comment.
Good catch — fixed in 28a0e91. The PIN branch now builds the AdvanceAuthentication URI from $ISPSSSession.tenant_url so any PodFqdn redirect performed by Start-Authentication is honoured. Added a regression test in d62ba79 that mutates the session tenant_url in the Start-Authentication mock and asserts the PIN request targets the redirected host.
| #Undocumented endpoint for checking the IdpLoginSessionId's status. Sniffed out from the ark-sdk-python project | ||
| $OobAuthStatusRequest['Uri'] = "$tenant_url/Security/OobAuthStatus" |
There was a problem hiding this comment.
The OOB IdP polling request uses "$tenant_url/Security/OobAuthStatus". If Start-Authentication redirected to a pod (via PodFqdn), $ISPSSSession.tenant_url will differ from $tenant_url and polling the original host may fail. Consider building this URI from $ISPSSSession.tenant_url to stay consistent with the redirected base URL.
| #Undocumented endpoint for checking the IdpLoginSessionId's status. Sniffed out from the ark-sdk-python project | |
| $OobAuthStatusRequest['Uri'] = "$tenant_url/Security/OobAuthStatus" | |
| $oobAuthStatusTenantUrl = if ($ISPSSSession.tenant_url) { $ISPSSSession.tenant_url } else { $tenant_url } | |
| #Undocumented endpoint for checking the IdpLoginSessionId's status. Sniffed out from the ark-sdk-python project | |
| $OobAuthStatusRequest['Uri'] = "$oobAuthStatusTenantUrl/Security/OobAuthStatus" |
There was a problem hiding this comment.
Confirmed and fixed in 28a0e91. This was actually a pre-existing issue in the polling branch (shipped with the initial OOB IdP support in 8fc35b8) that my PR inherited when I nested it under the new else. Since the surrounding code was already being touched here, I took the fix in this PR rather than leaving the two branches inconsistent. Used $ISPSSSession.tenant_url directly — no fallback needed, since New-IDSession assigns it unconditionally at line 53 before Start-Authentication runs. Regression test added in d62ba79. Happy to split that hunk into a follow-up if you'd prefer to keep this PR strictly scoped to the PIN flow.
simulates Start-Authentication mutating ISPSSSession.tenant_url via a PodFqdn redirect and asserts the PIN and polling requests target the redirected host rather than the stale local tenant_url parameter.
Start-Authentication rewrites the session tenant_url when the server returns a PodFqdn redirect, so the OOB IdP PIN submission and the polling fallback must build their request URIs from the session variable rather than the original function parameter. previously both branches used the stale local tenant_url and would hit the wrong host for tenants that redirect to a pod. flagged by Copilot on PR pspete#23.
Description
Tenants configured to require a PIN code after external IdP authentication hang forever in
New-IDSession. After the user logs in to the IdP, the browser displays a PIN that must be submitted to/Security/AdvanceAuthentication, but the existing OOB IdP branch only handles the polling variant via/Security/OobAuthStatus, so users see the PIN with no way to enter it.This PR adds a new branch in
New-IDSessionthat detectsIdpOobAuthPinRequiredon theStart-Authenticationresponse, prompts the user for the PIN (as aSecureString, matchingGet-MechanismAnswerconventions), and submits it with theOOBAUTHPINmechanism id and theIdpLoginSessionIdfrom the start-auth response. The submission is wrapped in the sametry/catch+Clear-AdvanceAuthenticationcontract used byStart-AdvanceAuthentication, and the response is validated (Summary = LoginSuccess,Tokenpresent) before flowing into the existing session-output path.Reference:
ark-sdk-python—ark_sdk_python/auth/identity/ark_identity.py(__perform_pin_code_idp_authentication), the same project cited by the existing OOB IdP code.Type of change
How Has This Been Tested?
New
Context 'OOB IdP Authentication'inTests/New-IDSession.Tests.ps1covering:OOBAUTHPIN/IdpLoginSessionId/Answer), noOobAuthStatuscall,Start-AdvanceAuthenticationbypassed.Clear-AdvanceAuthenticationand re-throws; non-LoginSuccesssummary and missingTokenare rejected with cleanup.OobAuthStatusand does not prompt.Full suite run:
Invoke-Pester ./Tests→ 2789 passed, 2 failed. Both failures (Out-QRImage,Start-AdvanceAuthenticationcleanup) are pre-existing Linux/WSL path issues in files not touched by this PR, last modified in 2024 (7eda428).Pester test(s) update required
Pester test(s) updated
Pester test(s) passing
Checklist: