Merge pull request #786 from kmycode/upstream-20240731

Upstream 20240731
2024-08-01 07:24:31 +09:00 · 2024-08-01 07:24:31 +09:00 · 9e1c63aa2c
commit 9e1c63aa2c
parent 0c37295624 8b09a57a91
320 changed files with 3132 additions and 1643 deletions
--- a/spec/controllers/admin/tags_controller_spec.rb
+++ b/spec/controllers/admin/tags_controller_spec.rb
@ -9,6 +9,43 @@ RSpec.describe Admin::TagsController do
    sign_in Fabricate(:user, role: UserRole.find_by(name: 'Admin'))
  end

+  describe 'GET #index' do
+    before do
+      Fabricate(:tag)
+
+      tag_filter = instance_double(Admin::TagFilter, results: Tag.all)
+      allow(Admin::TagFilter).to receive(:new).and_return(tag_filter)
+    end
+
+    let(:params) { { order: 'newest' } }
+
+    it 'returns http success' do
+      get :index
+
+      expect(response).to have_http_status(200)
+      expect(response).to render_template(:index)
+
+      expect(Admin::TagFilter)
+        .to have_received(:new)
+        .with(hash_including(params))
+    end
+
+    describe 'with filters' do
+      let(:params) { { order: 'newest', name: 'test' } }
+
+      it 'returns http success' do
+        get :index, params: { name: 'test' }
+
+        expect(response).to have_http_status(200)
+        expect(response).to render_template(:index)
+
+        expect(Admin::TagFilter)
+          .to have_received(:new)
+          .with(hash_including(params))
+      end
+    end
+  end
+
  describe 'GET #show' do
    let!(:tag) { Fabricate(:tag) }

--- a/spec/controllers/oauth/authorizations_controller_spec.rb
+++ b/spec/controllers/oauth/authorizations_controller_spec.rb
@ -3,8 +3,6 @@
 require 'rails_helper'

 RSpec.describe Oauth::AuthorizationsController do
-  render_views
-
  let(:app) { Doorkeeper::Application.create!(name: 'test', redirect_uri: 'http://localhost/', scopes: 'read') }

  describe 'GET #new' do
@ -36,11 +34,6 @@ RSpec.describe Oauth::AuthorizationsController do
        expect(response.headers['Cache-Control']).to include('private, no-store')
      end

-      it 'gives options to authorize and deny' do
-        subject
-        expect(response.body).to match(/Authorize/)
-      end
-
      include_examples 'stores location for user'

      context 'when app is already authorized' do
@ -61,7 +54,8 @@ RSpec.describe Oauth::AuthorizationsController do

        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' }
-          expect(response.body).to match(/Authorize/)
+
+          expect(response).to have_http_status(:success)
        end
      end
    end
--- a/spec/lib/link_details_extractor_spec.rb
+++ b/spec/lib/link_details_extractor_spec.rb
@ -129,6 +129,24 @@ RSpec.describe LinkDetailsExtractor do
      include_examples 'structured data'
    end

+    context 'with the first tag is null' do
+      let(:html) { <<~HTML }
+        <!doctype html>
+        <html>
+        <body>
+          <script type="application/ld+json">
+            null
+          </script>
+          <script type="application/ld+json">
+            #{ld_json}
+          </script>
+        </body>
+        </html>
+      HTML
+
+      include_examples 'structured data'
+    end
+
    context 'with preceding block of unsupported LD+JSON' do
      let(:html) { <<~HTML }
        <!doctype html>
--- a/spec/lib/text_formatter_spec.rb
+++ b/spec/lib/text_formatter_spec.rb
@ -224,6 +224,14 @@ RSpec.describe TextFormatter do
      end
    end

+    context 'when given a URL with trailing @ symbol' do
+      let(:text) { 'https://gta.fandom.com/wiki/TW@ Content' }
+
+      it 'matches the full URL' do
+        expect(subject).to include 'href="https://gta.fandom.com/wiki/TW@"'
+      end
+    end
+
    context 'when given a URL containing unsafe code (XSS attack, visible part)' do
      let(:text) { 'http://example.com/b<del>b</del>' }

--- a/spec/models/account_spec.rb
+++ b/spec/models/account_spec.rb
@ -934,6 +934,14 @@ RSpec.describe Account do
    it 'does not match URL query string' do
      expect(subject.match('https://example.com/?x=@alice')).to be_nil
    end
+
+    it 'matches usernames immediately following the letter ß' do
+      expect(subject.match('Hello toß @alice from me')[1]).to eq 'alice'
+    end
+
+    it 'matches usernames containing uppercase characters' do
+      expect(subject.match('Hello to @aLice@Example.com from me')[1]).to eq 'aLice@Example.com'
+    end
  end

  describe 'validations' do
--- a/spec/models/admin/tag_filter_spec.rb
+++ b/spec/models/admin/tag_filter_spec.rb
@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Admin::TagFilter do
+  describe 'with invalid params' do
+    it 'raises with key error' do
+      filter = described_class.new(wrong: true)
+
+      expect { filter.results }.to raise_error(/wrong/)
+    end
+
+    it 'raises with status scope error' do
+      filter = described_class.new(status: 'unknown')
+
+      expect { filter.results }.to raise_error(/Unknown status: unknown/)
+    end
+
+    it 'raises with order value error' do
+      filter = described_class.new(order: 'unknown')
+
+      expect { filter.results }.to raise_error(/Unknown order: unknown/)
+    end
+  end
+
+  describe '#results' do
+    let(:listable_tag) { Fabricate(:tag, name: 'test1', listable: true) }
+    let(:not_listable_tag) { Fabricate(:tag, name: 'test2', listable: false) }
+
+    it 'returns tags filtered by name' do
+      filter = described_class.new(name: 'test')
+
+      expect(filter.results).to eq([listable_tag, not_listable_tag])
+    end
+  end
+end
--- a/spec/models/tag_spec.rb
+++ b/spec/models/tag_spec.rb
@ -95,6 +95,14 @@ RSpec.describe Tag do
    it 'does not match purely-numeric hashtags' do
      expect(subject.match('hello #0123456')).to be_nil
    end
+
+    it 'matches hashtags immediately following the letter ß' do
+      expect(subject.match('Hello toß #ruby').to_s).to eq '#ruby'
+    end
+
+    it 'matches hashtags containing uppercase characters' do
+      expect(subject.match('Hello #rubyOnRails').to_s).to eq '#rubyOnRails'
+    end
  end

  describe '#to_param' do
@ -104,6 +112,18 @@ RSpec.describe Tag do
    end
  end

+  describe '#formatted_name' do
+    it 'returns name with a proceeding hash symbol' do
+      tag = Fabricate(:tag, name: 'foo')
+      expect(tag.formatted_name).to eq '#foo'
+    end
+
+    it 'returns display_name with a proceeding hash symbol, if display name present' do
+      tag = Fabricate(:tag, name: 'foobar', display_name: 'FooBar')
+      expect(tag.formatted_name).to eq '#FooBar'
+    end
+  end
+
  describe '.recently_used' do
    let(:account) { Fabricate(:account) }
    let(:other_person_status) { Fabricate(:status) }
@ -232,5 +252,23 @@ RSpec.describe Tag do

      expect(results).to eq [tag, similar_tag]
    end
+
+    it 'finds only listable tags' do
+      tag = Fabricate(:tag, name: 'match')
+      _miss_tag = Fabricate(:tag, name: 'matchunlisted', listable: false)
+
+      results = described_class.search_for('match')
+
+      expect(results).to eq [tag]
+    end
+
+    it 'finds non-listable tags as well via option' do
+      tag = Fabricate(:tag, name: 'match')
+      unlisted_tag = Fabricate(:tag, name: 'matchunlisted', listable: false)
+
+      results = described_class.search_for('match', 5, 0, exclude_unlistable: false)
+
+      expect(results).to eq [tag, unlisted_tag]
+    end
  end
 end
--- a/spec/requests/api/v1/notifications_spec.rb
+++ b/spec/requests/api/v1/notifications_spec.rb
@ -8,6 +8,83 @@ RSpec.describe 'Notifications' do
  let(:scopes)  { 'read:notifications write:notifications' }
  let(:headers) { { 'Authorization' => "Bearer #{token.token}" } }

+  describe 'GET /api/v1/notifications/unread_count', :inline_jobs do
+    subject do
+      get '/api/v1/notifications/unread_count', headers: headers, params: params
+    end
+
+    let(:params) { {} }
+
+    before do
+      first_status = PostStatusService.new.call(user.account, text: 'Test')
+      ReblogService.new.call(Fabricate(:account), first_status)
+      PostStatusService.new.call(Fabricate(:account), text: 'Hello @alice')
+      FavouriteService.new.call(Fabricate(:account), first_status)
+      FavouriteService.new.call(Fabricate(:account), first_status)
+      FollowService.new.call(Fabricate(:account), user.account)
+    end
+
+    it_behaves_like 'forbidden for wrong scope', 'write write:notifications'
+
+    context 'with no options' do
+      it 'returns expected notifications count' do
+        subject
+
+        expect(response).to have_http_status(200)
+        expect(body_as_json[:count]).to eq 5
+      end
+    end
+
+    context 'with a read marker' do
+      before do
+        id = user.account.notifications.browserable.order(id: :desc).offset(2).first.id
+        user.markers.create!(timeline: 'notifications', last_read_id: id)
+      end
+
+      it 'returns expected notifications count' do
+        subject
+
+        expect(response).to have_http_status(200)
+        expect(body_as_json[:count]).to eq 2
+      end
+    end
+
+    context 'with exclude_types param' do
+      let(:params) { { exclude_types: %w(mention) } }
+
+      it 'returns expected notifications count' do
+        subject
+
+        expect(response).to have_http_status(200)
+        expect(body_as_json[:count]).to eq 4
+      end
+    end
+
+    context 'with a user-provided limit' do
+      let(:params) { { limit: 2 } }
+
+      it 'returns a capped value' do
+        subject
+
+        expect(response).to have_http_status(200)
+        expect(body_as_json[:count]).to eq 2
+      end
+    end
+
+    context 'when there are more notifications than the limit' do
+      before do
+        stub_const('Api::V1::NotificationsController::DEFAULT_NOTIFICATIONS_COUNT_LIMIT', 2)
+      end
+
+      it 'returns a capped value' do
+        subject
+
+        expect(response).to have_http_status(200)
+        expect(body_as_json[:count]).to eq Api::V1::NotificationsController::DEFAULT_NOTIFICATIONS_COUNT_LIMIT
+      end
+    end
+  end
+
  describe 'GET /api/v1/notifications', :inline_jobs do
    subject do
      get '/api/v1/notifications', headers: headers, params: params
--- a/spec/requests/api/v2_alpha/notifications_spec.rb
+++ b/spec/requests/api/v2_alpha/notifications_spec.rb
@ -8,6 +8,83 @@ RSpec.describe 'Notifications' do
  let(:scopes)  { 'read:notifications write:notifications' }
  let(:headers) { { 'Authorization' => "Bearer #{token.token}" } }

+  describe 'GET /api/v2_alpha/notifications/unread_count', :inline_jobs do
+    subject do
+      get '/api/v2_alpha/notifications/unread_count', headers: headers, params: params
+    end
+
+    let(:params) { {} }
+
+    before do
+      first_status = PostStatusService.new.call(user.account, text: 'Test')
+      ReblogService.new.call(Fabricate(:account), first_status)
+      PostStatusService.new.call(Fabricate(:account), text: 'Hello @alice')
+      FavouriteService.new.call(Fabricate(:account), first_status)
+      FavouriteService.new.call(Fabricate(:account), first_status)
+      FollowService.new.call(Fabricate(:account), user.account)
+    end
+
+    it_behaves_like 'forbidden for wrong scope', 'write write:notifications'
+
+    context 'with no options' do
+      it 'returns expected notifications count' do
+        subject
+
+        expect(response).to have_http_status(200)
+        expect(body_as_json[:count]).to eq 4
+      end
+    end
+
+    context 'with a read marker' do
+      before do
+        id = user.account.notifications.browserable.order(id: :desc).offset(2).first.id
+        user.markers.create!(timeline: 'notifications', last_read_id: id)
+      end
+
+      it 'returns expected notifications count' do
+        subject
+
+        expect(response).to have_http_status(200)
+        expect(body_as_json[:count]).to eq 2
+      end
+    end
+
+    context 'with exclude_types param' do
+      let(:params) { { exclude_types: %w(mention) } }
+
+      it 'returns expected notifications count' do
+        subject
+
+        expect(response).to have_http_status(200)
+        expect(body_as_json[:count]).to eq 3
+      end
+    end
+
+    context 'with a user-provided limit' do
+      let(:params) { { limit: 2 } }
+
+      it 'returns a capped value' do
+        subject
+
+        expect(response).to have_http_status(200)
+        expect(body_as_json[:count]).to eq 2
+      end
+    end
+
+    context 'when there are more notifications than the limit' do
+      before do
+        stub_const('Api::V2Alpha::NotificationsController::DEFAULT_NOTIFICATIONS_COUNT_LIMIT', 2)
+      end
+
+      it 'returns a capped value' do
+        subject
+
+        expect(response).to have_http_status(200)
+        expect(body_as_json[:count]).to eq Api::V2Alpha::NotificationsController::DEFAULT_NOTIFICATIONS_COUNT_LIMIT
+      end
+    end
+  end
+
  describe 'GET /api/v2_alpha/notifications', :inline_jobs do
    subject do
      get '/api/v2_alpha/notifications', headers: headers, params: params
--- a/spec/requests/well_known/oauth_metadata_spec.rb
+++ b/spec/requests/well_known/oauth_metadata_spec.rb
@ -29,7 +29,10 @@ describe 'The /.well-known/oauth-authorization-server request' do
      revocation_endpoint: oauth_revoke_url(protocol: protocol),
      scopes_supported: Doorkeeper.configuration.scopes.map(&:to_s),
      response_types_supported: Doorkeeper.configuration.authorization_response_types,
+      response_modes_supported: Doorkeeper.configuration.authorization_response_flows.flat_map(&:response_mode_matches).uniq,
+      token_endpoint_auth_methods_supported: %w(client_secret_basic client_secret_post),
      grant_types_supported: grant_types_supported,
+      code_challenge_methods_supported: ['S256'],
      # non-standard extension:
      app_registration_endpoint: api_v1_apps_url(protocol: protocol)
    )
--- a/spec/routing/accounts_routing_spec.rb
+++ b/spec/routing/accounts_routing_spec.rb
@ -47,6 +47,61 @@ describe 'Routes under accounts/' do
    end
  end

+  context 'with local username encoded at' do
+    include RSpec::Rails::RequestExampleGroup
+    let(:username) { 'alice' }
+
+    it 'routes /%40:username' do
+      get "/%40#{username}"
+      expect(response).to redirect_to("/@#{username}")
+    end
+
+    it 'routes /%40:username.json' do
+      get("/%40#{username}.json")
+      expect(response).to redirect_to("/@#{username}.json")
+    end
+
+    it 'routes /%40:username.rss' do
+      get("/%40#{username}.rss")
+      expect(response).to redirect_to("/@#{username}.rss")
+    end
+
+    it 'routes /%40:username/:id' do
+      get("/%40#{username}/123")
+      expect(response).to redirect_to("/@#{username}/123")
+    end
+
+    it 'routes /%40:username/:id/embed' do
+      get("/%40#{username}/123/embed")
+      expect(response).to redirect_to("/@#{username}/123/embed")
+    end
+
+    it 'routes /%40:username/following' do
+      get("/%40#{username}/following")
+      expect(response).to redirect_to("/@#{username}/following")
+    end
+
+    it 'routes /%40:username/followers' do
+      get("/%40#{username}/followers")
+      expect(response).to redirect_to("/@#{username}/followers")
+    end
+
+    it 'routes /%40:username/with_replies' do
+      get("/%40#{username}/with_replies")
+      expect(response).to redirect_to("/@#{username}/with_replies")
+    end
+
+    it 'routes /%40:username/media' do
+      get("/%40#{username}/media")
+      expect(response).to redirect_to("/@#{username}/media")
+    end
+
+    it 'routes /%40:username/tagged/:tag' do
+      get("/%40#{username}/tagged/foo")
+      expect(response).to redirect_to("/@#{username}/tagged/foo")
+    end
+  end
+
  context 'with remote username' do
    let(:username) { 'alice@example.com' }

@ -82,4 +137,50 @@ describe 'Routes under accounts/' do
      expect(get("/@#{username}/tagged/foo")).to route_to('home#index', username_with_domain: username, any: 'tagged/foo')
    end
  end
+
+  context 'with remote username encoded at' do
+    include RSpec::Rails::RequestExampleGroup
+    let(:username) { 'alice%40example.com' }
+    let(:username_decoded) { 'alice@example.com' }
+
+    it 'routes /%40:username' do
+      get("/%40#{username}")
+      expect(response).to redirect_to("/@#{username_decoded}")
+    end
+
+    it 'routes /%40:username/:id' do
+      get("/%40#{username}/123")
+      expect(response).to redirect_to("/@#{username_decoded}/123")
+    end
+
+    it 'routes /%40:username/:id/embed' do
+      get("/%40#{username}/123/embed")
+      expect(response).to redirect_to("/@#{username_decoded}/123/embed")
+    end
+
+    it 'routes /%40:username/following' do
+      get("/%40#{username}/following")
+      expect(response).to redirect_to("/@#{username_decoded}/following")
+    end
+
+    it 'routes /%40:username/followers' do
+      get("/%40#{username}/followers")
+      expect(response).to redirect_to("/@#{username_decoded}/followers")
+    end
+
+    it 'routes /%40:username/with_replies' do
+      get("/%40#{username}/with_replies")
+      expect(response).to redirect_to("/@#{username_decoded}/with_replies")
+    end
+
+    it 'routes /%40:username/media' do
+      get("/%40#{username}/media")
+      expect(response).to redirect_to("/@#{username_decoded}/media")
+    end
+
+    it 'routes /%40:username/tagged/:tag' do
+      get("/%40#{username}/tagged/foo")
+      expect(response).to redirect_to("/@#{username_decoded}/tagged/foo")
+    end
+  end
 end
--- a/spec/support/matchers/api_pagination.rb
+++ b/spec/support/matchers/api_pagination.rb
@ -7,7 +7,7 @@ RSpec::Matchers.define :include_pagination_headers do |links|
    end.all?
  end

-  failure_message do |header|
-    "expected that #{header} would have the same values as #{links}."
+  failure_message do |response|
+    "expected that #{response.headers['Link']} would have the same values as #{links}."
  end
 end
--- a/spec/support/stories/profile_stories.rb
+++ b/spec/support/stories/profile_stories.rb
@ -3,6 +3,12 @@
 module ProfileStories
  attr_reader :bob, :alice, :alice_bio

+  def fill_in_auth_details(email, password)
+    fill_in 'user_email', with: email
+    fill_in 'user_password', with: password
+    click_on I18n.t('auth.login')
+  end
+
  def as_a_registered_user
    @bob = Fabricate(
      :user,
@ -17,9 +23,7 @@ module ProfileStories
  def as_a_logged_in_user
    as_a_registered_user
    visit new_user_session_path
-    fill_in 'user_email', with: email
-    fill_in 'user_password', with: password
-    click_on I18n.t('auth.login')
+    fill_in_auth_details(email, password)
  end

  def as_a_logged_in_admin
--- a/spec/system/log_in_spec.rb
+++ b/spec/system/log_in_spec.rb
@ -17,17 +17,13 @@ describe 'Log in' do
  end

  it 'A valid email and password user is able to log in' do
-    fill_in 'user_email', with: email
-    fill_in 'user_password', with: password
-    click_on I18n.t('auth.login')
+    fill_in_auth_details(email, password)

    expect(subject).to have_css('div.app-holder')
  end

  it 'A invalid email and password user is not able to log in' do
-    fill_in 'user_email', with: 'invalid_email'
-    fill_in 'user_password', with: 'invalid_password'
-    click_on I18n.t('auth.login')
+    fill_in_auth_details('invalid_email', 'invalid_password')

    expect(subject).to have_css('.flash-message', text: failure_message('invalid'))
  end
@ -36,9 +32,7 @@ describe 'Log in' do
    let(:confirmed_at) { nil }

    it 'A unconfirmed user is able to log in' do
-      fill_in 'user_email', with: email
-      fill_in 'user_password', with: password
-      click_on I18n.t('auth.login')
+      fill_in_auth_details(email, password)

      expect(subject).to have_css('div.admin-wrapper')
    end
--- a/spec/system/oauth_spec.rb
+++ b/spec/system/oauth_spec.rb
@ -2,8 +2,15 @@

 require 'rails_helper'

-describe 'Using OAuth from an external app', :js, :streaming do
+describe 'Using OAuth from an external app' do
+  include ProfileStories
+
+  subject { visit "/oauth/authorize?#{params.to_query}" }
+
  let(:client_app) { Doorkeeper::Application.create!(name: 'test', redirect_uri: about_url(host: Rails.application.config.x.local_domain), scopes: 'read') }
+  let(:params) do
+    { client_id: client_app.uid, response_type: 'code', redirect_uri: client_app.redirect_uri, scope: 'read' }
+  end

  context 'when the user is already logged in' do
    let!(:user) { Fabricate(:user) }
@ -14,8 +21,7 @@ describe 'Using OAuth from an external app', :js, :streaming 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}"
+      subject

      # It presents the user with an authorization page
      expect(page).to have_content(I18n.t('doorkeeper.authorizations.buttons.authorize'))
@ -29,8 +35,7 @@ describe 'Using OAuth from an external app', :js, :streaming do
    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}"
+      subject

      # It presents the user with an authorization page
      expect(page).to have_content(I18n.t('doorkeeper.authorizations.buttons.deny'))
@ -42,6 +47,79 @@ describe 'Using OAuth from an external app', :js, :streaming do
      # 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
    end
+
+    # The tests in this context ensures that requests without PKCE parameters
+    # still work; In the future we likely want to force usage of PKCE for
+    # security reasons, as per:
+    #
+    # https://www.ietf.org/archive/id/draft-ietf-oauth-security-topics-27.html#section-2.1.1-9
+    context 'when not using PKCE' do
+      it 'does not include the PKCE values in the hidden inputs' do
+        subject
+
+        code_challenge_inputs = all('.oauth-prompt input[name=code_challenge]', visible: false)
+        code_challenge_method_inputs = all('.oauth-prompt input[name=code_challenge_method]', visible: false)
+
+        expect(code_challenge_inputs).to_not be_empty
+        expect(code_challenge_method_inputs).to_not be_empty
+
+        (code_challenge_inputs.to_a + code_challenge_method_inputs.to_a).each do |input|
+          expect(input.value).to be_nil
+        end
+      end
+    end
+
+    context 'when using PKCE' do
+      let(:params) do
+        { client_id: client_app.uid, response_type: 'code', redirect_uri: client_app.redirect_uri, scope: 'read', code_challenge_method: pkce_code_challenge_method, code_challenge: pkce_code_challenge }
+      end
+      let(:pkce_code_challenge) { SecureRandom.hex(32) }
+      let(:pkce_code_challenge_method) { 'S256' }
+
+      context 'when using S256 code challenge method' do
+        it 'includes the PKCE values in the hidden inputs' do
+          subject
+
+          code_challenge_inputs = all('.oauth-prompt input[name=code_challenge]', visible: false)
+          code_challenge_method_inputs = all('.oauth-prompt input[name=code_challenge_method]', visible: false)
+
+          expect(code_challenge_inputs).to_not be_empty
+          expect(code_challenge_method_inputs).to_not be_empty
+
+          code_challenge_inputs.each do |input|
+            expect(input.value).to eq pkce_code_challenge
+          end
+          code_challenge_method_inputs.each do |input|
+            expect(input.value).to eq pkce_code_challenge_method
+          end
+        end
+      end
+
+      context 'when using plain code challenge method' do
+        let(:pkce_code_challenge_method) { 'plain' }
+
+        it 'does not include the PKCE values in the response' do
+          subject
+
+          expect(page).to have_no_css('.oauth-prompt input[name=code_challenge]')
+          expect(page).to have_no_css('.oauth-prompt input[name=code_challenge_method]')
+        end
+
+        it 'does not include the authorize button' do
+          subject
+
+          expect(page).to have_no_css('.oauth-prompt button[type="submit"]')
+        end
+
+        it 'includes an error message' do
+          subject
+
+          within '.form-container .flash-message' do
+            expect(page).to have_content(I18n.t('doorkeeper.errors.messages.invalid_code_challenge_method'))
+          end
+        end
+      end
+    end
  end

  context 'when the user is not already logged in' do
@ -170,12 +248,6 @@ describe 'Using OAuth from an external app', :js, :streaming do

  private

-  def fill_in_auth_details(email, password)
-    fill_in 'user_email', with: email
-    fill_in 'user_password', with: password
-    click_on I18n.t('auth.login')
-  end
-
  def fill_in_otp_details(value)
    fill_in 'user_otp_attempt', with: value
    click_on I18n.t('auth.login')
--- a/spec/workers/scheduler/user_cleanup_scheduler_spec.rb
+++ b/spec/workers/scheduler/user_cleanup_scheduler_spec.rb
@ -12,29 +12,31 @@ describe Scheduler::UserCleanupScheduler do

  describe '#perform' do
    before do
-      # Need to update the already-existing users because their initialization overrides confirmation_sent_at
+      # Update already-existing users because initialization overrides `confirmation_sent_at`
      new_unconfirmed_user.update!(confirmed_at: nil, confirmation_sent_at: Time.now.utc)
      old_unconfirmed_user.update!(confirmed_at: nil, confirmation_sent_at: 10.days.ago)
      confirmed_user.update!(confirmed_at: 1.day.ago)
    end

-    it 'deletes the old unconfirmed user, their account, and the moderation note' do
+    it 'deletes the old unconfirmed user and metadata while preserving confirmed user and newer unconfirmed user' do
      expect { subject.perform }
-        .to change { User.exists?(old_unconfirmed_user.id) }.from(true).to(false)
-        .and change { Account.exists?(old_unconfirmed_user.account_id) }.from(true).to(false)
-      expect { moderation_note.reload }.to raise_error(ActiveRecord::RecordNotFound)
+        .to change { User.exists?(old_unconfirmed_user.id) }
+        .from(true).to(false)
+        .and change { Account.exists?(old_unconfirmed_user.account_id) }
+        .from(true).to(false)
+      expect { moderation_note.reload }
+        .to raise_error(ActiveRecord::RecordNotFound)
+      expect_preservation_of(new_unconfirmed_user)
+      expect_preservation_of(confirmed_user)
    end

-    it 'does not delete the new unconfirmed user or their account' do
-      subject.perform
-      expect(User.exists?(new_unconfirmed_user.id)).to be true
-      expect(Account.exists?(new_unconfirmed_user.account_id)).to be true
-    end
+    private

-    it 'does not delete the confirmed user or their account' do
-      subject.perform
-      expect(User.exists?(confirmed_user.id)).to be true
-      expect(Account.exists?(confirmed_user.account_id)).to be true
+    def expect_preservation_of(user)
+      expect(User.exists?(user.id))
+        .to be true
+      expect(Account.exists?(user.account_id))
+        .to be true
    end
  end
 end