Add auth to sdk

[justindbaur]

🎟️ Tracking

📔 Objective

This adds the now published Bitwarden.Server.Sdk.Authentication library to Bitwarden.Server.Sdk. It's enabled by default but able to be remove using <BitIncludeAuthentication>false</BitIncludeAuthentication>

I am also following the pattern of using README.md as the more detailed documentation about how the package works or how it should be developed and using PACKAGE.md for the documentation about how someone would consume the package.

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[claude]

Claude finished @justindbaur's task —— View job

---

Code Review Complete

I've thoroughly reviewed this PR which adds the Bitwarden.Server.Sdk.Authentication package to the main SDK. Here's my comprehensive analysis:

---

Summary of Changes

This PR integrates authentication capabilities into the Bitwarden Server SDK with the following key changes:

1. MSBuild Integration: Added BitIncludeAuthentication property (defaults to true) with opt-out support
2. Service Registration: Integrated AddBitwardenAuthentication() into both IHostApplicationBuilder and IHostBuilder paths
3. Documentation: Added PACKAGE.md for consumer documentation, updated README.md for developer documentation
4. Testing: Expanded test matrix from 2D (telemetry × features) to 3D (telemetry × features × authentication)
5. Minor Cleanup: Removed BitIncludeFeatures overrides in test fixtures

---

Critical Issues

1. ⚠️ Missing Middleware Integration for Web SDK Users

Location: extensions/Bitwarden.Server.Sdk/src/Content/HostBuilderExtensions.cs:43-45

The SDK registers authentication services via AddBitwardenAuthentication(), but it never calls UseBitwardenAuthentication() to add the middleware to the pipeline.

The authentication package provides AuthenticationApplicationBuilderExtensions.UseBitwardenAuthentication() which:

• Calls UseAuthentication()
• Adds PostAuthenticationLoggingMiddleware

However, SDK consumers using UseBitwardenSdk() will need to:

1. Manually call app.UseAuthentication() or app.UseBitwardenAuthentication()
2. Ensure correct middleware ordering (after UseRouting(), before UseAuthorization())

Impact: The authentication feature is only half-integrated. Services are registered but middleware is not, which means authentication won't actually work unless consumers manually add the middleware.

Recommendation: Either:

• Option A (Preferred): Add automatic middleware registration when using WebApplication in UseBitwardenSdk():

  public static TBuilder UseBitwardenSdk<TBuilder>(this TBuilder builder)
      where TBuilder : IHostApplicationBuilder
  {
      // ... existing service registrations ...
      
      #if BIT_INCLUDE_AUTHENTICATION
      builder.Services.AddBitwardenAuthentication();
      
      // Register middleware automatically for WebApplication
      if (builder is WebApplication app)
      {
          app.UseBitwardenAuthentication();
      }
      #endif
      
      return builder;
  }

• Option B: Update PACKAGE.md to clearly document that consumers must manually call app.UseBitwardenAuthentication() and show proper middleware ordering

2. ⚠️ Version Pinning Inconsistency

Location: extensions/Bitwarden.Server.Sdk/src/Sdk/Sdk.targets:53

<PackageReference Include="Bitwarden.Server.Sdk.Authentication" Version="[0.1.0]" />

The Authentication package's actual version is 0.2.0-beta.1 (per Bitwarden.Server.Sdk.Authentication.csproj:11), but the SDK references [0.1.0]. This will cause a runtime package resolution failure.

Recommendation: Update to Version="[0.2.0]" to match the actual package version being published.

---

Suggested Improvements

3. 🎨 Documentation Clarity

Location: extensions/Bitwarden.Server.Sdk/src/PACKAGE.md:25-32

The authentication section mentions the feature "registers Bitwarden style authentication" but doesn't explain:

• What "Bitwarden style" means (JWT Bearer with specific claim mapping)
• That middleware setup may be required
• Configuration options (e.g., Authentication:Schemes:Bearer config section)

Recommendation: Expand the documentation to include:

## Authentication

Enabled by default and able to be removed using
`<BitIncludeAuthentication>false</BitIncludeAuthentication>` in your project file.

This feature automatically includes the `Bitwarden.Server.Sdk.Authentication` library and when using
the `Microsoft.NET.Sdk.Web` SDK it will register Bitwarden style authentication services in
`UseBitwardenSdk()`. 

### What's Included
- JWT Bearer authentication with Bitwarden-specific configuration
- Claim mapping disabled for consistency between issuing and consuming sides
- Audience validation disabled (per Bitwarden identity service behavior)
- Automatic subject claim logging via `PostAuthenticationLoggingMiddleware`

### Configuration
Configure JWT Bearer options via the `Authentication:Schemes:Bearer` configuration section.
See the [Authentication package README](../Bitwarden.Server.Sdk.Authentication/README.md) for details.

### Usage Notes
[Document whether middleware is automatic or manual]

4. 🎨 Preprocessor Directive Naming Convention

Location: Multiple files

The codebase uses BIT_INCLUDE_TELEMETRY, BIT_INCLUDE_FEATURES, and BIT_INCLUDE_AUTHENTICATION as preprocessor directives. Consider documenting this pattern in the SDK README since these constants are surfaced to consuming projects and could be useful for conditional compilation.

5. 📝 Test Cleanup Explanation

Location: ConfigTestFixture.cs:106 and TelemetryProjectFixture.cs:75

The removal of { "BitIncludeFeatures", "false" } from test fixtures appears intentional but unexplained.

Impact: Tests now include Features and Authentication packages by default where they previously only included the base SDK.

Recommendation: Consider if this was intentional or if these tests should disable all optional features for isolation:

{ "BitIncludeFeatures", "false" },
{ "BitIncludeAuthentication", "false" },

6. 🤔 Missing Integration Test for Authentication Usage

Location: extensions/Bitwarden.Server.Sdk/tests/Bitwarden.Server.Sdk.IntegrationTests/SdkTests.cs

Similar to the FeaturesTurnedOn_CanUseFeatureService test (line 93), there should be a test that verifies authentication services are actually registered and usable.

Recommendation: Add test:

[Fact]
public void AuthenticationTurnedOn_CanUseAuthenticationServices()
{
    ProjectCreator.Templates.SdkProject(
        out var result,
        out var buildOutput,
        customAction: (project) =>
        {
            project.Property("BitIncludeAuthentication", bool.TrueString);
        },
        additional: """
            app.MapGet("/test", (HttpContext context) => 
            {
                var authService = context.RequestServices.GetRequiredService<Microsoft.AspNetCore.Authentication.IAuthenticationService>();
                return Results.Ok(authService != null);
            });
            """
    );

    Assert.True(result, buildOutput.GetConsoleLog());
}

---

Good Practices Observed

• ✅ Consistent feature flag pattern matching Telemetry and Features
• ✅ Comprehensive test matrix covering all feature combinations
• ✅ Proper use of preprocessor directives for conditional compilation
• ✅ Documentation split between developer (README.md) and consumer (PACKAGE.md) perspectives
• ✅ Integration tests verify compilation with feature enabled/disabled

---

Action Items for Author

1. Critical: Fix version pinning from [0.1.0] to [0.2.0] in Sdk.targets:53
2. Critical: Address middleware registration (either automatic or document manual requirement)
3. High: Expand PACKAGE.md documentation to explain authentication configuration and usage
4. Medium: Consider adding integration test for authentication service registration
5. Low: Document whether test fixture cleanup (removal of BitIncludeFeatures=false) was intentional

---

[github-actions]

Checkmarx One – Scan Summary & Details – 7db86fe3-173c-4d26-b5d0-f2c547c38a3c

Great job! No new security vulnerabilities introduced in this pull request

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 39.56%. Comparing base (08e6877) to head (3933a4c).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #192   +/-   ##
=======================================
  Coverage   39.56%   39.56%           
=======================================
  Files          40       40           
  Lines        1049     1049           
  Branches       85       85           
=======================================
  Hits          415      415           
  Misses        605      605           
  Partials       29       29           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[withinfocus]

The first Claude-reported issue isn't accurate but there are some other decent suggested improvements -- how about a follow-up PR with those improvements plus instructions file updates to clarify how middleware is used?