Skip to content

Digital Credentials: Fix user activation tests for early validation errors#57231

Merged
marcoscaceres merged 6 commits into
masterfrom
user_activation_after
Jan 20, 2026
Merged

Digital Credentials: Fix user activation tests for early validation errors#57231
marcoscaceres merged 6 commits into
masterfrom
user_activation_after

Conversation

@marcoscaceres

Copy link
Copy Markdown
Contributor

Validation errors (TypeError) no longer consume user activation since validation now happens before user activation checking.

Spec change: w3c-fedid/digital-credentials#418

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates digital credentials tests to reflect a specification change where validation errors (TypeError) no longer consume user activation, as validation now happens before user activation checking.

Changes:

  • Modified existing user activation consumption test to assert that activation is preserved on validation errors
  • Added user activation preservation assertions to all TypeError test cases in get() and create() methods
  • Added missing user activation bless calls to ensure proper test setup

Reviewed changes

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

File Description
digital-credentials/user-activation.https.html Updated test assertion from assert_false to assert_true to verify user activation is NOT consumed on early validation errors
digital-credentials/get.tentative.https.html Added three user activation preservation checks for TypeError cases and one missing test_driver.bless() call
digital-credentials/create.tentative.https.html Added two user activation preservation checks for TypeError cases and one missing test_driver.bless() call

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

Comment thread digital-credentials/user-activation.https.html Outdated
@mohamedamir

Copy link
Copy Markdown
Contributor

.. validation now happens before user activation checking.

Thinking about this a bit more, this indicates the verifier can keep reusing the same user activation in case of validation errors.
I don't find a reason why it's a bad idea.
I don't think any verifier can abuse it to do fingerprinting for example. (apart from know the user agent which is already available)
WDYT?

@mohamedamir mohamedamir left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM because it's aligned with the spec change, but I added some comments regarding the main idea.
Feel free to discuss here or in the spec change PR.

@wpt-pr-bot wpt-pr-bot added infra wptrunner The automated test runner, commonly called through ./wpt run labels Jan 19, 2026
@marcoscaceres marcoscaceres force-pushed the user_activation_after branch 4 times, most recently from 57ed232 to a89a72d Compare January 19, 2026 10:15
Comment thread digital-credentials/get.tentative.https.html

@mohamedamir mohamedamir left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still LGTM
Thank you!

Comment thread digital-credentials/user-activation-create.tentative.https.html Outdated
Comment thread digital-credentials/user-activation-get.https.html Outdated
Comment thread digital-credentials/create.tentative.https.html
@marcoscaceres

Copy link
Copy Markdown
Contributor Author

Thinking about this a bit more, this indicates the verifier can keep reusing the same user activation in case of validation errors.

They can't do much, because transient activation bombs out after a few seconds.

I don't find a reason why it's a bad idea.
I don't think any verifier can abuse it to do fingerprinting for example. (apart from know the user agent which is already available)
WDYT?

yeah, it's not very useful or something that can really be exploited in a meaningful way.

mohamedamir and others added 2 commits January 20, 2026 12:09
This is a minor fix in the user activation tests in issuance to be compatible with the latest testing infrastructure.

After this fix, the issuance user activation tests will be aligned with presentation tests.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

digital-credentials/user-activation-get.https.html:36

  • The test description mentions "empty requests" but the test actually passes an empty protocol array. Consider updating the description to say "with empty protocol array" for clarity and accuracy.

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

Comment thread digital-credentials/user-activation-create.tentative.https.html Outdated
Comment thread digital-credentials/user-activation-get.https.html Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@marcoscaceres marcoscaceres merged commit b6d0fe8 into master Jan 20, 2026
21 checks passed
@marcoscaceres marcoscaceres deleted the user_activation_after branch January 20, 2026 05:04
KurtCattiSchmidt pushed a commit to KurtCattiSchmidt/wpt-ahem that referenced this pull request Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

digital-credentials infra wg-s_fedid wptrunner The automated test runner, commonly called through ./wpt run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants