Api/Auth: Adds AuthorizationHandler and refactor authcontrollers by losolio · Pull Request #12 · incendilabs/ignis

losolio · 2026-03-10T17:29:36Z

This adds an AuthorizationHandler that provides an easy way for api/web projects to add auth with a thin controller. It also split AddIgnisAuth into three focused extension methods: AddIgnisAuthServer and AddIgnisClientSync.

The motivation is to move toward a library model where the host application has full control over what gets registered and when. Instead of Ignis.Auth silently adding controllers and removing them via ApplicationPartManager hacks, the host now owns the controller and explicitly opts in to each capability.

Copilot

Pull request overview

Refactors Ignis authentication to expose the token endpoint logic via a reusable AuthorizationHandler, shifting host apps (e.g., Ignis.Api) to provide a thin /connect/token controller and wiring auth via more focused extension methods.

Changes:

Introduces AuthorizationHandler in Ignis.Auth and removes the Ignis.Auth MVC controller.
Splits auth wiring into AddIgnisAuthServer, AddIgnisClientSync, and SyncOAuthClientsAsync, and updates Ignis.Api startup accordingly.
Updates tests and Ignis.Auth README to match the new configuration surface (e.g., AllowedGrantTypes, removal of endpoint settings/redirect URIs).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/Ignis.Api.Tests/IntegrationFixture.cs	Updates required env vars for auth client configuration.
tests/Ignis.Api.Tests/IgnisApiFactory.cs	Simplifies in-memory auth config for tests.
src/Ignis.Auth/README.md	Updates documentation to describe the handler + thin-controller approach and config changes.
src/Ignis.Auth/Extensions/AuthServiceExtensions.cs	Adds client-sync registration and startup execution helper (`SyncOAuthClientsAsync`).
src/Ignis.Auth/Extensions/AuthServerExtensions.cs	Renames/refactors server registration into `AddIgnisAuthServer` and registers the handler.
src/Ignis.Auth/Controllers/AuthorizationController.cs	Removes the built-in token endpoint controller from the auth library.
src/Ignis.Auth/AuthorizationHandler.cs	Adds reusable token endpoint logic to be invoked by host controllers.
src/Ignis.Auth/AuthSettings.cs	Removes endpoint settings and keeps client grant types + certificates.
src/Ignis.Api/Program.cs	Updates auth wiring, client sync startup call, and middleware setup.
src/Ignis.Api/Controllers/AuthorizationController.cs	Adds thin token endpoint controller delegating to `AuthorizationHandler`.

Comments suppressed due to low confidence (2)

src/Ignis.Auth/Extensions/AuthServerExtensions.cs:42

AuthorizationHandler is referenced unqualified from the Ignis.Auth.Extensions namespace, but the type is declared in Ignis.Auth. This will not compile unless you add using Ignis.Auth; (or fully-qualify the type name) before calling AddTransient.
src/Ignis.Auth/Extensions/AuthServerExtensions.cs:24
ArgumentException.ThrowIfNullOrWhiteSpace(settings.ConnectionString) is called without a parameter name, which produces a much less actionable exception than the previous explicit config-path message. Consider providing a meaningful paramName (e.g., "AuthSettings:ConnectionString") or throwing an ArgumentException with a clear message so misconfiguration is easier to diagnose.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-10T17:36:56Z

src/Ignis.Api/Program.cs

 builder.Services.AddFhir(sparkSettings);

-// The project reference to Ignis.Auth causes auto-discovery of its controllers.
-// Remove them when auth is disabled to avoid DI resolution failures.
-builder.Services.AddControllers()
-    .ConfigureApplicationPartManager(manager =>
-    {
-        if (!authSettings.Enabled)
-        {
-            var authAssemblyName = typeof(AuthSettings).Assembly.GetName().Name;
-            var authPart = manager.ApplicationParts
-                .FirstOrDefault(p => p.Name == authAssemblyName);
-            if (authPart != null)
-                manager.ApplicationParts.Remove(authPart);
-        }
-    });
+builder.Services.AddControllers();



Controllers are now always registered, and the /connect/token controller lives in Ignis.Api, so the token endpoint will show up in API explorer/OpenAPI even when AuthSettings.Enabled is false (it will just return 404). If the intent is to hide the token endpoint when auth is disabled (as the previous ApplicationPart removal did), consider conditionally mapping the token endpoint only when auth is enabled (e.g., via a minimal API route) or otherwise excluding it from OpenAPI when disabled.

Addressed — the controller now uses constructor injection and AuthSettings.Enabled has been removed entirely. Auth is always registered when Ignis.Auth is referenced, so there's no longer a state where the handler is missing. The OpenAPI visibility follows naturally from that.

Copilot · 2026-03-10T17:36:57Z

src/Ignis.Api/Controllers/AuthorizationController.cs

+using Microsoft.Extensions.DependencyInjection;
+
+namespace Ignis.Api.Controllers;
+
+[ApiController]
+public class AuthorizationController : ControllerBase
+{
+    private AuthorizationHandler? Handler =>
+        HttpContext.RequestServices.GetService<AuthorizationHandler>();
+
+    /// <summary>Exchange credentials for an access token (OAuth 2.0 client_credentials grant).</summary>
+    [HttpPost("~/connect/token")]
+    [ProducesResponseType(typeof(object), StatusCodes.Status200OK, "application/json")]
+    [ProducesResponseType(StatusCodes.Status400BadRequest)]
+    [ProducesResponseType(StatusCodes.Status401Unauthorized)]
+    public Task<IActionResult> Exchange() =>
+        Handler?.ExchangeAsync(HttpContext) ?? Task.FromResult<IActionResult>(NotFound());


This controller uses HttpContext.RequestServices.GetService(...) (service locator) and returns NotFound() when the handler isn't registered. That makes misconfiguration harder to detect and is inconsistent with the constructor-injection pattern used by other controllers in this project. Consider using constructor injection for AuthorizationHandler and registering/mapping this endpoint only when auth is enabled (so startup fails fast if auth is enabled but not configured).

Suggested change

using Microsoft.Extensions.DependencyInjection;

namespace Ignis.Api.Controllers;

[ApiController]

public class AuthorizationController : ControllerBase

{

private AuthorizationHandler? Handler =>

HttpContext.RequestServices.GetService<AuthorizationHandler>();

/// <summary>Exchange credentials for an access token (OAuth 2.0 client_credentials grant).</summary>

[HttpPost("~/connect/token")]

[ProducesResponseType(typeof(object), StatusCodes.Status200OK, "application/json")]

[ProducesResponseType(StatusCodes.Status400BadRequest)]

[ProducesResponseType(StatusCodes.Status401Unauthorized)]

public Task<IActionResult> Exchange() =>

Handler?.ExchangeAsync(HttpContext) ?? Task.FromResult<IActionResult>(NotFound());

namespace Ignis.Api.Controllers;

[ApiController]

public class AuthorizationController : ControllerBase

{

private readonly AuthorizationHandler _handler;

public AuthorizationController(AuthorizationHandler handler)

{

_handler = handler;

}

/// <summary>Exchange credentials for an access token (OAuth 2.0 client_credentials grant).</summary>

[HttpPost("~/connect/token")]

[ProducesResponseType(typeof(object), StatusCodes.Status200OK, "application/json")]

[ProducesResponseType(StatusCodes.Status400BadRequest)]

[ProducesResponseType(StatusCodes.Status401Unauthorized)]

public Task<IActionResult> Exchange() =>

_handler.ExchangeAsync(HttpContext);

Agree, the PR has been updated to reduce the config complexity. Ignis.Auth is now more of a library, and the Ignis.Api project is using it.

Move endpoint logic from Ignis.Auth controller into AuthorizationHandler, letting the host wire up a thin controller with constructor injection. Split AddIgnisAuth into AddIgnisAuthServer and AddIgnisClientSync. Remove the Enabled flag — as the consuming project now has control.

Copilot AI review requested due to automatic review settings March 10, 2026 17:29

Copilot started reviewing on behalf of losolio March 10, 2026 17:30 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

losolio force-pushed the refactor/auth-controller branch from 4190022 to a8aeb0d Compare March 10, 2026 18:00

losolio force-pushed the refactor/auth-controller branch from a8aeb0d to 59e6624 Compare March 10, 2026 18:17

losolio requested review from kennethmyhra March 10, 2026 18:38

kennethmyhra approved these changes Mar 14, 2026

View reviewed changes

kennethmyhra merged commit d97b4c8 into main Mar 14, 2026
5 checks passed

kennethmyhra deleted the refactor/auth-controller branch March 14, 2026 10:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Api/Auth: Adds AuthorizationHandler and refactor authcontrollers#12

Api/Auth: Adds AuthorizationHandler and refactor authcontrollers#12
kennethmyhra merged 1 commit intomainfrom
refactor/auth-controller

losolio commented Mar 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

losolio Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

losolio Mar 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

losolio commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

losolio Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

losolio Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

losolio commented Mar 10, 2026 •

edited

Loading