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

fix!: Authz request parameter wallet_issuer renamed to wallet_id #142

Closed
wants to merge 2 commits into from

Conversation

peppelinux
Copy link
Member

@peppelinux peppelinux commented Dec 9, 2023

This PR:

  • resolves Should the parameter wallet_issuer be changed to wallet_provider? #103, renaming wallet_issuer to wallet_id. The term Wallet Issuer may sound misleading since it may be confused with Wallet Provider that's a consolidated term within the eIDAS Wallet ecosystem
  • aligns the terminology about End-User, in particular in the sequence diagrams
  • removes the mention of OAuth 2.0 AS in the definition of the term Wallet, since in this specs the AS for authorizing the issuance of a credential is elsewhere from the Wallet that acts like a client. Another reason for this removal is that in the SIOPv2 specs the wallet instance acts as an OAuth 2.0 AS, while in this specification the wallet instance acts like an OAuth 2.0 Client.

@paulbastian
Copy link
Contributor

My colleagues and I never really understood the parameters wallet_issuer and user_hint and this PR doesn't solve that either. I think the specification is missing text on this, but unsure if this should be a target of this PR

Co-authored-by: Kristina <[email protected]>
@@ -103,7 +103,7 @@ Biometrics-based Holder Binding:
: Ability of the Holder to prove legitimate possession of a Verifiable Credential by demonstrating a certain biometric trait, such as fingerprint or face. One example of a Verifiable Credential with Biometrics-based Holder Binding is a mobile driving license [@ISO.18013-5], which contains a portrait of the holder.

Wallet:
: An entity used by the Holder to receive, store, present, and manage Verifiable Credentials and key material. There is no single deployment model of a Wallet: Verifiable Credentials and keys can both be stored/managed locally, or by using a remote self-hosted service, or a remote third-party service. In the context of this specification, the Wallet acts as an OAuth 2.0 Authorization Server (see [@!RFC6749]) towards the Credential Verifier which acts as the OAuth 2.0 Client.
: An entity used by the Holder to request, receive, store, present, and manage Verifiable Credentials and key material. There is no single deployment model of a Wallet: Verifiable Credentials and keys can both be stored and managed locally, or by using a remote self-hosted service, or a remote third-party service. In the context of this specification, during Credential Issuance the Wallet acts as an OAuth 2.0 Client (see [@!RFC6749]) towards the Credential Issuer which acts as the OAuth 2.0 Authorization Server / Resource Server.
Copy link
Member Author

@peppelinux peppelinux Dec 10, 2023

Choose a reason for hiding this comment

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

Suggested change
: An entity used by the Holder to request, receive, store, present, and manage Verifiable Credentials and key material. There is no single deployment model of a Wallet: Verifiable Credentials and keys can both be stored and managed locally, or by using a remote self-hosted service, or a remote third-party service. In the context of this specification, during Credential Issuance the Wallet acts as an OAuth 2.0 Client (see [@!RFC6749]) towards the Credential Issuer which acts as the OAuth 2.0 Authorization Server / Resource Server.
: An entity used by the Holder to request, receive, store, present, and manage Verifiable Credentials and key material. There is no single deployment model of a Wallet: Verifiable Credentials and keys can both be stored and managed locally, or by using a remote self-hosted service, or a remote third-party service. In the context of this specification, the Wallet acts as an OAuth 2.0 Client (see [@!RFC6749]) since the Credential Issuer is the OAuth 2.0 Resource Server and, in certain cases, it may also implement an OAuth 2.0 Authorization Server.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Sakurann WDYT?

@peppelinux
Copy link
Member Author

My colleagues and I never really understood the parameters wallet_issuer and user_hint and this PR doesn't solve that either. I think the specification is missing text on this, but unsure if this should be a target of this PR

yes, your concerns are understandable and I have moved your comment in the following issue:

#145

@tplooker
Copy link
Contributor

With regard to the change around wallet_issuer -> wallet_provider, I still don't see a need to have an alternative identifier for the wallet. See my comment in issue #145, but I think the client_id should suffice.

@peppelinux
Copy link
Member Author

peppelinux commented Dec 12, 2023

With regard to the change around wallet_issuer -> wallet_provider, I still don't see a need to have an alternative identifier for the wallet. See my comment in issue #145, but I think the client_id should suffice.

it makes sense, if I got it well, you're suggesting to replace wallet_issuer with client_id?
if yes, I agree and I can change this part of the text in this PR accordingly

@Sakurann
Copy link
Collaborator

if we want to revisit the fact if wallet_issuer parameter itself is needed, I don't think this PR need to go in before ID-1.

@tplooker
Copy link
Contributor

if we want to revisit the fact if wallet_issuer parameter itself is needed, I don't think this PR need to go in before ID-1.

Agreed. I think we should revisit this.

@Sakurann Sakurann added this to the post ID-1 milestone Dec 18, 2023
@awoie
Copy link
Contributor

awoie commented Dec 20, 2023

if we want to revisit the fact if wallet_issuer parameter itself is needed, I don't think this PR need to go in before ID-1.

Agreed. I think we should revisit this.

I think it would be great if we could reuse client_id for this because this is what the wallet effectively is -> an OAuth2 client to the AS that wants to get access to a Credential Issuer.

@selfissued
Copy link
Member

It seems to me that we should discuss whether to have a wallet identifier at all that is distinct from the Client ID before the Implementer's Draft, as it would result in normative changes.

It might be more appropriate to discuss this in issue #103 than this PR, because depending on the outcome of the discussion, this PR might not be used at all.

@Sakurann
Copy link
Collaborator

closing in a week unless strong objections, since there seems to be a consensus that we should go back to the issue discussion whether wallet_issuer is needed at all.

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.

Should the parameter wallet_issuer be changed to wallet_provider?
6 participants