From e26fe9cc50934f991943eca70436372493cfbd84 Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Wed, 4 Dec 2019 14:37:30 +0800 Subject: [PATCH] Refactoring: Remove test redundancies (from 9409d17de) 9409d17de added test coverage that turned out to overlap with existing tests. This commit removes the old test cases to eliminate redundancy, and polishes some of the language in the new test case descriptions. --- spec/requests/channels_twitter_spec.rb | 17 +-- .../integration/twitter_webhook_spec.rb | 101 ------------------ 2 files changed, 9 insertions(+), 109 deletions(-) diff --git a/spec/requests/channels_twitter_spec.rb b/spec/requests/channels_twitter_spec.rb index 8247e2236..129edadb8 100644 --- a/spec/requests/channels_twitter_spec.rb +++ b/spec/requests/channels_twitter_spec.rb @@ -22,8 +22,10 @@ RSpec.describe 'Twitter channel API endpoints', type: :request do end context 'without valid twitter credentials in the DB' do - let!(:twitter_channel) { create(:twitter_channel, external_credential: twitter_credential) } - let(:twitter_credential) { create(:twitter_credential, credentials: { foo: 'bar' }) } + before do + twitter_credential.credentials.delete(:consumer_secret) + twitter_credential.save! + end it 'responds 422 Unprocessable Entity' do get '/api/v1/channels_twitter_webhook', params: params, as: :json @@ -33,7 +35,7 @@ RSpec.describe 'Twitter channel API endpoints', type: :request do end context 'without "crc_token" param' do - let(:params) { {} } + before { params.delete(:crc_token) } it 'responds 422 Unprocessable Entity' do get '/api/v1/channels_twitter_webhook', params: params, as: :json @@ -44,10 +46,9 @@ RSpec.describe 'Twitter channel API endpoints', type: :request do end describe 'POST /api/v1/channels_twitter_webhook' do - let(:payload) { params.stringify_keys.to_s.gsub(/=>/, ':').tr(' ', '') } + let(:payload) { params.stringify_keys.to_s.gsub(/=>/, ':').delete(' ') } let(:headers) { { 'x-twitter-webhooks-signature': hash_signature } } - let(:params) { { foo: 'bar', for_user_id: user_id } } - let(:user_id) { twitter_channel.options[:user][:id] } + let(:params) { { foo: 'bar', for_user_id: twitter_channel.options[:user][:id] } } # What's this all about? See the "Optional signature header validation" section of this article: # https://developer.twitter.com/en/docs/accounts-and-users/subscribe-account-activity/guides/securing-webhooks @@ -71,8 +72,8 @@ RSpec.describe 'Twitter channel API endpoints', type: :request do end end - context 'when payload doesn’t match' do - let(:headers) { { 'x-twitter-webhooks-signature': 'Not actually a signature' } } + context 'when invalid (not based on consumer secret + payload)' do + let(:headers) { { 'x-twitter-webhooks-signature': 'Not a valid signature' } } it 'responds 401 Not Authorized' do post '/api/v1/channels_twitter_webhook', params: params, headers: headers, as: :json diff --git a/spec/requests/integration/twitter_webhook_spec.rb b/spec/requests/integration/twitter_webhook_spec.rb index bd13bbc34..67e833409 100644 --- a/spec/requests/integration/twitter_webhook_spec.rb +++ b/spec/requests/integration/twitter_webhook_spec.rb @@ -4,36 +4,6 @@ RSpec.describe 'Twitter Webhook Integration', type: :request do let!(:external_credential) { create(:twitter_credential, credentials: credentials) } let(:credentials) { { consumer_key: 'CCC', consumer_secret: 'DDD' } } - describe '#webhook_verify (for Twitter to confirm Zammad’s credentials)' do - context 'with only cached credentials' do - let!(:external_credential) { nil } - - before { Cache.write('external_credential_twitter', credentials) } - - it 'returns an HMAC signature for cached credentials plus params[:crc_token]' do - get '/api/v1/channels_twitter_webhook', - params: { crc_token: 'some_random', nonce: 'some_nonce' }, - headers: { 'x-twitter-webhooks-signature' => 'something' }, - as: :json - - expect(response).to have_http_status(:ok) - expect(json_response).to include('response_token' => 'sha256=VE19eUk6krbdSqWPdvH71xtFhApBAU81uPW3UT65vOs=') - end - end - - context 'with only credentials stored in DB' do - it 'returns an HMAC signature for stored credentials plus params[:crc_token]' do - get '/api/v1/channels_twitter_webhook', - params: { crc_token: 'some_random', nonce: 'some_nonce' }, - headers: { 'x-twitter-webhooks-signature' => 'something' }, - as: :json - - expect(response).to have_http_status(:ok) - expect(json_response).to include('response_token' => 'sha256=VE19eUk6krbdSqWPdvH71xtFhApBAU81uPW3UT65vOs=') - end - end - end - describe '#webhook_incoming' do let!(:channel) do create( @@ -67,77 +37,6 @@ RSpec.describe 'Twitter Webhook Integration', type: :request do ) end - describe 'confirming authenticity of incoming Twitter webhook' do - context 'with valid headers & parameters' do - it 'returns 200 success' do - post '/api/v1/channels_twitter_webhook', - params: { for_user_id: channel.options[:user][:id], key: 'value' }, - headers: { 'x-twitter-webhooks-signature' => 'sha256=JjEmBe1lVKT8XldrYUKibF+D5ehp8f0jDk3PXZSHEWI=' }, - as: :json - - expect(response).to have_http_status(:ok) - end - end - - context 'when request is missing x-twitter-webhooks-signature header' do - it 'returns 422 with error message' do - post '/api/v1/channels_twitter_webhook', as: :json - - expect(response).to have_http_status(:unprocessable_entity) - expect(json_response).to include('error' => "Missing 'x-twitter-webhooks-signature' header") - end - end - - context 'when Zammad has no Twitter credentials (in DB or cache)' do - let!(:external_credential) { nil } - let!(:channel) { nil } - - it 'returns 422 with error message' do - post '/api/v1/channels_twitter_webhook', - headers: { 'x-twitter-webhooks-signature' => 'something' }, - as: :json - - expect(response).to have_http_status(:unprocessable_entity) - expect(json_response).to include('error' => "No such external_credential 'twitter'!") - end - end - - context 'with invalid token in x-twitter-webhooks-signature header' do - it 'returns 401 with error message' do - post '/api/v1/channels_twitter_webhook', - headers: { 'x-twitter-webhooks-signature' => 'something' }, - as: :json - - expect(response).to have_http_status(:unauthorized) - expect(json_response).to include('error' => 'Not authorized') - end - end - - context 'with :for_user_id request parameter' do - it 'returns 422 with error message' do - post '/api/v1/channels_twitter_webhook', - params: { key: 'value' }, - headers: { 'x-twitter-webhooks-signature' => 'sha256=EERHBy/k17v+SuT+K0OXuwhJtKnPtxi0n/Y4Wye4kVU=' }, - as: :json - - expect(response).to have_http_status(:unprocessable_entity) - expect(json_response).to include('error' => "Missing 'for_user_id' in payload!") - end - end - - context 'with invalid :for_user_id request parameter' do - it 'returns 422 with error message' do - post '/api/v1/channels_twitter_webhook', - params: { for_user_id: 'not_existing', key: 'value' }, - headers: { 'x-twitter-webhooks-signature' => 'sha256=QaJiQl/4WRp/GF37b+eAdF6kPgptjDCLOgAIIbB1s0I=' }, - as: :json - - expect(response).to have_http_status(:unprocessable_entity) - expect(json_response).to include('error' => "No such channel for user id 'not_existing'!") - end - end - end - describe 'auto-creation of tickets/articles on webhook receipt' do let(:webhook_payload) do JSON.parse(File.read(Rails.root.join('test/data/twitter', payload_file))).symbolize_keys