Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Domain-locked web tokens. #5894

Merged
merged 3 commits into from
Jun 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 5 additions & 5 deletions dapps/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ use std::path::PathBuf;
use std::sync::Arc;
use std::collections::HashMap;

use jsonrpc_http_server::{self as http, hyper};
use jsonrpc_http_server::{self as http, hyper, Origin};

use fetch::Fetch;
use parity_reactor::Remote;
Expand All @@ -90,12 +90,12 @@ impl<F> SyncStatus for F where F: Fn() -> bool + Send + Sync {

/// Validates Web Proxy tokens
pub trait WebProxyTokens: Send + Sync {
/// Should return true if token is a valid web proxy access token.
fn is_web_proxy_token_valid(&self, token: &str) -> bool;
/// Should return a domain allowed to be accessed by this token or `None` if the token is not valid
fn domain(&self, token: &str) -> Option<Origin>;
}

impl<F> WebProxyTokens for F where F: Fn(String) -> bool + Send + Sync {
fn is_web_proxy_token_valid(&self, token: &str) -> bool { self(token.to_owned()) }
impl<F> WebProxyTokens for F where F: Fn(String) -> Option<Origin> + Send + Sync {
fn domain(&self, token: &str) -> Option<Origin> { self(token.to_owned()) }
}

/// Current supported endpoints.
Expand Down
40 changes: 31 additions & 9 deletions dapps/src/tests/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ fn should_encode_and_decode_base32() {
#[test]
fn should_stream_web_content() {
// given
let (server, fetch) = serve_with_fetch("token");
let (server, fetch) = serve_with_fetch("token", "https://parity.io");

// when
let response = request(server,
Expand All @@ -335,7 +335,7 @@ fn should_stream_web_content() {
#[test]
fn should_support_base32_encoded_web_urls() {
// given
let (server, fetch) = serve_with_fetch("token");
let (server, fetch) = serve_with_fetch("token", "https://parity.io");

// when
let response = request(server,
Expand All @@ -358,7 +358,7 @@ fn should_support_base32_encoded_web_urls() {
#[test]
fn should_correctly_handle_long_label_when_splitted() {
// given
let (server, fetch) = serve_with_fetch("xolrg9fePeQyKLnL");
let (server, fetch) = serve_with_fetch("xolrg9fePeQyKLnL", "https://contribution.melonport.com");

// when
let response = request(server,
Expand All @@ -382,7 +382,7 @@ fn should_correctly_handle_long_label_when_splitted() {
#[test]
fn should_support_base32_encoded_web_urls_as_path() {
// given
let (server, fetch) = serve_with_fetch("token");
let (server, fetch) = serve_with_fetch("token", "https://parity.io");

// when
let response = request(server,
Expand All @@ -402,10 +402,32 @@ fn should_support_base32_encoded_web_urls_as_path() {
fetch.assert_no_more_requests();
}

#[test]
fn should_return_error_on_non_whitelisted_domain() {
// given
let (server, fetch) = serve_with_fetch("token", "https://ethcore.io");

// when
let response = request(server,
"\
GET / HTTP/1.1\r\n\
Host: EHQPPSBE5DM78X3GECX2YBVGC5S6JX3S5SMPY.web.web3.site\r\n\
Connection: close\r\n\
\r\n\
"
);

// then
response.assert_status("HTTP/1.1 400 Bad Request");
assert_security_headers_for_embed(&response.headers);

fetch.assert_no_more_requests();
}

#[test]
fn should_return_error_on_invalid_token() {
// given
let (server, fetch) = serve_with_fetch("test");
let (server, fetch) = serve_with_fetch("test", "https://parity.io");

// when
let response = request(server,
Expand All @@ -427,7 +449,7 @@ fn should_return_error_on_invalid_token() {
#[test]
fn should_return_error_on_invalid_protocol() {
// given
let (server, fetch) = serve_with_fetch("token");
let (server, fetch) = serve_with_fetch("token", "ftp://parity.io");

// when
let response = request(server,
Expand All @@ -449,7 +471,7 @@ fn should_return_error_on_invalid_protocol() {
#[test]
fn should_disallow_non_get_requests() {
// given
let (server, fetch) = serve_with_fetch("token");
let (server, fetch) = serve_with_fetch("token", "https://parity.io");

// when
let response = request(server,
Expand All @@ -474,7 +496,7 @@ fn should_disallow_non_get_requests() {
#[test]
fn should_fix_absolute_requests_based_on_referer() {
// given
let (server, fetch) = serve_with_fetch("token");
let (server, fetch) = serve_with_fetch("token", "https://parity.io");

// when
let response = request(server,
Expand All @@ -497,7 +519,7 @@ fn should_fix_absolute_requests_based_on_referer() {
#[test]
fn should_fix_absolute_requests_based_on_referer_in_url() {
// given
let (server, fetch) = serve_with_fetch("token");
let (server, fetch) = serve_with_fetch("token", "https://parity.io");

// when
let response = request(server,
Expand Down
8 changes: 5 additions & 3 deletions dapps/src/tests/helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,15 @@ pub fn serve_with_registrar_and_fetch_and_threads(multi_threaded: bool) -> (Serv
(server, fetch, reg)
}

pub fn serve_with_fetch(web_token: &'static str) -> (Server, FakeFetch) {
pub fn serve_with_fetch(web_token: &'static str, domain: &'static str) -> (Server, FakeFetch) {
let fetch = FakeFetch::default();
let f = fetch.clone();
let (server, _) = init_server(move |builder| {
builder
.fetch(f.clone())
.web_proxy_tokens(Arc::new(move |token| &token == web_token))
.web_proxy_tokens(Arc::new(move |token| {
if &token == web_token { Some(domain.into()) } else { None }
}))
}, Default::default(), Remote::new_sync());

(server, fetch)
Expand Down Expand Up @@ -147,7 +149,7 @@ impl ServerBuilder {
dapps_path: dapps_path.as_ref().to_owned(),
registrar: registrar,
sync_status: Arc::new(|| false),
web_proxy_tokens: Arc::new(|_| false),
web_proxy_tokens: Arc::new(|_| None),
signer_address: None,
allowed_hosts: DomainsValidation::Disabled,
remote: remote,
Expand Down
12 changes: 9 additions & 3 deletions dapps/src/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,14 @@ impl<F: Fetch> WebHandler<F> {
let target_url = token_it.next();

// Check if token supplied in URL is correct.
match token {
Some(token) if self.web_proxy_tokens.is_web_proxy_token_valid(token) => {},
let domain = match token.and_then(|token| self.web_proxy_tokens.domain(token)) {
Some(domain) => domain,
_ => {
return Err(State::Error(ContentHandler::error(
StatusCode::BadRequest, "Invalid Access Token", "Invalid or old web proxy access token supplied.", Some("Try refreshing the page."), self.embeddable_on.clone()
)));
}
}
};

// Validate protocol
let mut target_url = match target_url {
Expand All @@ -152,6 +152,12 @@ impl<F: Fetch> WebHandler<F> {
}
};

if !target_url.starts_with(&*domain) {
return Err(State::Error(ContentHandler::error(
StatusCode::BadRequest, "Invalid Domain", "Dapp attempted to access invalid domain.", Some(&target_url), self.embeddable_on.clone(),
)));
}

if !target_url.ends_with("/") {
target_url = format!("{}/", target_url);
}
Expand Down
4 changes: 2 additions & 2 deletions js/src/api/rpc/signer/signer.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ export default class Signer {
.execute('signer_generateAuthorizationToken');
}

generateWebProxyAccessToken () {
generateWebProxyAccessToken (domain) {
return this._transport
.execute('signer_generateWebProxyAccessToken');
.execute('signer_generateWebProxyAccessToken', domain);
}

rejectRequest (requestId) {
Expand Down
6 changes: 5 additions & 1 deletion js/src/jsonrpc/interfaces/signer.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ export default {

generateWebProxyAccessToken: {
desc: 'Generates a new web proxy access token.',
params: [],
params: [{
type: String,
desc: 'Domain for which the token is valid. Only requests to this domain will be allowed.',
example: 'https://parity.io'
}],
returns: {
type: String,
desc: 'The new web proxy access token.',
Expand Down
20 changes: 11 additions & 9 deletions js/src/views/Web/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,17 @@ export default class Store {
}

@action gotoUrl = (_url) => {
transaction(() => {
let url = (_url || this.nextUrl).trim().replace(/\/+$/, '');
let url = (_url || this.nextUrl).trim().replace(/\/+$/, '');

if (!hasProtocol.test(url)) {
url = `https://${url}`;
}
if (!hasProtocol.test(url)) {
url = `https://${url}`;
}

this.setNextUrl(url);
this.setCurrentUrl(this.nextUrl);
return this.generateToken(url).then(() => {
transaction(() => {
this.setNextUrl(url);
this.setCurrentUrl(this.nextUrl);
});
});
}

Expand Down Expand Up @@ -134,11 +136,11 @@ export default class Store {
this.nextUrl = url;
}

generateToken = () => {
generateToken = (_url) => {
this.setToken(null);

return this._api.signer
.generateWebProxyAccessToken()
.generateWebProxyAccessToken(_url)
.then((token) => {
this.setToken(token);
})
Expand Down
11 changes: 6 additions & 5 deletions js/src/views/Web/store.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,16 @@ describe('views/Web/Store', () => {
describe('gotoUrl', () => {
it('uses the nextUrl when none specified', () => {
store.setNextUrl('https://parity.io');
store.gotoUrl();

expect(store.currentUrl).to.equal('https://parity.io');
return store.gotoUrl().then(() => {
expect(store.currentUrl).to.equal('https://parity.io');
});
});

it('adds https when no protocol', () => {
store.gotoUrl('google.com');

expect(store.currentUrl).to.equal('https://google.com');
return store.gotoUrl('google.com').then(() => {
expect(store.currentUrl).to.equal('https://google.com');
});
});
});

Expand Down
1 change: 0 additions & 1 deletion js/src/views/Web/web.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export default class Web extends Component {

componentDidMount () {
this.store.gotoUrl(this.props.params.url);
return this.store.generateToken();
}

componentWillReceiveProps (props) {
Expand Down
2 changes: 1 addition & 1 deletion parity/dapps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ mod server {
) -> Result<Middleware, String> {
let signer = deps.signer;
let parity_remote = parity_reactor::Remote::new(deps.remote.clone());
let web_proxy_tokens = Arc::new(move |token| signer.is_valid_web_proxy_access_token(&token));
let web_proxy_tokens = Arc::new(move |token| signer.web_proxy_access_token_domain(&token));

Ok(parity_dapps::Middleware::dapps(
parity_remote,
Expand Down
11 changes: 6 additions & 5 deletions rpc/src/v1/helpers/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

use std::sync::Arc;
use std::ops::Deref;
use http::Origin;
use util::Mutex;
use transient_hashmap::TransientHashMap;

Expand All @@ -29,7 +30,7 @@ const TOKEN_LIFETIME_SECS: u32 = 3600;
pub struct SignerService {
is_enabled: bool,
queue: Arc<ConfirmationsQueue>,
web_proxy_tokens: Mutex<TransientHashMap<String, ()>>,
web_proxy_tokens: Mutex<TransientHashMap<String, Origin>>,
generate_new_token: Box<Fn() -> Result<String, String> + Send + Sync + 'static>,
}

Expand All @@ -46,16 +47,16 @@ impl SignerService {
}

/// Checks if the token is valid web proxy access token.
pub fn is_valid_web_proxy_access_token(&self, token: &String) -> bool {
self.web_proxy_tokens.lock().contains_key(&token)
pub fn web_proxy_access_token_domain(&self, token: &String) -> Option<Origin> {
self.web_proxy_tokens.lock().get(token).cloned()
}

/// Generates a new web proxy access token.
pub fn generate_web_proxy_access_token(&self) -> String {
pub fn generate_web_proxy_access_token(&self, domain: Origin) -> String {
let token = random_string(16);
let mut tokens = self.web_proxy_tokens.lock();
tokens.prune();
tokens.insert(token.clone(), ());
tokens.insert(token.clone(), domain);
token
}

Expand Down
4 changes: 2 additions & 2 deletions rpc/src/v1/impls/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ impl<D: Dispatcher + 'static> Signer for SignerClient<D> {
.map_err(|e| errors::token(e))
}

fn generate_web_proxy_token(&self) -> Result<String, Error> {
Ok(self.signer.generate_web_proxy_access_token())
fn generate_web_proxy_token(&self, domain: String) -> Result<String, Error> {
Ok(self.signer.generate_web_proxy_access_token(domain.into()))
}

fn subscribe_pending(&self, _meta: Self::Metadata, sub: Subscriber<Vec<ConfirmationRequest>>) {
Expand Down
4 changes: 2 additions & 2 deletions rpc/src/v1/traits/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ build_rpc_trait! {
#[rpc(name = "signer_generateAuthorizationToken")]
fn generate_token(&self) -> Result<String, Error>;

/// Generates new web proxy access token.
/// Generates new web proxy access token for particular domain.
#[rpc(name = "signer_generateWebProxyAccessToken")]
fn generate_web_proxy_token(&self) -> Result<String, Error>;
fn generate_web_proxy_token(&self, String) -> Result<String, Error>;

#[pubsub(name = "signer_pending")] {
/// Subscribe to new pending requests on signer interface.
Expand Down