Fix csrf_detect error with concurrent authorisations

.rubocop.yml

@@ -38,5 +38,11 @@ Style/SpaceInsideHashLiteralBraces:
 Style/StringLiterals:
   EnforcedStyle: double_quotes
 
-Style/TrailingComma:
+Style/TrailingCommaInLiteral:
   EnforcedStyleForMultiline: 'comma'
+
+Style/TrailingCommaInArguments:
+  EnforcedStyleForMultiline: 'comma'
+
+Style/MutableConstant:
+  Enabled: false

lib/omniauth/strategies/oauth2.rb

@@ -42,9 +42,9 @@ def callback_url
 
       credentials do
         hash = {"token" => access_token.token}
-        hash.merge!("refresh_token" => access_token.refresh_token) if access_token.expires? && access_token.refresh_token
-        hash.merge!("expires_at" => access_token.expires_at) if access_token.expires?
-        hash.merge!("expires" => access_token.expires?)
+        hash["refresh_token"] = access_token.refresh_token if access_token.expires? && access_token.refresh_token
+        hash["expires_at"] = access_token.expires_at if access_token.expires?
+        hash["expires"] = access_token.expires?
         hash
       end
 
@@ -59,7 +59,7 @@ def authorize_params
           @env ||= {}
           @env["rack.session"] ||= {}
         end
-        session["omniauth.state"] = params[:state]
+        session["omniauth.state.#{params[:state]}"] = "true"
         params
       end
 
@@ -71,7 +71,7 @@ def callback_phase # rubocop:disable AbcSize, CyclomaticComplexity, MethodLength
         error = request.params["error_reason"] || request.params["error"]
         if error
           fail!(error, CallbackError.new(request.params["error"], request.params["error_description"] || request.params["error_reason"], request.params["error_uri"]))
-        elsif !options.provider_ignores_state && (request.params["state"].to_s.empty? || request.params["state"] != session.delete("omniauth.state"))
+        elsif !options.provider_ignores_state && (request.params["state"].to_s.empty? || session.delete("omniauth.state.#{request.params['state']}") != "true")
           fail!(:csrf_detected, CallbackError.new(:csrf_detected, "CSRF detected"))
         else
           self.access_token = build_access_token

spec/omniauth/strategies/oauth2_spec.rb

@@ -10,10 +10,12 @@ def app
 
   before do
     OmniAuth.config.test_mode = true
+    OmniAuth.config.full_host = "test"
   end
 
   after do
     OmniAuth.config.test_mode = false
+    OmniAuth.config.full_host = "test"
   end
 
   describe "Subclassing Behavior" do
@@ -56,8 +58,9 @@ def app
 
     it "includes random state in the authorize params" do
       instance = subject.new("abc", "def")
-      expect(instance.authorize_params.keys).to eq(["state"])
-      expect(instance.session["omniauth.state"]).not_to be_empty
+      params = instance.authorize_params
+      expect(params.keys).to eq(["state"])
+      expect(instance.session["omniauth.state.#{params[:state]}"]).not_to be_empty
     end
   end
 
@@ -79,12 +82,68 @@ def app
     subject { fresh_strategy }
     it "calls fail with the client error received" do
       instance = subject.new("abc", "def")
+      instance.authorize_params # init env and set state
       allow(instance).to receive(:request) do
         double("Request", :params => {"error_reason" => "user_denied", "error" => "access_denied"})
       end
 
       expect(instance).to receive(:fail!).with("user_denied", anything)
       instance.callback_phase
+      expect(instance.env["omniauth.auth"]).to be_nil
+    end
+
+    it "calls fail with csrf_detected when state is incorrect" do
+      instance = subject.new("abc", "def")
+      instance.authorize_params # init env and set state
+      allow(instance).to receive(:request) do
+        double("Request", :params => {"state" => "invalid_state"})
+      end
+
+      expect(instance).to receive(:fail!).with(:csrf_detected, anything)
+      instance.callback_phase
+      expect(instance.env["omniauth.auth"]).to be_nil
+    end
+
+    it "succeeds" do
+      app = double("RackApp")
+      instance = subject.new(app, "def")
+      params = instance.authorize_params
+      allow(instance).to receive(:request) do
+        double("Request", :params => {"state" => params[:state]})
+      end
+      allow(instance).to receive(:build_access_token) do
+        double("OAuht2::AccessToken", :expires? => false, :expired? => false, :token => "access token")
+      end
+
+      expect(app).to receive(:call) do |env|
+        expect(env["omniauth.auth"]["credentials"]).to eq("token" => "access token", "expires" => false)
+      end
+      instance.callback_phase
+      expect(instance.env["omniauth.auth"]).to_not be_nil
+    end
+
+    it "allows for two concurrent authorizations on the same session" do
+      app = double("RackApp")
+      instance = subject.new(app, "def")
+      allow(instance).to receive(:build_access_token) do
+        double("OAuht2::AccessToken", :expires? => false, :expired? => false, :token => "access token")
+      end
+
+      states = []
+
+      2.times do
+        params = instance.authorize_params
+        states << params[:state]
+      end
+
+      expect(app).to receive(:call).twice
+      expect(instance).to_not receive(:fail!)
+      states.each do |state|
+        allow(instance).to receive(:request) do
+          double("Request", :params => {"state" => state})
+        end
+        instance.callback_phase
+      end
     end
   end
 end