Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require HTTPS before trusting publisher payment information #109

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Require HTTPS before trusting publisher payment information #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
816911a
Require HTTPS when retrieving payment info
da2x Feb 16, 2017
9b694f6
Support 304 Not Modified responses (caching)
da2x Feb 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 37 additions & 31 deletions shared/scripts/contentscript.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,50 +86,56 @@ function findDomain() {
return domain;
}

// Find all links on the page.
var links = selectAll('link');

// Build up an object with page and author details for the extension.
var messageBody = {
hostname: findDomain(),
list: []
};

// Iterate over all links and filter down to the last link that contains the
// correct metadata.

if (!domains[messageBody.hostname]) {
// Enforce HTTPS requirement for parsing payment info from page. Page
// can still be HTTP, but then /tipsy.txt must be served over HTTPS.
if (document.location.protocol == 'https:') {

messageBody.list = links.filter(function(link) {
return link.rel === 'author';
}).map(function(link) {
var author = {};
// Find all links on the page.
var links = selectAll('link');

// Personal identification.
author.name = link.getAttribute('name');
author.href = link.getAttribute('href');
author.gravatar = link.getAttribute('gravatar');
// Iterate over all links and filter down to the last link that contains the
// correct metadata.

// Payment information.
author.dwolla = link.getAttribute('dwolla') || link.getAttribute('data-dwolla');
author.bitcoin = link.getAttribute('bitcoin') || link.getAttribute('data-bitcoin');
author.paypal = link.getAttribute('paypal') || link.getAttribute('data-paypal');
author.stripe = link.getAttribute('stripe') || link.getAttribute('data-stripe');
return author;
});

} else {
var newArray = [];
newArray[0] = domains[messageBody.hostname];
var author = newArray;
messageBody.list = author;
}
if (!domains[messageBody.hostname]) {

messageBody.list = links.filter(function(link) {
return link.rel === 'author';
}).map(function(link) {
var author = {};

// if nothing, check for tipsy.txt
if (messageBody.list.length === 0) {
// Personal identification.
author.name = link.getAttribute('name');
author.href = link.getAttribute('href');
author.gravatar = link.getAttribute('gravatar');

// Payment information.
author.dwolla = link.getAttribute('dwolla') || link.getAttribute('data-dwolla');
author.bitcoin = link.getAttribute('bitcoin') || link.getAttribute('data-bitcoin');
author.paypal = link.getAttribute('paypal') || link.getAttribute('data-paypal');
author.stripe = link.getAttribute('stripe') || link.getAttribute('data-stripe');
return author;
});


} else {
var newArray = [];
newArray[0] = domains[messageBody.hostname];
var author = newArray;
messageBody.list = author;
}

// if nothing, check for tipsy.txt
if (messageBody.list.length === 0) {
messageBody.list = parseTxt();
}
} else {
// Page loaded over HTTP, but give /tipsy.txt a shot (also requires HTTPS)
messageBody.list = parseTxt();
}

Expand Down
15 changes: 13 additions & 2 deletions shared/scripts/lib/utils/tipsy-txt-parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,12 @@ export function parseTxt() {
}
}


if (info == null || shouldRenew) {
var req = new XMLHttpRequest();
try {
req.open('GET', "/tipsy.txt", false);
try {
// Enforce HTTPS requirement when retrieving payment info.
req.open('GET', 'https://' + document.domain + '/tipsy.txt', false);
} catch (e) {

}
Expand All @@ -109,6 +111,15 @@ export function parseTxt() {
serialInfo.prevTime = Date.now().toString();
localStorage.setItem(document.domain, JSON.stringify(serialInfo));
}
} if (req.status == 304) {
info = localStorage.getItem(document.domain);
if (info !== undefined || info !== '') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too nested here. In the last else clause, there is another info = localStorage.getItem(document.domain), which also needs a check if info is not empty. Actually, these steps could be packaged into {getInfo, updateInfo, initInfo} (local operation) and syncInfo (remote operation). The if (info != null) check afterward can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I don’t get emails from this repo, so I’m not notified of the review.) You’re complaining about existing code and not my code changes, right? I’m checking to make sure there is a locally stored item (cached information) before updating the prevTime. All other uses of setItem replaces the existing item where as this updates the item already in storage. I can refactor everything here, but that would be a separate issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, to be precise. The diff algo / git-blame couldn't capture that the lines are being moved instead of rewritten. Yes, it was a cleanup request, and I could do it later as well once this PR is merged.

info = JSON.parse(info);
info.prevTime = Date.now().toString();
localStorage.setItem(document.domain, JSON.stringify(info));
} else {
localStorage.setItem(document.domain, JSON.stringify({'tipsyTried': Date.now()}));
}
} else {
localStorage.setItem(document.domain, JSON.stringify({'tipsyTried': Date.now()}));
}
Expand Down