From f49161ab1d57969b105bd115af8c827fc817db61 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 8 Oct 2024 09:30:54 -0400 Subject: [PATCH] Oauth system spec cleanup / helper method extraction (#32287) --- spec/system/oauth_spec.rb | 96 +++++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 40 deletions(-) diff --git a/spec/system/oauth_spec.rb b/spec/system/oauth_spec.rb index 64ac75879e..14ffc163f0 100644 --- a/spec/system/oauth_spec.rb +++ b/spec/system/oauth_spec.rb @@ -24,28 +24,28 @@ RSpec.describe 'Using OAuth from an external app' do subject # It presents the user with an authorization page - expect(page).to have_content(I18n.t('doorkeeper.authorizations.buttons.authorize')) - - # Upon authorizing, it redirects to the apps' callback URL - click_on I18n.t('doorkeeper.authorizations.buttons.authorize') - expect(page).to have_current_path(/\A#{client_app.redirect_uri}/, url: true) + expect(page).to have_content(oauth_authorize_text) # It grants the app access to the account - expect(Doorkeeper::AccessGrant.exists?(application: client_app, resource_owner_id: user.id)).to be true + expect { click_on oauth_authorize_text } + .to change { user_has_grant_with_client_app? }.to(true) + + # Upon authorizing, it redirects to the apps' callback URL + expect(page).to redirect_to_callback_url end it 'when rejecting the authorization request' do subject # It presents the user with an authorization page - expect(page).to have_content(I18n.t('doorkeeper.authorizations.buttons.deny')) - - # Upon denying, it redirects to the apps' callback URL - click_on I18n.t('doorkeeper.authorizations.buttons.deny') - expect(page).to have_current_path(/\A#{client_app.redirect_uri}/, url: true) + expect(page).to have_content(oauth_deny_text) # It does not grant the app access to the account - expect(Doorkeeper::AccessGrant.exists?(application: client_app, resource_owner_id: user.id)).to be false + expect { click_on oauth_deny_text } + .to_not change { user_has_grant_with_client_app? }.from(false) + + # Upon denying, it redirects to the apps' callback URL + expect(page).to redirect_to_callback_url end # The tests in this context ensures that requests without PKCE parameters @@ -133,7 +133,6 @@ RSpec.describe 'Using OAuth from an external app' do end it 'when accepting the authorization request' do - params = { client_id: client_app.uid, response_type: 'code', redirect_uri: client_app.redirect_uri, scope: 'read' } visit "/oauth/authorize?#{params.to_query}" # It presents the user with a log-in page @@ -145,18 +144,17 @@ RSpec.describe 'Using OAuth from an external app' do # Logging in redirects to an authorization page fill_in_auth_details(email, password) - expect(page).to have_content(I18n.t('doorkeeper.authorizations.buttons.authorize')) - - # Upon authorizing, it redirects to the apps' callback URL - click_on I18n.t('doorkeeper.authorizations.buttons.authorize') - expect(page).to have_current_path(/\A#{client_app.redirect_uri}/, url: true) + expect(page).to have_content(oauth_authorize_text) # It grants the app access to the account - expect(Doorkeeper::AccessGrant.exists?(application: client_app, resource_owner_id: user.id)).to be true + expect { click_on oauth_authorize_text } + .to change { user_has_grant_with_client_app? }.to(true) + + # Upon authorizing, it redirects to the apps' callback URL + expect(page).to redirect_to_callback_url end it 'when rejecting the authorization request' do - params = { client_id: client_app.uid, response_type: 'code', redirect_uri: client_app.redirect_uri, scope: 'read' } visit "/oauth/authorize?#{params.to_query}" # It presents the user with a log-in page @@ -168,21 +166,20 @@ RSpec.describe 'Using OAuth from an external app' do # Logging in redirects to an authorization page fill_in_auth_details(email, password) - expect(page).to have_content(I18n.t('doorkeeper.authorizations.buttons.authorize')) - - # Upon denying, it redirects to the apps' callback URL - click_on I18n.t('doorkeeper.authorizations.buttons.deny') - expect(page).to have_current_path(/\A#{client_app.redirect_uri}/, url: true) + expect(page).to have_content(oauth_authorize_text) # It does not grant the app access to the account - expect(Doorkeeper::AccessGrant.exists?(application: client_app, resource_owner_id: user.id)).to be false + expect { click_on oauth_deny_text } + .to_not change { user_has_grant_with_client_app? }.from(false) + + # Upon denying, it redirects to the apps' callback URL + expect(page).to redirect_to_callback_url end context 'when the user has set up TOTP' do let(:user) { Fabricate(:user, email: email, password: password, otp_required_for_login: true, otp_secret: User.generate_otp_secret) } it 'when accepting the authorization request' do - params = { client_id: client_app.uid, response_type: 'code', redirect_uri: client_app.redirect_uri, scope: 'read' } visit "/oauth/authorize?#{params.to_query}" # It presents the user with a log-in page @@ -202,18 +199,17 @@ RSpec.describe 'Using OAuth from an external app' do # Filling in the correct TOTP code redirects to an app authorization page fill_in_otp_details(user.current_otp) - expect(page).to have_content(I18n.t('doorkeeper.authorizations.buttons.authorize')) - - # Upon authorizing, it redirects to the apps' callback URL - click_on I18n.t('doorkeeper.authorizations.buttons.authorize') - expect(page).to have_current_path(/\A#{client_app.redirect_uri}/, url: true) + expect(page).to have_content(oauth_authorize_text) # It grants the app access to the account - expect(Doorkeeper::AccessGrant.exists?(application: client_app, resource_owner_id: user.id)).to be true + expect { click_on oauth_authorize_text } + .to change { user_has_grant_with_client_app? }.to(true) + + # Upon authorizing, it redirects to the apps' callback URL + expect(page).to redirect_to_callback_url end it 'when rejecting the authorization request' do - params = { client_id: client_app.uid, response_type: 'code', redirect_uri: client_app.redirect_uri, scope: 'read' } visit "/oauth/authorize?#{params.to_query}" # It presents the user with a log-in page @@ -233,14 +229,14 @@ RSpec.describe 'Using OAuth from an external app' do # Filling in the correct TOTP code redirects to an app authorization page fill_in_otp_details(user.current_otp) - expect(page).to have_content(I18n.t('doorkeeper.authorizations.buttons.authorize')) - - # Upon denying, it redirects to the apps' callback URL - click_on I18n.t('doorkeeper.authorizations.buttons.deny') - expect(page).to have_current_path(/\A#{client_app.redirect_uri}/, url: true) + expect(page).to have_content(oauth_authorize_text) # It does not grant the app access to the account - expect(Doorkeeper::AccessGrant.exists?(application: client_app, resource_owner_id: user.id)).to be false + expect { click_on oauth_deny_text } + .to_not change { user_has_grant_with_client_app? }.from(false) + + # Upon denying, it redirects to the apps' callback URL + expect(page).to redirect_to_callback_url end end # TODO: external auth @@ -252,4 +248,24 @@ RSpec.describe 'Using OAuth from an external app' do fill_in 'user_otp_attempt', with: value click_on I18n.t('auth.login') end + + def oauth_authorize_text + I18n.t('doorkeeper.authorizations.buttons.authorize') + end + + def oauth_deny_text + I18n.t('doorkeeper.authorizations.buttons.deny') + end + + def redirect_to_callback_url + have_current_path(/\A#{client_app.redirect_uri}/, url: true) + end + + def user_has_grant_with_client_app? + Doorkeeper::AccessGrant + .exists?( + application: client_app, + resource_owner_id: user.id + ) + end end