Update OAuth2 section

[mmccool]

Updates intended to satisfy the resolutions of the Security TF as described in #926
Basically adds client and device flows, while noting (with an assertion!) that all other flows should be specified using an extension.

---

Preview | Diff

[mmccool]

To check:

• Do vocabulary items need to be added for "device" and "client"?
• Do vocabulary items need to be removed? In particular, "implicit" and "password" should NOT be in the base vocabulary
• Need to double-check if both "authorization" and "token" are needed for all flows.
  Until the above are resolved I'm marking it as a draft.

[takuki]

TD task force discussed in 2020-07-15 TD teleconference (see minutes).

This change should not break any existing TD, therefore should be 1.1 compatible.
For example. no change to the JSON schema file.

Security TF will further discuss the change next week. We can hold this PR for now.

[mmccool]

We may need to update the security best practices document to note that implicit and password flows are deprecated, and to recommended which use cases are suitable for device, code, and client. We should probably review the main security and privacy guidelines document as well.

[relu91]

Need to double-check if both "authorization" and "token" are needed for all flows.

I did a review of the RFC 6749 and 8628. My conclusion is that the authorization endpoint is mandatory for code and implicit flow. It is clearly stated that this endpoint is used to interact with the Resource Owner:

The authorization endpoint is used to interact with the resource
owner and obtain an authorization grant.

client_credentials flow does not require an interaction with the Resource Owner and the password flow supposes that the interaction is direct (i.e. no intermediary service is used).

On the other hand, the device flow uses a dedicated authorization endpoint called device authorization endpoint. The RFC 8628 uses this new term to distinguish between the normal OAuth 2 authorization endpoint (see section 3.1). We might use the term authorization as well since it is clear that we are referring to the device authorization endpoint when we define a device flow SecurityDefinition.

Finally, about the token endpoint. RFC 6749 states:

The token endpoint is used by the client to obtain an access token by
presenting its authorization grant or refresh token. The token
endpoint is used with every authorization grant except for the
implicit grant type (since an access token is issued directly)

Also the device flow, clearly needs a reference to a token endpoint to complete the protocol. Therefore, I'd say it is mandatory for each flow except the implicit.

[farshidtz]

For clarity, I suggest having a seperate vocabulary for device authorization endpoint. After all, this is an independent endpoint of the authorization server, and a flow in future may need to combine it with authorization. Also necessary for proposal #929.

Some other useful references:

• Swagger OAuth2 (table with endpoints, excl. device flow)
• Endpoints with descriptions

I think it would be more readable to specify required and relevant endpoints in tabular form instead of text; see swagger link above.

[relu91]

I agree that the table helps a lot. I made a draft to summarize my comment above:

field name	code	implicit	password	client	device
authorization	+	+	-	-	-
token	+	-	+	+	+
refresh (optional)	+	+	+	+	+
device_authorization	-	-	-	-	+

+: applies to the flow
-: not applies to the flow

As @farshidtz suggested I introduced a new field for the device authorization endpoint. I agree that it is clearer.

[mmccool]

Regarding "device_authorization", my feeling is that since we are in the "device" flow it is implicit that we are talking about the "device_authorization" endpoint when we use the "authorization" field and that we don't need another field.

[mmccool]

I updated the PR to use "authorization" in the device flow for "device_authorization". Note however that if we do this then the proposal in #929 (extending the "flow" field to be an array of flows) won't work since the "authorization" interpretation will depend on the flow.

I also review the ontology and it seems the flows are not defined as formal vocabulary terms. We might want to reconsider that and use an enumeration rather than a string field for the flows, but if we do this to avoid a breaking change we would still have to accept the "simple" strings. Will discuss in the TD meeting...

[farshidtz]

Regarding "device_authorization", my feeling is that since we are in the "device" flow it is implicit that we are talking about the "device_authorization" endpoint when we use the "authorization" field and that we don't need another field.

This is true if we assume that producers of TDs are all aware of this. In many cases, developers don't know about fine details of OAuth2 and don't read the TD spec. Since there is an "authorization" endpoint on every OAuth2 server, there is a good chance that they would set that as the value of "authorization" endpoint in TD without a second thought.

Even here, we originally thought they were the same endpoint until @relu91 pointed it out.

[mmccool]

I made some updates, will also add an informative table as in comments above regarding what's required and not to make this clearer. I'll keep the text assertions though. I may also see if I can change "optional" to "depends on the flow" in the main table.

[mmccool]

I will also add an editor's note to mention that we are still discussing device_authorization. After that, though, I would like to merge it in the TD draft spec.

[mmccool]

Edits done, upgraded to "ready to merge" status.

[mmccool]

ugh, still need to update the ontology files to reflect what's in index.template.html. It doesn't matter right now but will if render.sh is ever fixed to pull info from the ttl files. I will do that shortly...

[mmccool]

Also new table uses the "def" class to get formatting right... but this is also used to indicate tables with assertions. So I need to add a new class, "nodef", to the CSS for non-normative tables (maybe using a different color, also).

[mmccool]

updated ontology to be consistent with main text, but could not find definition of table "def" class anywhere. Left it as it so it renders correctly... it seems a row needs to be marked with rfc2119-table-assertion to affect the implementation report anyhow, but it would be good to mark this table as being informative (although it does reference normative assertions in the main text, it is merely redundant).

[sebastiankb]

In today's TD call we decided to merge this PR.