From 5024593550603bcb2f2a304a84b6614ba46738ae Mon Sep 17 00:00:00 2001 From: Jamie Rodriguez Date: Tue, 16 Jul 2024 15:49:25 +0200 Subject: [PATCH] WIP --- .../AvailableSoftwareUpdates.jsx | 7 +- .../AvailableSoftwareUpdates.test.jsx | 7 +- .../js/pages/HostDetailsPage/HostDetails.jsx | 4 +- .../pages/HostDetailsPage/HostDetailsPage.jsx | 20 +- assets/js/state/sagas/softwareUpdates.js | 15 + assets/js/state/sagas/softwareUpdates.test.js | 56 ++++ assets/js/state/selectors/softwareUpdates.js | 5 + assets/js/state/softwareUpdates.js | 9 + .../infrastructure/software_updates/suma.ex | 6 +- .../software_updates/suma_api.ex | 23 +- .../controllers/fallback_controller.ex | 7 + .../v1/suse_manager_controller_test.exs | 286 ++++++++---------- 12 files changed, 247 insertions(+), 198 deletions(-) diff --git a/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.jsx b/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.jsx index 81407781af..1d6f1ba59f 100644 --- a/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.jsx +++ b/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.jsx @@ -21,8 +21,7 @@ const containerClassNames = classNames( function AvailableSoftwareUpdates({ className, settingsConfigured = false, - softwareUpdatesSettingsLoading, - softwareUpdatesLoading, + loading, relevantPatches, upgradablePackages, tooltip, @@ -30,7 +29,7 @@ function AvailableSoftwareUpdates({ onNavigateToPatches = noop, onNavigateToPackages = noop, }) { - if (softwareUpdatesSettingsLoading) { + if (loading) { return ; } @@ -60,7 +59,6 @@ function AvailableSoftwareUpdates({ title="Relevant Patches" critical={gt(relevantPatches, 0)} tooltip={tooltip} - loading={softwareUpdatesLoading} icon={} onNavigate={onNavigateToPatches} > @@ -70,7 +68,6 @@ function AvailableSoftwareUpdates({ } onNavigate={onNavigateToPackages} > diff --git a/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.test.jsx b/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.test.jsx index 0a5363aac6..891c774aa8 100644 --- a/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.test.jsx +++ b/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.test.jsx @@ -54,12 +54,7 @@ describe('AvailableSoftwareUpdates component', () => { }); it('renders Software Updates Settings Loading status', () => { - render( - - ); + render(); expect(screen.getAllByLabelText('Loading')).toHaveLength(1); }); diff --git a/assets/js/pages/HostDetailsPage/HostDetails.jsx b/assets/js/pages/HostDetailsPage/HostDetails.jsx index a0e12d642f..408cf56de7 100644 --- a/assets/js/pages/HostDetailsPage/HostDetails.jsx +++ b/assets/js/pages/HostDetailsPage/HostDetails.jsx @@ -73,7 +73,6 @@ function HostDetails({ upgradablePackages, softwareUpdatesLoading, softwareUpdatesSettingsSaved, - softwareUpdatesSettingsLoading, softwareUpdatesTooltip, userAbilities, cleanUpHost, @@ -245,8 +244,7 @@ function HostDetails({ relevantPatches={relevantPatches} upgradablePackages={upgradablePackages} tooltip={softwareUpdatesTooltip} - softwareUpdatesSettingsLoading={softwareUpdatesSettingsLoading} - softwareUpdatesLoading={softwareUpdatesLoading} + loading={softwareUpdatesLoading} onBackToSettings={() => navigate(`/settings`)} onNavigateToPatches={() => navigate(`/hosts/${hostID}/patches`)} onNavigateToPackages={() => navigate(`/hosts/${hostID}/packages`)} diff --git a/assets/js/pages/HostDetailsPage/HostDetailsPage.jsx b/assets/js/pages/HostDetailsPage/HostDetailsPage.jsx index c44e762e79..ed906e777d 100644 --- a/assets/js/pages/HostDetailsPage/HostDetailsPage.jsx +++ b/assets/js/pages/HostDetailsPage/HostDetailsPage.jsx @@ -14,10 +14,7 @@ import { getInstancesOnHost } from '@state/selectors/sapSystem'; import { getCatalog } from '@state/selectors/catalog'; import { getLastExecution } from '@state/selectors/lastExecutions'; import { - getSoftwareUpdatesSettingsLoading, - getSoftwareUpdatesSettingsSaved, -} from '@state/selectors/softwareUpdatesSettings'; -import { + getSoftwareUpdatesSettingsConfigured, getSoftwareUpdatesLoading, getSoftwareUpdatesStats, } from '@state/selectors/softwareUpdates'; @@ -31,7 +28,6 @@ import { } from '@state/lastExecutions'; import { deregisterHost } from '@state/hosts'; -import { fetchSoftwareUpdatesSettings } from '@state/softwareUpdatesSettings'; import { fetchSoftwareUpdates } from '@state/softwareUpdates'; import HostDetails from './HostDetails'; @@ -60,12 +56,10 @@ function HostDetailsPage() { const [exportersStatus, setExportersStatus] = useState([]); - const softwareUpdatesSettingsLoading = useSelector((state) => - getSoftwareUpdatesSettingsLoading(state) - ); - const softwareUpdatesConnectionSaved = useSelector((state) => - getSoftwareUpdatesSettingsSaved(state) + const settingsConfigured = useSelector((state) => + getSoftwareUpdatesSettingsConfigured(state) ); + const { numRelevantPatches, numUpgradablePackages } = useSelector((state) => getSoftwareUpdatesStats(state, hostID) ); @@ -90,11 +84,10 @@ function HostDetailsPage() { ); useEffect(() => { + dispatch(fetchSoftwareUpdates(hostID)); getExportersStatus(); refreshCatalog(); dispatch(updateLastExecution(hostID)); - dispatch(fetchSoftwareUpdates(hostID)); - dispatch(fetchSoftwareUpdatesSettings()); }, []); if (!host) { @@ -125,8 +118,7 @@ function HostDetailsPage() { suseManagerEnabled={suseManagerEnabled} relevantPatches={numRelevantPatches} upgradablePackages={numUpgradablePackages} - softwareUpdatesSettingsSaved={softwareUpdatesConnectionSaved} - softwareUpdatesSettingsLoading={softwareUpdatesSettingsLoading} + softwareUpdatesSettingsSaved={settingsConfigured} softwareUpdatesLoading={softwareUpdatesLoading} softwareUpdatesTooltip={ numRelevantPatches === undefined && numUpgradablePackages === undefined diff --git a/assets/js/state/sagas/softwareUpdates.js b/assets/js/state/sagas/softwareUpdates.js index 8e4cbf695f..457fab475b 100644 --- a/assets/js/state/sagas/softwareUpdates.js +++ b/assets/js/state/sagas/softwareUpdates.js @@ -8,6 +8,8 @@ import { import { FETCH_UPGRADABLE_PACKAGES_PATCHES, FETCH_SOFTWARE_UPDATES, + setSettingsConfigured, + setSettingsNotConfigured, startLoadingSoftwareUpdates, setSoftwareUpdates, setEmptySoftwareUpdates, @@ -21,9 +23,16 @@ export function* fetchSoftwareUpdates({ payload: hostID }) { try { const response = yield call(getSoftwareUpdates, hostID); yield put(setSoftwareUpdates({ hostID, ...response.data })); + yield put(setSettingsConfigured()); } catch (error) { yield put(setEmptySoftwareUpdates({ hostID })); + const errorCode = get(error, ['response', 'status']); + + if (errorCode === 403) { + yield put(setSettingsNotConfigured()); + } + const errors = get(error, ['response', 'data'], []); yield put(setSoftwareUpdatesErrors({ hostID, errors })); } @@ -37,7 +46,13 @@ export function* fetchUpgradablePackagesPatches({ data: { patches }, } = yield call(getPatchesForPackages, packageIDs); yield put(setPatchesForPackages({ hostID, patches })); + yield put(setSettingsConfigured()); } catch (error) { + const errorCode = get(error, ['response', 'status']); + + if (errorCode === 403) { + yield put(setSettingsNotConfigured()); + } yield put(setPatchesForPackages({ hostID, patches: [] })); } } diff --git a/assets/js/state/sagas/softwareUpdates.test.js b/assets/js/state/sagas/softwareUpdates.test.js index 8304df1e4d..a25f64a2af 100644 --- a/assets/js/state/sagas/softwareUpdates.test.js +++ b/assets/js/state/sagas/softwareUpdates.test.js @@ -7,6 +7,8 @@ import { patchForPackageFactory } from '@lib/test-utils/factories'; import { networkClient } from '@lib/network'; import { + setSettingsConfigured, + setSettingsNotConfigured, startLoadingSoftwareUpdates, setSoftwareUpdates, setSoftwareUpdatesErrors, @@ -64,6 +66,7 @@ describe('Software Updates saga', () => { expect(dispatched).toEqual([ startLoadingSoftwareUpdates({ hostID }), setSoftwareUpdates({ hostID, ...response }), + setSettingsConfigured(), ]); }); @@ -91,6 +94,32 @@ describe('Software Updates saga', () => { ]); } ); + + it('should set settings not configured when 403', async () => { + const axiosMock = new MockAdapter(networkClient); + const hostID = faker.string.uuid(); + + const errorBody = { + errors: [ + { title: 'Forbidden', detail: 'SUSE Manager authentication error.' }, + ], + }; + + axiosMock + .onGet(`/api/v1/hosts/${hostID}/software_updates`) + .reply(403, errorBody); + + const dispatched = await recordSaga(fetchSoftwareUpdates, { + payload: hostID, + }); + + expect(dispatched).toEqual([ + startLoadingSoftwareUpdates({ hostID }), + setEmptySoftwareUpdates({ hostID }), + setSettingsNotConfigured(), + setSoftwareUpdatesErrors({ hostID, errors: errorBody }), + ]); + }); }); describe('Fetching patches for packages', () => { @@ -114,6 +143,33 @@ describe('Software Updates saga', () => { expect(dispatched).toEqual([ setPatchesForPackages({ hostID, patches: response.patches }), + setSettingsConfigured(), + ]); + }); + + it('should set settings not configured when 403', async () => { + const axiosMock = new MockAdapter(networkClient); + const hostID = faker.string.uuid(); + + const errorBody = { + errors: [ + { title: 'Forbidden', detail: 'SUSE Manager authentication error.' }, + ], + }; + + axiosMock + .onGet(`/api/v1/hosts/${hostID}/software_updates`) + .reply(403, errorBody); + + const dispatched = await recordSaga(fetchSoftwareUpdates, { + payload: hostID, + }); + + expect(dispatched).toEqual([ + startLoadingSoftwareUpdates({ hostID }), + setEmptySoftwareUpdates({ hostID }), + setSettingsNotConfigured(), + setSoftwareUpdatesErrors({ hostID, errors: errorBody }), ]); }); }); diff --git a/assets/js/state/selectors/softwareUpdates.js b/assets/js/state/selectors/softwareUpdates.js index a565aec2b3..ef98b48867 100644 --- a/assets/js/state/selectors/softwareUpdates.js +++ b/assets/js/state/selectors/softwareUpdates.js @@ -17,6 +17,11 @@ export const getSoftwareUpdatesStats = createSelector( }) ); +export const getSoftwareUpdatesSettingsConfigured = createSelector( + [(state) => getSoftwareUpdates(state)], + (softwareUpdates) => get(softwareUpdates, ['settingsConfigured'], false) +); + export const getSoftwareUpdatesPatches = createSelector( [(state, id) => getSoftwareUpdatesForHost(id)(state)], (softwareUpdates) => get(softwareUpdates, ['relevant_patches'], []) diff --git a/assets/js/state/softwareUpdates.js b/assets/js/state/softwareUpdates.js index 2653705f66..9557eb42a6 100644 --- a/assets/js/state/softwareUpdates.js +++ b/assets/js/state/softwareUpdates.js @@ -2,6 +2,7 @@ import { createAction, createSlice } from '@reduxjs/toolkit'; import { find, get, isEmpty } from 'lodash'; const initialState = { + settingsConfigured: true, softwareUpdates: {}, }; @@ -14,6 +15,12 @@ export const softwareUpdatesSlice = createSlice({ name: 'softwareUpdates', initialState, reducers: { + setSettingsConfigured: (state) => { + state.settingsConfigured = true; + }, + setSettingsNotConfigured: (state) => { + state.settingsConfigured = false; + }, startLoadingSoftwareUpdates: (state, { payload: { hostID } }) => { state.softwareUpdates = { ...state.softwareUpdates, @@ -81,6 +88,8 @@ export const fetchUpgradablePackagesPatches = createAction( ); export const { + setSettingsConfigured, + setSettingsNotConfigured, startLoadingSoftwareUpdates, setSoftwareUpdates, setEmptySoftwareUpdates, diff --git a/lib/trento/infrastructure/software_updates/suma.ex b/lib/trento/infrastructure/software_updates/suma.ex index 1afa241267..657c12898c 100644 --- a/lib/trento/infrastructure/software_updates/suma.ex +++ b/lib/trento/infrastructure/software_updates/suma.ex @@ -62,19 +62,19 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma do {:ok, new_state} -> request |> do_handle(new_state) - |> handle_authentication_error(request) + |> handle_suma_authentication_error(request) error -> error end end - defp handle_authentication_error({:error, :authentication_error}, request) do + defp handle_suma_authentication_error({:error, :suma_authentication_error}, request) do clear() handle_request(request) end - defp handle_authentication_error(result, _), do: result + defp handle_suma_authentication_error(result, _), do: result defp do_handle({:get_system_id, fully_qualified_domain_name}, %State{ url: url, diff --git a/lib/trento/infrastructure/software_updates/suma_api.ex b/lib/trento/infrastructure/software_updates/suma_api.ex index 50598938ea..40b87d4706 100644 --- a/lib/trento/infrastructure/software_updates/suma_api.ex +++ b/lib/trento/infrastructure/software_updates/suma_api.ex @@ -27,7 +27,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do fully_qualified_domain_name :: String.t(), ca_cert :: String.t() | nil ) :: - {:ok, pos_integer()} | {:error, :system_id_not_found | :authentication_error} + {:ok, pos_integer()} | {:error, :system_id_not_found | :suma_authentication_error} def get_system_id(url, auth, fully_qualified_domain_name, ca_cert) do url |> get_suma_api_url() @@ -47,7 +47,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do ca_cert :: String.t() | nil ) :: {:ok, [map()]} - | {:error, :error_getting_patches | :authentication_error} + | {:error, :error_getting_patches | :suma_authentication_error} def get_relevant_patches(url, auth, system_id, ca_cert) do url |> get_suma_api_url() @@ -67,7 +67,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do ca_cert :: String.t() | nil ) :: {:ok, [map()]} - | {:error, :error_getting_packages | :authentication_error} + | {:error, :error_getting_packages | :suma_authentication_error} def get_upgradable_packages(url, auth, system_id, ca_cert) do url |> get_suma_api_url() @@ -87,7 +87,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do ca_cert :: String.t() | nil ) :: {:ok, [map()]} - | {:error, :error_getting_patches | :authentication_error} + | {:error, :error_getting_patches | :suma_authentication_error} def get_patches_for_package(url, auth, package_id, ca_cert) do url |> get_suma_api_url() @@ -107,7 +107,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do ca_cert :: String.t() | nil ) :: {:ok, [map()]} - | {:error, :error_getting_affected_systems | :authentication_error} + | {:error, :error_getting_affected_systems | :suma_authentication_error} def get_affected_systems(url, auth, advisory_name, ca_cert) do url |> get_suma_api_url() @@ -127,7 +127,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do ca_cert :: String.t() | nil ) :: {:ok, [map()]} - | {:error, :error_getting_errata_details | :authentication_error} + | {:error, :error_getting_errata_details | :suma_authentication_error} def get_errata_details(url, auth, advisory_name, ca_cert) do url |> get_suma_api_url() @@ -147,7 +147,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do ca_cert :: String.t() | nil ) :: {:ok, [map()]} - | {:error, :error_getting_cves | :authentication_error} + | {:error, :error_getting_cves | :suma_authentication_error} def get_cves(url, auth, advisory_name, ca_cert) do url |> get_suma_api_url() @@ -167,7 +167,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do ca_cert :: String.t() | nil ) :: {:ok, [map()]} - | {:error, :error_getting_affected_packages | :authentication_error} + | {:error, :error_getting_affected_packages | :suma_authentication_error} def get_affected_packages(url, auth, advisory_name, ca_cert) do url |> get_suma_api_url() @@ -187,7 +187,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do ca_cert :: String.t() | nil ) :: {:ok, [map()]} - | {:error, :error_getting_fixes | :authentication_error} + | {:error, :error_getting_fixes | :suma_authentication_error} def get_bugzilla_fixes(url, auth, advisory_name, ca_cert) do url |> get_suma_api_url() @@ -201,7 +201,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do end defp handle_auth_error({:ok, %HTTPoison.Response{status_code: 401}}), - do: {:error, :authentication_error} + do: {:error, :suma_authentication_error} defp handle_auth_error({:ok, %HTTPoison.Response{status_code: _, body: body}}), do: {:ok, body} @@ -226,7 +226,8 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do end end - defp decode_response({:error, :authentication_error}, _), do: {:error, :authentication_error} + defp decode_response({:error, :suma_authentication_error}, _), + do: {:error, :suma_authentication_error} defp decode_response({:error, _} = error, error_atom: error_atom, error_log: error_log) do Logger.error("#{error_log} Error: #{inspect(error)}") diff --git a/lib/trento_web/controllers/fallback_controller.ex b/lib/trento_web/controllers/fallback_controller.ex index 5a3eed9717..dd98bcaba7 100644 --- a/lib/trento_web/controllers/fallback_controller.ex +++ b/lib/trento_web/controllers/fallback_controller.ex @@ -17,6 +17,13 @@ defmodule TrentoWeb.FallbackController do |> render(:"404", reason: "SUSE Manager settings not configured.") end + def call(conn, {:error, :suma_authentication_error}) do + conn + |> put_status(:forbidden) + |> put_view(ErrorView) + |> render(:"403", reason: "SUSE Manager authentication error.") + end + def call(conn, {:error, :invalid_refresh_token}) do conn |> put_status(:unauthorized) diff --git a/test/trento_web/controllers/v1/suse_manager_controller_test.exs b/test/trento_web/controllers/v1/suse_manager_controller_test.exs index 234948e3c8..0cd6135ed8 100644 --- a/test/trento_web/controllers/v1/suse_manager_controller_test.exs +++ b/test/trento_web/controllers/v1/suse_manager_controller_test.exs @@ -227,174 +227,148 @@ defmodule TrentoWeb.V1.SUSEManagerControllerTest do } = result end - test "should return 422 when advisory details are not found", %{ - conn: conn, - api_spec: api_spec - } do - insert_software_updates_settings() - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_errata_details, 1, fn _ -> - {:error, :error_getting_errata_details} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_cves, 1, fn _ -> - {:ok, build_list(10, :cve)} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_bugzilla_fixes, 1, fn _ -> - {:ok, build(:bugzilla_fix)} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_affected_packages, 1, fn _ -> - {:ok, build_list(10, :affected_package)} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_affected_systems, 1, fn _ -> - {:ok, build_list(10, :affected_system)} - end) - - advisory_name = Faker.Pokemon.name() - - conn - |> get("/api/v1/software_updates/errata_details/#{advisory_name}") - |> json_response(:unprocessable_entity) - |> assert_schema("UnprocessableEntity", api_spec) - end - - test "should return 422 when advisory CVEs are not found", %{ - conn: conn, - api_spec: api_spec - } do - insert_software_updates_settings() - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_errata_details, 1, fn _ -> - {:ok, build(:errata_details)} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_cves, 1, fn _ -> - {:error, :error_getting_cves} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_bugzilla_fixes, 1, fn _ -> - {:ok, build(:bugzilla_fix)} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_affected_packages, 1, fn _ -> - {:ok, build_list(10, :affected_package)} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_affected_systems, 1, fn _ -> - {:ok, build_list(10, :affected_system)} - end) - - advisory_name = Faker.Pokemon.name() - - conn - |> get("/api/v1/software_updates/errata_details/#{advisory_name}") - |> json_response(:unprocessable_entity) - |> assert_schema("UnprocessableEntity", api_spec) - end - - test "should return 422 when advisory fixes are not found", %{ - conn: conn, - api_spec: api_spec - } do - insert_software_updates_settings() - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_errata_details, 1, fn _ -> - {:ok, build(:errata_details)} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_cves, 1, fn _ -> - {:ok, build_list(10, :cve)} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_bugzilla_fixes, 1, fn _ -> - {:error, :error_getting_fixes} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_affected_packages, 1, fn _ -> - {:ok, build_list(10, :affected_package)} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_affected_systems, 1, fn _ -> - {:ok, build_list(10, :affected_system)} - end) - - advisory_name = Faker.Pokemon.name() - - conn - |> get("/api/v1/software_updates/errata_details/#{advisory_name}") - |> json_response(:unprocessable_entity) - |> assert_schema("UnprocessableEntity", api_spec) - end - - test "should return 422 when advisory affected packages are not found", %{ - conn: conn, - api_spec: api_spec - } do - insert_software_updates_settings() - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_errata_details, 1, fn _ -> - {:ok, build(:errata_details)} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_cves, 1, fn _ -> - {:ok, build_list(10, :cve)} - end) + error_scenarios = [ + %{ + name: "advisory details not found", + mocks: [ + get_errata_details: {:error, :error_getting_errata_details}, + get_cves: {:ok, build_list(10, :cve)}, + get_bugzilla_fixes: {:ok, build(:bugzilla_fix)}, + get_affected_packages: {:ok, build_list(10, :affected_package)}, + get_affected_systems: {:ok, build_list(10, :affected_system)} + ] + }, + %{ + name: "CVEs not found", + mocks: [ + get_errata_details: {:ok, build(:errata_details)}, + get_cves: {:error, :error_getting_cves}, + get_bugzilla_fixes: {:ok, build(:bugzilla_fix)}, + get_affected_packages: {:ok, build_list(10, :affected_package)}, + get_affected_systems: {:ok, build_list(10, :affected_system)} + ] + }, + %{ + name: "advisory fixes not found", + mocks: [ + get_errata_details: {:ok, build(:errata_details)}, + get_cves: {:ok, build_list(10, :cve)}, + get_bugzilla_fixes: {:error, :error_getting_fixes}, + get_affected_packages: {:ok, build_list(10, :affected_package)}, + get_affected_systems: {:ok, build_list(10, :affected_system)} + ] + }, + %{ + name: "affected packages not found", + mocks: [ + get_errata_details: {:ok, build(:errata_details)}, + get_cves: {:ok, build_list(10, :cve)}, + get_bugzilla_fixes: {:ok, build(:bugzilla_fix)}, + get_affected_packages: {:error, :error_getting_affected_packages}, + get_affected_systems: {:ok, build_list(10, :affected_system)} + ] + }, + %{ + name: "affected systems not found", + mocks: [ + get_errata_details: {:ok, build(:errata_details)}, + get_cves: {:ok, build_list(10, :cve)}, + get_bugzilla_fixes: {:ok, build(:bugzilla_fix)}, + get_affected_packages: {:ok, build_list(10, :affected_package)}, + get_affected_systems: {:error, :error_getting_affected_systems} + ] + } + ] - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_bugzilla_fixes, 1, fn _ -> - {:ok, build(:bugzilla_fix)} - end) + for %{name: name} = scenario <- error_scenarios do + @scenario scenario - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_affected_packages, 1, fn _ -> - {:error, :error_getting_affected_packages} - end) + test "should return 422 when #{name}", %{conn: conn, api_spec: api_spec} do + %{mocks: mocks} = @scenario - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_affected_systems, 1, fn _ -> - {:ok, build_list(10, :affected_system)} - end) + Enum.each(mocks, fn {network_call, response} -> + expect(Trento.SoftwareUpdates.Discovery.Mock, network_call, 1, fn _ -> response end) + end) - advisory_name = Faker.Pokemon.name() + advisory_name = Faker.Pokemon.name() - conn - |> get("/api/v1/software_updates/errata_details/#{advisory_name}") - |> json_response(:unprocessable_entity) - |> assert_schema("UnprocessableEntity", api_spec) + conn + |> get("/api/v1/software_updates/errata_details/#{advisory_name}") + |> json_response(:unprocessable_entity) + |> assert_schema("UnprocessableEntity", api_spec) + end end - test "should return 422 when advisory affected systems are not found", %{ - conn: conn, - api_spec: api_spec - } do - insert_software_updates_settings() - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_errata_details, 1, fn _ -> - {:ok, build(:errata_details)} - end) - - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_cves, 1, fn _ -> - {:ok, build_list(10, :cve)} - end) + unauthorized_scenarios = [ + %{ + name: "get_errata_details unauthorized", + mocks: [ + get_errata_details: {:error, :suma_authentication_error}, + get_cves: {:ok, build_list(10, :cve)}, + get_bugzilla_fixes: {:ok, build(:bugzilla_fix)}, + get_affected_packages: {:ok, build_list(10, :affected_package)}, + get_affected_systems: {:ok, build_list(10, :affected_system)} + ] + }, + %{ + name: "get_cves unauthorized", + mocks: [ + get_errata_details: {:ok, build(:errata_details)}, + get_cves: {:error, :suma_authentication_error}, + get_bugzilla_fixes: {:ok, build(:bugzilla_fix)}, + get_affected_packages: {:ok, build_list(10, :affected_package)}, + get_affected_systems: {:ok, build_list(10, :affected_system)} + ] + }, + %{ + name: "get_bugzilla_fixes unauthorized", + mocks: [ + get_errata_details: {:ok, build(:errata_details)}, + get_cves: {:ok, build_list(10, :cve)}, + get_bugzilla_fixes: {:error, :suma_authentication_error}, + get_affected_packages: {:ok, build_list(10, :affected_package)}, + get_affected_systems: {:ok, build_list(10, :affected_system)} + ] + }, + %{ + name: "get_affected_packages unauthorized", + mocks: [ + get_errata_details: {:ok, build(:errata_details)}, + get_cves: {:ok, build_list(10, :cve)}, + get_bugzilla_fixes: {:ok, build(:bugzilla_fix)}, + get_affected_packages: {:error, :suma_authentication_error}, + get_affected_systems: {:ok, build_list(10, :affected_system)} + ] + }, + %{ + name: "get_affected_systems unauthorized", + mocks: [ + get_errata_details: {:ok, build(:errata_details)}, + get_cves: {:ok, build_list(10, :cve)}, + get_bugzilla_fixes: {:ok, build(:bugzilla_fix)}, + get_affected_packages: {:ok, build_list(10, :affected_package)}, + get_affected_systems: {:error, :suma_authentication_error} + ] + } + ] - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_bugzilla_fixes, 1, fn _ -> - {:ok, build(:bugzilla_fix)} - end) + for %{name: name} = scenario <- unauthorized_scenarios do + @scenario scenario - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_affected_packages, 1, fn _ -> - {:ok, build_list(10, :affected_package)} - end) + test "should return 403 for #{name}", %{conn: conn, api_spec: api_spec} do + %{mocks: mocks} = @scenario - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_affected_systems, 1, fn _ -> - {:error, :error_getting_affected_systems} - end) + Enum.each(mocks, fn {network_call, response} -> + expect(Trento.SoftwareUpdates.Discovery.Mock, network_call, 1, fn _ -> response end) + end) - advisory_name = Faker.Pokemon.name() + advisory_name = Faker.Pokemon.name() - conn - |> get("/api/v1/software_updates/errata_details/#{advisory_name}") - |> json_response(:unprocessable_entity) - |> assert_schema("UnprocessableEntity", api_spec) + conn + |> get("/api/v1/software_updates/errata_details/#{advisory_name}") + |> json_response(:forbidden) + |> assert_schema("Forbidden", api_spec) + end end end end