feat(evals): add Swift and Android MFA step-up evals - #196
Conversation
Mirrors the React MFA eval for mobile: gate a Transfer Funds action behind step-up (acr_values + max_age 0 through Universal Login) and the amr claim. Both ship a shared login-only scaffold on Auth0.swift 3.x / Auth0.Android 4.x and skip the native build, like the mobile quickstarts. Also backfills the MFA and DPoP rows missing from the eval table, and ignores the .build / .gradle residue those scaffolds pick up locally.
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📝 WalkthroughWalkthroughThe change adds Android and Swift MFA evaluation prompts and graders, registers new evals, and introduces Auth0 authentication scaffolds for Compose and SwiftUI apps. ChangesMobile MFA evaluations
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/auth0-evals/src/evals/mfa/android/PROMPT.md`:
- Line 10: The Android prompt at
apps/auth0-evals/src/evals/mfa/android/PROMPT.md:10-10 must require a new
Universal Login step-up request using the MFA acr_values policy URI and max_age:
0, followed by ID-token amr validation requiring "mfa" before the transfer.
Apply the same requirements to the Swift prompt at
apps/auth0-evals/src/evals/mfa/swift/PROMPT.md:10-10.
In `@apps/auth0-evals/src/evals/scaffolds/swift/auth0/Package.swift`:
- Around line 9-23: Update Package.swift so BarkbookApp is an executable product
and target owning ContentView.swift’s `@main` entry point, and declare the root
Auth0.plist as a processed resource for that target. Ensure the executable
target carries the app’s callback URL scheme configuration while preserving the
Auth0.swift dependency; do not run a Swift build.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 487296ee-0d9f-4b8b-911a-d21d651753f0
📒 Files selected for processing (16)
.gitignoreapps/auth0-evals/README.mdapps/auth0-evals/src/evals/mfa/android/PROMPT.mdapps/auth0-evals/src/evals/mfa/android/graders.tsapps/auth0-evals/src/evals/mfa/swift/PROMPT.mdapps/auth0-evals/src/evals/mfa/swift/graders.tsapps/auth0-evals/src/evals/scaffolds/android/auth0/AGENTS.mdapps/auth0-evals/src/evals/scaffolds/android/auth0/app/build.gradleapps/auth0-evals/src/evals/scaffolds/android/auth0/app/src/main/AndroidManifest.xmlapps/auth0-evals/src/evals/scaffolds/android/auth0/app/src/main/java/com/barkbook/app/MainActivity.ktapps/auth0-evals/src/evals/scaffolds/android/auth0/app/src/main/res/values/strings.xmlapps/auth0-evals/src/evals/scaffolds/swift/auth0/AGENTS.mdapps/auth0-evals/src/evals/scaffolds/swift/auth0/Auth0.plistapps/auth0-evals/src/evals/scaffolds/swift/auth0/Package.swiftapps/auth0-evals/src/evals/scaffolds/swift/auth0/Sources/BarkbookApp/AuthenticationService.swiftapps/auth0-evals/src/evals/scaffolds/swift/auth0/Sources/BarkbookApp/ContentView.swift
| 'Uses correct multi-factor acr_values policy URI', | ||
| GraderLevel.L1, | ||
| ), | ||
| contains('withParameters', 'Passes acr_values through WebAuthProvider withParameters', GraderLevel.L1), |
There was a problem hiding this comment.
The withParameters check at L1 creates a false-negative risk. Any correct solution that uses only withMaxAge(0) (without calling withParameters) will fail this grader despite being correct. The contains('acr_values', ...) check on line 6 already verifies the parameter is present somewhere — this method-specific check is redundant.
Suggested fix: Remove this line. If you want to verify the current builder API is used, that belongs in an L5 judge.
There was a problem hiding this comment.
Checked WebAuthProvider.kt on main: the login builder has withMaxAge(Int) and withParameters(Map<String, Any?>), no singular withParameter and no acr_values builder. So acr_values can only reach the request through withParameters — anything that fails this grader also fails contains('acr_values'). Near-redundant, agreed, but it catches one thing that grader can't: an agent hand-building an /authorize URL with acr_values. Keeping it.
| // Auth0.Android has no Credentials.claims property — claims come from credentials.user. | ||
| notContains('credentials.claims', 'No hallucinated Credentials.claims property', GraderLevel.L2), | ||
| notContains( | ||
| 'mfaClient', |
There was a problem hiding this comment.
This grader checks for lowercase 'mfaClient' (variable/method form) but misses the uppercase 'MFAClient' class name. An agent that imports the class without assigning it to a mfaClient-named variable bypasses this check:
import com.auth0.android.provider.MFAClient // 'mfaClient' never appears → grader passes
val client = MFAClient.create(...) // sameSuggested fix: Add a second grader after this one:
notContains('MFAClient', 'No hallucinated MFAClient class import', GraderLevel.L2),There was a problem hiding this comment.
Good catch, and it's worse than the casing: notContains defaults to caseSensitive: true, and the needle names the wrong symbol on each platform. In Auth0.swift MFAClient is a real protocol reached via Auth0.mfa(), so the type may never appear in the code at all; in Auth0.Android the class is MfaApiClient, which mfaclient doesn't match even case-insensitively — so this grader couldn't fire on Android.
Switched both to the one thing every embedded-MFA path must carry, case-insensitive:
notContains(
'mfaToken',
'Does not use the embedded MFA grant (MfaApiClient) — wrong approach for a Universal Login app',
GraderLevel.L2,
{ caseSensitive: false },
),Went with a replacement rather than a second grader — an L2 slot that can never fail would inflate the Hallucination dimension.
|
|
||
| // ── L2: Hallucination / wrong approach ──────────────────────────────── | ||
| notContains('Auth0SDK', 'No hallucinated Auth0SDK package name (correct package is Auth0)', GraderLevel.L2), | ||
| notContains( |
There was a problem hiding this comment.
Same issue as the Android grader — checks for lowercase 'mfaClient' but misses the uppercase 'MFAClient' class name. If an agent imports or references the class by its proper PascalCase name without using a lowercase-named variable, the hallucination goes undetected.
Suggested fix: Add a second grader after this one:
notContains('MFAClient', 'No hallucinated MFAClient class', GraderLevel.L2),There was a problem hiding this comment.
Good catch, and it's worse than the casing: notContains defaults to caseSensitive: true, and the needle names the wrong symbol on each platform. In Auth0.swift MFAClient is a real protocol reached via Auth0.mfa(), so the type may never appear in the code at all; in Auth0.Android the class is MfaApiClient, which mfaclient doesn't match even case-insensitively — so this grader couldn't fire on Android.
Switched both to the one thing every embedded-MFA path must carry, case-insensitive:
notContains(
'mfaToken',
'Does not use the embedded MFA grant (Auth0.mfa()/MFAClient) — wrong approach for a Universal Login app',
GraderLevel.L2,
{ caseSensitive: false },
),Went with a replacement rather than a second grader — an L2 slot that can never fail would inflate the Hallucination dimension.
|
|
||
| // ── L5: Current API patterns ────────────────────────────────────────── | ||
| // `.parameters` / `.maxAge` are the current builders for extra authorization | ||
| // parameters. The scaffold's login call uses neither, so this only passes if |
There was a problem hiding this comment.
This deterministic matches check is location-agnostic — it only verifies .parameters( or .maxAge( exists somewhere in the workspace, not that it's used in the step-up request specifically. The L4 judge already validates the step-up flow is correct; this check adds little and could fail correct solutions that pass acr_values via a different-but-valid mechanism.
Suggested fix: Either drop this in favor of relying on the L5 judge below it (line 71), or add a comment making the assumption explicit.
There was a problem hiding this comment.
Location-agnostic, true, but there's no valid alternative mechanism to miss: Auth0.swift's WebAuth builder has no acr_values method, so .parameters([String: String]) is the only way to pass it, and .maxAge(Int) is the only dedicated max_age builder. The scaffold's login call uses neither, so this can only pass if the agent built the step-up request. Keeping it as the deterministic counterpart to the judge below it.
…raders notContains is case-sensitive, and 'mfaClient' names neither SDK's symbol — Auth0.swift has an MFAClient protocol reached via Auth0.mfa() (the type is often never named), Auth0.Android has MfaApiClient. Match mfaToken case-insensitively instead, which every embedded-MFA path carries.
2 participants
✏️ Changes
Adds MFA step-up evals for the two mobile SDKs:
swift_mfaandandroid_mfa. Same shape asmfa/react— gate a Transfer Funds action behind MFA by requesting step-up through Universal Login withacr_valuesandmax_age0, readingamroff the ID token, and only then running the transfer. Mobile had no MFA coverage before this.Both use a new shared scaffold (
src/evals/scaffolds/{swift,android}/auth0): a login-only app with Auth0 already wired, no Transfer Funds UI, since building the feature is the task. Nocompile_command— there is no native toolchain in the sandbox, same as the mobile quickstarts.The graders accept either legitimate way of doing each step, so correct solutions aren't failed on style:
amrvia JWTDecode /customClaimson Swift andgetExtraInfo()/ jwtdecode on Android, andmax_agevia the dedicated builder or amax_ageparameter. L2 penalises the embedded MFA grant, which is wrong for a Universal Login app.Two side fixes: the README eval table listed quickstarts only, so the existing MFA and DPoP evals are backfilled; and
.build//.gradle/are gitignored, since both scaffolds pick up build residue locally.🔮 Type of Change
🔗 References
SDK-10701, SDK-10702 — under epic SDK-10349
Reference eval:
apps/auth0-evals/src/evals/mfa/react/I added at least one link (task, Slack thread, etc.) to explain why this change is needed.
📖 Documentation
Updated the eval table in
apps/auth0-evals/README.md. No new primitives, levels or flags, so the other docs stay accurate.🎯 Testing
npm run build && npm test && npm run lintpass. No framework logic changed, so no new package tests.Ran the deterministic graders against the bare scaffolds and against a hand-written correct solution for each platform: everything positive fails on the scaffold, everything passes on the correct solution. That caught two graders the scaffold itself already satisfied, now scoped to the step-up request.
10/10 jobs green:
swift_mfaandroid_mfaFailures are specific, not noisy: one run never used
acr_valuesat all, two hand-rolled Base64 JWT parsing, one setwithMaxAge(0)only behind a flag the MFA path passed asfalse, one short-circuited theamrcheck on a session boolean.Ignore the baseline column — two pre-existing framework bugs make it meaningless.
runBaselinesets nomaxOutputTokensand doesn't disable thinking, so on Opus 5 the answer goes to reasoning tokens (swift_mfareturned 0 characters). And baseline judges always see an empty corpus, becausegradeTextwrites tollm_response.txtand the judge excludes.txt. Follow-ups to come; neither is caused by this PR.