Skip to content

Commit

Permalink
fix: avoid email delegation via GET request (#430)
Browse files Browse the repository at this point in the history
The email validation approval process is now split into two stages: a
GET request with no side effects except to load a page, that then
auto-submits a POST request to actually continue the flow.

## Summary of problem

This fixes the API so as to follow [proper HTTP
semantics](#333 (comment)):

> The purpose of distinguishing between safe [i.e. like GET] and unsafe
[like PUT/POST] methods is to allow automated retrieval processes
(spiders) and cache performance optimization (pre-fetching) to work
without fear of causing harm. In addition, it allows a user agent to
apply appropriate constraints on the automated use of unsafe methods
when processing potentially untrusted content.

That is, a `PUT` or `POST` (rather than a `GET`) **must** be the method
used in order to do things like

* cause a message to be sent (forwarding a UCAN delegation via a
separate connection's websocket)
* cause an untrusted keypair to be associated with a billable email
address (which is the outcome of that forwarding, in practice!)

Fixing the HTTP semantics should address all of #348, and is the first
step to addressing the security concerns in #333.

## Summary of solution

Clicking (or scanning/pre-fetching/previewing/etc.) the link in the
email no longer finishes the validation process. Instead, it loads a
(harmless to scan/pre-fetch/preview) landing page which simply says
"Validating Email" while using JavaScript to auto-complete the process.

This patch is able to fix the core HTTP semantics in a very self-contained way:

* no changes needed to the email templates
* will not break any existing unexpired links at the moment it is
deployed
* is essentially the exact same UX from a user's perspective (they might
notice just a little extra blink)
* does degrade gracefully if user has JS disabled, and any non-browser
clients could still trigger the `POST` ± just as easy as before
* no changes needed on the `w3ui` side for this part of the email
validation improvements

---------

Co-authored-by: Nathan Vander Wilt <[email protected]>
  • Loading branch information
travis and natevw authored Feb 10, 2023
1 parent a53d6e6 commit d282d6a
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 5 deletions.
5 changes: 3 additions & 2 deletions packages/access-api/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { notFound } from '@web3-storage/worker-utils/response'
import { Router } from '@web3-storage/worker-utils/router'
import { postRaw } from './routes/raw.js'
import { postRoot } from './routes/root.js'
import { validateEmail } from './routes/validate-email.js'
import { preValidateEmail, validateEmail } from './routes/validate-email.js'
import { validateWS } from './routes/validate-ws.js'
import { version } from './routes/version.js'
import { getContext } from './utils/context.js'
Expand All @@ -14,7 +14,8 @@ const r = new Router({ onNotFound: notFound })

r.add('options', '*', preflight)
r.add('get', '/version', version)
r.add('get', '/validate-email', validateEmail)
r.add('get', '/validate-email', preValidateEmail)
r.add('post', '/validate-email', validateEmail)
r.add('get', '/validate-ws', validateWS)
r.add('post', '/', postRoot)
r.add('post', '/raw', postRaw)
Expand Down
15 changes: 15 additions & 0 deletions packages/access-api/src/routes/validate-email.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,23 @@ import {
HtmlResponse,
ValidateEmail,
ValidateEmailError,
PendingValidateEmail,
} from '../utils/html.js'

/**
* @param {import('@web3-storage/worker-utils/router').ParsedRequest} req
* @param {import('../bindings.js').RouteContext} env
*/
export async function preValidateEmail(req, env) {
if (!req.query?.ucan) {
return new HtmlResponse(
<ValidateEmailError msg={'Missing delegation in the URL.'} />
)
}

return new HtmlResponse(<PendingValidateEmail autoApprove={true} />)
}

/**
* @param {import('@web3-storage/worker-utils/router').ParsedRequest} req
* @param {import('../bindings.js').RouteContext} env
Expand Down
34 changes: 34 additions & 0 deletions packages/access-api/src/utils/html.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,40 @@ export class HtmlResponse extends Response {
}
}

/**
*
* @param {object} props
* @param {boolean} [props.autoApprove]
*/
export const PendingValidateEmail = ({ autoApprove }) => (
<div class="fcenter">
<img
src="https://web3.storage/android-chrome-512x512.png"
height="80"
width="80"
/>
<div>
<h1>Validating Email</h1>
<form id="approval" method="post" class="fcenter">
<button class="mcenter">Approve</button>
</form>
{autoApprove ? (
<script
dangerouslySetInnerHTML={{
// NOTE: this script sticks to ES3-era syntax for compat with more browsers
__html: `(function () {
// auto-submit the form for any user w/JS enabled
var form = document.getElementById('approval');
form.style.display = 'none';
form.submit();
})();`,
}}
/>
) : undefined}
</div>
</div>
)

/**
*
* @param {object} param0
Expand Down
4 changes: 2 additions & 2 deletions packages/access-api/test/access-authorize.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('access/authorize', function () {
/** @type {import('@web3-storage/access/types').EncodedDelegation<[import('@web3-storage/capabilities/types').AccessSession]>} */ (
url.searchParams.get('ucan')
)
const rsp = await mf.dispatchFetch(url)
const rsp = await mf.dispatchFetch(url, { method: 'POST' })
const html = await rsp.text()

assert(html.includes(encoded))
Expand Down Expand Up @@ -119,7 +119,7 @@ describe('access/authorize', function () {

const url = new URL(inv)
// click email url
await mf.dispatchFetch(url)
await mf.dispatchFetch(url, { method: 'POST' })

// ws
const res = await mf.dispatchFetch('http://localhost:8787/validate-ws', {
Expand Down
2 changes: 1 addition & 1 deletion packages/access-api/test/space-recover.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ describe('space-recover', function () {
assert.deepEqual(del.audience.did(), issuer.did())
assert.deepEqual(del.issuer.did(), service.did())
assert.deepEqual(del.capabilities[0].can, 'space/recover')
const rsp = await mf.dispatchFetch(url)
const rsp = await mf.dispatchFetch(url, { method: 'POST' })
const html = await rsp.text()

assert(html.includes(encoded))
Expand Down

0 comments on commit d282d6a

Please sign in to comment.