From 8fdd789474ca1151dfc945fd404883ebb2ffedf1 Mon Sep 17 00:00:00 2001 From: Dmitry Seregin Date: Fri, 10 Sep 2021 17:57:09 +0300 Subject: [PATCH] Pull request: 3419 client allowlist collision Updates #3419. Squashed commit of the following: commit 370094c00d9c15b1336fbedb1e233bd4436c9898 Author: Dmitriy Seregin Date: Fri Sep 10 17:31:16 2021 +0300 added link to github issue commit 407ba9b2db46b887a30ddb081bd37c56e56b0496 Merge: 426c8146 80548233 Author: Dmitriy Seregin Date: Fri Sep 10 17:29:52 2021 +0300 Merge branch 'master' into 3419-client-allowlist-collision commit 426c8146cff5c112ebb25192af276c6601200528 Author: Dmitriy Seregin Date: Fri Sep 10 16:28:11 2021 +0300 fix en commit d28c6022321828c6bdc55c3f9a4f655b26d146d2 Author: Dmitriy Seregin Date: Fri Sep 10 15:49:12 2021 +0300 added missing space commit b374a09327968ca5343c1595d1ab8cf317c15ffe Author: Dmitriy Seregin Date: Fri Sep 10 15:43:55 2021 +0300 fixes after review commit 2be629d66e4703e2f5a85615bf1eaaa92e03c6fd Author: Dmitriy Seregin Date: Thu Sep 9 14:17:19 2021 +0300 fixes commit 5c2aa6201cc0ecf404d4057e354fbb0bdadcdd6d Author: Dmitriy Seregin Date: Wed Sep 8 15:04:30 2021 +0300 return empty line to locale file commit 3631c3772babbd595b1c3de4a7e91be6bac3e80f Author: Dmitriy Seregin Date: Wed Sep 8 13:57:51 2021 +0300 all: fix collisions in access lists && expand block/unblock client --- CHANGELOG.md | 3 + client/src/__locales/en.json | 3 +- client/src/actions/access.js | 25 ++++-- client/src/components/Dashboard/Clients.js | 23 +++-- .../src/components/Logs/Cells/ClientCell.js | 7 +- .../components/Logs/Cells/helpers/index.js | 19 +++-- client/src/components/Logs/Cells/index.js | 7 +- client/src/components/Logs/index.js | 2 + .../components/Settings/Dns/Access/Form.js | 42 ++++++++-- internal/dnsforward/access.go | 62 ++++++++++++++ internal/home/clients.go | 83 +++++++++++++------ openapi/CHANGELOG.md | 19 ++++- openapi/openapi.yaml | 8 +- 13 files changed, 238 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74ab273b..79fd7709 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,8 @@ and this project adheres to ### Changed +- Items in allowed clients, disallowed clients, and blocked hosts lists must + be unique ([#3419]). - The TLS private key previously saved as a string isn't shown in API responses any more ([#1898]). - Better OpenWrt detection ([#3435]). @@ -178,6 +180,7 @@ In this release, the schema version has changed from 10 to 12. [#3351]: https://github.com/AdguardTeam/AdGuardHome/issues/3351 [#3372]: https://github.com/AdguardTeam/AdGuardHome/issues/3372 [#3417]: https://github.com/AdguardTeam/AdGuardHome/issues/3417 +[#3419]: https://github.com/AdguardTeam/AdGuardHome/issues/3419 [#3435]: https://github.com/AdguardTeam/AdGuardHome/issues/3435 [#3437]: https://github.com/AdguardTeam/AdGuardHome/issues/3437 [#3443]: https://github.com/AdguardTeam/AdGuardHome/issues/3443 diff --git a/client/src/__locales/en.json b/client/src/__locales/en.json index 2df2d93b..30dcb0bc 100644 --- a/client/src/__locales/en.json +++ b/client/src/__locales/en.json @@ -613,7 +613,8 @@ "click_to_view_queries": "Click to view queries", "port_53_faq_link": "Port 53 is often occupied by \"DNSStubListener\" or \"systemd-resolved\" services. Please read <0>this instruction on how to resolve this.", "adg_will_drop_dns_queries": "AdGuard Home will be dropping all DNS queries from this client.", - "client_not_in_allowed_clients": "The client is not allowed because it is not in the \"Allowed clients\" list.", + "filter_allowlist": "WARNING: This action also will exclude the rule \"{{disallowed_rule}}\" from the list of allowed clients.", + "last_rule_in_allowlist": "Cannot disallow this client because excluding the rule \"{{disallowed_rule}}\" will DISABLE \"Allowed clients\" list.", "experimental": "Experimental", "use_saved_key": "Use the previously saved key" } diff --git a/client/src/actions/access.js b/client/src/actions/access.js index 3cbc8016..710159ad 100644 --- a/client/src/actions/access.js +++ b/client/src/actions/access.js @@ -52,25 +52,34 @@ export const toggleClientBlock = (ip, disallowed, disallowed_rule) => async (dis dispatch(toggleClientBlockRequest()); try { const accessList = await apiClient.getAccessList(); - const allowed_clients = accessList.allowed_clients ?? []; const blocked_hosts = accessList.blocked_hosts ?? []; - const disallowed_clients = accessList.disallowed_clients ?? []; - - const updatedDisallowedClients = disallowed - ? disallowed_clients.filter((client) => client !== disallowed_rule) - : disallowed_clients.concat(ip); + let allowed_clients = accessList.allowed_clients ?? []; + let disallowed_clients = accessList.disallowed_clients ?? []; + if (disallowed) { + if (!disallowed_rule) { + allowed_clients = allowed_clients.concat(ip); + } else { + disallowed_clients = disallowed_clients + .filter((client) => client !== disallowed_rule); + } + } else if (allowed_clients.length > 1) { + allowed_clients = allowed_clients + .filter((client) => client !== disallowed_rule); + } else { + disallowed_clients = disallowed_clients.concat(ip); + } const values = { allowed_clients, blocked_hosts, - disallowed_clients: updatedDisallowedClients, + disallowed_clients, }; await apiClient.setAccessList(values); dispatch(toggleClientBlockSuccess(values)); if (disallowed) { - dispatch(addSuccessToast(i18next.t('client_unblocked', { ip: disallowed_rule }))); + dispatch(addSuccessToast(i18next.t('client_unblocked', { ip: disallowed_rule || ip }))); } else { dispatch(addSuccessToast(i18next.t('client_blocked', { ip }))); } diff --git a/client/src/components/Dashboard/Clients.js b/client/src/components/Dashboard/Clients.js index cc11b915..76cf998a 100644 --- a/client/src/components/Dashboard/Clients.js +++ b/client/src/components/Dashboard/Clients.js @@ -38,15 +38,23 @@ const renderBlockingButton = (ip, disallowed, disallowed_rule) => { const dispatch = useDispatch(); const { t } = useTranslation(); const processingSet = useSelector((state) => state.access.processingSet); + const allowedСlients = useSelector((state) => state.access.allowed_clients, shallowEqual); const buttonClass = classNames('button-action button-action--main', { 'button-action--unblock': disallowed, }); const toggleClientStatus = async (ip, disallowed, disallowed_rule) => { - const confirmMessage = disallowed - ? t('client_confirm_unblock', { ip: disallowed_rule }) - : `${t('adg_will_drop_dns_queries')} ${t('client_confirm_block', { ip })}`; + let confirmMessage; + + if (disallowed) { + confirmMessage = t('client_confirm_unblock', { ip: disallowed_rule || ip }); + } else { + confirmMessage = `${t('adg_will_drop_dns_queries')} ${t('client_confirm_block', { ip })}`; + if (allowedСlients.length > 0) { + confirmMessage = confirmMessage.concat(`\n\n${t('filter_allowlist', { disallowed_rule })}`); + } + } if (window.confirm(confirmMessage)) { await dispatch(toggleClientBlock(ip, disallowed, disallowed_rule)); @@ -58,15 +66,16 @@ const renderBlockingButton = (ip, disallowed, disallowed_rule) => { const text = disallowed ? BLOCK_ACTIONS.UNBLOCK : BLOCK_ACTIONS.BLOCK; - const isNotInAllowedList = disallowed && disallowed_rule === ''; + const lastRuleInAllowlist = !disallowed && allowedСlients === disallowed_rule; + const disabled = processingSet || lastRuleInAllowlist; return (
diff --git a/client/src/components/Logs/Cells/ClientCell.js b/client/src/components/Logs/Cells/ClientCell.js index 99dfcd4b..cfcf19ec 100644 --- a/client/src/components/Logs/Cells/ClientCell.js +++ b/client/src/components/Logs/Cells/ClientCell.js @@ -28,6 +28,8 @@ const ClientCell = ({ const autoClients = useSelector((state) => state.dashboard.autoClients, shallowEqual); const processingRules = useSelector((state) => state.filtering.processingRules); const isDetailed = useSelector((state) => state.queryLogs.isDetailed); + const processingSet = useSelector((state) => state.access.processingSet); + const allowedСlients = useSelector((state) => state.access.allowed_clients, shallowEqual); const [isOptionsOpened, setOptionsOpened] = useState(false); const autoClient = autoClients.find((autoClient) => autoClient.name === client); @@ -71,11 +73,12 @@ const ClientCell = ({ const { confirmMessage, buttonKey: blockingClientKey, - isNotInAllowedList, + lastRuleInAllowlist, } = getBlockClientInfo( client, client_info?.disallowed || false, client_info?.disallowed_rule || '', + allowedСlients, ); const blockingForClientKey = isFiltered ? 'unblock_for_this_client_only' : 'block_for_this_client_only'; @@ -100,7 +103,7 @@ const ClientCell = ({ await dispatch(updateLogs()); } }, - disabled: isNotInAllowedList, + disabled: processingSet || lastRuleInAllowlist, }, ]; diff --git a/client/src/components/Logs/Cells/helpers/index.js b/client/src/components/Logs/Cells/helpers/index.js index b843ed99..2309b340 100644 --- a/client/src/components/Logs/Cells/helpers/index.js +++ b/client/src/components/Logs/Cells/helpers/index.js @@ -2,17 +2,24 @@ import i18next from 'i18next'; export const BUTTON_PREFIX = 'btn_'; -export const getBlockClientInfo = (ip, disallowed, disallowed_rule) => { - const confirmMessage = disallowed - ? i18next.t('client_confirm_unblock', { ip: disallowed_rule }) - : `${i18next.t('adg_will_drop_dns_queries')} ${i18next.t('client_confirm_block', { ip })}`; +export const getBlockClientInfo = (ip, disallowed, disallowed_rule, allowedСlients) => { + let confirmMessage; + + if (disallowed) { + confirmMessage = i18next.t('client_confirm_unblock', { ip: disallowed_rule || ip }); + } else { + confirmMessage = `${i18next.t('adg_will_drop_dns_queries')} ${i18next.t('client_confirm_block', { ip })}`; + if (allowedСlients.length > 0) { + confirmMessage = confirmMessage.concat(`\n\n${i18next.t('filter_allowlist', { disallowed_rule })}`); + } + } const buttonKey = i18next.t(disallowed ? 'allow_this_client' : 'disallow_this_client'); - const isNotInAllowedList = disallowed && disallowed_rule === ''; + const lastRuleInAllowlist = !disallowed && allowedСlients === disallowed_rule; return { confirmMessage, buttonKey, - isNotInAllowedList, + lastRuleInAllowlist, }; }; diff --git a/client/src/components/Logs/Cells/index.js b/client/src/components/Logs/Cells/index.js index 25d6584e..df36f1d2 100644 --- a/client/src/components/Logs/Cells/index.js +++ b/client/src/components/Logs/Cells/index.js @@ -50,6 +50,8 @@ const Row = memo(({ const filters = useSelector((state) => state.filtering.filters, shallowEqual); const whitelistFilters = useSelector((state) => state.filtering.whitelistFilters, shallowEqual); const autoClients = useSelector((state) => state.dashboard.autoClients, shallowEqual); + const processingSet = useSelector((state) => state.access.processingSet); + const allowedСlients = useSelector((state) => state.access.allowed_clients, shallowEqual); const clients = useSelector((state) => state.dashboard.clients); @@ -104,11 +106,12 @@ const Row = memo(({ const { confirmMessage, buttonKey: blockingClientKey, - isNotInAllowedList, + lastRuleInAllowlist, } = getBlockClientInfo( client, client_info?.disallowed || false, client_info?.disallowed_rule || '', + allowedСlients, ); const blockingForClientKey = isFiltered ? 'unblock_for_this_client_only' : 'block_for_this_client_only'; @@ -147,7 +150,7 @@ const Row = memo(({ const blockClientButton = ; diff --git a/client/src/components/Logs/index.js b/client/src/components/Logs/index.js index 1416d21d..c3b30703 100644 --- a/client/src/components/Logs/index.js +++ b/client/src/components/Logs/index.js @@ -15,6 +15,7 @@ import Disabled from './Disabled'; import { getFilteringStatus } from '../../actions/filtering'; import { getClients } from '../../actions'; import { getDnsConfig } from '../../actions/dnsConfig'; +import { getAccessList } from '../../actions/access'; import { getLogsConfig, resetFilteredLogs, @@ -126,6 +127,7 @@ const Logs = () => { await Promise.all([ dispatch(getLogsConfig()), dispatch(getDnsConfig()), + dispatch(getAccessList()), ]); } catch (err) { console.error(err); diff --git a/client/src/components/Settings/Dns/Access/Form.js b/client/src/components/Settings/Dns/Access/Form.js index 4f8342f0..aaf60412 100644 --- a/client/src/components/Settings/Dns/Access/Form.js +++ b/client/src/components/Settings/Dns/Access/Form.js @@ -1,6 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { Field, reduxForm } from 'redux-form'; +import { connect } from 'react-redux'; +import { Field, reduxForm, formValueSelector } from 'redux-form'; import { Trans, withTranslation } from 'react-i18next'; import flow from 'lodash/flow'; import { renderTextareaField } from '../../../../helpers/form'; @@ -31,16 +32,20 @@ const fields = [ }, ]; -const Form = (props) => { +let Form = (props) => { const { - handleSubmit, submitting, invalid, processingSet, + allowedClients, handleSubmit, submitting, invalid, processingSet, } = props; const renderField = ({ - id, title, subtitle, disabled = processingSet, normalizeOnBlur, + id, title, subtitle, disabled = false, processingSet, normalizeOnBlur, }) =>
{subtitle} @@ -51,7 +56,7 @@ const Form = (props) => { component={renderTextareaField} type="text" className="form-control form-control--textarea font-monospace" - disabled={disabled} + disabled={disabled || processingSet} normalizeOnBlur={normalizeOnBlur} />
; @@ -66,7 +71,15 @@ const Form = (props) => { return (
- {fields.map(renderField)} + { + fields.map((f) => { + const props = { ...f }; + if (allowedClients && f.id === 'disallowed_clients') { + props.disabled = true; + } + return renderField(props); + }) + }