Skip to content

Commit b25b459

Browse files
committed
fix(auth): prevent 2FA bypass when user lacks required role in alternative flows
1 parent 933cd80 commit b25b459

File tree

4 files changed

+165
-3
lines changed

4 files changed

+165
-3
lines changed

README.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,17 @@ The authenticator is built and tested with multiple Keycloak versions:
106106
While the builds differ slightly for each version, the core functionality remains the same. The version-specific builds ensure compatibility and proper integration with each Keycloak release.
107107

108108

109+
## Known Limitations
110+
111+
### Email OTP appears in "Try Another Way" for users without the required role
112+
113+
When using role-based filtering in an ALTERNATIVE flow alongside other 2FA methods (e.g., TOTP), Email OTP may still appear in Keycloak's "Try another way" selection list for users who don't have the required role. This is a Keycloak architectural limitation - the selection list is built using a different code path that doesn't fully respect our role filtering.
114+
115+
**Important:** Selecting Email OTP in this case will **not** bypass 2FA. The user will be redirected back to complete their other configured 2FA method (e.g., TOTP).
116+
117+
**Workaround:** If you need to completely hide Email OTP for certain users, use Keycloak's built-in **Condition - User Role** execution in a conditional subflow instead of the authenticator's role filtering option.
118+
119+
109120
## License
110121

111122
This project is released under the [Unlicense](./UNLICENSE). This means you can copy, modify, publish, use, compile, sell, or distribute this software, either in source code form or as a compiled binary, for any purpose, commercial or non-commercial, and by any means.

src/main/java/ch/jacem/for_keycloak/email_otp_authenticator/EmailOTPFormAuthenticator.java

Lines changed: 82 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
import org.keycloak.email.EmailTemplateProvider;
1616
import org.keycloak.events.Errors;
1717
import org.keycloak.forms.login.LoginFormsProvider;
18+
import org.keycloak.models.AuthenticationExecutionModel;
19+
import org.keycloak.models.AuthenticationFlowModel;
1820
import org.keycloak.models.AuthenticatorConfigModel;
1921
import org.keycloak.models.KeycloakSession;
2022
import org.keycloak.models.RealmModel;
@@ -123,9 +125,15 @@ public void action(AuthenticationFlowContext context) {
123125

124126
@Override
125127
public void authenticate(AuthenticationFlowContext context) {
126-
// Check role condition - skip OTP if user doesn't match criteria
128+
// Check role condition - if user doesn't match criteria, skip this authenticator
127129
if (!this.shouldRequireOtp(context)) {
128-
context.success();
130+
// In REQUIRED mode: use success() to skip (allows role-based filtering)
131+
// In ALTERNATIVE mode: use attempted() to prevent 2FA bypass
132+
if (context.getExecution().isRequired()) {
133+
context.success();
134+
} else {
135+
context.attempted();
136+
}
129137
return;
130138
}
131139

@@ -192,7 +200,78 @@ public boolean configuredFor(AuthenticationFlowContext context, AuthenticatorCon
192200

193201
@Override
194202
public boolean configuredFor(KeycloakSession session, RealmModel realm, UserModel user) {
195-
return null != user.getEmail() && !user.getEmail().isEmpty();
203+
if (user == null || user.getEmail() == null || user.getEmail().isEmpty()) {
204+
logger.debugf("configuredFor: user is null or has no email");
205+
return false;
206+
}
207+
208+
// Check all authentication flows for email-otp-form executions (including nested subflows)
209+
// Return true only if user matches at least one execution's role requirement
210+
for (AuthenticationFlowModel flow : realm.getAuthenticationFlowsStream().toList()) {
211+
logger.debugf("configuredFor: checking flow %s", flow.getAlias());
212+
if (isUserConfiguredForEmailOtpInFlow(realm, user, flow.getId())) {
213+
logger.debugf("configuredFor: user %s is configured for email-otp in flow %s", user.getUsername(), flow.getAlias());
214+
return true;
215+
}
216+
}
217+
218+
// User doesn't match any email-otp-form execution's requirements
219+
logger.debugf("configuredFor: user %s is NOT configured for any email-otp execution", user.getUsername());
220+
return false;
221+
}
222+
223+
private boolean isUserConfiguredForEmailOtpInFlow(RealmModel realm, UserModel user, String flowId) {
224+
for (AuthenticationExecutionModel execution : realm.getAuthenticationExecutionsStream(flowId).toList()) {
225+
// If this is a subflow, recursively check it
226+
if (execution.isAuthenticatorFlow()) {
227+
String subflowId = execution.getFlowId();
228+
if (subflowId != null && isUserConfiguredForEmailOtpInFlow(realm, user, subflowId)) {
229+
return true;
230+
}
231+
continue;
232+
}
233+
234+
// Check if this is an email-otp-form execution
235+
if (!EmailOTPFormAuthenticatorFactory.PROVIDER_ID.equals(execution.getAuthenticator())) {
236+
continue;
237+
}
238+
239+
// Get the config for this execution
240+
String configId = execution.getAuthenticatorConfig();
241+
if (configId == null) {
242+
// No config means no role restriction - user is eligible
243+
return true;
244+
}
245+
246+
AuthenticatorConfigModel config = realm.getAuthenticatorConfigById(configId);
247+
if (config == null) {
248+
// Config not found, treat as no restriction
249+
return true;
250+
}
251+
252+
// Check role requirement
253+
String configuredRole = ConfigHelper.getRole(config);
254+
if (configuredRole == null || configuredRole.isEmpty()) {
255+
// No role configured - user is eligible
256+
return true;
257+
}
258+
259+
RoleModel role = realm.getRole(configuredRole);
260+
if (role == null) {
261+
// Role doesn't exist - treat as no restriction
262+
return true;
263+
}
264+
265+
// Check if user matches the role requirement (considering negation)
266+
boolean hasRole = user.hasRole(role);
267+
boolean negateRole = ConfigHelper.getNegateRole(config);
268+
if (hasRole != negateRole) {
269+
// User matches this execution's requirement
270+
return true;
271+
}
272+
}
273+
274+
return false;
196275
}
197276

198277
@Override

tests/e2e/helpers/otp-form.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,29 @@ export class TwoFactorChoice {
102102
}
103103
}
104104
}
105+
106+
async isEmailOtpOptionVisible(): Promise<boolean> {
107+
// Check specifically for Email OTP in the authentication method selection list
108+
// Keycloak shows options as clickable divs/links with the authenticator display name
109+
const selectors = [
110+
'.select-auth-box-parent:has-text("Email OTP")',
111+
'.select-auth-box-desc:has-text("Email")',
112+
'#kc-select-credential-form :has-text("Email OTP")',
113+
'a[id*="email-otp"]',
114+
'div.pf-c-tile:has-text("Email")',
115+
];
116+
117+
for (const selector of selectors) {
118+
const element = this.page.locator(selector).first();
119+
if (await element.isVisible({ timeout: 1000 }).catch(() => false)) {
120+
return true;
121+
}
122+
}
123+
return false;
124+
}
125+
126+
async isTryAnotherWayVisible(): Promise<boolean> {
127+
const tryAnotherLink = this.page.locator('a:has-text("Try another way"), a:has-text("try another way"), #try-another-way');
128+
return await tryAnotherLink.isVisible({ timeout: 2000 }).catch(() => false);
129+
}
105130
}

tests/e2e/specs/otp-alternative.spec.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,53 @@ test.describe('Email OTP Alternative Flow', () => {
6969
await loginPage.expectLoggedIn();
7070
});
7171

72+
test('user without role, has TOTP - selecting Email OTP does not bypass 2FA', async ({ page }) => {
73+
const loginPage = new LoginPage(page);
74+
const totpForm = new TotpForm(page);
75+
const choice = new TwoFactorChoice(page);
76+
77+
await loginPage.goto(REALM);
78+
await loginPage.login('user-totp-only', TEST_PASSWORD);
79+
80+
// Should see TOTP form
81+
await totpForm.expectVisible();
82+
83+
// If "Try another way" is visible, try selecting Email OTP
84+
const hasTryAnotherWay = await choice.isTryAnotherWayVisible();
85+
if (hasTryAnotherWay) {
86+
await choice.clickTryAnotherWay();
87+
await page.waitForTimeout(500);
88+
89+
// Try to select Email OTP (if visible)
90+
const emailOtpOption = page.locator('text="Email OTP"').first();
91+
if (await emailOtpOption.isVisible({ timeout: 1000 }).catch(() => false)) {
92+
await emailOtpOption.click();
93+
await page.waitForLoadState('networkidle');
94+
95+
// User should NOT be logged in - should still be on a 2FA page
96+
// The flow should either show an error or redirect back to TOTP
97+
const isLoggedIn = await page.url().includes('/account') ||
98+
await page.locator('text="Sign out"').isVisible({ timeout: 1000 }).catch(() => false);
99+
expect(isLoggedIn).toBe(false);
100+
101+
// Should be back on TOTP form or still in auth flow
102+
const onAuthPage = await page.url().includes('/login-actions') ||
103+
await totpForm.expectVisible().then(() => true).catch(() => false);
104+
expect(onAuthPage).toBe(true);
105+
}
106+
}
107+
108+
// Complete TOTP login normally
109+
// First go back if needed
110+
if (!await page.locator('#otp').isVisible({ timeout: 1000 }).catch(() => false)) {
111+
await page.goBack();
112+
}
113+
await totpForm.expectVisible();
114+
const totpCode = generateTotpCode('user-totp-only');
115+
await totpForm.enterCode(totpCode);
116+
await loginPage.expectLoggedIn();
117+
});
118+
72119
test('user with role, no TOTP - sees Email OTP', async ({ page }) => {
73120
const loginPage = new LoginPage(page);
74121
const otpForm = new OtpForm(page);

0 commit comments

Comments
 (0)