Skip to content

Conversation

@adeinega
Copy link
Member

@adeinega adeinega commented Feb 6, 2024

A Credential Issuer URL should not already include the query component per 11.2.2.

If an intent for the wallet was to not add any query parameters to the Credential Issuer Identifier when it fetches the metadata then this sentence should be changed to better reflect this.

A Credential Issuer URL shouldn't already include the query component per 11.2.2.

If an intent for the Wallet was to not add any query parameters for the Credential Issuer Identifier then this sentence should be changed to better reflect this.
Copy link
Contributor

@tplooker tplooker left a comment

Choose a reason for hiding this comment

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

I agree with @adeinega, I believe the normative requirement that this PR removes is needless duplication

Copy link
Collaborator

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

if you are removing a normative statement, it is not an editorial change. please change the PR title accordingly

@adeinega adeinega changed the title Mnr changes Remove the duplicate requirement about not including query parameters to Credential Issuer Identifier Feb 7, 2024
@pmhsfelix
Copy link
Contributor

Agree with the removal given both:

  • the restriction that the issuer ID cannot have query string.
  • the documented URI creation procedure for the metadata GET from the issuer ID.

Copy link
Contributor

@paulbastian paulbastian left a comment

Choose a reason for hiding this comment

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

The existing query statement is only refering to the the Credential Issuer Identifier.

I would propose to move this statement one paragraph earlier as:
"Credential Issuers publishing metadata MUST make a JSON document available at the path formed by concatenating the string /.well-known/openid-credential-issuer to the Credential Issuer Identifier. The resulting URL SHOULD NOT contain any query parameters"

@Sakurann Sakurann requested a review from paulbastian February 8, 2024 16:21
Copy link
Contributor

@paulbastian paulbastian left a comment

Choose a reason for hiding this comment

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

agreed afet wg call

@Sakurann Sakurann merged commit 9638828 into main Feb 8, 2024
@adeinega adeinega deleted the adeinega-patch-1 branch February 8, 2024 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants