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.

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

feat: improved experience on DNSLink websites #826

Merged
merged 3 commits into from
Dec 6, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
20 changes: 18 additions & 2 deletions add-on/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,18 @@
"message": "Redirect requests for IPFS resources to the Custom gateway",
"description": "An option description on the Preferences screen (option_useCustomGateway_description)"
},
"option_dnslinkRedirect_title": {
"message": "Force load from Custom Gateway",
Copy link

Choose a reason for hiding this comment

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

Maybe change to: "Force page load from custom gateway"

  1. Explicit language specifying object that is being loaded
  2. Consistent capitalization

Copy link

Choose a reason for hiding this comment

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

Probably fine as is, but I am a little on the fence about "force".

"Always load pages from custom gateway"?

Copy link
Member

Choose a reason for hiding this comment

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

I agree with the language tweak here. This might be out of scope for this PR but do you think it's widely understood what a Custom Gateway is? I would be inclined to be more specific, such as: Always load DNSLink websites via IPFS Companion's local gateway. (if that is a correct statement?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Will tweak the label, but want to keep "force" for now to emphasize its "over the top" behavior (until we can redirect in a way that keeps origin sandbox)

@cwaring ack, lets track that as part of #741

"description": "An option title on the Preferences screen (option_dnslinkRedirect_title)"
},
"option_dnslinkRedirect_description": {
"message": "If global redirect is enabled, this will include DNSLink websites and redirect them to respective /ipns/{fqdn} paths at Custom Gateway",
"description": "An option description on the Preferences screen (option_dnslinkRedirect_description)"
},
"option_dnslinkRedirect_warning": {
"message": "Redirecting to a path-based gateway breaks Origin-based security isolation of DNSLink website! Please leave this disabled unless you are aware of (and ok with) related risks.",
"description": "A warning on the Preferences screen, displayed when URL does not belong to Secure Context (option_customGatewayUrl_warning)"
},
"option_noRedirectHostnames_title": {
"message": "Redirect Opt-Outs",
"description": "An option title on the Preferences screen (option_noRedirectHostnames_title)"
Expand Down Expand Up @@ -327,6 +339,10 @@
"message": "Toggle use of Custom Gateway when IPFS API availability changes",
"description": "An option description on the Preferences screen (option_automaticMode_description)"
},
"option_header_dnslink": {
"message": "DNSLink",
"description": "A section header on the Preferences screen (option_header_dnslink)"
},
"option_header_experiments": {
"message": "Experiments",
"description": "A section header on the Preferences screen (option_header_experiments)"
Expand Down Expand Up @@ -364,11 +380,11 @@
"description": "An option description on the Preferences screen (option_linkify_description)"
},
"option_dnslinkPolicy_title": {
"message": "DNSLink Support",
"message": "DNSLink Lookup",
"description": "An option title on the Preferences screen (option_dnslinkPolicy_title)"
},
"option_dnslinkPolicy_description": {
"message": "Select DNS TXT lookup policy to load IPFS hosted sites over IPFS where possible",
"message": "Lookup policy for displaying context actions on websites with DNSLink",
"description": "An option description on the Preferences screen (option_dnslinkPolicy_description)"
},
"option_dnslinkPolicy_disabled": {
Expand Down
1 change: 1 addition & 0 deletions add-on/src/lib/ipfs-companion.js
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,7 @@ module.exports = async function init () {
case 'preloadAtPublicGateway':
case 'openViaWebUI':
case 'noRedirectHostnames':
case 'dnslinkRedirect':
state[key] = change.newValue
break
}
Expand Down
7 changes: 3 additions & 4 deletions add-on/src/lib/ipfs-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
return redirectToGateway(request.url, state, ipfsPathValidator)
}
// Detect dnslink using heuristics enabled in Preferences
if (state.dnslinkPolicy && dnslinkResolver.canLookupURL(request.url)) {
if (state.dnslinkRedirect && state.dnslinkPolicy && dnslinkResolver.canLookupURL(request.url)) {
const dnslinkRedirect = dnslinkResolver.dnslinkRedirect(request.url)
if (dnslinkRedirect && isSafeToRedirect(request, runtime)) {
// console.log('onBeforeRequest.dnslinkRedirect', dnslinkRedirect)
Expand Down Expand Up @@ -339,10 +339,9 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
if (header.name.toLowerCase() === 'x-ipfs-path' && isSafeToRedirect(request, runtime)) {
// console.log('onHeadersReceived.request.responseHeaders', request.responseHeaders.length)
const xIpfsPath = header.value
log(`detected x-ipfs-path for ${request.url}: ${xIpfsPath}`)
// First: Check if dnslink heuristic yields any results
// Note: this depends on which dnslink lookup policy is selecten in Preferences
if (state.dnslinkPolicy && dnslinkResolver.canLookupURL(request.url)) {
if (state.dnslinkRedirect && state.dnslinkPolicy && dnslinkResolver.canLookupURL(request.url)) {
// x-ipfs-path is a strong indicator of IPFS support
// so we force dnslink lookup to pre-populate dnslink cache
// in a way that works even when state.dnslinkPolicy !== 'enabled'
Expand All @@ -358,7 +357,7 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
if (IsIpfs.ipnsPath(xIpfsPath)) {
// Ignore unhandled IPNS path by this point
// (means DNSLink is disabled so we don't want to make a redirect that works like DNSLink)
log(`onHeadersReceived: ignoring x-ipfs-path=${xIpfsPath} (dnslinkPolicy=false or missing DNS TXT record)`)
// log(`onHeadersReceived: ignoring x-ipfs-path=${xIpfsPath} (dnslinkRedirect=false, dnslinkPolicy=false or missing DNS TXT record)`)
} else if (IsIpfs.ipfsPath(xIpfsPath)) {
// It is possible that someone exposed /ipfs/<cid>/ under /
// and our path-based onBeforeRequest heuristics were unable
Expand Down
1 change: 1 addition & 0 deletions add-on/src/lib/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ exports.optionDefaults = Object.freeze({
automaticMode: true,
linkify: false,
dnslinkPolicy: 'best-effort',
dnslinkRedirect: false,
recoverFailedHttpRequests: true,
detectIpfsPathHeader: true,
preloadAtPublicGateway: true,
Expand Down
70 changes: 70 additions & 0 deletions add-on/src/options/forms/dnslink-form.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
'use strict'
/* eslint-env browser, webextensions */

const browser = require('webextension-polyfill')
const html = require('choo/html')
const switchToggle = require('../../pages/components/switch-toggle')

function dnslinkForm ({
dnslinkPolicy,
dnslinkRedirect,
onOptionChange
}) {
const onDnslinkPolicyChange = onOptionChange('dnslinkPolicy')
const onDnslinkRedirectChange = onOptionChange('dnslinkRedirect')

return html`
<form>
<fieldset>
<legend>${browser.i18n.getMessage('option_header_dnslink')}</legend>
<div>
<label for="dnslinkPolicy">
<dl>
<dt>${browser.i18n.getMessage('option_dnslinkPolicy_title')}</dt>
<dd>
${browser.i18n.getMessage('option_dnslinkPolicy_description')}
<p><a href="https://github.com/ipfs-shipyard/ipfs-companion/blob/master/docs/dnslink.md#dnslink-support-in-ipfs-companion" target="_blank">
${browser.i18n.getMessage('option_legend_readMore')}
</a></p>
</dd>
</dl>
</label>
<select id="dnslinkPolicy" name='dnslinkPolicy' onchange=${onDnslinkPolicyChange}>
<option
value='false'
selected=${String(dnslinkPolicy) === 'false'}>
${browser.i18n.getMessage('option_dnslinkPolicy_disabled')}
</option>
<option
value='best-effort'
selected=${dnslinkPolicy === 'best-effort'}>
${browser.i18n.getMessage('option_dnslinkPolicy_bestEffort')}
</option>
<option
value='enabled'
selected=${dnslinkPolicy === 'enabled'}>
${browser.i18n.getMessage('option_dnslinkPolicy_enabled')}
</option>
</select>
</div>
<div>
<label for="dnslinkRedirect">
<dl>
<dt>${browser.i18n.getMessage('option_dnslinkRedirect_title')}</dt>
<dd>
${browser.i18n.getMessage('option_dnslinkRedirect_description')}
${dnslinkRedirect ? html`<p class="red i">${browser.i18n.getMessage('option_dnslinkRedirect_warning')}</p>` : null}
<p><a href="https://github.com/ipfs-shipyard/ipfs-companion/issues/667" target="_blank">
${browser.i18n.getMessage('option_legend_readMore')}
</a></p>
</dd>
</dl>
</label>
<div>${switchToggle({ id: 'dnslinkRedirect', checked: dnslinkRedirect, onchange: onDnslinkRedirectChange })}</div>
</div>
</fieldset>
</form>
`
}

module.exports = dnslinkForm
32 changes: 0 additions & 32 deletions add-on/src/options/forms/experiments-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ function experimentsForm ({
displayNotifications,
catchUnhandledProtocols,
linkify,
dnslinkPolicy,
recoverFailedHttpRequests,
detectIpfsPathHeader,
ipfsProxy,
Expand All @@ -20,7 +19,6 @@ function experimentsForm ({
const onDisplayNotificationsChange = onOptionChange('displayNotifications')
const onCatchUnhandledProtocolsChange = onOptionChange('catchUnhandledProtocols')
const onLinkifyChange = onOptionChange('linkify')
const onDnslinkPolicyChange = onOptionChange('dnslinkPolicy')
const onrecoverFailedHttpRequestsChange = onOptionChange('recoverFailedHttpRequests')
const onDetectIpfsPathHeaderChange = onOptionChange('detectIpfsPathHeader')
const onIpfsProxyChange = onOptionChange('ipfsProxy')
Expand Down Expand Up @@ -66,36 +64,6 @@ function experimentsForm ({
</label>
<div>${switchToggle({ id: 'linkify', checked: linkify, onchange: onLinkifyChange })}</div>
</div>
<div>
<label for="dnslinkPolicy">
<dl>
<dt>${browser.i18n.getMessage('option_dnslinkPolicy_title')}</dt>
<dd>
${browser.i18n.getMessage('option_dnslinkPolicy_description')}
<p><a href="https://github.com/ipfs-shipyard/ipfs-companion/blob/master/docs/dnslink.md#dnslink-support-in-ipfs-companion" target="_blank">
${browser.i18n.getMessage('option_legend_readMore')}
</a></p>
</dd>
</dl>
</label>
<select id="dnslinkPolicy" name='dnslinkPolicy' onchange=${onDnslinkPolicyChange}>
<option
value='false'
selected=${String(dnslinkPolicy) === 'false'}>
${browser.i18n.getMessage('option_dnslinkPolicy_disabled')}
</option>
<option
value='best-effort'
selected=${dnslinkPolicy === 'best-effort'}>
${browser.i18n.getMessage('option_dnslinkPolicy_bestEffort')}
</option>
<option
value='enabled'
selected=${dnslinkPolicy === 'enabled'}>
${browser.i18n.getMessage('option_dnslinkPolicy_enabled')}
</option>
</select>
</div>
<div>
<label for="detectIpfsPathHeader">
<dl>
Expand Down
7 changes: 6 additions & 1 deletion add-on/src/options/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const html = require('choo/html')
const globalToggleForm = require('./forms/global-toggle-form')
const ipfsNodeForm = require('./forms/ipfs-node-form')
const fileImportForm = require('./forms/file-import-form')
const dnslinkForm = require('./forms/dnslink-form')
const gatewaysForm = require('./forms/gateways-form')
const apiForm = require('./forms/api-form')
const experimentsForm = require('./forms/experiments-form')
Expand Down Expand Up @@ -77,11 +78,15 @@ module.exports = function optionsPage (state, emit) {
preloadAtPublicGateway: state.options.preloadAtPublicGateway,
onOptionChange
})}
${dnslinkForm({
dnslinkPolicy: state.options.dnslinkPolicy,
dnslinkRedirect: state.options.dnslinkRedirect,
onOptionChange
})}
${experimentsForm({
displayNotifications: state.options.displayNotifications,
catchUnhandledProtocols: state.options.catchUnhandledProtocols,
linkify: state.options.linkify,
dnslinkPolicy: state.options.dnslinkPolicy,
recoverFailedHttpRequests: state.options.recoverFailedHttpRequests,
detectIpfsPathHeader: state.options.detectIpfsPathHeader,
ipfsProxy: state.options.ipfsProxy,
Expand Down
3 changes: 2 additions & 1 deletion test/functional/lib/ipfs-request-dnslink.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const fakeRequestId = () => {

// const nodeTypes = ['external', 'embedded']

describe('modifyRequest processing', function () {
describe('modifyRequest processing of DNSLinks', function () {
let state, dnslinkResolver, ipfsPathValidator, modifyRequest, runtime

before(function () {
Expand All @@ -34,6 +34,7 @@ describe('modifyRequest processing', function () {
ipfsNodeType: 'external',
peerCount: 1,
redirect: true,
dnslinkRedirect: true, // NOTE: this is opt-in now
catchUnhandledProtocols: true,
gwURLString: 'http://127.0.0.1:8080',
pubGwURLString: 'https://ipfs.io'
Expand Down