setup-auth: multi-provider authentication and SPA flows support - #173
Merged
Dhiresh Chawla (dhiresh06) merged 67 commits intoMay 28, 2026
Conversation
Fix Facebook OAuth settings (AppId/AppSecret), local login Email/Username field mapping, OIDC AuthenticationType setting, and validate-auth.js check. Add RememberMe support, Azure AD B2C provider, two-factor authentication, invitation-based registration, and secret management warnings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ards - Unify login() signature with invitationCode as 3rd parameter - Add AUTH_PROVIDERS array + loginWithProvider() for multi-social OAuth - Add WS-Federation AuthenticationType site setting - Guard getTenantId() to throw instead of constructing broken URL - Add RequireInvitationCode and OpenRegistrationEnabled settings - Add SAML2 AuthenticationType exact-match warning - Add response.ok check in fetchAntiForgeryToken() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace Azure AD B2C with Entra External ID (ciamlogin.com) - Use create-site-setting.js and create-environment-variable.js scripts instead of raw YAML templates for site setting creation - Ask user for ClientId during the flow instead of placeholder - Handle secrets via environment variables (never in YAML files) - Social OAuth: ask user which specific providers they want - Add mandatory identity provider question enforcement Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Multi-select identity providers (users can pick multiple)
- Skip provider question if already specified in prompt
- Add explicit 2FA/invitation AskUserQuestion in Phase 2.1
- Fix validate-auth.js: skip authz check for login-only scope
- Fix eval 5: expect collected ClientId, not placeholder
- Fix eval 8: match individual Google/Facebook selection
- Add eval 13: OIDC with advanced claims mapping
- Add {ProviderName} naming convention (OpenIdConnect_1, SAML2_1)
- Explicit Entra External ID site settings (not just "same as OIDC")
- Handle re-runs when site settings already exist
- Fix writeYaml to quote YAML-special characters
- SSR guard for getCurrentUser() in Astro
- Dev mode logout via sessionStorage sign-out flag
- Separate social providers in provider table (not grouped)
- Clarify Entra External ID is NOT Microsoft Account
- Generic auth description in create-site feature options
- Align secret management guidance with env-var approach
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Mixed external+local multi-provider pattern (AUTH_PROVIDERS array with loginWithProvider handling both ExternalLogin and Login endpoints) - Registration flow documentation (ExternalLoginConfirmation) - Password reset flow for local auth (ForgotPassword/ResetPassword) - Detailed logout flow (RP-initiated, cookie cleanup, security stamp) - RememberMeEnabled site setting in 2FA block - SAML2 AuthenticationType must-match-exactly warning - Explicit create-site-setting commands for social secrets - Empty AUTH_PROVIDERS guard - Fix "formerly Azure AD B2C" — separate product - Secret management: env vars only, no placeholder YAML - Guard loginWithProvider local path when credentials missing - Eval 14: mixed Local + Entra External ID Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Local auth button in multi-provider view now expands to show email/password form on click instead of always showing inline. Adds Back button to collapse and Forgot password link. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- getAuthError() parses server error query params (no raw echo - phishing safe)
- register() function with returnUrl, identity guard, anti-forgery token
- RegisterForm component at /register for local auth self-registration
- LocalLoginForm shows server errors, forgot password, conditional register link
- Multi-provider AuthButton shows server errors on failed external login
- OpenRegistrationEnabled and ResetPasswordEnabled site settings for local auth
- Error handling uses finally{} for isSubmitting to prevent locked forms
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Critical: - Add RPInitiatedLogout site setting for OIDC providers (mutually exclusive with ExternalLogoutEnabled — prevents open IdP sessions) - Add MetadataAddress as alternative to Authority for OIDC providers - Add Registration/Enabled global toggle to always-create settings - Add ExternalLoginEnabled to always-create settings Medium - missing flows documented in reference: - ExternalPasswordReset flow (OIDC providers with PasswordResetPolicyId) - ExternalProfileEdit flow (OIDC providers with ProfileEditPolicyId) - Terms & Conditions gate - Invitation redemption flow - Session expiry re-auth with prompt=login and sessionExpired param - Account management endpoints table (ChangePassword, SetPassword, ChangeEmail, LinkLogin, RemoveLogin, ConfirmEmail, ChangeTwoFactor) Medium - missing settings added to optional tables: - RPInitiatedLogout, MetadataAddress, Caption, RequireUniqueEmail, RefreshOnIssuerKeyNotFound, PasswordResetPolicyId, ProfileEditPolicyId, DefaultPolicyId, TokenEndPointAuthenticatedMethod, AllowedDynamicAuthorizationParameters - ResetPasswordEnabled, ResetPasswordRequiresConfirmedEmail, IsCaptchaEnabledForRegistration, TriggerLockoutOnFailedPassword, DenyMinorsWithoutParentalConsent - ApplicationCookie: AbsoluteSlidingExpireTimeSpan, CookiePath, CookieHttpOnly, CookieSecure, LoginPath, SecurityStampValidator - Global toggles: AzureADLoginEnabled, ExternalLoginEnabled, SignOutEverywhereEnabled Medium - missing error codes added: - invalid_username_or_password, too_many_attempts, email_required, username_required, password_required, password_confirmation_failure, invalid_two_factor_code, duplicate_email, duplicate_username, deny_minors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace plain-text environment variable pattern with Key Vault flow: - List available Key Vaults via list-azure-keyvaults.js - Create new Key Vault if none found via create-azure-keyvault.js - User stores secrets via CLI (store-keyvault-secret.js) or Azure Portal - Create env var with --type secret referencing Key Vault secret URI - Link env var to site setting via --envVarSchema - Fallback to plain env var with placeholder if user skips Key Vault - Secret values never pass through the conversation - Updated reference doc secret management best practices Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
User feedback implementation: - Phase 2.0: Smart auth inference from site context (internal → Entra ID + invite-only, customer-facing → External ID + open registration, financial → External ID + invite-only, etc.) with rationale explanation and opt-out to manual selection - Remove Local Authentication from provider choices — only configure if user explicitly requests it - Always present Entra External ID as an option - Add Microsoft Learn doc links for each provider's follow-up questions so users know where to get values - Phase 5.1.1: Create dedicated /signin page when multiple providers are configured (multi-provider login page) - Single provider: nav button calls login() directly - Multi-provider: nav button navigates to /signin page Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Each provider follow-up question now shows both: - Example value format (e.g., a1b2c3d4-e5f6-..., 1234567890123456) - Link to relevant Microsoft Learn documentation page Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add return before approve() in validate-auth.js (lines 12, 18) to prevent crash on non-Power-Pages projects - Wire getSessionExpiredMessage() into Phase 5.1.1 signin page - Remove duplicate LoginButtonAuthenticationType row Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move Entra External ID to top of provider list with (Recommended) label. Entra ID listed second without recommended tag — it's for internal sites. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sked Local authentication must NEVER be set up by default, recommended in smart inference, or included in the provider selection list. Only configure if user explicitly asks for username/password login. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…or local login - Phase 2.1: Ask user whether local login should use email or username - Phase 3.2: Strengthen register() function as required for local auth - Phase 5.1.1: Update sign-in page to use correct credential field type - Phase 5.1.2: NEW section for creating registration page when local auth + open registration - Phase 7.1: Add registration page to file verification inventory - Phase 8.1: Use user's email/username choice for LocalLoginByEmail site setting - Phase 8.3: Add registration page to summary table - Remove buried one-liner about /register route (replaced by Phase 5.1.2) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…te, CAPTCHA SKILL.md changes: - Phase 5.1.1: Use /login route (not /signin) to avoid server conflict - Phase 5.1.2: Use /registration route (not /register), add ViewState note, add dev-mode auth redirect skip - Phase 5.2: Emphasize replacing hardcoded nav links with AuthButton - Phase 3.2: Fix local login to use /SignIn endpoint and PasswordValue field - Phase 8.1: Add ProfileRedirectEnabled overwrite note, add CAPTCHA disable settings (CaptchaEnabled, IsCaptchaEnabledForRegistration) authentication-reference.md changes: - Local Login Flow: Update to /SignIn endpoint, PasswordValue field name - Add Local Registration Flow section explaining Web Forms ViewState pattern - register() function: Replace simple POST with ViewState-based implementation (fetch server page -> parse ViewState + control names -> POST) - Update all route references: /signin -> /login, /register -> /registration - loginWithProvider local branch: Fix endpoint and field name - RegisterForm: Update route, add dev-mode note Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…auth SKILL.md changes: - Phase 3.2: Add parseServerErrors(), forgotPassword() to auth service function list - Phase 3.2: Add critical note about using fetch() not form.submit() for local auth - Phase 5.1.3: NEW — Forgot Password page creation (/forgot-password route) - Phase 5.1.4: NEW — Validation pattern for all auth pages (validate-on-blur, clear-on-change) - Phase 5.1.4: Validation rules table for login, registration, forgot password - Phase 7.1: Add forgot password page to file inventory, add validation checks - Phase 8.3: Add forgot password page to summary table authentication-reference.md changes: - Add parseServerErrors() helper that extracts errors from server HTML responses - Replace form.submit() with fetch() in login (single-provider and multi-provider) - Replace form.submit() with fetch() in register - Add forgotPassword() function (MVC form POST via fetch) - All three functions now throw parsed server errors for SPA display Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ndling - Add optional MetadataAddress question for Entra External ID (not always derivable from authority for some tenant configs) - Add optional MetadataAddress question for generic OIDC - Add server-rendered page handling guidance in Phase 3.2: ExternalLoginConfirmation, SendCode/VerifyCode, TermsAndConditions are server-side and cannot be intercepted by SPA - Document how to minimize server redirects (correct registration settings, disable terms unless needed, refresh auth on return) - Add MetadataAddress to Entra External ID Phase 8.1 site settings Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Phase 2.1: Ask for full Authority URL instead of tenant name (supports ciamlogin.com and custom domains like login.contoso.com) - Phase 8.1: Use authority URL from user, not hardcoded template - Update all ciamlogin.com references to show as example, not default - Reference doc: note custom domain support in provider descriptions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Eval 15: Microsoft Account standalone (Vue) - Eval 16: Smart inference for internal HR dashboard - Eval 17: Custom domain Entra External ID + Google + /login page - Eval 18: Local auth with registration, forgot password, error handling - Eval 19: Key Vault secret flow for Entra External ID - Eval 20: Auth error handling (getAuthError, getSessionExpiredMessage) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The PostToolUse:Skill hook was firing immediately after the skill's instructions loaded into context — before the skill had done any work. Validators were running against the initial project state, which caused false-positive failures (e.g., existing authService.ts from a previous attempt didn't yet contain AUTH_PROVIDER, blocking the skill from running at all). Fix: - PostToolUse:Skill hook now just RECORDS which skills were invoked to a session state file in OS temp dir (keyed by session_id) - New Stop hook reads the state file and runs validators AFTER the skill has finished its work - State file is cleaned up after validation runs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Based on https://learn.microsoft.com/en-us/power-pages/security/authentication/entra-external-id and deep code review of crm.solutions.portal repo: 1. MetadataAddress is REQUIRED for Entra External ID (not optional) - Per Learn doc: collected from App Registrations -> Endpoints blade - Phase 2.1: question now asks for it as a required field - Phase 8.1: creates the MetadataAddress site setting unconditionally 2. Button name warning — do NOT use "Sign in with Microsoft" - Conflicts with Microsoft Account social provider - Learn doc recommends "Microsoft Entra External ID" or "Sign in with External ID" 3. Docs link updated from /openid-settings to /entra-external-id 4. Added implementation note: server treats External ID as generic OIDC (no CIAM-specific handling in StartupSettingsManager.cs or LoginController.cs) 5. CRITICAL: Redirect URI / CallbackPath collision prevention - OWIN OIDC middleware defaults CallbackPath to /signin-oidc for ALL OIDC providers — multiple OIDC providers collide without unique CallbackPath - Phase 8.1 now creates unique CallbackPath per provider (/signin-oidc-<suffix>) - RedirectUri pattern updated to /signin-oidc-<unique-suffix> - Added workflow to derive unique suffix from existing OIDC providers 6. Documented duplicate site setting detection (already in create-site-setting.js, checks by both name and filename, case-insensitive, no overwrites) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SKILL.md: - Phase 3.5: NEW — Create Session KeepAlive hook with full requirements - Phase 7.1: Add keepalive to file inventory - Phase 8.3: Add keepalive to summary table authentication-reference.md: - NEW section: Session KeepAlive for SPA Sites - React useSessionKeepAlive hook implementation - Layout integration pattern - Explanation of why /_layout/tokenhtml is the best endpoint Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…defaults SKILL.md Phase 3.5: - Define SESSION_EXPIRE_MS from the configured ExpireTimeSpan - intervalMs = min(SESSION_EXPIRE_MS / 3, 15min) — before halfway renewal point - idleTimeoutMs = min(SESSION_EXPIRE_MS * 0.9, 30min) - Examples: 10min session → 3.3min ping, 9min idle. 24h → 15min ping, 30min idle. authentication-reference.md: - Updated useSessionKeepAlive with SESSION_EXPIRE_MS constant - Math.min formulas for both intervals - Comments explaining the timing logic Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SKILL.md: - Phase 2: Add Terms to optional features, collect terms content + labels, GDPR prerequisite warning, TermsPublicationDate option - Phase 3.2: Add TermsRequiredError, acceptTerms(), terms detection in both loginLocal() and register() (two different server redirect URLs) - Phase 5.1.5: NEW — Terms page creation with hardcoded snippet content, TermsRequiredError catching in Login and Registration pages - Phase 7.1: Add Terms page to file inventory - Phase 8.1: Add TermsAgreementEnabled setting, TermsPublicationDate, content snippet creation (Account/Signin/TermsAndConditionsCopy) - Phase 8.3: Add Terms page and snippet to summary table authentication-reference.md: - NEW section: Terms and Conditions for SPA Sites - Prerequisites (GDPR solution, setting, snippet) - How server triggers terms (two redirect URLs for login vs registration) - TermsRequiredError class and acceptTerms() function - React Terms page component with hardcoded snippet constants - Login/Registration TermsRequiredError catch patterns - Content snippet table and re-consent via TermsPublicationDate Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Phase 2.1: Ask user if Entra External ID app registration uses a client secret (default: No, since public clients use PKCE) - Phase 8.1.1: Add conditional execution — only run Key Vault setup when at least one configured provider needs a secret - Document which providers require secrets vs which skip the phase - Entra External ID: clarify ClientSecret is optional (PKCE default) Fixes the issue where users were prompted for Key Vault setup even when their Entra External ID configuration didn't require a secret. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace vague "unique-suffix" placeholder with deterministic pattern:
- CallbackPath = /signin-{ProviderName-lowercased}
- RedirectUri = {site-url}/signin-{ProviderName-lowercased}
Since ProviderName is already guaranteed unique via the naming
convention (OpenIdConnect_1, OpenIdConnect_2, EntraExternalId, etc.),
deriving CallbackPath from it automatically gives uniqueness.
Previously the LLM was told to "derive a unique suffix" which was
fragile — two runs could pick the same suffix. Now the suffix is
mechanically derived from an already-unique ProviderName.
Still checks for existing collisions before creating and increments
the numeric suffix on ProviderName if collision is detected.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Based on deep research of crm.solutions.portal source code:
1. User display name fallback hierarchy (getUserDisplayName):
- firstName + lastName → firstName → userName → email → 'User'
- Only contactId and userName are guaranteed on Portal.User
- firstName/lastName/email require RegistrationClaimsMapping site
setting AND the IdP actually emitting the claim
- Same fallback logic applied to getUserInitials()
2. Auth failure redirect workaround:
- /Account/Login/ExternalAuthenticationFailed path is hardcoded
in OWIN startup (Startup.Auth.cs:30) — no site setting override
- Only ?message=access_denied query param is ever passed
- SPA workaround: edit Dataverse content snippets
'Account/Register/ExternalAuthenticationFailed' and
'.../AccessDenied' to inject a <script> that redirects to
/login?message={code}
- Documented the script pattern and limitations in reference doc
- Added post-deploy reminder in SKILL.md
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
firstName/lastName/email on Portal.User are populated from standard OIDC claims (given_name, family_name, email) by default. Explicit RegistrationClaimsMapping is NOT required for these standard claims — only for mapping non-standard/custom claims to contact fields. Previous wording incorrectly implied RegistrationClaimsMapping was required for these common fields to populate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Dhiresh Chawla (dhiresh06)
enabled auto-merge (squash)
May 27, 2026 09:05
Contributor
There was a problem hiding this comment.
Pull request overview
This PR significantly expands the setup-auth skill from a primarily Entra ID-focused scaffolder into a broad multi-provider, SPA-aware authentication setup tool. It adds reference documentation, helper scripts, a new HTML report renderer, a Dataverse solution detector, hook architecture changes (PostToolUse → Stop), YAML quoting in site-setting creation, and broad eval coverage for the new provider matrix and optional /user-profile flow.
Changes:
- Adds multi-provider authentication patterns (Entra ID, Entra External ID, OIDC, SAML2, WS-Federation, local, social) plus invitation, terms, reset-password, redeem-invitation, external-login-confirmation, profile page flows in
authentication-reference.md. - New helper scripts (
check-solution-installed.js,render-auth-report.js), new HTML report template, YAML quoting increate-site-setting.js, and centralized hook restructure (run-skill-stop-validation.js+ modifiedrun-skill-posttool-validation.js). - Broadly expanded eval coverage (25 setup-auth scenarios; new SAML2/local/OIDC scenarios in test-site).
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/power-pages/skills/setup-auth/references/authentication-reference.md | Adds large multi-provider reference, profile/terms/invitation/reset-password flows |
| plugins/power-pages/skills/setup-auth/scripts/validate-auth.js | Adds provider-config marker check; scopes authorization-utils check to relevant content |
| plugins/power-pages/skills/setup-auth/assets/auth-report.html | New Fluent-styled HTML report template |
| plugins/power-pages/scripts/render-auth-report.js | New script wrapping renderTemplate() for the auth report |
| plugins/power-pages/scripts/check-solution-installed.js | New Dataverse solution presence probe |
| plugins/power-pages/scripts/create-site-setting.js | Adds YAML scalar quoting helpers |
| plugins/power-pages/hooks/run-skill-posttool-validation.js | Now records skills to a session file instead of running validators |
| plugins/power-pages/hooks/run-skill-stop-validation.js | New Stop hook that runs validators at session end |
| plugins/power-pages/hooks/hooks.json | Registers the new Stop hook |
| plugins/power-pages/skills/create-site/SKILL.md | Tells create-site to describe Auth features generically |
| evals/power-pages/setup-auth/evals.json | Adds 16 new provider/flow scenarios |
| evals/power-pages/test-site/evals.json | Adds private/auth-gated site test scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Three eval scenarios asserted behavior that the canonical reference documents as wrong: - Eval #7 (local auth): said the auth service posts to /Account/Login/Login. The actual endpoint is /SignIn, and the password field name is PasswordValue (not Password). - Eval #12 (invitation-only registration): asserted the skill creates Authentication/Registration/RequireInvitationCode = true. That setting is not read by the Power Pages server. Invitation-only behavior is enforced by InvitationEnabled = true paired with OpenRegistrationEnabled = false. - Eval #13 (OIDC + custom claims): called RegistrationClaimsMapping a JSON value. The server parses it as comma-separated contactfield=claimtype pairs (CSV), not JSON. All three are flagged by Copilot review on PR #173. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reorganizes the report into 8 tabbed sections (Overview, Providers, Local Auth, Features, Site Settings, Permissions, Files, Next Steps) with a left sidebar matching the audit-report / permissions-plan / create-site-plan templates: 220px sidebar, Unicode glyph icons, count badges, accent left-border on active, and a sticky position so the nav stays visible while scrolling. Adds the standard one-line fixed footer "AI-generated content may be incorrect" used by every other report template in the plugin. Overview tab includes a "Highlights" card summarizing the most load-bearing facts (provider count, profile-page status, T&C enforcement state, federated logout) so reviewers don't need to click through every tab. Deep-linking via URL hash works — sharing auth-setup-report.html#settings activates that tab on load. Addresses PR #173 comment from priyanshu92 on render-auth-report.js. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The canonical pattern declared in authentication-reference.md and SKILL.md is `export const AUTH_PROVIDERS: AuthProviderConfig[] = [...]` even for single-provider sites — but the validator was checking for the singular `AUTH_PROVIDER` constant and the reference's first example declared the singular form, contradicting the guidance further down. - validate-auth.js now checks for AUTH_PROVIDERS (or AuthProviderConfig as a secondary marker), with an error message that names the exact canonical shape so a failing run is self-explanatory. - authentication-reference.md's first auth-service example now declares AUTH_PROVIDERS (one-element array) as the canonical form and keeps AUTH_PROVIDER as a derived alias so the dozens of downstream `AUTH_PROVIDER.x` references in the same file still resolve. - The "Provider-Specific AUTH_PROVIDER Configuration Examples" section heading is renamed to "Provider-Specific Configuration Entries" with a leading callout clarifying that each snippet is an *entry* in the AUTH_PROVIDERS array, not a standalone singular constant. Addresses Copilot review comments C7 and C8 on PR #173. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AGENTS.md requires node:test coverage under scripts/tests/ for new and modified scripts. This PR added check-solution-installed.js and render-auth-report.js without tests, and modified validate-auth.js without test coverage. - scripts/lib/check-solution-installed.js: extract the Dataverse query logic from the CLI so it can be unit-tested by mocking helpers.makeRequest (matching the pattern used by lib/discover-env-var-definitions.js). - scripts/check-solution-installed.js: thin CLI wrapper that resolves env URL + auth token and delegates to the lib function. Same exit codes, same JSON output, same stderr messages. - scripts/tests/check-solution-installed.test.js (10 tests): input validation, installed-true with version, installed-false, version=null fallback, network error, 401/403, 500, malformed JSON. - scripts/tests/render-auth-report.test.js (5 tests): happy path, missing args, missing required keys, refuse-to-overwrite, empty- arrays edge case. - scripts/tests/validate-auth.test.js (13 tests): skip paths, complete setup, each individual failure condition, AuthProviderConfig fallback marker (regression guard for the canonical AUTH_PROVIDERS pattern established by the C7 fix), Angular component-path convention. 28 new tests, all passing. Addresses Copilot review comment C4 and priyanshu92's P4 on PR #173. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The needsQuoting helper only checked for special characters anywhere in
the value. That misses cases where the LEADING character changes the
parser's interpretation even when no other special character appears:
- "-foo": leading "-" is ambiguous — parsers may read it as a block
sequence indicator or a numeric sign depending on context.
- "'foo" / '"foo': opens a quoted scalar that never terminates.
- "\tfoo": YAML 1.2 disallows leading tabs in plain scalars.
Adds a leading-character check covering these four cases. Refactors the
function from one long boolean OR into a sequence of self-documenting
early returns so each rule is annotated with its rationale.
Six new tests cover each leading-character case plus a regression guard
asserting that ordinary values ("Contoso Portal", "/signin-EntraExternal")
still emit as bareword scalars.
Addresses Copilot review comment C5 on PR #173.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The C5 quoting fix made the writer correctly emit `value: "-foo"` for values that would otherwise change meaning when read as YAML. But the loader's parseYamlScalar didn't unwrap quoted scalars — it just trimmed and returned the literal string, including the quote characters. So a read-modify-write cycle would silently mutate the value. Teach parseYamlScalar to unwrap both double-quoted scalars (\ and \" escapes, matching the writer) and single-quoted scalars (with the YAML 1.2 '' escape) BEFORE the bareword tests, so "true" / "false" / "null" round-trip as strings instead of being coerced to JS literals. This also fixes a pre-existing latent bug in manage-headers' transform- headers.js, which reads CSP-style HTTP header values written by `pac pages download-code-site` (those values contain `:` / `;` / `'` and are written quoted). Before this change the manage-headers UX was showing the literal value-with-quotes; now it shows the actual content. Adds 8 tests: 4 unwrap cases in powerpages-config.test.js (double-quote with escapes, single-quote with '' escape, plain-scalars-untouched regression guard, bareword coercion regression guard) and 4 writer-then- loader round-trip tests in create-site-setting.test.js (leading hyphen, embedded quotes, reserved barewords, literal backslash). Full audit of the 9 callers of powerpages-config.js found zero regression risks — every caller either reads name/id/integer fields (unaffected) or reads value fields whose pre-existing behavior was broken in the same way (improved). Addresses Copilot review comment N4 on PR #173. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CLI took --envUrl from the user and passed it straight to
helpers.getAuthToken, which interpolates the URL into
`az account get-access-token --resource "${url}"` via execSync. A
malicious --envUrl like 'x"; rm -rf ~; echo "' would break out of the
shell quotes and execute arbitrary commands.
New sanitizeEnvUrl() helper in lib/check-solution-installed.js parses
the URL via new URL(), requires https, rejects userinfo, and returns
url.origin only (scheme + host + port). URL.origin contains no shell-
special characters by the URL spec, so the result is safe to
interpolate. Applied to both the --envUrl CLI flag and the URL returned
by getEnvironmentUrl() for defense in depth.
Adds 8 tests: happy path, strips path/query/fragment/trailing slash,
preserves explicit port, neutralizes shell-injection payloads in path/
query, normalizes WHATWG-stripped newlines/tabs, rejects non-https
protocols, rejects userinfo, rejects garbage input.
This fixes the symptom at the call site. The underlying getAuthToken
helper still uses execSync with string interpolation — hardening it
(switch to execFile with an args array, etc.) affects 30+ callers and
is out of scope for this PR.
Addresses Copilot review comment N2 on PR #173.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Priyanshu Agrawal (priyanshu92)
approved these changes
May 27, 2026
This reverts commit d9941de.
The previous commit reverted the Stop hook architecture and restored PostToolUse:Skill as the only validation trigger. PostToolUse fires when the Skill tool LOADS the skill instructions — before the skill has done any work — which historically caused false-positive failures when re-running setup-auth on a project that had stale auth code from a previous attempt. Fix that without re-introducing the Stop hook: gate the real validation on a finishing marker. The skill writes docs/auth-setup-report.html at the end of Phase 8.3.5; the validator silent-approves until that file exists. This is the marker-file pattern documented in plugins/power-pages/PLUGIN_DEVELOPMENT_GUIDE.md as the recommended alternative to Stop hooks for "did this complete?" checks. Order of checks in validate-auth.js: 1. Not a Power Pages project → silent approve 2. No auth files at all → silent approve (not an auth session) 3. Auth files exist but report not written → silent approve (in-progress) 4. Auth files + report present → run the real validation Test changes: - New withMarker() helper writes the report file - The 11 tests that exercise real validation now call withMarker - New "silent-approves when auth artifacts exist but the finishing report has not been written yet" test for the in-progress path - New "also silent-approves when a date-suffixed report is the only marker on disk" test for repeat runs (the renderer date-suffixes the file when one already exists) Together with the previous revert, this addresses C6, N1, P1, P2 on PR #173. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@microsoft-github-policy-service agree company="Microsoft" |
T-Nid (tyaginidhi)
approved these changes
May 28, 2026
Hjalmar Otto Fjøsne (Hjaf)
added a commit
to equinor/power-platform-skills
that referenced
this pull request
May 28, 2026
Upstream range: 2923950..b4c46e9 Affected plugins: power-pages Merge strategy: direct-copy (17 files) Upstream PR microsoft#173: setup-auth multi-provider authentication and SPA flows support - Multi-provider auth (Entra ID, OIDC, SAML2, WS-Federation, local, social) - SPA-ified auth pages via Code-Site-Shell-Header - Optional /user-profile SPA page - New scripts: check-solution-installed.js, render-auth-report.js - YAML quoting fix in create-site-setting.js - 25 eval scenarios (was 9) - Comprehensive unit tests for all scripts
Hjalmar Otto Fjøsne (Hjaf)
added a commit
to equinor/power-platform-skills
that referenced
this pull request
May 28, 2026
- Updated scope summary to include multi-provider auth - Added evidence for upstream PR microsoft#173 - Updated notes with security assessment of new scripts - Status remains 'defer' (existing blockers unchanged)
4 tasks
5 tasks
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.
Summary
AUTH_PROVIDERSarray drives the SPA; Login page renders providers from configuration with the layout chosen by the maker (single button / multi-button / dropdown).Code-Site-Shell-Header—/Account/Login/{ResetPassword,RedeemInvitation,Register,ExternalLoginCallback,TermsAndConditions,ExternalAuthenticationFailed}and/SignInredirect to SPA equivalents, preserving query params. Server-bounce paths (e.g.,/profile→/SignIn?ReturnUrl=...) stay inside the SPA./user-profileSPA page (opt-in in Phase 2.1) —getMyProfile/updateMyProfilevia Power Pages Web API with anti-forgery +If-Match, Self-scope table permission oncontact, simplified form (firstname, lastname, mobile, address), read-only Account Details, header repaints instantly after save viaapplyContactUpdateLocally+ spread-on-refresh fix.check-solution-installed.jsauto-detects the GDPR Privacy Extensions solution for the Terms prereq (falls back to manual prompt on infra failure).render-auth-report.jsproduces a Fluent-styled HTML report atdocs/auth-setup-report.htmlafter every run, masking any setting whose name containsSecret.upn(v1.0 tokens lackemail);getUserDisplayNamefalls back to email before the OIDC subject; external provider button text wraps instead of mid-string ellipsis; defaults flipped to "First sign-in only" sync and "Link to existing contact" linking to match the common CRM workflow; built-in 2FA explicitly NOT scaffolded (server-rendered SendCode/VerifyCode can't be SPA-ified — recommend IdP-level MFA).