Skip to content

Commit

Permalink
Fix miscellaneous allow list QA issues (#460)
Browse files Browse the repository at this point in the history
* Fix: Prevent clearing content settings on a new window creation

* Fix parent domain issue

* Do not clear content settings on install

* Fix parent domain issue

* Update context menu text

* ref: refactor var name

* ref: not using getDotPrefixedDomain as isCookieDomainInAllowList function internally calls it.

* ref: update var name

* Fix undefined domain issue in cookie.parsedCookie

---------

Co-authored-by: Mayank Rana <mayankranax1@gmail.com>
  • Loading branch information
mohdsayed and mayan-000 authored Jan 30, 2024
1 parent 0bc4677 commit de6ac15
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 34 deletions.
10 changes: 7 additions & 3 deletions packages/extension/src/serviceWorker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ chrome.runtime.onInstalled.addListener(async (details) => {
PROMISE_QUEUE.clear();
await PROMISE_QUEUE.add(async () => {
await chrome.storage.local.clear();
chrome.contentSettings.cookies.clear({});

if (details.reason === 'install') {
await chrome.storage.sync.clear();
Expand Down Expand Up @@ -537,8 +536,13 @@ chrome.storage.local.onChanged.addListener(
* Fires when the browser window is opened.
* @see https://developer.chrome.com/docs/extensions/reference/api/windows#event-onCreated
*/
chrome.windows.onCreated.addListener(() => {
chrome.contentSettings.cookies.clear({});
chrome.windows.onCreated.addListener(async () => {
const totalWindows = await chrome.windows.getAll();

// We do not want to clear content settings if a user has create one more window.
if (totalWindows.length < 2) {
chrome.contentSettings.cookies.clear({});
}
});

chrome.storage.sync.onChanged.addListener(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import setParentDomain from './useAllowedList/setParentDomain';
import onAllowListClick from './useAllowedList/onAllowListClick';
import reloadCurrentTab from '../../../../../utils/reloadCurrentTab';
import getDotPrefixedDomain from './useAllowedList/getDotPrefixedDomain';
import isCookieDomainInAllowList from './useAllowedList/isCookieDomainInAllowList';

interface RowContextMenuProps {
domainsInAllowList: Set<string>;
Expand Down Expand Up @@ -69,16 +70,15 @@ const RowContextMenu = forwardRef<
const [parentDomain, setParentDomainCallback] = useState<string>('');
const [selectedCookie, setSelectedCookie] = useState<CookieTableData>();

const [domain, name] = useMemo(
const [domain, dotPrefixedDomain, name] = useMemo(
() => [
selectedCookie?.parsedCookie?.domain || '',
getDotPrefixedDomain(selectedCookie?.parsedCookie?.domain || ''),
selectedCookie?.parsedCookie?.name,
],
[selectedCookie]
);

const _domain = useMemo(() => getDotPrefixedDomain(domain), [domain]);

const handleRightClick = useCallback(
(e: React.MouseEvent<HTMLElement>, { originalData }: TableRow) => {
e.preventDefault();
Expand All @@ -94,28 +94,20 @@ const RowContextMenu = forwardRef<

useEffect(() => {
(async () => {
await setParentDomain(_domain || '', setParentDomainCallback);
await setParentDomain(dotPrefixedDomain || '', setParentDomainCallback);
})();
}, [_domain, setParentDomainCallback]);
}, [dotPrefixedDomain, setParentDomainCallback, contextMenuOpen]);

useImperativeHandle(ref, () => ({
onRowContextMenu(e, row) {
handleRightClick(e, row);
},
}));

const isDomainInAllowList = useMemo(() => {
let _isDomainInAllowList = domainsInAllowList.has(_domain);

if (!_isDomainInAllowList) {
_isDomainInAllowList = [...domainsInAllowList].some((storedDomain) => {
// For example xyz.bbc.com and .bbc.com
return _domain.endsWith(storedDomain) && _domain !== storedDomain;
});
}

return _isDomainInAllowList;
}, [_domain, domainsInAllowList]);
const isDomainInAllowList = isCookieDomainInAllowList(
dotPrefixedDomain,
domainsInAllowList
);

const handleCopy = useCallback(() => {
try {
Expand All @@ -139,18 +131,18 @@ const RowContextMenu = forwardRef<

removeSelectedRow();
await onAllowListClick(
_domain,
dotPrefixedDomain,
tabUrl || '',
isIncognito,
domainsInAllowList.has(_domain),
domainsInAllowList.has(dotPrefixedDomain),
domainsInAllowList,
setDomainsInAllowListCallback
);
setContextMenuOpen(false);
await reloadCurrentTab();
},
[
_domain,
dotPrefixedDomain,
domainsInAllowList,
isIncognito,
removeSelectedRow,
Expand Down Expand Up @@ -222,7 +214,7 @@ const RowContextMenu = forwardRef<
<span id="allow-list-option">
{isDomainInAllowList
? 'Remove Domain from Allow List'
: 'Add Domain to Allow List'}
: 'Allow Domain During Session'}
</span>
</button>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,27 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* External dependencies.
*/
import type { CookieTableData } from '@ps-analysis-tool/common';

/**
* Internal dependencies.
*/
import getDotPrefixedDomain from './getDotPrefixedDomain';

const isCookieDomainInAllowList = (
cookie: CookieTableData,
domain: string,
domainsInAllowList: Set<string>
) => {
const domain = getDotPrefixedDomain(cookie.parsedCookie?.domain || '');
const dotPrefixedDomain = getDotPrefixedDomain(domain);

let isDomainInAllowList = domainsInAllowList.has(domain);
let isDomainInAllowList = domainsInAllowList.has(dotPrefixedDomain);

if (!isDomainInAllowList) {
isDomainInAllowList = [...domainsInAllowList].some((storedDomain) => {
// For example xyz.bbc.com and .bbc.com
return domain.endsWith(storedDomain) && domain !== storedDomain;
return (
dotPrefixedDomain.endsWith(storedDomain) &&
dotPrefixedDomain !== storedDomain
);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const setParentDomain = async (
let parentDomainValue = '';

if (!allowListSessionStorage || allowListSessionStorage?.length === 0) {
setter(() => '');
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const useHighlighting = (
acc[key] = {
...cookie,
isDomainInAllowList: isCookieDomainInAllowList(
cookie,
cookie.parsedCookie?.domain || '',
domainsInAllowList
),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ describe('RowContextMenu', () => {
);
});

const rowContextMenu = await screen.findByText('Add Domain to Allow List');
const rowContextMenu = await screen.findByText(
'Allow Domain During Session'
);

expect(rowContextMenu).toBeVisible();
});
Expand Down

0 comments on commit de6ac15

Please sign in to comment.