Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a new optional rpId to Credential Record #2258

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MasterKale
Copy link
Contributor

@MasterKale MasterKale commented Feb 12, 2025

Closes #2257

This PR adds rpId to Credential Record. This optional field is defined to try and help guide RPs, that wish to use Related Origins, to store values that will streamline adoption of the feature.


Preview | Diff

@emlun
Copy link
Member

emlun commented Feb 12, 2025

I figured this seems fair at first, but as I read the PR I started thinking this might be a bit of a trap. See: #2257 (comment)

@sbweeden
Copy link
Contributor

I'd like to see this sentence augmented with a few words indicating why its useful to store the rpId as part of the credential record. Just something small to help folks understand the value of this.

: <dfn>rpId</dfn>
:: The value of the <code>{{PublicKeyCredentialCreationOptions/rp}}.{{PublicKeyCredentialRpEntity/id}}</code> parameter
specified in the {{CredentialsContainer/create()}} operation during credential registration.
Storing this enables the [=[RP]=] to use the credential across different domains later
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Storing this enables the [=[RP]=] to use the credential across different domains later
This is a core property of a credential and storing the value at creation time is recommended to assist with future operations such as audits, troubleshooting, or leveraging features such as [[#sctn-related-origins|Related Origin Requests]].

Copy link
Member

Choose a reason for hiding this comment

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

some starter text. needs some work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Credential Record to suggest storing RP ID as well for better Related Origins support
5 participants