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

Issue #86 closed
Sascha Preibisch created an issue

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.

Comments (14)

  1. 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.

  2. Edmund Jay

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

  3. 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.

  4. 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)

  5. 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.

  6. 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.

  7. 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>>

  8. Log in to comment