Skip to content

Commit

Permalink
Fix initialization of "detection-repo" span (#58)
Browse files Browse the repository at this point in the history
The "detection-repo" span lists which repositories/projects are
responsible for this advisory. The idea was that we would update this
dynamically to tell the user who was responsible for the block.

This initialization was moved from the HTML document to `index.ts`,
which is where most of the other page initialization logic is.

The markup was updated to provide a default value, so that the page
looks less broken when we don't know the source.

But most importantly, the way we're checking the source was fixed.
Previously we were relying on `window.location.href` including a string
that would indicate the source, but this is never set in practice.

Instead it now looks at the `newIssueUrl` query parameter, which is
currently how the extension communicates the source of each block.
  • Loading branch information
Gudahtt authored Feb 16, 2023
1 parent ee2db54 commit c267dbb
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 11 deletions.
49 changes: 49 additions & 0 deletions src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,55 @@ describe('Phishing warning page', () => {
expect(suspectLink?.innerText).toBe('https://example.com');
});

it('should default to crediting both projects', async () => {
const detectionRepo = window.document.getElementById('detection-repo');

expect(detectionRepo?.innerHTML).toBe(
'Ethereum Phishing Detector and PhishFort',
);
});

it('should credit Ethereum Phishing Detector when the source is not provided', async () => {
window.document.location.href = getUrl(
'example.com',
'https://example.com',
);
// non-null assertion used because TypeScript doesn't know the event handler was run
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
onDomContentLoad!(new Event('DOMContentLoaded'));
const detectionRepo = window.document.getElementById('detection-repo');

expect(detectionRepo?.innerText).toBe('Ethereum Phishing Detector');
});

it('should credit Ethereum Phishing Detector when the block source is MetaMask', async () => {
window.document.location.href = getUrl(
'example.com',
'https://example.com',
'https://github.com/metamask/eth-phishing-detect/issues/new',
);
// non-null assertion used because TypeScript doesn't know the event handler was run
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
onDomContentLoad!(new Event('DOMContentLoaded'));
const detectionRepo = window.document.getElementById('detection-repo');

expect(detectionRepo?.innerText).toBe('Ethereum Phishing Detector');
});

it('should credit PhishFort when the block source is PhishFort', async () => {
window.document.location.href = getUrl(
'example.com',
'https://example.com',
'https://github.com/phishfort/phishfort-lists/issues/new',
);
// non-null assertion used because TypeScript doesn't know the event handler was run
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
onDomContentLoad!(new Event('DOMContentLoaded'));
const detectionRepo = window.document.getElementById('detection-repo');

expect(detectionRepo?.innerText).toBe('PhishFort');
});

it.todo(
'should add site to safelist when the user continues at their own risk',
);
Expand Down
12 changes: 12 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ function start() {
throw new Error('Unable to locate new issue link');
}

const detectionRepo = document.getElementById('detection-repo');
if (!detectionRepo) {
throw new Error('Unable to locate detection repo span');
}

const newIssueUrl =
hashQueryString.get('newIssueUrl') ||
`https://github.com/MetaMask/eth-phishing-detect/issues/new`;
Expand All @@ -138,6 +143,13 @@ function start() {
)}&body=${encodeURIComponent(suspectHref)}`;
newIssueLink.setAttribute('href', `${newIssueUrl}${newIssueParams}`);

const blockedByMetamask = newIssueUrl.includes('eth-phishing-detect');
if (blockedByMetamask) {
detectionRepo.innerText = 'Ethereum Phishing Detector';
} else {
detectionRepo.innerText = 'PhishFort';
}

const continueLink = document.getElementById('unsafe-continue');
if (!continueLink) {
throw new Error('Unable to locate unsafe continue link');
Expand Down
12 changes: 1 addition & 11 deletions static/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,6 @@
document.getElementById('antiClickjack').innerHTML =
'#content__framed-body { display: none !important; }';
}

window.onload = function() {
if((window.location.href).includes('eth-phishing-detect')){
document.getElementById("detection-repo").innerText = 'Ethereum Phishing Detector';
}
if((window.location.href).includes('phishfort-lists')){
document.getElementById("detection-repo").innerText = 'PhishFort';
}

}
</script>
<title>MetaMask Phishing Detection</title>
<script
Expand Down Expand Up @@ -76,7 +66,7 @@ <h1>
</ul>
</p>
</br>
<p>Advisory provided by <span id="detection-repo"></span>.</p>
<p>Advisory provided by <span id="detection-repo">Ethereum Phishing Detector and PhishFort</span>.</p>
</br>
</br>
<p>If we're flagging a legitimate website, please <a id="new-issue-link" rel="noopener" target='_blank' href="https://github.com/metamask/eth-phishing-detect/issues/new">report a detection problem.</a></p>
Expand Down

0 comments on commit c267dbb

Please sign in to comment.