-
Notifications
You must be signed in to change notification settings - Fork 20
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
clarify what is mutually exclusive (client_metadata and pre-registered clients) #165
Conversation
@@ -266,6 +266,8 @@ This specification defines the following new parameters: | |||
`client_metadata_uri`: | |||
: OPTIONAL. A string containing an HTTPS URL pointing to a resource where a JSON object with the Verifier metadata can be retrieved. The scheme used in the `client_metadata_uri` value MUST be `https`. The `client_metadata_uri` value MUST be reachable by the Wallet. It MUST NOT be present if `client_metadata` parameter is present. | |||
|
|||
`client-metadata` and `client_metadata_uri` parameters MUST NOT be used when the Wallet knows Client Identifier and corresponding Client metadata prior to the transaction (i.e., Client Identifier scheme is absent or has value `pre-registered`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this again it looks odd to me.
-
I can't see why we'd single out
pre-registered
, it seems to me like the same concerns would apply toentity_id
given that is very very similar to a pre-registered client. -
It overlaps with the discussion we had about what metadata goes into client_metadata parameter? #17 where people were saying the
client_metadata
could be used to pass ephemeral encryption keys, and I can't see why we'd want to disallow ephemeral encryptions keys forpre-registered
clients.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on 1, I disageee. it is only pre-registered or when client id scheme is absent that requires wallet to know verifier's client metadata, entity_id might be similar, but it does not require wallet knowing a client_id and i think there are use-cases where it might be useful to use client metadata with entity_id scheme.
On 2, can we please take it step by step? i want to resolve #96 first because that sentence is confusing and this PR updates the text. once we have agreement on #17, we can update this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think there are use-cases where it might be useful to use client metadata with entity_id scheme.
I think there's something that you're seeing that I'm not yet getting - I'm not sure which cases you are thinking of? Or can you explain a case where using client_metadata is very obviously wrong for pre-registered but not for any other client_id_scheme?
I think there are use cases where we need client metadata with pre_registered too? i.e. The ephemeral encryption key case, particularly once you're using https://datatracker.ietf.org/doc/draft-ietf-oauth-attestation-based-client-auth/ in combination with pre-registered client_ids.
I think I'd feel more comfortable removing the sentence at this stage. It feels like we're deciding on one part of #19 but without a full justification why it's sensible.
If we really want to keep the sentence we should fix it, as "Wallet knows Client Identifier and corresponding Client metadata prior to the transaction" is also true for entity_id the second time a wallet sees a particular verifier (federation requires the AS to register the client_id once it's seen it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are use cases where we need client metadata with pre_registered too? i.e. The ephemeral encryption key case, particularly once you're using https://datatracker.ietf.org/doc/draft-ietf-oauth-attestation-based-client-auth/ in combination with pre-registered client_ids.
I agree with this point, although I think we also need to also go further here e.g
- Establish an order of precedent too, for example if there is metadata from registration that takes priority over what is specified in this parameter
- We also need to either make this parameter work with only a subset of metadata when the request isn't signed as its not authoritative in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
per #165 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer we discuss this wider there is more discussion to be had on how this parameter is handled in different contexts.
ok, in this PR let's simply delete the confusing paragraph for now, without introducing a clarification text (I agree it makes sense to use ephemeral encryption key with pre-registered clients). and I made a note in #17 (comment) that we should revisit if the usage of client metadata is affected by client_id_scheme or not to make sure this does not get forgotten. |
fixes #96.