[Openid-specs-ab] Review Comments on Discovery

[Torsten Lodderstedt]

Hi Mike,

here are my comments (some of them have already been discussed during 
the meeting on Sunday). Note: For your convenience, I also attached the 
word file containing my comments.

regards,
Torsten.

-----

Sections 2 & 4 describe a discovery process composed of three (mostly 
optional) steps (issuer discovery, create config URI, retrieve config 
data). I think it would be helpful to describe the whole process first, 
before diving into the details. This would support the reader to get 
guidance and allow to point out optional and mandatory parts of this 
spec.

section 2 heading - [already discussed] Isn’t this section about the 
discovery of the OP’s issuer URI? This should be spelled out. I got 
screwed since I thought the URI discovered here is directly used to 
retrieve config data. I may not be the only one.

2.1.1.

1. step - Why is XRI mention given all input must be a URI? I thought 
XRIs are a super-set of URIs.

2. step - Seems the URI must always contain an authority component? I 
think this should be stated explicitly.

2.1.2

This is stuff is hard to read and understand. Some example strings would 
be helpful for the reader. This would also lay a bridge to the next 
section.

"A string of any other type" - What ist he other type you refer to? Any 
other than a URI?

"2. If the userinfo component is present " - What about the host?

3.

"issuer" - Is this value required to be the same as the issuer URI 
(possibly) discovered by the process described in section 2? [already 
discussed, yes]

"authorization_endpoint
OPTIONAL. URL of the OP's Authentication and Authorization Endpoint" - 
This endpoint is also the OAuth 2.0 authz endpoint, this should be 
mentioned (as for the following parameter).

"userinfo_endpoint"
"This URL MUST use the https scheme and MAY contain port, path, and 
query parameter components. " - I think this particular normative text 
should be given in the core, only. Otherwise we end up with normative 
text regarding a certain concept spread/copied over the OIDC document 
suite, which makes maintaining consistence more difficult.

"scopes_supported
... The server MUST support the openid scope value. " - See above, this 
is not about discovery. I think it should at most be a note, but I would 
prefer a reference to the respective section in core.

"response_types_supported
... Dynamic OpenID Providers MUST support the code, id_token, and the 
token id_token response type values." - Same here, could refer to the 
MTI section in core.

grant_types_supported
... Dynamic OpenID Providers MUST support the authorization_code and 
implicit grant type values and MAY support the 
urn:ietf:params:oauth:grant-type:jwt-bearer grant type defined in JSON 
Web Token (JWT) Profile for OAuth 2.0 Client Authentication and 
Authorization Grants [OAuth.JWT]." - Same here. Moreover, I’m surprised 
this text mentions the JWT Bearer token profile. This is not even 
mentioned in core. What is the rationale?

"subject_types_supported
... Valid types include pairwise and public." - Are there other types 
envisioned?

"... The algorithm RS256 MUST be included. The value none MAY be 
supported, but MUST only be used with the Authorization Code Flow." - 
Normative text on non-discovery aspects again. I would suggest to make 
it a note or a reference instead.

"request_object_encryption_alg_values_supported
... Authorization Server ..." - I would suggest to call it “OP” 
consistently.

"token_endpoint_auth_methods_supported
OPTIONAL. JSON array containing a list of authentication methods 
supported by this Token Endpoint. The options are client_secret_post, 
client_secret_basic, client_secret_jwt, and private_key_jwt, ... " - 
Same here about normative text

"request_uri_parameter_supported
OPTIONAL. Boolean value specifying whether the OP supports use of the 
request_uri parameter, with true indicating support. If omitted, the 
default value is true. " - I’m wondering why the default is different 
for request object and reference. I would expect false for both of them.

4.

"This step is OPTIONAL. The OpenID Provider endpoints and configuration 
information MAY be obtained out-of-band." - I’m wondering why this is 
optional as I consider this the main part of discovery. Or does it refer 
to derivation of the config URI based on the issuer URI? Then I would 
suggest to change section heading to something like “Deriving OpenID 
Provider Config URI”. This might also require to push 4.1. to the top 
level.

".. Providers supporting discovery MUST support receiving WebFinger 
requests via TLS" - Is this statement misplaced. As it refers to 
WebFinger, it might belong to section 2?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Draft  OpenID Connect Discovery 1.0 - draft 19_tlt.docx
Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document
Size: 77006 bytes
Desc: not available
URL: <http://lists.openid.net/pipermail/openid-specs-ab/attachments/20131106/55579465/attachment.docx>