Skip to content

Commit

Permalink
+ clients: improve logic when processing "disallowed IP" property for…
Browse files Browse the repository at this point in the history 
… a client

#1925

* commit 'c72cd58f69b71e4981d8b182b2f7d53ea5e30868':
  + client: Move the client access check to the server-side
  Improve the clients/find API response
  + GET /control/clients/find: add "disallowed" property
  • Loading branch information
@szolin
szolin committed Sep 24, 2020
2 parents 10f67bd + c72cd58 commit 6222d17
Show file tree
Hide file tree
Showing 23 changed files with 348 additions and 443 deletions.
7 changes: 6 additions & 1 deletion AGHTechDoc.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,7 @@ Error response (Client not found):
### API: Find clients by IP

This method returns the list of clients (manual and auto-clients) matching the IP list.
For auto-clients only `name`, `ids` and `whois_info` fields are set. Other fields are empty.
For auto-clients only `name`, `ids`, `whois_info`, `disallowed`, and `disallowed_rule` fields are set. Other fields are empty.

Request:

Expand All @@ -968,11 +968,16 @@ Response:
key: "value"
...
}

"disallowed": false,
"disallowed_rule": "..."
}
}
...
]

* `disallowed` - whether the client's IP is blocked or not.
* `disallowed_rule` - the rule due to which the client is disallowed. If `disallowed` is `true`, and this string is empty - it means that the client IP is disallowed by the "allowed IP list", i.e. it is not included in allowed.

## DNS general settings

Expand Down
1 change: 1 addition & 0 deletions client/src/__locales/en.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,5 +582,6 @@
"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</0> 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.",
"experimental": "Experimental"
}
135 changes: 2 additions & 133 deletions client/src/__tests__/helpers.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,136 +1,5 @@
import {
countClientsStatistics, findAddressType, getIpMatchListStatus, sortIp,
} from '../helpers/helpers';
import { ADDRESS_TYPES, IP_MATCH_LIST_STATUS } from '../helpers/constants';

describe('getIpMatchListStatus', () => {
describe('IPv4', () => {
test('should return EXACT on find the exact ip match', () => {
const list = `127.0.0.2
2001:db8:11a3:9d7:0:0:0:0
192.168.0.1/8
127.0.0.1
127.0.0.3`;
expect(getIpMatchListStatus('127.0.0.1', list))
.toEqual(IP_MATCH_LIST_STATUS.EXACT);
});

test('should return CIDR on find the cidr match', () => {
const list = `127.0.0.2
2001:db8:11a3:9d7:0:0:0:0
192.168.0.1/8
127.0.0.0/24
127.0.0.3`;
expect(getIpMatchListStatus('127.0.0.1', list))
.toEqual(IP_MATCH_LIST_STATUS.CIDR);
});

test('should return NOT_FOUND if the ip is not in the list', () => {
const list = `127.0.0.1
2001:db8:11a3:9d7:0:0:0:0
192.168.0.1/8
127.0.0.2
127.0.0.3`;
expect(getIpMatchListStatus('127.0.0.4', list))
.toEqual(IP_MATCH_LIST_STATUS.NOT_FOUND);
});

test('should return the first EXACT or CIDR match in the list', () => {
const list1 = `2001:db8:11a3:9d7:0:0:0:0
127.0.0.1
127.0.0.8/24
127.0.0.3`;
expect(getIpMatchListStatus('127.0.0.1', list1))
.toEqual(IP_MATCH_LIST_STATUS.EXACT);

const list2 = `2001:db8:11a3:9d7:ffff:ffff:ffff:ffff
2001:0db8:11a3:09d7:0000:0000:0000:0000/64
127.0.0.0/24
127.0.0.1
127.0.0.8/24
127.0.0.3`;
expect(getIpMatchListStatus('127.0.0.1', list2))
.toEqual(IP_MATCH_LIST_STATUS.CIDR);
});
});

describe('IPv6', () => {
test('should return EXACT on find the exact ip match', () => {
const list = `127.0.0.0
2001:db8:11a3:9d7:0:0:0:0
2001:db8:11a3:9d7:ffff:ffff:ffff:ffff
127.0.0.1`;
expect(getIpMatchListStatus('2001:db8:11a3:9d7:0:0:0:0', list))
.toEqual(IP_MATCH_LIST_STATUS.EXACT);
});

test('should return EXACT on find the exact ip match of short and long notation', () => {
const list = `127.0.0.0
192.168.0.1/8
2001:db8::
127.0.0.2`;
expect(getIpMatchListStatus('2001:db8:0:0:0:0:0:0', list))
.toEqual(IP_MATCH_LIST_STATUS.EXACT);
});

test('should return CIDR on find the cidr match', () => {
const list1 = `2001:0db8:11a3:09d7:0000:0000:0000:0000/64
127.0.0.1
127.0.0.2`;
expect(getIpMatchListStatus('2001:db8:11a3:9d7:0:0:0:0', list1))
.toEqual(IP_MATCH_LIST_STATUS.CIDR);

const list2 = `2001:0db8::/16
127.0.0.0
2001:db8:11a3:9d7:0:0:0:0
2001:db8::
2001:db8:11a3:9d7:ffff:ffff:ffff:ffff
127.0.0.1`;
expect(getIpMatchListStatus('2001:db1::', list2))
.toEqual(IP_MATCH_LIST_STATUS.CIDR);
});

test('should return NOT_FOUND if the ip is not in the list', () => {
const list = `2001:db8:11a3:9d7:0:0:0:0
2001:0db8:11a3:09d7:0000:0000:0000:0000/64
127.0.0.1
127.0.0.2`;
expect(getIpMatchListStatus('::', list))
.toEqual(IP_MATCH_LIST_STATUS.NOT_FOUND);
});

test('should return the first EXACT or CIDR match in the list', () => {
const list1 = `2001:db8:11a3:9d7:0:0:0:0
2001:0db8:11a3:09d7:0000:0000:0000:0000/64
127.0.0.3`;
expect(getIpMatchListStatus('2001:db8:11a3:9d7:0:0:0:0', list1))
.toEqual(IP_MATCH_LIST_STATUS.EXACT);

const list2 = `2001:0db8:11a3:09d7:0000:0000:0000:0000/64
2001:db8:11a3:9d7:0:0:0:0
127.0.0.3`;
expect(getIpMatchListStatus('2001:db8:11a3:9d7:0:0:0:0', list2))
.toEqual(IP_MATCH_LIST_STATUS.CIDR);
});
});

describe('Empty list or IP', () => {
test('should return NOT_FOUND on empty ip', () => {
const list = `127.0.0.0
2001:db8:11a3:9d7:0:0:0:0
2001:db8:11a3:9d7:ffff:ffff:ffff:ffff
127.0.0.1`;
expect(getIpMatchListStatus('', list))
.toEqual(IP_MATCH_LIST_STATUS.NOT_FOUND);
});

test('should return NOT_FOUND on empty list', () => {
const list = '';
expect(getIpMatchListStatus('127.0.0.1', list))
.toEqual(IP_MATCH_LIST_STATUS.NOT_FOUND);
});
});
});
import { sortIp, countClientsStatistics, findAddressType } from '../helpers/helpers';
import { ADDRESS_TYPES } from '../helpers/constants';

describe('sortIp', () => {
describe('ipv4', () => {
Expand Down
20 changes: 8 additions & 12 deletions client/src/actions/access.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import i18next from 'i18next';

import apiClient from '../api/Api';
import { addErrorToast, addSuccessToast } from './toasts';
import { BLOCK_ACTIONS } from '../helpers/constants';
import { splitByNewLine } from '../helpers/helpers';

export const getAccessListRequest = createAction('GET_ACCESS_LIST_REQUEST');
Expand Down Expand Up @@ -49,19 +48,16 @@ export const toggleClientBlockRequest = createAction('TOGGLE_CLIENT_BLOCK_REQUES
export const toggleClientBlockFailure = createAction('TOGGLE_CLIENT_BLOCK_FAILURE');
export const toggleClientBlockSuccess = createAction('TOGGLE_CLIENT_BLOCK_SUCCESS');

export const toggleClientBlock = (type, ip) => async (dispatch) => {
export const toggleClientBlock = (ip, disallowed, disallowed_rule) => async (dispatch) => {
dispatch(toggleClientBlockRequest());
try {
const {
allowed_clients, disallowed_clients, blocked_hosts,
allowed_clients, blocked_hosts, disallowed_clients = [],
} = await apiClient.getAccessList();
let updatedDisallowedClients = disallowed_clients || [];

if (type === BLOCK_ACTIONS.UNBLOCK && updatedDisallowedClients.includes(ip)) {
updatedDisallowedClients = updatedDisallowedClients.filter((client) => client !== ip);
} else if (type === BLOCK_ACTIONS.BLOCK && !updatedDisallowedClients.includes(ip)) {
updatedDisallowedClients.push(ip);
}
const updatedDisallowedClients = disallowed
? disallowed_clients.filter((client) => client !== disallowed_rule)
: disallowed_clients.concat(ip);

const values = {
allowed_clients,
Expand All @@ -72,9 +68,9 @@ export const toggleClientBlock = (type, ip) => async (dispatch) => {
await apiClient.setAccessList(values);
dispatch(toggleClientBlockSuccess(values));

if (type === BLOCK_ACTIONS.UNBLOCK) {
dispatch(addSuccessToast(i18next.t('client_unblocked', { ip })));
} else if (type === BLOCK_ACTIONS.BLOCK) {
if (disallowed) {
dispatch(addSuccessToast(i18next.t('client_unblocked', { ip: disallowed_rule })));
} else {
dispatch(addSuccessToast(i18next.t('client_blocked', { ip })));
}
} catch (error) {
Expand Down
37 changes: 30 additions & 7 deletions client/src/actions/queryLogs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,26 @@ import {
} from '../helpers/constants';
import { addErrorToast, addSuccessToast } from './toasts';

const enrichWithClientInfo = async (logs) => {
const clientsParams = getParamsForClientsSearch(logs, 'client');

if (Object.keys(clientsParams).length > 0) {
const clients = await apiClient.findClients(clientsParams);
return addClientInfo(logs, clients, 'client');
}

return logs;
};

const getLogsWithParams = async (config) => {
const { older_than, filter, ...values } = config;
const rawLogs = await apiClient.getQueryLog({
...filter,
older_than,
});
const { data, oldest } = rawLogs;
let logs = normalizeLogs(data);
const clientsParams = getParamsForClientsSearch(logs, 'client');

if (Object.keys(clientsParams).length > 0) {
const clients = await apiClient.findClients(clientsParams);
logs = addClientInfo(logs, clients, 'client');
}
const normalizedLogs = normalizeLogs(data);
const logs = await enrichWithClientInfo(normalizedLogs);

return {
logs,
Expand Down Expand Up @@ -82,6 +88,23 @@ export const getLogsRequest = createAction('GET_LOGS_REQUEST');
export const getLogsFailure = createAction('GET_LOGS_FAILURE');
export const getLogsSuccess = createAction('GET_LOGS_SUCCESS');

export const updateLogs = () => async (dispatch, getState) => {
try {
const { logs, oldest, older_than } = getState().queryLogs;

const enrichedLogs = await enrichWithClientInfo(logs);

dispatch(getLogsSuccess({
logs: enrichedLogs,
oldest,
older_than,
}));
} catch (error) {
dispatch(addErrorToast({ error }));
dispatch(getLogsFailure(error));
}
};

export const getLogs = () => async (dispatch, getState) => {
dispatch(getLogsRequest());
try {
Expand Down
66 changes: 29 additions & 37 deletions client/src/components/Dashboard/Clients.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ import classNames from 'classnames';
import Card from '../ui/Card';
import Cell from '../ui/Cell';

import { getPercent, getIpMatchListStatus, sortIp } from '../../helpers/helpers';
import { BLOCK_ACTIONS, IP_MATCH_LIST_STATUS, STATUS_COLORS } from '../../helpers/constants';
import { getPercent, sortIp } from '../../helpers/helpers';
import { BLOCK_ACTIONS, STATUS_COLORS } from '../../helpers/constants';
import { toggleClientBlock } from '../../actions/access';
import { renderFormattedClientCell } from '../../helpers/renderFormattedClientCell';
import { getStats } from '../../actions/stats';

const getClientsPercentColor = (percent) => {
if (percent > 50) {
Expand All @@ -33,59 +34,51 @@ const CountCell = (row) => {
return <Cell value={value} percent={percent} color={percentColor} search={ip} />;
};

const renderBlockingButton = (ip) => {
const renderBlockingButton = (ip, disallowed, disallowed_rule) => {
const dispatch = useDispatch();
const { t } = useTranslation();
const processingSet = useSelector((state) => state.access.processingSet);
const disallowed_clients = useSelector(
(state) => state.access.disallowed_clients, shallowEqual,
);

const ipMatchListStatus = getIpMatchListStatus(ip, disallowed_clients);

if (ipMatchListStatus === IP_MATCH_LIST_STATUS.CIDR) {
return null;
}

const isNotFound = ipMatchListStatus === IP_MATCH_LIST_STATUS.NOT_FOUND;
const type = isNotFound ? BLOCK_ACTIONS.BLOCK : BLOCK_ACTIONS.UNBLOCK;
const text = type;

const buttonClass = classNames('button-action button-action--main', {
'button-action--unblock': !isNotFound,
'button-action--unblock': disallowed,
});

const toggleClientStatus = (type, ip) => {
const confirmMessage = type === BLOCK_ACTIONS.BLOCK
? `${t('adg_will_drop_dns_queries')} ${t('client_confirm_block', { ip })}`
: t('client_confirm_unblock', { ip });
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 })}`;

if (window.confirm(confirmMessage)) {
dispatch(toggleClientBlock(type, ip));
await dispatch(toggleClientBlock(ip, disallowed, disallowed_rule));
await dispatch(getStats());
}
};

const onClick = () => toggleClientStatus(type, ip);
const onClick = () => toggleClientStatus(ip, disallowed, disallowed_rule);

const text = disallowed ? BLOCK_ACTIONS.UNBLOCK : BLOCK_ACTIONS.BLOCK;

const isNotInAllowedList = disallowed && disallowed_rule === '';
return <div className="table__action pl-4">
<button
type="button"
className={buttonClass}
onClick={onClick}
disabled={processingSet}
>
<Trans>{text}</Trans>
</button>
</div>;
<button
type="button"
className={buttonClass}
onClick={isNotInAllowedList ? undefined : onClick}
disabled={isNotInAllowedList || processingSet}
title={t(isNotInAllowedList ? 'client_not_in_allowed_clients' : text)}
>
<Trans>{text}</Trans>
</button>
</div>;
};

const ClientCell = (row) => {
const { value, original: { info } } = row;
const { value, original: { info, info: { disallowed, disallowed_rule } } } = row;

return <>
<div className="logs__row logs__row--overflow logs__row--column d-flex align-items-center">
{renderFormattedClientCell(value, info, true)}
{renderBlockingButton(value)}
{renderBlockingButton(value, disallowed, disallowed_rule)}
</div>
</>;
};
Expand All @@ -96,7 +89,6 @@ const Clients = ({
}) => {
const { t } = useTranslation();
const topClients = useSelector((state) => state.stats.topClients, shallowEqual);
const disallowedClients = useSelector((state) => state.access.disallowed_clients, shallowEqual);

return <Card
title={t('top_clients')}
Expand Down Expand Up @@ -138,9 +130,9 @@ const Clients = ({
return {};
}

const { ip } = rowInfo.original;
const { info: { disallowed } } = rowInfo.original;

return getIpMatchListStatus(ip, disallowedClients) === IP_MATCH_LIST_STATUS.NOT_FOUND ? {} : { className: 'logs__row--red' };
return disallowed ? { className: 'logs__row--red' } : {};
}}
/>
</Card>;
Expand Down
Loading

0 comments on commit 6222d17

Please sign in to comment.