Skip to content

Do not register SWA auth scheme when auth property is omitted - #3813

Open
aaronburtle wants to merge 2 commits into
mainfrom
dev/aaronburtle/auth-MSRC-investigation
Open

aaronburtle wants to merge 2 commits into
mainfrom
dev/aaronburtle/auth-MSRC-investigation

Conversation

@aaronburtle

Copy link
Copy Markdown
Contributor

Why make this change?

Closes #3812

What is this change?

  • A loaded configuration that omits runtime.host.authentication now uses Unauthenticated, matching the documented default, rather than falling back to EasyAuth.
  • Separates two startup cases:
    • Configuration loaded, authentication omitted: register Unauthenticated as the production default.
    • No configuration available yet: preserve the existing hosted late-configuration bootstrap.
  • Updates request-time scheme resolution so a missing or blank provider selects Unauthenticated. This also protects development mode, where EasyAuth handlers remain registered for hot reload.
  • Preserves existing explicit-provider checks and hosted late-configuration behavior.
  • Adds a design note documenting the fix, compatibility boundaries, and existing hosted EasyAuth requirements: trusted identity injection, header sanitization, and prevention of direct-origin access.

How was this tested?

  • Integration Tests
    • Added full ASP.NET startup/request-pipeline tests using TestServer, with the authentication property literally omitted and both Azure hosting indicators unset.
    • Covered production and development modes with both SWA- and App Service principals
    • Verified existing bootstrap behavior through /configuration and /configuration/v2, including readiness before hydration and authentication afterward.
    • Verified all six new cases with DAB_CONFIG_AUTH_TOKEN both unset and set. The compatibility test supplies the token only on its configuration POST.
  • Unit Tests

Sample Request(s)

N/A

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The reviewed changes meet the intended authentication behavior and include integration coverage.

Pull request overview

Updates authentication defaults so omitted configuration uses Unauthenticated while preserving late-configuration EasyAuth behavior.

Changes:

  • Adjusts startup authentication registration.
  • Resolves missing or blank providers to Unauthenticated.
  • Adds integration coverage for startup and request-pipeline scenarios.
File summaries
File Description
src/Service/Startup.cs Separates loaded-configuration defaults from late-configuration bootstrap.
src/Service.Tests/Configuration/ConfigurationTests.cs Tests omitted authentication and late-configuration scenarios.
src/Core/AuthenticationHelpers/ClientRoleHeaderAuthenticationMiddleware.cs Selects unauthenticated handling for missing or blank providers.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@RubenCerna2079 RubenCerna2079 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Approving as I assume comment will be addressed.

]);
HttpContext context = await server.SendAsync(requestContext =>
{
requestContext.Request.Path = "/api/not-an-entity";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we use a non-existent entity? Wouldn't it be better to use one that already exists?

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

Labels

Projects

Status: Review In Progress

Development

Successfully merging this pull request may close these issues.

Avoid registering SWA as an auth scheme with authentication property is omitted

4 participants