From c3b6a7a29792ff55706a0741f88bdc43b741c46b Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 25 Sep 2024 03:56:08 -0400 Subject: [PATCH] Reduce factory creation (36 -> 12) in `spec/controllers/oauth/*` area (#32045) --- .../oauth/authorizations_controller_spec.rb | 42 +++++++------- ...authorized_applications_controller_spec.rb | 57 ++++++++----------- .../oauth/tokens_controller_spec.rb | 19 +++---- 3 files changed, 51 insertions(+), 67 deletions(-) diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index 9bb520211c2..cfc80b86503 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -10,13 +10,6 @@ RSpec.describe Oauth::AuthorizationsController do get :new, params: { client_id: app.uid, response_type: 'code', redirect_uri: 'http://localhost/', scope: 'read' } end - shared_examples 'stores location for user' do - it 'stores location for user' do - subject - expect(controller.stored_location_for(:user)).to eq "/oauth/authorize?client_id=#{app.uid}&redirect_uri=http%3A%2F%2Flocalhost%2F&response_type=code&scope=read" - end - end - context 'when signed in' do let!(:user) { Fabricate(:user) } @@ -24,18 +17,17 @@ RSpec.describe Oauth::AuthorizationsController do sign_in user, scope: :user end - it 'returns http success' do + it 'returns http success and private cache control headers' do subject - expect(response).to have_http_status(200) - end - it 'returns private cache control headers' do - subject - expect(response.headers['Cache-Control']).to include('private, no-store') + expect(response) + .to have_http_status(200) + expect(response.headers['Cache-Control']) + .to include('private, no-store') + expect(controller.stored_location_for(:user)) + .to eq authorize_path_for(app) end - include_examples 'stores location for user' - context 'when app is already authorized' do before do Doorkeeper::AccessToken.find_or_create_for( @@ -52,10 +44,12 @@ RSpec.describe Oauth::AuthorizationsController do expect(response).to redirect_to(/\A#{app.redirect_uri}/) end - it 'does not redirect to callback with force_login=true' do - get :new, params: { client_id: app.uid, response_type: 'code', redirect_uri: 'http://localhost/', scope: 'read', force_login: 'true' } + context 'with `force_login` param true' do + subject do + get :new, params: { client_id: app.uid, response_type: 'code', redirect_uri: 'http://localhost/', scope: 'read', force_login: 'true' } + end - expect(response).to have_http_status(:success) + it { is_expected.to have_http_status(:success) } end end end @@ -63,10 +57,16 @@ RSpec.describe Oauth::AuthorizationsController do context 'when not signed in' do it 'redirects' do subject - expect(response).to redirect_to '/auth/sign_in' - end - include_examples 'stores location for user' + expect(response) + .to redirect_to '/auth/sign_in' + expect(controller.stored_location_for(:user)) + .to eq authorize_path_for(app) + end + end + + def authorize_path_for(app) + "/oauth/authorize?client_id=#{app.uid}&redirect_uri=http%3A%2F%2Flocalhost%2F&response_type=code&scope=read" end end end diff --git a/spec/controllers/oauth/authorized_applications_controller_spec.rb b/spec/controllers/oauth/authorized_applications_controller_spec.rb index 52d3dbde83e..1cf0984abea 100644 --- a/spec/controllers/oauth/authorized_applications_controller_spec.rb +++ b/spec/controllers/oauth/authorized_applications_controller_spec.rb @@ -10,38 +10,31 @@ RSpec.describe Oauth::AuthorizedApplicationsController do get :index end - shared_examples 'stores location for user' do - it 'stores location for user' do - subject - expect(controller.stored_location_for(:user)).to eq '/oauth/authorized_applications' - end - end - context 'when signed in' do before do sign_in Fabricate(:user), scope: :user end - it 'returns http success' do + it 'returns http success with private cache control headers' do subject - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + expect(response.headers['Cache-Control']) + .to include('private, no-store') + expect(controller.stored_location_for(:user)) + .to eq '/oauth/authorized_applications' end - - it 'returns private cache control headers' do - subject - expect(response.headers['Cache-Control']).to include('private, no-store') - end - - include_examples 'stores location for user' end context 'when not signed in' do it 'redirects' do subject - expect(response).to redirect_to '/auth/sign_in' - end - include_examples 'stores location for user' + expect(response) + .to redirect_to '/auth/sign_in' + expect(controller.stored_location_for(:user)) + .to eq '/oauth/authorized_applications' + end end end @@ -55,23 +48,19 @@ RSpec.describe Oauth::AuthorizedApplicationsController do before do sign_in user, scope: :user allow(redis).to receive(:pipelined).and_yield(redis_pipeline_stub) + end + + it 'revokes access tokens for the application and removes subscriptions and sends kill payload to streaming' do post :destroy, params: { id: application.id } - end - it 'revokes access tokens for the application' do - expect(Doorkeeper::AccessToken.where(application: application).first.revoked_at).to_not be_nil - end - - it 'removes subscriptions for the application\'s access tokens' do - expect(Web::PushSubscription.where(user: user).count).to eq 0 - end - - it 'removes the web_push_subscription' do - expect { web_push_subscription.reload }.to raise_error(ActiveRecord::RecordNotFound) - end - - it 'sends a session kill payload to the streaming server' do - expect(redis_pipeline_stub).to have_received(:publish).with("timeline:access_token:#{access_token.id}", '{"event":"kill"}') + expect(Doorkeeper::AccessToken.where(application: application).first.revoked_at) + .to_not be_nil + expect(Web::PushSubscription.where(user: user).count) + .to eq(0) + expect { web_push_subscription.reload } + .to raise_error(ActiveRecord::RecordNotFound) + expect(redis_pipeline_stub) + .to have_received(:publish).with("timeline:access_token:#{access_token.id}", '{"event":"kill"}') end end end diff --git a/spec/controllers/oauth/tokens_controller_spec.rb b/spec/controllers/oauth/tokens_controller_spec.rb index dd2d8ca70cb..a2eed797e07 100644 --- a/spec/controllers/oauth/tokens_controller_spec.rb +++ b/spec/controllers/oauth/tokens_controller_spec.rb @@ -9,20 +9,15 @@ RSpec.describe Oauth::TokensController do let!(:access_token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, application: application) } let!(:web_push_subscription) { Fabricate(:web_push_subscription, user: user, access_token: access_token) } - before do + it 'revokes the token and removes subscriptions' do post :revoke, params: { client_id: application.uid, token: access_token.token } - end - it 'revokes the token' do - expect(access_token.reload.revoked_at).to_not be_nil - end - - it 'removes web push subscription for token' do - expect(Web::PushSubscription.where(access_token: access_token).count).to eq 0 - end - - it 'removes the web_push_subscription' do - expect { web_push_subscription.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(access_token.reload.revoked_at) + .to_not be_nil + expect(Web::PushSubscription.where(access_token: access_token).count) + .to eq(0) + expect { web_push_subscription.reload } + .to raise_error(ActiveRecord::RecordNotFound) end end end