Digital Credentials: implement CANONICAL_REQUEST_OBJECTS with actual mDoc data #56352

marcoscaceres · 2025-12-01T00:27:44Z

Part 4:

Add CANONICAL_REQUEST_OBJECTS with real mDoc deviceRequest and encryptionInfo data
Update factory functions to use canonical objects as defaults
Refactor internal architecture with new helpers
Make mediation optional in type definitions (no longer defaults to 'required')
Add comprehensive data parameter support to config interfaces
Complete property bag + signal + protocol selection + canonical objects integration

Copilot

Pull request overview

This PR implements canonical request objects with real mDoc data for the Digital Credentials API and refactors the internal architecture to support flexible protocol and data configuration.

Key changes:

Adds CANONICAL_REQUEST_OBJECTS with actual ISO mDoc deviceRequest and encryptionInfo data (base64-encoded)
Refactors helper functions to use configuration object pattern with makeGetOptions(config) and makeCreateOptions(config) signatures
Makes mediation and signal optional in type definitions while maintaining sensible defaults in implementations
Introduces new internal helpers buildRequests() and makeOptionsInternal() for cleaner architecture

Reviewed changes

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

Show a summary per file

File	Description
digital-credentials/dc-types.ts	Adds type definitions for OpenID protocols, mDocRequest interface, configuration interfaces (MakeGetOptionsConfig, MakeCreateOptionsConfig), and makes mediation/signal optional in option types
digital-credentials/support/helper.js	Major refactor: adds CANONICAL_REQUEST_OBJECTS with mDoc data, implements new buildRequests/makeOptionsInternal helpers, updates factory functions to use config object pattern, adds protocol detection logic
digital-credentials/user-activation.https.html	Updates test calls from legacy API `makeGetOptions("protocol")` to new config pattern `makeGetOptions({protocol})`
digital-credentials/get.tentative.https.html	Updates test calls to use new config object pattern with `{protocol: [], signal}` syntax
digital-credentials/create.tentative.https.html	Updates test calls to use new config object pattern with `{protocol: [], signal}` syntax
digital-credentials/create.disabled-by-permissions-policy.https.sub.html	Updates test call to use new config pattern `{protocol: []}`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

digital-credentials/support/helper.js

Copilot · 2025-12-01T01:01:36Z

@marcoscaceres I've opened a new pull request, #56353, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

digital-credentials/support/helper.js

marcoscaceres · 2025-12-05T05:54:50Z

@mohamedamir, ok, I introduced a .requests = [] array that can now be passed to pass requests with custom data.

The .protocol are converted to then converted to Request types, so now we can do any combination of:

makeGetRequest({protocol: "openid4vp-whatever", requests: [orangeRequest, bananaRequest] });

And they get nicely merged together.

this is nice too:

makeGetRequest({requests: [orangeRequest, bananaRequest] });

And, so is:

makeGetRequest({protocol: ["x", "y"], requests: [orangeRequest, bananaRequest] });

marcoscaceres · 2025-12-05T05:57:10Z

In the future, we could combine them:

makeGetRequest({requests: [orangeRequest, "openid4vp-v1-unsigned", bananaRequest, "org-iso-mdoc"] });

That can be a nice followup.

digital-credentials/support/helper.js

mohamedamir · 2025-12-05T07:29:22Z

digital-credentials/support/helper.js

+ * @template Req
+ * @param {Protocol[]} protocols
+ * @param {Record<Protocol, (data?: MobileDocumentRequest | object) => Req>} mapping
+ * @param {MobileDocumentRequest | object} [explicitData] - Explicit data for create operations


create operation has a different meaning in this context. I got confused some time why this is specific for create and not for get.
I believe this refers to building the request.
So may be use build instead of create in multiple comments introduce in this PR?

…mDoc data - Add CANONICAL_REQUEST_OBJECTS with real mDoc deviceRequest and encryptionInfo data - Update factory functions to use canonical objects as defaults with data spread - Refactor internal architecture with new buildRequests and makeOptionsInternal helpers - Make mediation optional in type definitions (no longer defaults to 'required') - Add comprehensive data parameter support to config interfaces - Complete property bag + signal + protocol selection + canonical objects integration

…quests parameter - Add requests parameter to MakeGetOptionsConfig and MakeCreateOptionsConfig interfaces - Replace complex makeOptionsUnified/makeOptionsInternal system with simpler makeCredentialOptionsFromConfig - Convert buildRequestsFromProtocols to functional programming approach using .map() - Remove factory functions (makeOID4VPDict, makeMDocRequest) in favor of inline functions in allMappings - Make mediation optional in CredentialRequestOptions and CredentialCreationOptions - Enhance TypeScript annotations with generics for better type safety - Support empty protocol arrays (protocol: []) for testing scenarios - Clean up unused imports and improve documentation alignment

mohamedamir · 2025-12-09T06:29:39Z

digital-credentials/support/helper.js

-    throw new Error(`Internal error: Invalid options type specified: ${type}`);
-  }
+function makeCredentialOptionsFromConfig(config, mapping) {
+  const { protocol, requests = [], data, mediation = "required", signal } = config;


IMHO, we should also remove populating the default mediation to make sure browsers don't reply on it being set.

Absolutely. That's in the next PR.... just making sure this all continues to work as it was.

mohamedamir · 2025-12-09T06:32:18Z

digital-credentials/dc-types.ts

+  /**
+   * Optional data to override canonical data for protocol-based requests.
+   */
+  data?: MobileDocumentRequest | object;


I recall that some tests are testing passing here invalid data, which aren't necessarily objects.
Maybe we should make this to capture that?

Suggested change

data?: MobileDocumentRequest | object;

data?: MobileDocumentRequest | any;

That's true. However, we don't actually validate these inputs in the tests themselves (maybe we should.... will check if we can do that).

mohamedamir

This looks solid now!
Thanks a lot Marcos for the revisions!
Some minor comments are left!
But otherwise, LGTM

… data override pattern - Add data parameter to MakeGetOptionsConfig and MakeCreateOptionsConfig for overriding canonical data - Wire explicitData parameter through buildRequestsFromProtocols call chain properly - Update tests to use clean makeGetOptions({data}) pattern instead of manual object construction - Replace post-construction data manipulation with clean data override during construction - Maintain separation of concerns: protocol selection automatic, data override optional

marcoscaceres requested review from Copilot and mohamedamir and removed request for Copilot December 1, 2025 00:27

wpt-pr-bot added digital-credentials wg-s_fedid labels Dec 1, 2025

wpt-pr-bot assigned tplooker Dec 1, 2025

wpt-pr-bot requested review from samuelgoto and tplooker December 1, 2025 00:28

Copilot started reviewing on behalf of marcoscaceres December 1, 2025 00:28 View session

Copilot finished reviewing on behalf of marcoscaceres December 1, 2025 00:31

Copilot AI reviewed Dec 1, 2025

View reviewed changes

digital-credentials/support/helper.js Outdated Show resolved Hide resolved

digital-credentials/support/helper.js Outdated Show resolved Hide resolved

digital-credentials/support/helper.js Outdated Show resolved Hide resolved

Copilot AI mentioned this pull request Dec 1, 2025

Document that explicit data overrides canonical request objects #56353

Merged

Copilot AI reviewed Dec 1, 2025

View reviewed changes

marcoscaceres force-pushed the update-canonical-request-objects branch 2 times, most recently from c160952 to 4841b68 Compare December 4, 2025 10:09

mohamedamir reviewed Dec 4, 2025

View reviewed changes

digital-credentials/support/helper.js Outdated Show resolved Hide resolved

mohamedamir reviewed Dec 4, 2025

View reviewed changes

digital-credentials/support/helper.js Outdated Show resolved Hide resolved

marcoscaceres requested a review from mohamedamir December 5, 2025 05:25

marcoscaceres force-pushed the update-canonical-request-objects branch from 532af27 to 95ffbec Compare December 5, 2025 05:39

mohamedamir reviewed Dec 5, 2025

View reviewed changes

digital-credentials/support/helper.js Show resolved Hide resolved

mohamedamir reviewed Dec 5, 2025

View reviewed changes

marcoscaceres added 2 commits December 9, 2025 14:11

marcoscaceres force-pushed the update-canonical-request-objects branch from 95ffbec to de0d785 Compare December 9, 2025 04:57

mohamedamir reviewed Dec 9, 2025

View reviewed changes

mohamedamir approved these changes Dec 9, 2025

View reviewed changes

marcoscaceres force-pushed the update-canonical-request-objects branch from de0d785 to e8894c4 Compare December 10, 2025 06:06

marcoscaceres merged commit db7e907 into master Dec 10, 2025
16 of 18 checks passed

marcoscaceres deleted the update-canonical-request-objects branch December 10, 2025 06:28

	data?: MobileDocumentRequest \| object;
	data?: MobileDocumentRequest \| any;

Digital Credentials: implement CANONICAL_REQUEST_OBJECTS with actual mDoc data #56352

Digital Credentials: implement CANONICAL_REQUEST_OBJECTS with actual mDoc data #56352

Uh oh!

Conversation

marcoscaceres commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Dec 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

marcoscaceres commented Dec 5, 2025

Uh oh!

marcoscaceres commented Dec 5, 2025

Uh oh!

Uh oh!

mohamedamir Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

mohamedamir Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

marcoscaceres Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

mohamedamir Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

marcoscaceres Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

mohamedamir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

marcoscaceres commented Dec 1, 2025 •

edited

Loading