diff --git a/lib/travis/api/app/endpoint/authorization.rb b/lib/travis/api/app/endpoint/authorization.rb index 888d26b1..94b3f9ad 100644 --- a/lib/travis/api/app/endpoint/authorization.rb +++ b/lib/travis/api/app/endpoint/authorization.rb @@ -163,7 +163,8 @@ class Travis::Api::App redirect_uri: oauth_endpoint } - if params[:code] and state_ok?(params[:state]) + if params[:code] + halt 400, 'state mismatch' unless state_ok?(params[:state]) endpoint.path = config.access_token_path values[:state] = params[:state] values[:code] = params[:code] @@ -187,11 +188,13 @@ class Travis::Api::App redis.expire('github:states', 1800) payload = params[:origin] || params[:redirect_uri] state << ":::" << payload if payload + response.set_cookie('travis.state', state) state end def state_ok?(state) - redis.srem('github:states', state.split(":::", 1)) if state + cookie_state = request.cookies['travis.state'] + state == cookie_state and redis.srem('github:states', state.to_s.split(":::", 1)) end def github_to_travis(token, options = {}) diff --git a/spec/unit/endpoint/authorization_spec.rb b/spec/unit/endpoint/authorization_spec.rb index bb91f4bc..ad8ae799 100644 --- a/spec/unit/endpoint/authorization_spec.rb +++ b/spec/unit/endpoint/authorization_spec.rb @@ -38,9 +38,20 @@ describe Travis::Api::App::Endpoint::Authorization do end describe "GET /auth/handshake" do + describe 'evil hackers messing with the state' do + it 'does not succeed if state cookie mismatches' do + Travis.redis.sadd('github:states', 'github-state') + response = get '/auth/handshake?state=github-state&code=oauth-code' + response.status.should be == 400 + response.body.should be == "state mismatch" + Travis.redis.srem('github:states', 'github-state') + end + end + describe 'with insufficient oauth permissions' do before do Travis.redis.sadd('github:states', 'github-state') + rack_mock_session.cookie_jar['travis.state'] = 'github-state' response = mock('response') response.expects(:body).returns('access_token=foobarbaz-token')