Allowing the previous refresh token makes RT rotation useless

[neilmadden-teya]

The point of requiring a new refresh token to be issued each time is to allow some (weak) level of protection to public clients using refresh tokens. OAuth 2.1 only requires this for non-confidential clients. As per section 4.3.1:

If a refresh token is compromised and subsequently used by both the attacker and the legitimate client, one of them will present an invalidated refresh token, which will inform the authorization server of the breach. The authorization server cannot determine which party submitted the invalid refresh token, but it will revoke the active refresh token as well as the access authorization grant associated with it. This stops the attack at the cost of forcing the legitimate client to obtain a fresh authorization grant.

Allowing the previous refresh token to be used entirely disables this protection.

Some OAuth providers have tried to solve this issue by putting a strict time limit on how long the previous RT remains valid after it has been rotated.

[kentonv]

This decision is discussed in the readme:

https://github.com/cloudflare/workers-oauth-provider?tab=readme-ov-file#single-use-refresh-tokens

There is a tension here between risk management and availability.

The requirements in the spec are well-meaning but ignore the realities of distributed systems. The requirement will necessarily lead to grants being incorrectly revoked as a result of simple network partitions or machine failures -- situations where no attack took place. Pulling a network cable could cause someone's system to suddenly and permanently stop working (until they manually issue a new grant). I felt this was an unreasonable trade-off.

Note that this was my intentional decision, not something the LLM did. The LLM implemented the spec correctly and I asked for the change. It is perhaps arrogant of me to think I know better than the spec authors and perhaps I will end up losing this argument and looking bad but if so that's 100% on me the human, not the LLM.

[kentonv]

OAuth 2.1 only requires this for non-confidential clients.

That's not quite right: It requires either strict refresh token rotation, or that the implementation "cryptographically binds the refresh token to a certain client, e.g., by utilizing DPoP [RFC9449] or mTLS [RFC8705]."

Unfortunately the clients we aim to support with this library do not implement such "cryptographic binding" mechanisms. (A major weakness of the OAuth standard is that it tends to assume that clients are purpose-built to work with a particular OAuth provider, and thus each OAuth provider has the power to implement additional requirements on its clients. The OAuth spec likes to suggest possible additional restrictions, without actually fully specifying them, figuring the provider can specify whatever they want. In our use case, though -- MCP -- the clients are not purpose-built for any particular provider, but rather all clients are expected to talk to all providers, so no provider can realistically place unique requirements on a client.)

I do agree that it would be reasonable to require strict rotation for non-confidential clients, as such clients are likely mobile or desktop apps that can easily ask the user to re-authenticate after an error. That said, in our particular use case (MCP), clients instead, by convention, use dynamic client registration. So all clients end up being confidential clients anyway (even the ones that are desktop apps). (Open-ended dynamic client registration arguably defeats the purpose of even having client registration to start with, but this part is not up to me...)

[neilmadden-teya]

The spec says (emphasis mine):

Authorization servers MUST utilize one of these methods to detect refresh token replay by malicious actors for public clients

There's no need to rotate RTs for confidential clients.

Open-ended dynamic client registration arguably defeats the purpose of even having client registration to start with...

Indeed. I would not issue RTs at all in this use-case, as there is no way to make them secure. Normal access tokens with an idle timeout seems better.

[kentonv]

The spec says (emphasis mine):

OMG, I have read this multiple times and missed the words "for public clients" there somehow! 🤦

OK, I feel quite silly now. Clearly my whole rant is misguided then, and for confidential clients (which are most if not all users of this library at present) the implementation is compliant. We should adjust the implementation to require rotation for public clients specifically.

Indeed. I would not issue RTs at all in this use-case, as there is no way to make them secure. Normal access tokens with an idle timeout seems better.

This is an interesting debate that I think goes beyond this library.

I disagree that static vs. dynamic registration is the difference between "secure" and "insecure", but I would agree with "more secure" and "less secure". It's a risk that can be weighed against benefits.

In the MCP ecosystem, we want a client and a server to be able to connect without having previously known anything about each other. The client is some AI agent (there are tons of these), the server is some API provider (there are even more of these). They can't all register with each other in advance.

It is also sometimes necessary for the client to retain offline access. A user might instruct an agent to, say, monitor airline ticket prices using some MCP API and notify them when a good price appears.

So the entire client registration of OAuth turns out to be a huge problem, and so the MCP designers mandated dynamic client registration.

For the record, this is not my doing -- we're just following the MCP spec here. But I do think client registration has always been a barrier to interoperability. I'm not quite sure what is the right way to solve that.