Update origin verification flow by kj84park · Pull Request #68 · line/line-fido2-server

kj84park · 2025-09-15T03:42:04Z

What is this PR for?

Resolves #60

Overview or reasons

Introduce a web-origin (https) allowlist to support platform passkeys and refactor the checkOrigin flow for clarity.
Preserve existing native app facet validation (android:/ios:).
For environments without configured web origins, fall back to strict string equality between request origin and
clientDataJSON.origin for backward compatibility.

Tasks

Code
- Add: fido2-core/.../helper/OriginValidationHelper.java (isAppFacet, validateAppFacet, validateWeb)
- Change: ResponseServiceImpl#checkOrigin → delegate to helper with early-return branching
- Build: add JUnit 5 test setup in fido2-core/build.gradle
Tests
- Add: fido2-core/.../test/.../OriginValidationHelperTest.java (app/web/fallback/multi-origin cases)
Docs
- Update README.md with configuration notes

Result

App facets: exact string match against the configured allowlist
Web origins: if app.origins contains any http(s) entries, enforce allowlist and require exact match
Fallback: if no http(s) entries exist, require strict equality between request origin and clientDataJSON.origin
Security: exact matching only (no wildcards/normalization); rpIdHash verification unchanged
Compatibility: unchanged behavior when no web origins are configured; no API/schema changes

Copilot

Pull Request Overview

This PR refactors the origin validation flow in the FIDO2 server to support web-origin allowlists for platform passkeys while maintaining backward compatibility. The refactoring extracts origin validation logic from ResponseServiceImpl into a dedicated helper class for better code organization and clarity.

Key changes:

Introduces a web-origin allowlist mechanism to support HTTPS origins alongside existing app facet validation
Refactors origin validation logic into a dedicated OriginValidationHelper class
Adds comprehensive test coverage for the new validation scenarios

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
OriginValidationHelper.java	New helper class containing extracted origin validation logic with support for app facets and web origins
OriginValidationHelperTest.java	Comprehensive test suite covering app facet, web allowlist, and fallback validation scenarios
ResponseServiceImpl.java	Refactored checkOrigin method to delegate to the new helper class
build.gradle	Added JUnit 5 dependencies and test platform configuration
README.md	Updated documentation to explain the new web origin allowlist functionality

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

gibeom-gwon

Thank you for the PR @kj84park nim,
I left some comments.

By this change, VerifyCredential.origin and RegisterCredential.origin fields are ineffective when web origin is set.

gibeom-gwon · 2025-09-15T08:14:20Z

...re/src/main/java/com/linecorp/line/auth/fido/fido2/server/helper/OriginValidationHelper.java

+        }
+    }
+
+    public static void validateWeb(URI client, URI rp, List<String> allowed) {


How about change rp to originFromRp and client to originFromClient?

Suggested change

public static void validateWeb(URI client, URI rp, List<String> allowed) {

public static void validateWeb(URI originFromClient, URI originFromRp, List<String> allowed) {

gibeom-gwon · 2025-09-15T08:23:08Z

...core/src/main/java/com/linecorp/line/auth/fido/fido2/server/service/ResponseServiceImpl.java

    protected void checkOrigin(URI originFromClientData, URI originFromRp) {
-        final String ANDROID_FACET_SCHEME = "android";
-        final String IOS_FACET_SCHEME = "ios";
+        List<String> configuredOrigins = appOriginService.getOrigins(originFromRp.toString());


Now AppOriginService can contain web origin also, maybe it is good to change name to OriginService. But it is just minor, not essential in this PR.

I found that getOrigins parameter name is rpId, but originFromRp is provided. I think rpId is more appropriate for this parameter because origin can be variable.

gibeom-gwon · 2025-09-15T08:34:15Z

...core/src/main/java/com/linecorp/line/auth/fido/fido2/server/service/ResponseServiceImpl.java

+        if (OriginValidationHelper.isAppFacet(originFromClientData)) {
+            OriginValidationHelper.validateAppFacet(originFromClientData, configuredOrigins);
+            return;
        }
+
+        OriginValidationHelper.validateWeb(originFromClientData, originFromRp, configuredOrigins);


Maybe we can create OriginValidationService and provide this logic as method validate.
In OriginValidationService, we can move OriginValidationHelper logics as private method. How do you think about this direction?

@gibeom-gwon

Thanks for the suggestion! How about we merge AppOriginService and OriginValidationHelper into one OriginValidationService that handles both origin lookups and validation?

@kj84park

This will add validation logic into OriginValidationService and library user need to implement OriginValidationServiceImpl that provides origin lookup?
I think it would be also good direction!

I implemented OriginValidationService as you originally suggested.
I think it's better that way.

gibeom-gwon · 2025-09-15T08:42:07Z

...re/src/main/java/com/linecorp/line/auth/fido/fido2/server/helper/OriginValidationHelper.java

+    private static boolean hasWebAllowlist(List<String> allowed) {
+        for (String o : allowed) {
+            if (o.startsWith("https://") || o.startsWith("http://")) {
+                return true;
+            }
+        }
+        return false;
+    }


This will be called in every assertion and attestation.
If we can get the web origin list as seperate with app origin list, this can be easily check with webOriginAllowed.size() > 0.
I don't think the current implementation has a significant impact on performance, so we can improve this in the future.

I agree. I'll update this later.

gibeom-gwon · 2025-09-15T08:44:57Z

.gitignore

 /.nb-gradle/
 **/build/
+
+.DS_Store


Just a minor thing, how about add new line in the bottom?

kj84park · 2025-09-17T05:59:48Z

Thank you for the PR @kj84park nim, I left some comments.

By this change, VerifyCredential.origin and RegisterCredential.origin fields are ineffective when web origin is set.

@gibeom-gwon nim
Yes. With web origins configured, the server validates clientDataJSON.origin against the allowlist and ignores the request origin fields.

Also, I applied most of your comments and updated the code. Please review.

gibeom-gwon

Sorry for the late review.
LGTM! 👍

kj84park · 2025-09-18T22:35:39Z

README.md

+- Android native (FIDO2 API/Credential Manager): `clientDataJSON.origin` starts with `android:...` (app facet). See "Verify origin" in Android Credential Manager docs: https://developer.android.com/identity/sign-in/credential-manager#verify-origin
+- iOS native (AuthenticationServices, passkeys): `clientDataJSON.origin` is an `https` web origin (no `ios:` prefix), e.g., `https://example.com`. For iOS/macOS passkeys, configure a web-origin allowlist.


@kimdora
I added the details based on your feedback. Could you check it?

README.md

kimdora

LGTM

Update origin verification flow

e89c398

kj84park requested review from Copilot and kimdora September 15, 2025 03:42

kj84park self-assigned this Sep 15, 2025

Copilot AI reviewed Sep 15, 2025

View reviewed changes

kj84park requested a review from gibeom-gwon September 15, 2025 03:50

kj84park added 2 commits September 15, 2025 17:45

Update README.md

81b30cc

Update README.md

7624257

gibeom-gwon reviewed Sep 15, 2025

View reviewed changes

Apply review comments

b5416f6

gibeom-gwon previously approved these changes Sep 18, 2025

View reviewed changes

kj84park commented Sep 18, 2025

View reviewed changes

kimdora reviewed Sep 23, 2025

View reviewed changes

README.md Show resolved Hide resolved

kimdora previously approved these changes Sep 23, 2025

View reviewed changes

Update README.md

7322800

kj84park dismissed stale reviews from kimdora and gibeom-gwon via 7322800 September 23, 2025 08:22

kj84park requested a review from kimdora September 23, 2025 08:26

kimdora approved these changes Sep 23, 2025

View reviewed changes

kj84park merged commit 34cc4cf into line:main Sep 23, 2025
4 checks passed

kj84park deleted the update-origin branch September 23, 2025 09:37

	public static void validateWeb(URI client, URI rp, List<String> allowed) {
	public static void validateWeb(URI originFromClient, URI originFromRp, List<String> allowed) {

		- Android native (FIDO2 API/Credential Manager): `clientDataJSON.origin` starts with `android:...` (app facet). See "Verify origin" in Android Credential Manager docs: https://developer.android.com/identity/sign-in/credential-manager#verify-origin
		- iOS native (AuthenticationServices, passkeys): `clientDataJSON.origin` is an `https` web origin (no `ios:` prefix), e.g., `https://example.com`. For iOS/macOS passkeys, configure a web-origin allowlist.

Conversation

kj84park commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is this PR for?

Overview or reasons

Tasks

Result

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

gibeom-gwon left a comment

Choose a reason for hiding this comment

Uh oh!

gibeom-gwon Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kj84park Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kj84park commented Sep 17, 2025

Uh oh!

gibeom-gwon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kimdora left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kj84park commented Sep 15, 2025 •

edited

Loading

gibeom-gwon Sep 15, 2025 •

edited

Loading

kj84park Sep 16, 2025 •

edited

Loading