VPR-61 feat(auth): add Entra ID login alongside CAS - #298
Open
rlorenzo wants to merge 1 commit into
Open
Conversation
Campus is retiring CAS in favor of Entra ID. Both providers now sign in to the same cookie, so a session is identical downstream whichever was used, and the cutover becomes a config change rather than a code change. - Register an OpenID Connect handler (auth code + PKCE) and remap Entra claims to the bare, lowercased kerberos id ClaimsTransformer resolves users by. Casing matters: UserHelper.IsInRole and the emulation cache key both compare login ids with an ordinal ==, so a mixed-case UPN would break emulation - Gate providers on Authentication:EnabledProviders (Cas, EntraId or Both). TEST runs both, PROD stays CAS - Widen the default and SVMUser policies, which required a "CAS" authentication-method claim and would have rejected every Entra session - Translate Entra's amr=mfa into the credentialType claim the Duo 2FA policy reads, so both providers satisfy [Authorize(Policy = "2faAuthentication")] - Keep /login as the single entry point every existing sign-in link uses; it picks the provider, or defers to the splash when both are offered - Fall back to CAS when no provider is usable. Throwing killed the host, so a half-finished cutover would have taken CAS down with it - Send an Entra user to the app root on logout once Entra is switched off, rather than to a CAS logout they never had a session for
rlorenzo
force-pushed
the
feature/VPR-61-entra-id-login
branch
from
August 6, 2026 20:57
eebcda2 to
7a9de4a
Compare
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.
Stacked on #216 (
feature/VPR-151-dynamic-login-screen) — review that one first; this PR targets it, so the diff here is only the Entra work.Campus is retiring CAS in favor of Microsoft Entra ID. This adds Entra as a second login provider so TEST can run both side by side, and makes the eventual cutover a config change rather than a code change.
Closes VPR-61.
Config
New
Authentication:EnabledProviders, acceptingCas,EntraId, orBoth.CasBothCasCutover is editing that one value to
EntraId.EntraId:ClientSecretis not committed. It follows the existing secret pattern: AWS SSM Parameter Store at/{Environment}/EntraId/ClientSecretin TEST/PROD,EntraId__ClientSecretin.env.locallocally.TenantIdandClientIdare not secrets and live inappsettings.{Env}.json.How it works
Entra is a standard OIDC handler (auth code + PKCE) that signs in to the same cookie CAS already uses, so nothing downstream needs to know which provider a user picked.
OnTokenValidatedrewrites the principal into the exact claim shapeAuthenticateCasLoginproduces. That matters becauseClaimsTransformerdoesprincipal.Identity.Name→UserHelper.GetByLoginId, soClaimTypes.Namehas to be the bare kerberos id. Entra hands backpreferred_usernameasjdoe@ucdavis.edu, so the domain is stripped. Both the source claim and the stripping are configurable, so if theonpremisessamaccountnameoptional claim gets enabled on the app registration later, that is a config switch. If no usable id resolves, the login is rejected rather than allowed through role-less./loginstays the single entry point every existing sign-in link already uses (Razor layout, session timeout, profile menu, VuebuildLoginUrl) — none of those call sites changed. It picks the provider when only one is enabled, and defers to the splash when both are. The splash's own buttons pass?provider=explicitly, which is what stops the CAS button from bouncing back to the splash.Two things that would have broken every Entra login
Program.csgated both theSVMUserpolicy and the app-wideDefaultPolicyonAuthenticationMethod == "CAS"literally. Entra sessions would have failed authorization on every request. Both now accept either provider.[Authorize(Policy = "2faAuthentication")]) checks acredentialTypeclaim that only CAS emits. Entra reports MFA inamrinstead, so that is translated into an accepted credential type.Failure modes found in review and fixed
This went through an automated review loop plus an adversarial pass. Five real defects came out of it, all fixed here:
EnabledProvidersstops includingEntraIdwhile Entra cookies are still valid (12h expiry), the user fell through to the CAS logout redirect. They never had a CAS session, so they got the wrong page and their upstream Entra session was never ended. The branch keyed off current config where it should key off how the user actually authenticated.EnabledProviders = EntraIdwith any one Entra setting missing narrowed the set toNone, which threw, which propagated out of the startuptryand killed the host, taking CAS with it. Now it degrades to CAS with a fatal log. Losing the site is a far worse outcome than serving the provider that still works./apiguard ran after provider dispatch inLogin, so an/api/...ReturnUrl reached the Entra challenge or the chooser splash instead of 401ing, whenever Entra was enabled./welcome?ReturnUrl=/api/...returned 200 inBothmode. That mode skips the deep-link branch, which is what enforced the guard. Now stated explicitly onWelcome, so all provider modes agree.AuthorityOverridewas unused and madeIsConfiguredaccept a tenant or an override, letting a tenant-less config register the handler. Removed.Tests
2922 backend tests pass. New coverage: the claim mapper (domain stripping, fallback ordering, casing,
amrhandling, principal shape),EntraIdSettings.IsConfigured, the Duo 2FA gate end-to-end through the mapper, and controller tests for provider selection, the/apiguard in every provider mode, all four logout paths, and a regression guard for the/welcome↔/loginloop.Note on the unrelated RAPS one-liner
Adding the OIDC package upgrades
Microsoft.IdentityModel.*, which drops the incidentalIsNullOrEmpty()extensionRAPSController.cs:234was relying on. Swapped for.Count > 0, matching whatRAPSSecurityService.cs:162already does for the same call. It was a compile break, not optional.Before TEST can actually use this
Two things are needed, both outside this repo:
/Test/EntraId/ClientSecret.https://secure-test.vetmed.ucdavis.edu/2/signin-entraas a redirect URI on the app registration (the/2PathBase is included automatically by ASP.NET, so it must be part of the registered URI).Until the secret exists, startup logs a fatal and falls back to CAS-only rather than showing a button that dead-ends.
TenantIdis set to the UC Davis campus tenant, verified against the published OIDC discovery document forucdavis.edu(and matching what VIPER 1's alumni portal already uses).Not smoke-tested against the real tenant, since the secret isn't provisioned. The claim mapping is reasoned from the OIDC spec and Entra's documented token shape, not observed. The first real login is the thing to watch: specifically that
preferred_usernamearrives in the shape we expect.One known gap left deliberately:
SaveTokens = false, so federated sign-out has noid_token_hintand Entra may show an account picker at logout instead of signing straight out. Storing the id_token would fix it, but the retrieval path is framework-version-specific and I can't verify it without a working tenant, so I'd rather confirm the behavior on TEST first than ship unverified auth plumbing.Login id casing
Worth a reviewer's eye.
UserHelper.cs:94comparesAaudUser.LoginIdtoUser.Identity.Namewith an ordinal, case-sensitive==, and the emulation cache is keyed on this claim inClaimsTransformerbut onAaudUser.LoginIdinEmulateUser. A mixed-case Entra UPN would therefore silently break emulation and force a DB round trip on every role check. The resolved id is lowercased to match what CAS supplies. That assumes AAUD login ids are lowercase kerberos ids, which is what CAS has been feeding it all along.