Enforce the essential acr level when the flow finishes without a step-up by michalvavrik · Pull Request #53587 · keycloak/keycloak

michalvavrik · 2026-10-06T10:07:36Z

closes: [CVE-2026-97176] Essential (forced) acr requirement silently bypassed when a browser flow re-evaluates an already active session with no interactive step #53157

Copilot

Copilot review overview

🟡 Changes recommended

Forced ACR enforcement remains bypassable for browser flows without a conditional LoA authenticator.

Review effort: Balanced
Findings: 1

Open (1)

Enforce numeric ACR when no ConditionalLoaAuthenticator is configured · New

What changed in this PR

Enforces mandatory LoA checks when authentication finishes without executing a step-up condition, addressing CVE-2026-97176.

Changes:

Registers forced-LoA validation at top-level flow completion.
Adds protocol-specific OIDC and SAML error handling.
Adds regression tests and upgrade documentation.

File	Description
`testsuite/.../LevelOfAssuranceFlowSamlTest.java`	Adds SAML SSO regression tests.
`tests/.../LevelOfAssuranceEssentialAcrTest.java`	Adds OIDC essential-ACR coverage.
`services/.../OIDCLoginProtocol.java`	Maps LoA failures to the OIDC error.
`services/.../AuthorizationEndpointBase.java`	Handles passive LoA failures.
`services/.../DefaultAuthenticationFlow.java`	Registers final forced-LoA validation.
`services/.../AcrNotFulfilledException.java`	Introduces a dedicated exception.
`services/.../ConditionalLoaAuthenticator.java`	Uses the dedicated exception.
`docs/.../changes-27_0_0.adoc`	Documents stricter enforcement.
`core/.../OAuthErrorException.java`	Defines the new OIDC error constant.

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

Copilot

Copilot review overview

🟡 Changes recommended

The new production exception source is missing the package-standard license header.

Review effort: Balanced
Findings: 1 · 1

Open (2)

Enforce numeric ACR when no ConditionalLoaAuthenticator is configured
Add missing Apache 2.0 license header · New

Copilot

Copilot review overview

🔵 Needs a closer look

Security-sensitive authentication-flow and protocol-error changes warrant final human validation.

Review effort: Balanced
Findings: 1

Open (1)

Enforce numeric ACR when no ConditionalLoaAuthenticator is configured

Resolved since last review (1)

Add missing Apache 2.0 license header

keycloak-github-bot · 2026-10-07T10:32:43Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.broker.KcSamlBrokerTest#loginInResponseToEmpty

Keycloak CI - FIPS IT (strict)

jakarta.ws.rs.InternalServerErrorException: HTTP 500 Internal Server Error
	at org.jboss.resteasy.client.jaxrs.internal.ClientInvocation.handleErrorStatus(ClientInvocation.java:250)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.extractors.DefaultEntityExtractorFactory$3.extractEntity(DefaultEntityExtractorFactory.java:41)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.ClientInvoker.invokeSync(ClientInvoker.java:146)
	at org.jboss.resteasy.client.jaxrs.internal.proxy.ClientInvoker.invoke(ClientInvoker.java:104)
...

Report flaky test

keycloak-github-bot

Unreported flaky test detected, please review

mposolda

@michalvavrik Thanks, I've added inline comment in the DefaultAuthenticationFlow (related to copilot review). WDYT?

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

michalvavrik · 2026-10-08T10:10:38Z

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

I can't say I am sorry 😀 but @mposolda if you want to be safe, please request review on your behalf.

Copilot

🟡 Changes recommended

Selector-assigned LoA can bypass registration/reset enforcement, and condition-free SSO mishandles essential level 0.

6 open findings

Apply selector-assigned LoA only after selected browser flow completion · New
Enforce numeric ACR when no ConditionalLoaAuthenticator is configured
Qualify authFlowLoa fallback for selector flows without LoA conditions · New
Accept level-0 SSO sessions for flows without LoA conditions · New
Account for explicit LoA on condition-free selected flows · New
Document selector-assigned LoA for condition-free authentication flows · New

🧠 Review effort: Balanced

rmartinc

Thanks @michalvavrik! There is conflict now. I have just the same doubt that copilot is saying, my feeling is we can always set the callback for LoA if the level is enforced. It doesn't matter the LoA condition is executed later or not (that depends on the workflow, maybe it's not done OK and, even present, it's not executed). WDYT?

For the rest I'm OK. I think that your tests cover a lot of scenarios. So I think that it's complete. Just fix the conflict in the note and this is OK to me.

mabartos

@michalvavrik Thanks! Just a quick top-of-mind review, and rather implementation-wise suggestions. I think major concerns are brought up already by @mposolda and @rmartinc.

Copilot

🔵 Needs a closer look

Security-sensitive flow changes need human validation, including resolution of premature enforcement before post-broker step-up.

1 open issue

Defer step-up enforcement until post-broker authentication completes · New

1 resolved since last review

Enforce numeric ACR when no ConditionalLoaAuthenticator is configured

_{Review effort: Balanced}

… step-up closes: keycloak#53157 Signed-off-by: Michal Vavřík <dev@michalvavrik.net>

Copilot

🔵 Needs a closer look

Security-sensitive changes spanning SSO, passive authentication, brokered login, registration, and credential reset require final human validation.

No open issues 🎉

1 resolved since last review

Defer step-up enforcement until post-broker authentication completes

_{Review effort: Balanced}

rmartinc

Thanks @michalvavrik! LGTM!
@mposolda @mabartos Please re-review when you have time. Thanks too!

rmartinc

One more thing,maybe now we prefer to move the upgrading note to 26.8.x instead of 27.0.0?

michalvavrik · 2026-10-20T07:03:39Z

One more thing,maybe now we prefer to move the upgrading note to 26.8.x instead of 27.0.0?

that is what I had, but @mposolda and @mabartos preferred it the other way around; I don't know what is right, so I can't be helpful here. I can apply it if y'all agree though.

rmartinc · 2026-10-26T08:35:13Z

@mposolda @mabartos Would you like to take a second look to this?

mposolda

@michalvavrik @rmartinc @mabartos Thanks for the updates and review! LGTM

AFAIK on where the "Upgrading notes" are supposed to be documented. In upstream, it works like:

In main branch, it should go to the upcoming major/minor release. So right now to changes-27_0_0.adoc .
In the respective release* branch, it should be upcoming release. So for example changes-26_8_3.adoc for release/26.8 (the 26.8.2 not released yet, but that is going to be CVEB and payload for it from upstream changes like this one is already closed)

mposolda · 2026-10-26T11:18:08Z

Leaving open in case that @mabartos also wants to review?
@rmartinc Feel free to merge whenever it is OK from core-authn team

mabartos · 2026-10-26T11:23:34Z

@rmartinc @mposolda I'm already in the middle of reviewing that and checking last concerns.

mabartos

@michalvavrik LGTM for fixing the issue, thanks! I've added a comment for a possible follow-up issue on the step-up logic.

rmartinc · 2026-10-26T12:59:57Z

Thanks everybody!

rmartinc · 2026-10-26T13:00:50Z

@michalvavrik You can start working on the backports when you have time.

…thout a step-up closes: keycloak#53157 backports: keycloak#53587 Signed-off-by: Michal Vavřík <dev@michalvavrik.net>

michalvavrik · 2026-10-26T17:23:00Z

@michalvavrik You can start working on the backports when you have time.

Here you go:

[26.8] Enforce the essential acr level when the flow finishes without a step-up #54345
[26.6] Enforce the essential acr level when the flow finishes without a step-up #54346

…thout a step-up (#54345) closes: #53157 backports: #53587 Signed-off-by: Michal Vavřík <dev@michalvavrik.net> Signed-off-by: Martin Bartoš <mabartos@redhat.com> Signed-off-by: Ricardo Martin <rmartinc@redhat.com> Co-authored-by: Martin Bartoš <mabartos@redhat.com> Co-authored-by: Ricardo Martin <rmartinc@redhat.com>

…thout a step-up (#54346) closes: #53157 backports: #53587 Signed-off-by: Michal Vavřík <dev@michalvavrik.net> Signed-off-by: Ricardo Martin <rmartinc@redhat.com> Co-authored-by: Martin Bartoš <mabartos@redhat.com> Co-authored-by: Ricardo Martin <rmartinc@redhat.com>

Copilot AI balanced review requested due to automatic review settings October 6, 2026 10:07

michalvavrik requested review from a team as code owners October 6, 2026 10:07

Copilot started reviewing on behalf of michalvavrik October 6, 2026 10:08

View session

michalvavrik added the team/core-protocols label Oct 6, 2026

Copilot AI reviewed Oct 6, 2026

View reviewed changes

Comment thread services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java Resolved

Copilot AI balanced review requested due to automatic review settings October 6, 2026 11:22

michalvavrik force-pushed the in-review/53157 branch from 76427e1 to f3c169c Compare October 6, 2026 11:22

Copilot started reviewing on behalf of michalvavrik October 6, 2026 11:23

View session

Copilot AI reviewed Oct 6, 2026

View reviewed changes

Comment thread .../src/main/java/org/keycloak/authentication/authenticators/util/AcrNotFulfilledException.java Resolved

Copilot AI balanced review requested due to automatic review settings October 7, 2026 09:44

michalvavrik force-pushed the in-review/53157 branch from f3c169c to 71a2ab1 Compare October 7, 2026 09:44

Copilot started reviewing on behalf of michalvavrik October 7, 2026 09:44

View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

keycloak-github-bot Bot added the flaky-test label Oct 7, 2026

keycloak-github-bot Bot reviewed Oct 7, 2026

View reviewed changes

mposolda reviewed Oct 7, 2026

View reviewed changes

Comment thread services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java Resolved

mposolda self-assigned this Oct 7, 2026

Copilot AI balanced review requested due to automatic review settings October 8, 2026 09:48

michalvavrik force-pushed the in-review/53157 branch from 71a2ab1 to 1443540 Compare October 8, 2026 09:48

Copilot AI reviewed Oct 8, 2026

michalvavrik requested a review from mposolda October 8, 2026 10:53

mposolda requested a balanced review from Copilot October 9, 2026 13:58

Copilot started reviewing on behalf of mposolda October 9, 2026 13:58

View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 9, 2026 15:23

rmartinc requested changes Oct 14, 2026

View reviewed changes

Comment thread services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java Resolved

mabartos reviewed Oct 14, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 19, 2026 09:25

michalvavrik force-pushed the in-review/53157 branch from 463ec32 to 8a683f3 Compare October 19, 2026 09:25

Copilot started reviewing on behalf of michalvavrik October 19, 2026 09:26

View session

Copilot AI reviewed Oct 19, 2026

View reviewed changes

Comment thread services/src/main/java/org/keycloak/authentication/DefaultAuthenticationFlow.java Resolved

fix: enforce the essential acr level when the flow finishes without a…

a6cf386

… step-up closes: keycloak#53157 Signed-off-by: Michal Vavřík <dev@michalvavrik.net>

Copilot AI balanced review requested due to automatic review settings October 19, 2026 10:40

michalvavrik force-pushed the in-review/53157 branch from 8a683f3 to a6cf386 Compare October 19, 2026 10:40

Copilot started reviewing on behalf of michalvavrik October 19, 2026 10:40

View session

Copilot AI reviewed Oct 19, 2026

View reviewed changes

michalvavrik requested a review from rmartinc October 19, 2026 12:34

rmartinc approved these changes Oct 19, 2026

View reviewed changes

rmartinc reviewed Oct 20, 2026

View reviewed changes

mposolda approved these changes Oct 26, 2026

View reviewed changes

mabartos approved these changes Oct 26, 2026

View reviewed changes

rmartinc merged commit 791b93e into keycloak:main Oct 26, 2026
98 checks passed

michalvavrik deleted the in-review/53157 branch October 26, 2026 13:02

michalvavrik mentioned this pull request Oct 26, 2026

[26.8] Enforce the essential acr level when the flow finishes without a step-up #54345

Merged

michalvavrik mentioned this pull request Oct 26, 2026

[26.6] Enforce the essential acr level when the flow finishes without a step-up #54346

Merged

Conversation

Uh oh!

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Unreported flaky test detected

org.keycloak.testsuite.broker.KcSamlBrokerTest#loginInResponseToEmpty

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Uh oh!

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants