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

Complete IANA Considerations section #274

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

Conversation

selfissued
Copy link
Member

Fixes #239

Comment on lines 2204 to 2223
## OAuth URI Registry

This specification registers the following URNs
in the IANA "OAuth URI" registry [@IANA.OAuth.Parameters]
established by [@!RFC6755].

### urn:ietf:params:oauth:federation

* URN: urn:ietf:params:oauth:federation
* Common Name: OpenID Federation
* Change Controller: OpenID Foundation Artifact Binding Working Group - [email protected]
* Reference: (#federations) of this specification

### urn:ietf:params:oauth:federation_trust_mark

* URN: urn:ietf:params:oauth:federation_trust_mark
* Common Name: OpenID Federation Trust Mark
* Change Controller: OpenID Foundation Artifact Binding Working Group - [email protected]
* Reference: (#federations) of this specification

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am extremely unsure about this. First of all, these values are not defined as URIs in the spec. and I don't think the intent of the section was to define these parameters normatively... and if we are registering these, we should also register https://train.trust-scheme.de/info.

Copy link
Member Author

@selfissued selfissued Oct 3, 2024

Choose a reason for hiding this comment

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

The URNs are used normatively in the spec. To legitimately use them, we have to register them.

https://train.trust-scheme.de/info isn't eligible for registration. It's a URL which can be used as-is, without any form of registration.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Having read the relevant section of OID4VP I agree with Mike here - this does look like the correct outcome of the current spec language. It may well be appropriate to change the values, but we should probably do that in a separate issue if so, it would be out of scope for this PR.

Copy link
Member

Choose a reason for hiding this comment

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

I'd argue that it is appropriate to change the values and have #279 with the intent to do so.

This PR can just omit the registry requests so as to not further ingrain the erroneous values and avoid wasting the time of IANA and/or the DEs on that registry. See also #274 (comment) for related commentary.

I'd also agree with the statement that "these values are not defined as URIs in the spec" - a single passing mention does not constitute definition by a spec.

Lastly and only tangentially related - https://train.trust-scheme.de/ doesn't seem to resolve to anything and the provided reference for @TRAIN is seemingly to the website for a 2022 conference in Denmark https://oid2022.compute.dtu.dk/index.html, which all seems somewhat less than legitimately use of anything.

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.

please see my comments

Comment on lines +2212 to +2224
### urn:ietf:params:oauth:federation

* URN: `urn:ietf:params:oauth:federation`
* Common Name: OpenID Federation
* Change Controller: OpenID Foundation Artifact Binding Working Group - [email protected]
* Reference: (#federations) of this specification

### urn:ietf:params:oauth:federation_trust_mark

* URN: `urn:ietf:params:oauth:federation_trust_mark`
* Common Name: OpenID Federation Trust Mark
* Change Controller: OpenID Foundation Artifact Binding Working Group - [email protected]
* Reference: (#federations) of this specification
Copy link
Member

Choose a reason for hiding this comment

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

It seems entirely inappropriate for these to URNs to be in an IETF OAuth namespace.

Copy link
Member Author

Choose a reason for hiding this comment

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

What's an appropriate or inappropriate use of the OAuth URN namespace is likely in the eye of the beholder. For instance, the COSE Key Thumbprint spec uses it - paralleling its use by the JWK Thumbprint spec, with there also being its use in the JWK Thumbprint URI spec.

But as Elton John said, "Don't Shoot Me I'm Only the Piano Player".

My role in creating this PR was to register the registerable things that the specification already defined. I didn't try to pass judgement on whether they were the right things.

If you think these URNs already defined at 11.2. Support for Federations/Trust Schemes are the wrong things, convince the working group to change them, and I'll gladly update this PR to match.

Copy link
Member

Choose a reason for hiding this comment

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

As Billy Joel said in a different song about a piano player, "but there's some place that he'd rather be."

Indeed there is some element to 'eye of the beholder' here but, as co-author of the document that established the IANA "OAuth URI" registry, I behold these to be entirely inappropriate.

I would furthermore behold that JWK Thumbprint URI spec's use of it is pretty tenuous and the COSE Key Thumbprint spec's use is even more so. But it's way too late to do anything about the former and mostly too late for the latter.

There seem to be a lot of wrong things in 11.2. Support for Federations/Trust Schemes and I've put in #279 to try and make two of those wrong things less wrong.

In the meantime, this PR should not exacerbate the situation.

Comment on lines +2206 to +2224
## OAuth URI Registry

This specification registers the following URNs
in the IANA "OAuth URI" registry [@IANA.OAuth.Parameters]
established by [@!RFC6755].

### urn:ietf:params:oauth:federation

* URN: `urn:ietf:params:oauth:federation`
* Common Name: OpenID Federation
* Change Controller: OpenID Foundation Artifact Binding Working Group - [email protected]
* Reference: (#federations) of this specification

### urn:ietf:params:oauth:federation_trust_mark

* URN: `urn:ietf:params:oauth:federation_trust_mark`
* Common Name: OpenID Federation Trust Mark
* Change Controller: OpenID Foundation Artifact Binding Working Group - [email protected]
* Reference: (#federations) of this specification
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
## OAuth URI Registry
This specification registers the following URNs
in the IANA "OAuth URI" registry [@IANA.OAuth.Parameters]
established by [@!RFC6755].
### urn:ietf:params:oauth:federation
* URN: `urn:ietf:params:oauth:federation`
* Common Name: OpenID Federation
* Change Controller: OpenID Foundation Artifact Binding Working Group - [email protected]
* Reference: (#federations) of this specification
### urn:ietf:params:oauth:federation_trust_mark
* URN: `urn:ietf:params:oauth:federation_trust_mark`
* Common Name: OpenID Federation Trust Mark
* Change Controller: OpenID Foundation Artifact Binding Working Group - [email protected]
* Reference: (#federations) of this specification

per prior comments

Copy link
Member Author

Choose a reason for hiding this comment

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

Removing this text would currently make the spec inconsistent with itself. At such point as these URNs are no longer used, as suggested in #279, I'll remove it.

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.

IANA registrations missing for some defined parameters
6 participants