[Openid-specs-ab] Feedback on the spec drafts

[Casper Biering]

Hi,

I've been looking through the specs and have a number of comments.

First, I have fixed some seemingly minor naming and formatting issues,
which you can see here:
https://github.com/Peercraft/OpenID-Specs/compare/master...fixes

Next, my general review:

------------------------

== Discovery (draft 02) ==

*) Under Terminology, the description for Issuer is: "A verifiable
identifier for the OpenID Provider. An issuer is a HTTPS URL with no
path component."

Why does the examples show "https://example.com/auth" as a valid issuer.

*) It might be a good idea to give some kind of recommendation on how
long (if at all) to cache Provider Discovery and Provider Configuration.

== Dynamic Client Registration (draft 04) ==

*) What is js_origin_url used for? Does any examples exist?

*) In some places, it would be nice for the OP to redirect the user to
the RP. So a having a "application_url" is to be prefered.

== User Info (draft 06) ==

*) Phone_number should be changed to phone (to match email, rather than
email_address).

*) Verification claims are equally applicable to phone and address as to
email. Hence a rename from the attribute "verified" to "email_verified"
and the addition of phone_verified and address_verified would be
logical.

*) As in particular email addresses and phone numbers are often
shortlived, a boolean verification attribute will not persuade most RP's
to omit their own verification process. Instead the use of a datestamp
is proposed.

(Also most IDP's will probably have a general policy of either verifying
or not verifying emails, so an RP's basic trust in the email addresses
provided by an IDP would rather be a trust framework / policy issue)

*) As SMS has become a predominant way for RP's to contact customers by
phone, a phone number by itself is rather worthless. It is proposed to
add an attribute phone_support where the value could be a comma
separated string containing one or more of the substrings "voice",
"sms", and "fax".

*) Framework spec (chapter 8.3) mentions request verification of the
signature in a JWT for UserInfo endpoint, but the userinfo spec does not
cover it.

*) Why is id_token_audience not placed inside the OpenID Request Object,
as a submember of id_token?

== HTTP Redirect Binding (draft 04) ==

*) Remove artifact term (including Core and Framework)

*) Chapter 3.1.1 Both this spec and core says that redirect_uri is
required in the Authorization Request, but the OAuth specs says its
optional, if it is optained out-of-band (which in our case can be client
registration).

*) Chapter 3.1.1: It would be nice if all the parameters had references
to where is it defined, especially as Nat recommends reading the
http-redirect spec first.

*) Chapter 3.1.1.2 and 3.1.1.3 should be moved to Framework, to make
http-redirect spec more simple.

*) Chapter 3.1.6.1 should contain a description of user_id and domain.

*) Chapter 3.1.7.1, 3.1.8 and 3.1.8.1 should be removed, since its
covered in user-info spec.

== Core (draft 10) ==

*) With all the examples in core, it looks like its a shortcut version
of http-redirect. I suggest removing all examples, to make it less
confusing.

*) The prompt=consent param in the Authorization Request, what is the
current use-case for it? We were thinking of using it for redirecting
the user to our "which (additional) info do you want to disclose to this
RP?"-page.

== Common ==

*) From a specs writer point of view I understand why core and
http-redirect are separated, but as an implementer, its very confusing.
As mentioned above, removal of the http-redirect examples might help.

*) There are mixed use of hostnames (and urls) throughout the examples:
client.example.com
rp.example.com
op.example.com
example.com
server.example.com
example.net
client.com
A systematic alignment of example URLs would make it easier for
newcomers to read the specs.

== Session Management (draft 02) ==

*) A rewrite as proposed by Breno seems a good idea.

*) Chapter 3.2.1 & 3.2.2 & 3.2.3: Does not specify the error messages
and flow, if the action could not be completed.

*) Would be nice if the spec included single logout.

== Framework (draft 04) ==

*) Where is the documentation for ID token JSON response from the Token
Endpoint? I can only find an example in Framework chapter 5, and Session
chapter 3.2.2.

----------------------

I hope you will find some of this input useful.

-- 
Best Regards

Casper Biering
cb at peercraft.com