The read only spec should not contain 'if possible' in section 5.2.2

[Sascha Preibisch]

The read only spec. has this text within the list of requirements:

• shall verify that the Authorization Code (section 1.3.1 of [RFC6749]) has not been previously used if possible

I would like us to remove 'if possible'. This requirement starts with 'shall' which, if I am not mistaken, refers to something that has to be supported. If that is true, 'if possible' is not valid.

[Takahiko Kawasaki]

Changing 'shall' to 'should' is better than removing 'if possible' (if we should change the sentence).

Authorization codes are short-lived tokens. "RFC 6749, 4.1.2. Authorization Response" says "A maximum authorization code lifetime of 10 minutes is RECOMMENDED". If 'if possible' were removed, authorization server implementations would have to keep the short-lived tokens in their databases forever only to ensure that they are not reused even after the tokens have expired. Garbage records would be accumulated. It would be impractical.

[Dave Tonge]

I agree changing to "should" is a good idea.

[Nat Sakimura]

Accept "should"

[Edmund Jay]

fixes #86 - The read only spec should not contain 'if possible' in section 5.2.2

→ <<cset ea4cfabcda29>>

[Joseph Heenan]

Hi Edmund, it looks like the ‘if possible’ is still there after your commit?

[Edmund Jay]

From the previous comments, especially Takahiko's, it looks like it's agreed to change shall -> should and keep 'if possible'.

[Joseph Heenan]

Hm, yes - re-reading it I agree that did seem to be the consensus.

'should' already means "do this unless you have a good reason not to".

I think the new wording means "do this, if possible, unless you have a good reason not to". I don't really know what the addition of 'if possible' there means.

[Edmund Jay]

fixes #86 - The read only spec should not contain 'if possible' in section 5.2.2; dropped 'if possible'

→ <<cset af3085cb6bae>>

[Joseph Heenan]

I'd like to reopen this issue, as on further review I'm not 100% convinced by the text that is in the second implementers draft.

Current FAPI part 1 ID 2 says:

should verify that the authorization code (section 1.3.1 of [RFC6749]) has not been previously used;

and:

NOTE: Section 4.1.3 of [RFC6749] does not provide guidance regarding code reuse, but this document provides limitation on code reuse in Section 3.1.3.2 of [OIDC].

NOTE: If replay identification of the authorization code is not possible, it is desirable to set the validity period of the authorization code to one minute or a suitable short period of time. The validity period may act as a cache control indicator of when to clear the authorization code cache if one is used.

I think the requirement and the notes are generally weaker than https://tools.ietf.org/html/rfc6749#section-10.5 which says:

Authorization codes MUST be short lived and single-use.

My current opinion would be the text in FAPI should be strengthened to be consistent with RFC6749.

(See also https://bitbucket.org/openid/connect/issues/1057/oidcc-appears-to-override-single-use as the openid connect core spec seems to have a similar issue)

[Nat Sakimura]

The read only spec. has this text within the list of requirements:

• shall verify that the Authorization Code (section 1.3.1 of [RFC6749]) has not been previously used if possible

I would like us to remove 'if possible'. This requirement starts with 'shall' which, if I am not mistaken, refers to something that has to be supported. If that is true, 'if possible' is not valid.

[Joseph Heenan]

Mike added that the openid connect conformance suite issues a warning if a code can be reused within 30 seconds, and a hard failure if it can be reused after 30 seconds.

It can be hard to implement a hard single use requirement in large cloud systems.

The current conformance suite treats it as a fail if a code can be reused at all.

We didn’t come to any hard conclusion.

[Joseph Heenan]

pull request opened at https://bitbucket.org/openid/fapi/pull-requests/113/fapi-r-clarify-authorization-code-reuse/diff to ensure we at least reflect the position the connect WG has adopted in their certification tests. We may want to go further, but I see this as the minimum.

[Nat Sakimura]

FAPI-R: Clarify authorization code reuse requirements

The OpenID Connect and OAuth2 specifications in places use unclear language when talking about reuse of authorization codes.

This text attempts to state a clear position. The position chosen is that already documented in one section of RFC6749 4.1.2:

If an authorization code is used more than once, the authorization server MUST deny the request

In some ways it is not necessary to repeat this as it is already in RFC6749, however the clause is often missed and OIDCC adds confusion by adding 'if possible'.

closes #86

→ <<cset 32a8689b180e>>