jeremyevans/roda

route_csrf incompatible with rack-protection authenticity token checks

HoneyryderChuck opened this issue · 5 comments

While investigating a report about the oidc example failing in rodauth-oauth and updating the dependencies to the latest version, which upgraded omniauth 2.1.0, I've encountered what seems like an incompatibility between the route_csrf plugin and latest omniauth, which does CSRF validation by default.

For reference, the oidc client example uses the omniauth connect middleware in a roda which loads route_csrf. What's observed is that, when a POST request to /auth/openid_connect happens with the csrf generated by route_csrf (I changed the field to "authenticity_token", which is rack-protection's default), the processing goes down until this call, which tries to validate the token; this goes down til here, which just seems incompatible with how route_csrf generates and verifies tokens.

I'd say this is a bug, as roda should work seamless with legacy rack middlewares. Tell me what you think.

It looks like you expect tokens generated by route_csrf to be verifiable by other software. That seems like an unrealistic expectation. In addition to the software needing to have access to the underlying CSRF secret, it would have to do the verification the same way route_csrf does (which is more advanced/secure than most other implementations).

I think a better approach in rodauth-oath, when generating the CSRF token for the form that submits to the omniauth route, you should use an omniauth-compatible CSRF token instead of a route_csrf token. Alternatively, modify the omniauth CSRF validation support to do the same CSRF validation that the route_csrf plugin does.

Yes, I was feeling that that was going to be the answer as I was finalizing the description :) Would some documentation about the route_csrf not being compatible with other rack csrf alteratives be smth to consider?

On the rodauth-oauth side, there's no problem, given this was raised for the example "client app", which relies on omniauth_openid_connect, not rodauth-oauth.

I'm not opposed to such documentation, though I would consider it obvious that different CSRF implementations are not compatible, since there isn't a standard to follow. Can you provide an example of two CSRF implementations that are compatible, in that the tokens generated by one are verifiable by the other and vice-versa, without explicitly being designed with compatibility in mind?

Agreed, perhaps this shouldn't be an expectation at all.

Having given it another look, I think I found out the missing piece: omniauth protocols a callback for validating the request, which by default uses Rack::Protection::AuthenticityToken, itself compatible with rack_csrf.

This in turn is not compatible with rails csrf layer, and a separate gem has to be installed for omniauth compatibility, which overrides the omniauth callback with its own rails-compatible verifier.

This means that route_csrf could work with omniauth, as long as such a compatibility callback could be provided. Given the current state of the plugin, that'd be hard, given that the logic is inside a function which returns an error message. Nevertheless, this could be extracted.

Would this be something you'd be interested in? I can try and open a PR to refactor it in a way that it could be usable with omniauth, if so.

It's certainly something that I could consider, but it will depend on how invasive the patch is.