Possible race condition with multiple authorization requests causes CSRF error

[difernandez]

Description

When multiple authorize requests are made in quick succession, some times a CSRF error appears:

ERROR -- omniauth: (openid_connect) Authentication failure! csrf_detected: OmniAuth::Strategies::OpenIDConnect::CallbackError, csrf_detected | Invalid 'state' parameter

This can happen if the user clicks many times on a button or link that triggers authorization, if they open said links in many different tabs, or if silent authentication on render is implemented and the user reloads a page many times.

As far as I can tell, the problem comes from how the state is saved to storage:

def new_state
  state = if options.state.respond_to?(:call)
            if options.state.arity == 1
              options.state.call(env)
            else
              options.state.call
            end
          end
  session['omniauth.state'] = state || SecureRandom.hex(16)
end

State is always saved in session as a string, always under a fixed key. That means that if a second auth request is made before the callback for a first request has been received, the state for the first request is overwritten and when it's callback eventually is received, it fails with the csrf error because of the missing first state.

Possible solutions

I can think of two approaches to address this:

1. Define session['omniauth.state'] as an array. That way, new_state would push to it, and stored_state would find, delete and return the state if it's present in the array.
2. Use the state as key instead of value of the session stored state. That would also prevent collision/overwrite of stage, and it would also allow saving of other related state if you wanted (like a return_url to be used after a successful callback for example). This seems to be the approach used/recommenbed by Auth0, as seen in their documentation and in their Client Side SDK:

this.storage.setItem(this.namespace + state, transactionPayload, {
  expires: this.stateExpiration
});

I think the first approach is a bit simpler, but the second one comes with the added benefit of being able to associate a payload with the state, and the auth request by proxy, I don't know if that is something that would be of interest to this project.