Repository navigation
Add OpenID Connect sign-in - #2459
Merged
Merged
Conversation
pournasserian
added
type:feature
4 tasks done
pournasserian
added a commit
that referenced
this pull request
Oct 18, 2026
Closes #2458 Refs #2434 ## What The third slice of F-AUD (#2424): the first screen on the log, behind the sign-in from #2450. - **Core**: `IAuditExporter.ExportAsync` now commits an `audit.exported` event in its own unit of work before it writes anything. The event names the current initiator and, when the selection is one item's history, that item as its subject. An invalid query or format records nothing. The query runs only when the export starts writing, so a selection that reaches the present contains its own export event. `AuditErrorCodes` replaces the string literals in `AuditQuery` and the exporter. - **Admin**: `/audit`, server-rendered without interactivity like the account pages, with an "Audit log" link in the main layout. - The export form takes from and to as UTC dates (both days included, either optional), an optional item type and id, and Download JSON or Download CSV. - The history lookup lists an item's events oldest first. Each event shows its operation, a User or Agent badge, the initiator's name, the time, and the agent credential, model, prompt hash, sources, change set, previous version and event id when they are set. It shows at most 200 events and points to the export for the rest. - `AuditExportForm` maps the posted fields to an `AuditQuery` and a format and reports every problem at once, including unreadable dates and a malformed id. - Strings go through `IStringLocalizer`, and the layout uses logical utilities only. - **Host**: `POST /audit/export` maps the form, calls the exporter and streams the file with `Content-Disposition: attachment` and `Cache-Control: no-store`. The file name is `audit-log` or `audit-history-<id>`, so nothing typed into the form reaches the header. It requires a signed-in user, and form binding applies the anti-forgery check. A rejected form goes back to `/audit` with its values and error codes in the query, and the page puts each message next to its field. `GET /audit/export` redirects to the page, for someone whose session ran out, who signs in again and comes back with a GET. - **Component** (refs #2434): Badge, with the shadcn variants and bUnit tests. The rest of the page uses Input, FormField, Button and Card unchanged. - **CONTRIBUTING.md**: the recorded export and the download endpoint. ## Why - **An export records itself.** AUD-01 covers every operation, and an export is where data leaves the system. Recording it in Core means any later exposure, an API or an MCP tool, is recorded too without doing anything. Viewing an item's history on screen is a read and is not recorded. The event has no field for the window or the format, so they are not recorded. Adding them needs a details field on the audit event, which is a model change for a later slice. - **One HTTP endpoint despite ADR-0006's "no admin HTTP API".** A Razor page cannot answer with a file. The endpoint is not an API: it has no contract of its own, only maps the page's form onto the same Core service, and is protected like every page form. It lives in Host because Admin references only Core and the components package. - **The history is a list, not a table.** An event has up to fourteen fields, too wide for a table row. ADR-0011 puts tables on QuickGrid, a new package, which this screen does not need; the first tabular list, such as the browsable log in ADM-01, can bring it. - **A forged token gets a 400, not a 500.** The endpoint's 400 has no body, so the not-found page used to re-run as a POST and the anti-forgery check then threw on the same rejected form. #2459 since stopped status code pages for anything but GET and HEAD across the host, so the endpoint needs nothing of its own; the forged-token test covers it. No new NuGet dependency and no migration. ## Tests Unit: form mapping (UTC days with both ends included, empty fields select the full log, a one-day window, an item's history, every problem reported at once, unreadable dates per field, half an item, the return query keeps values and lists each code once). Exporter: the export event is committed before the first byte in both formats; an item's export names the item; an invalid query or format records nothing and opens no unit of work. bUnit: Badge default and variants. Integration through the running host: anonymous `GET /audit` and `POST /audit/export` go to sign-in and record nothing. A signed-in JSON download of an item's history and a CSV download of a date range return the selected events, each export's own event included, as attachments with the right content type, file name, `no-store` and the CSP; each records one `audit.exported` event naming the user. A rejected export redirects to the page with both messages next to their fields, the values kept, and nothing recorded. A forged anti-forgery token gets 400 and records nothing. The history lookup shows an item's events oldest first with every field, encodes a source containing a script tag, leaves out other items' events, and handles an unknown and a malformed id. The layout links to the page. The integration tests from #2449 now also check that an item's export contains its own record and that an empty selection is still recorded. Checked by hand in the browser: the page in light, dark and RTL, a rejected export coming back with its errors, the history of the signed-in user, and no CSP violations. ## Checklist - [x] Linked to an issue a maintainer has confirmed - [x] Targets `v2` (or `dev` for a v1 security fix) - [x] Tests or docs updated where behaviour changed - [x] No tool-generated co-author trailers or footers
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.
Closes #2457
What
The external identity option from USR-01: one OpenID Connect provider the owner configures. It is off until configured, so nothing external is needed (P10).
oidcscheme fromAuthentication:OpenIdConnect(Authority,ClientId,ClientSecret,DisplayName,AssumeVerifiedEmail), registered only when the authority and client id are set. It uses the authorization code flow with PKCE, the response posted back as a form, and theopenid profile emailscopes, and signs into Identity's external cookie.POST /sign-in/external/startstarts the challenge; its form binding makes the anti-forgery token required. An authority that is not HTTPS fails at startup, with plain HTTP allowed only in Development. Otherwise the handler would throw on every request.IUserSignIngainsGetExternalProvidersAsyncandSignInExternalAsync.CookieUserSignIn): a login is the provider's issuer plus subject, because a subject is unique only at its issuer. The first time, it links to the user with the same email, and only ifemail_verifiedis true or the owner setAssumeVerifiedEmail(for Entra, which never sends it). An identity with no matching user, an unverified email or a disabled account is refused and nothing is linked. Linking appendsuser.login_linkedand the sign-in appendsuser.signed_in, both with the user as initiator, in one unit of work, before the cookie is issued./sign-inwhen a provider is configured./sign-in/externalcompletes the sign-in and moves on to the return URL, or explains the refusal and links back to sign-in. Only existing components are used.form-actionnow also allows the provider's origin, because browsers check it on the redirect that leaves the start post. Nothing else in the policy changed./not-found, whose second anti-forgery check throws once the first has failed (500 instead of 400). This was latent since Add sign-in, first-run setup and web hardening #2450 and surfaced with the first minimal-API form.CONTRIBUTING.md: how to configure the provider and register the redirect URI.Why
USR-01 asks for at least one external identity option, and ADR-0009 settles it as a generic OpenID Connect provider. Choices made here:
New dependency:
Microsoft.AspNetCore.Authentication.OpenIdConnect10.0.13, Microsoft's handler at the version of the other ASP.NET Core packages. It bringsMicrosoft.IdentityModel.Protocols.OpenIdConnectandMicrosoft.IdentityModel.JsonWebTokenswith it. No migration: Identity'sUserLoginstable holds the links.Tests
Integration, through the host, against
TestOpenIdProvider, a small in-process provider in the test project (discovery, keys, authorize with form_post, token with PKCE andat_hash, signed with a test RSA key). It needs no external service and no test package.form-action 'self'only, and the start endpoint answers 404./ui-spikeand is audited asuser.login_linkedthenuser.signed_inwith the user as initiator.email_verifiedand a disabled account are each refused, with no cookie, no link and no audit event.AssumeVerifiedEmaillets a provider withoutemail_verifiedthrough.Checked by hand in the browser: the button under the divider, and the refusal page in RTL.
Checklist
v2(ordevfor a v1 security fix)