Refresh token clause readability

[Dave Tonge]

5.3.2.1. General requirements

10. shall not use refresh token rotation unless, in the case a response with a new refresh token is not received and stored by the client, retrying the request (with the previous refresh token) will succeed;

[Rifaat] I am having a hard time digesting this paragraph. I am not sure what it is trying to say.

[Joseph Heenan]

There is a note in the spec that tries to expand and is I think more readable:

“NOTE 2: Refresh token rotation is an optional feature defined in Section 6 of [RFC6749] where the authorization server issues a new refresh token to the client as part of the refresh_token grant. This document discourages the use of this feature as it does not bring any security benefits for confidential clients, and can cause significant operational issues. However, to allow for operational agility, authorization servers may implement it providing they meet the requirement in Clause 10”

I’m not 100% sure how to improve the clause itself.

‌

‌

[Dave Tonge]

I had a go at rewording it: https://bitbucket.org/openid/fapi/pull-requests/494

[Mark Verstege]

In Australia we do not allow refresh token cycling. This was after lengthy discussion with banks in our ecosystem as we moved towards FAPI 1 Final adoption. We identified large variation in how OPs were supporting refresh token cycling and it was actually harming interoperability with clients.

That said, we have recently heard of issues where OPs are changing brands or legal entities, which may result in a change of issuer. This would be a common ecosystem challenge where a solution where refresh token cycling is useful in certain business use cases to support continuity of service for end users.

‌

I agree that retaining “shall not use refresh token cycling” should be retained, but with additional notes. I think it is important to state (a) that refresh token cycling offers no security benefit, and (b) exceptions to allow refresh token cycling are an ecosystem consideration for example for OP migrations like change of issuer and mergers/demergers where continuity of service for the end-user is important to maintain where technical issues would otherwise break authorisations.

[Lukasz Jaromin]

@Mark

I am sending a list of options that we have discussed so far:

1. Ban refresh token cycling in FAPI2 SP

   • Description: Keep shall not use refresh token cycling in the AS requirements and that’s it.. The spec does not give an option to use RT cycling.
   • Pros: Interoperability.
   • Cons: No real option to use RT rotation from time to time for operational reasons and comply with the spec.
   • Issues and concerns: It is not allowed for target ecosystem to overwrite it in its specification

2. Ban it and allow it at the same time allow its constant use under certain conditions.

   • Description: See PR494.
   • Pros: RT rotation possible use operational reasons. Potentially easier migration to FAPI2.
   • Cons: Interoperability. No option to enforce OPs not to use it.
   • Issues and concerns: No clear requirements regarding implementation like acceptance of RT-1 by AS may lead to interoperability issues when RT rotation is used.

3. Like option number 2, but allow its use occasionally in extraordinary cases

   • Description: See PR505
   • Pros: RT use limited to extraordinary cases.
   • Cons: Some implementations may violate this requirement and keep rotating the RT even in non-extraordinary cases.
   • Issues and concerns: Like option number 2

4. Ban it, but clarify in a note that RT rotation can be used

   • Description: See Mark’s comment above
   • Pros: Interoperability as conformance tests would not allow RT rotation.
   • Cons: Divergence between the spec and conformance tests that would not allow it.
   • Issues and concerns: If RT rotation is occasionaly used and this feature is not covered by the conformance tests it would cause interoperability as there would likely be differences between implementations.

5. Like 3 above i.e. ban RT rotation, but allow occasional use in extraordinary cases and set precise additional rules regarding the old RT invalidation

   • Description: Option 3 as a baseline + specify that AS must keep the old RT until the client uses the new RT for the first time. If the new RT is used the AS shall invalidate the old RT.
   • Pros: RT is not used on regular basis. If it is used occasionally the ecosystem is interoperable as it is covered by the conformance tests. Even if implementation violates the spec and uses it on regular basis it is interoperable.
   • Cons: ? Are there any?
   • Issues and concerns: Additional implementation on AS side.

6. Like number 5 but without specifying that it shall happen only occasionally in the AS requirements. We move it to the note.

Please have a look at these options and state which one you like best by either commenting below or joining the conversation in the next WG meeting. If I have missed anything please feel free to add additional ones.

I am leaning towards 6 or 5.

[Nat Sakimura]

Thinking a bit more on it, I have started to wonder what are the concrete case for the “operational reasons”.

[Nat Sakimura]

• Extensive discussion on whether to allow refresh token rotation and under what circumstances.
• Lukasz presented 6 options, with debate focusing on options 1 (forbid rotation) vs 5/6 (allow occasional rotation).
• Concerns raised about interoperability and existing implementations.
• No consensus reached. Chair proposed to send out a vote on the mailing list for options 1 and 5.

‌

[Dima Postnikov]

Why Option 6 was not elaborated?

If I understand the options correctly this looks like the best of both worlds (1 and 5)

1 Ban RT rotation completely (SHALL NOT)

• Description: Keep shall not use refresh token cycling in the AS requirements and that’s it.. The spec does not give an option to use RT cycling.
• Pros: Interoperability. FAPI 2 proceeds to final now.
• Cons: No real option to use RT rotation from time to time for operational reasons and comply with the spec. DP this is a big CON potentially.
• Issues and concerns: It is not allowed for target ecosystem to overwrite it in its specification. It’s strange to not allow it on AS side and allow it on a client side. If both disallowed this creates a big problem when RT cycling is required.

5 Ban RT rotation on the AS for security purposes (SHALL NOT), allow for migration purposes (MAY) and prescribe a specific mechanism.

• Description: Option 3 as a baseline + specify that AS must keep the old RT until the client uses the new RT for the first time. If the new RT is used the AS shall invalidate the old RT.
• Pros: RT is not used on regular basis. If it is used occasionally the ecosystem is interoperable as it is covered by the conformance tests. Even if implementation violates the spec and uses it on regular basis it is interoperable.
• Cons: ? Are there any? DP Implementations will need much more guidance than above. All existing implementations and certifications are invalidated, all vendor solution do not support it. Need to wait for vendors to implement it before certifying anyone. Potentially we need a formal security analysis update. Feels like an overkill for something that unlkely to be used. FAPI 2 is delayed.
• Issues and concerns: Additional implementation on AS side. I don’t think we can provide a generic secure and fully specified mechanism for all exception use cases out there.

6 Ban RT rotation on the AS for security purposes (SHALL NOT), allow for migration purposes (MAY) and not prescribing a specific mechanism (may be just areas to be looked at : it should be implemented on AS side with minimal impact to RPs, security considerations to be reviewed, formal analysis doesn’t cover it).

Pros:

• interoperability for the rest of the spec is preserved
• exception is there if it’s required.
• flexibility for AS to implement the right way of RT rotation guarantee for each specific use case and still be compliant with the specs
• FAPI 2 proceeds to final now.

Cons:

• in an unlikely scenario when an IDP has to implement RT rotation, they are one their own. Minimal guidance.

‌

What did I miss?

[Dima Postnikov]

PR merged https://bitbucket.org/openid/fapi/pull-requests/509

[Dima Postnikov]

PR merged