Fix /api/v1/admin/accounts (#17887)

* Fix /api/v1/admin/accounts

Compatibility was broken since #17009 which changed the underlying filter class
without changing the controller.

This commits restore support for the old parameters.

* Add /api/v2/admin/accounts with the new parameters

* Add tests

* Add missing filter for `silenced` status

Co-authored-by: Eugen Rochko <eugen@zeonfederated.com>

Co-authored-by: Eugen Rochko <eugen@zeonfederated.com>

app/controllers/api/v1/admin/accounts_controller.rb

@@ -104,13 +104,27 @@ class Api::V1::Admin::AccountsController < Api::BaseController
   end
 
   def filtered_accounts
-    AccountFilter.new(filter_params).results
+    AccountFilter.new(translated_filter_params).results
   end
 
   def filter_params
     params.permit(*FILTER_PARAMS)
   end
 
+  def translated_filter_params
+    translated_params = { origin: 'local', status: 'active' }.merge(filter_params.slice(*AccountFilter::KEYS))
+
+    translated_params[:origin] = 'remote' if params[:remote].present?
+
+    %i(active pending disabled silenced suspended).each do |status|
+      translated_params[:status] = status.to_s if params[status].present?
+    end
+
+    translated_params[:permissions] = 'staff' if params[:staff].present?
+
+    translated_params
+  end
+
   def insert_pagination_headers
     set_pagination_headers(next_path, prev_path)
   end

app/controllers/api/v2/admin/accounts_controller.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+class Api::V2::Admin::AccountsController < Api::V1::Admin::AccountsController
+  FILTER_PARAMS = %i(
+    origin
+    status
+    permissions
+    username
+    by_domain
+    display_name
+    email
+    ip
+    invited_by
+  ).freeze
+
+  PAGINATION_PARAMS = (%i(limit) + FILTER_PARAMS).freeze
+
+  private
+
+  def filtered_accounts
+    AccountFilter.new(filter_params).results
+  end
+
+  def filter_params
+    params.permit(*FILTER_PARAMS)
+  end
+
+  def pagination_params(core_params)
+    params.slice(*PAGINATION_PARAMS).permit(*PAGINATION_PARAMS).merge(core_params)
+  end
+end

app/models/account_filter.rb

@@ -80,6 +80,10 @@ class AccountFilter
       accounts_with_users.merge(User.pending)
     when 'suspended'
       Account.suspended
+    when 'disabled'
+      accounts_with_users.merge(User.disabled)
+    when 'silenced'
+      Account.silenced
     else
       raise "Unknown status: #{value}"
     end

config/routes.rb

@@ -576,6 +576,10 @@ Rails.application.routes.draw do
       resources :media, only: [:create]
       get '/search', to: 'search#index', as: :search
       resources :suggestions, only: [:index]
+
+      namespace :admin do
+        resources :accounts, only: [:index]
+      end
     end
 
     namespace :web do

spec/controllers/api/v1/admin/accounts_controller_spec.rb

@@ -30,16 +30,45 @@ RSpec.describe Api::V1::Admin::AccountsController, type: :controller do
   end
 
   describe 'GET #index' do
+    let!(:remote_account)       { Fabricate(:account, domain: 'example.org') }
+    let!(:other_remote_account) { Fabricate(:account, domain: 'foo.bar') }
+    let!(:suspended_account)    { Fabricate(:account, suspended: true) }
+    let!(:suspended_remote)     { Fabricate(:account, domain: 'foo.bar', suspended: true) }
+    let!(:disabled_account)     { Fabricate(:user, disabled: true).account }
+    let!(:pending_account)      { Fabricate(:user, approved: false).account }
+    let!(:admin_account)        { user.account }
+
+    let(:params) { {} }
+
     before do
-      get :index
+      pending_account.user.update(approved: false)
+      get :index, params: params
     end
 
     it_behaves_like 'forbidden for wrong scope', 'write:statuses'
     it_behaves_like 'forbidden for wrong role', 'user'
 
+    [
+      [{ active: 'true', local: 'true', staff: 'true' }, [:admin_account]],
+      [{ by_domain: 'example.org', remote: 'true' }, [:remote_account]],
+      [{ suspended: 'true' }, [:suspended_account]],
+      [{ disabled: 'true' }, [:disabled_account]],
+      [{ pending: 'true' }, [:pending_account]],
+    ].each do |params, expected_results|
+      context "when called with #{params.inspect}" do
+        let(:params) { params }
+
         it 'returns http success' do
           expect(response).to have_http_status(200)
         end
+
+        it "returns the correct accounts (#{expected_results.inspect})" do
+          json = body_as_json
+
+          expect(json.map { |a| a[:id].to_i }).to eq (expected_results.map { |symbol| send(symbol).id })
+        end
+      end
+    end
   end
 
   describe 'GET #show' do

spec/controllers/api/v2/admin/accounts_controller_spec.rb

@@ -0,0 +1,73 @@
+require 'rails_helper'
+
+RSpec.describe Api::V2::Admin::AccountsController, type: :controller do
+  render_views
+
+  let(:role)   { 'moderator' }
+  let(:user)   { Fabricate(:user, role: role) }
+  let(:scopes) { 'admin:read admin:write' }
+  let(:token)  { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) }
+  let(:account) { Fabricate(:account) }
+
+  before do
+    allow(controller).to receive(:doorkeeper_token) { token }
+  end
+
+  shared_examples 'forbidden for wrong scope' do |wrong_scope|
+    let(:scopes) { wrong_scope }
+
+    it 'returns http forbidden' do
+      expect(response).to have_http_status(403)
+    end
+  end
+
+  shared_examples 'forbidden for wrong role' do |wrong_role|
+    let(:role) { wrong_role }
+
+    it 'returns http forbidden' do
+      expect(response).to have_http_status(403)
+    end
+  end
+
+  describe 'GET #index' do
+    let!(:remote_account)       { Fabricate(:account, domain: 'example.org') }
+    let!(:other_remote_account) { Fabricate(:account, domain: 'foo.bar') }
+    let!(:suspended_account)    { Fabricate(:account, suspended: true) }
+    let!(:suspended_remote)     { Fabricate(:account, domain: 'foo.bar', suspended: true) }
+    let!(:disabled_account)     { Fabricate(:user, disabled: true).account }
+    let!(:pending_account)      { Fabricate(:user, approved: false).account }
+    let!(:admin_account)        { user.account }
+
+    let(:params) { {} }
+
+    before do
+      pending_account.user.update(approved: false)
+      get :index, params: params
+    end
+
+    it_behaves_like 'forbidden for wrong scope', 'write:statuses'
+    it_behaves_like 'forbidden for wrong role', 'user'
+
+    [
+      [{ status: 'active', origin: 'local', permissions: 'staff' }, [:admin_account]],
+      [{ by_domain: 'example.org', origin: 'remote' }, [:remote_account]],
+      [{ status: 'suspended' }, [:suspended_remote, :suspended_account]],
+      [{ status: 'disabled' }, [:disabled_account]],
+      [{ status: 'pending' }, [:pending_account]],
+    ].each do |params, expected_results|
+      context "when called with #{params.inspect}" do
+        let(:params) { params }
+
+        it 'returns http success' do
+          expect(response).to have_http_status(200)
+        end
+
+        it "returns the correct accounts (#{expected_results.inspect})" do
+          json = body_as_json
+
+          expect(json.map { |a| a[:id].to_i }).to eq (expected_results.map { |symbol| send(symbol).id })
+        end
+      end
+    end
+  end
+end