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

Commit 53609f7

Browse files
tomusdrwgavofyork
authored andcommitted
Domain-locked web tokens. (#5894)
* Domain-locking web tokens. * JS part. * Fix linting issues.
1 parent 4d5280e commit 53609f7

File tree

13 files changed

+85
-48
lines changed

13 files changed

+85
-48
lines changed

Diff for: dapps/src/lib.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ use std::path::PathBuf;
7171
use std::sync::Arc;
7272
use std::collections::HashMap;
7373

74-
use jsonrpc_http_server::{self as http, hyper};
74+
use jsonrpc_http_server::{self as http, hyper, Origin};
7575

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

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

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

101101
/// Current supported endpoints.

Diff for: dapps/src/tests/fetch.rs

+31-9
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ fn should_encode_and_decode_base32() {
312312
#[test]
313313
fn should_stream_web_content() {
314314
// given
315-
let (server, fetch) = serve_with_fetch("token");
315+
let (server, fetch) = serve_with_fetch("token", "https://parity.io");
316316

317317
// when
318318
let response = request(server,
@@ -335,7 +335,7 @@ fn should_stream_web_content() {
335335
#[test]
336336
fn should_support_base32_encoded_web_urls() {
337337
// given
338-
let (server, fetch) = serve_with_fetch("token");
338+
let (server, fetch) = serve_with_fetch("token", "https://parity.io");
339339

340340
// when
341341
let response = request(server,
@@ -358,7 +358,7 @@ fn should_support_base32_encoded_web_urls() {
358358
#[test]
359359
fn should_correctly_handle_long_label_when_splitted() {
360360
// given
361-
let (server, fetch) = serve_with_fetch("xolrg9fePeQyKLnL");
361+
let (server, fetch) = serve_with_fetch("xolrg9fePeQyKLnL", "https://contribution.melonport.com");
362362

363363
// when
364364
let response = request(server,
@@ -382,7 +382,7 @@ fn should_correctly_handle_long_label_when_splitted() {
382382
#[test]
383383
fn should_support_base32_encoded_web_urls_as_path() {
384384
// given
385-
let (server, fetch) = serve_with_fetch("token");
385+
let (server, fetch) = serve_with_fetch("token", "https://parity.io");
386386

387387
// when
388388
let response = request(server,
@@ -402,10 +402,32 @@ fn should_support_base32_encoded_web_urls_as_path() {
402402
fetch.assert_no_more_requests();
403403
}
404404

405+
#[test]
406+
fn should_return_error_on_non_whitelisted_domain() {
407+
// given
408+
let (server, fetch) = serve_with_fetch("token", "https://ethcore.io");
409+
410+
// when
411+
let response = request(server,
412+
"\
413+
GET / HTTP/1.1\r\n\
414+
Host: EHQPPSBE5DM78X3GECX2YBVGC5S6JX3S5SMPY.web.web3.site\r\n\
415+
Connection: close\r\n\
416+
\r\n\
417+
"
418+
);
419+
420+
// then
421+
response.assert_status("HTTP/1.1 400 Bad Request");
422+
assert_security_headers_for_embed(&response.headers);
423+
424+
fetch.assert_no_more_requests();
425+
}
426+
405427
#[test]
406428
fn should_return_error_on_invalid_token() {
407429
// given
408-
let (server, fetch) = serve_with_fetch("test");
430+
let (server, fetch) = serve_with_fetch("test", "https://parity.io");
409431

410432
// when
411433
let response = request(server,
@@ -427,7 +449,7 @@ fn should_return_error_on_invalid_token() {
427449
#[test]
428450
fn should_return_error_on_invalid_protocol() {
429451
// given
430-
let (server, fetch) = serve_with_fetch("token");
452+
let (server, fetch) = serve_with_fetch("token", "ftp://parity.io");
431453

432454
// when
433455
let response = request(server,
@@ -449,7 +471,7 @@ fn should_return_error_on_invalid_protocol() {
449471
#[test]
450472
fn should_disallow_non_get_requests() {
451473
// given
452-
let (server, fetch) = serve_with_fetch("token");
474+
let (server, fetch) = serve_with_fetch("token", "https://parity.io");
453475

454476
// when
455477
let response = request(server,
@@ -474,7 +496,7 @@ fn should_disallow_non_get_requests() {
474496
#[test]
475497
fn should_fix_absolute_requests_based_on_referer() {
476498
// given
477-
let (server, fetch) = serve_with_fetch("token");
499+
let (server, fetch) = serve_with_fetch("token", "https://parity.io");
478500

479501
// when
480502
let response = request(server,
@@ -497,7 +519,7 @@ fn should_fix_absolute_requests_based_on_referer() {
497519
#[test]
498520
fn should_fix_absolute_requests_based_on_referer_in_url() {
499521
// given
500-
let (server, fetch) = serve_with_fetch("token");
522+
let (server, fetch) = serve_with_fetch("token", "https://parity.io");
501523

502524
// when
503525
let response = request(server,

Diff for: dapps/src/tests/helpers/mod.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,15 @@ pub fn serve_with_registrar_and_fetch_and_threads(multi_threaded: bool) -> (Serv
100100
(server, fetch, reg)
101101
}
102102

103-
pub fn serve_with_fetch(web_token: &'static str) -> (Server, FakeFetch) {
103+
pub fn serve_with_fetch(web_token: &'static str, domain: &'static str) -> (Server, FakeFetch) {
104104
let fetch = FakeFetch::default();
105105
let f = fetch.clone();
106106
let (server, _) = init_server(move |builder| {
107107
builder
108108
.fetch(f.clone())
109-
.web_proxy_tokens(Arc::new(move |token| &token == web_token))
109+
.web_proxy_tokens(Arc::new(move |token| {
110+
if &token == web_token { Some(domain.into()) } else { None }
111+
}))
110112
}, Default::default(), Remote::new_sync());
111113

112114
(server, fetch)
@@ -147,7 +149,7 @@ impl ServerBuilder {
147149
dapps_path: dapps_path.as_ref().to_owned(),
148150
registrar: registrar,
149151
sync_status: Arc::new(|| false),
150-
web_proxy_tokens: Arc::new(|_| false),
152+
web_proxy_tokens: Arc::new(|_| None),
151153
signer_address: None,
152154
allowed_hosts: DomainsValidation::Disabled,
153155
remote: remote,

Diff for: dapps/src/web.rs

+9-3
Original file line numberDiff line numberDiff line change
@@ -133,14 +133,14 @@ impl<F: Fetch> WebHandler<F> {
133133
let target_url = token_it.next();
134134

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

145145
// Validate protocol
146146
let mut target_url = match target_url {
@@ -152,6 +152,12 @@ impl<F: Fetch> WebHandler<F> {
152152
}
153153
};
154154

155+
if !target_url.starts_with(&*domain) {
156+
return Err(State::Error(ContentHandler::error(
157+
StatusCode::BadRequest, "Invalid Domain", "Dapp attempted to access invalid domain.", Some(&target_url), self.embeddable_on.clone(),
158+
)));
159+
}
160+
155161
if !target_url.ends_with("/") {
156162
target_url = format!("{}/", target_url);
157163
}

Diff for: js/src/api/rpc/signer/signer.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ export default class Signer {
4242
.execute('signer_generateAuthorizationToken');
4343
}
4444

45-
generateWebProxyAccessToken () {
45+
generateWebProxyAccessToken (domain) {
4646
return this._transport
47-
.execute('signer_generateWebProxyAccessToken');
47+
.execute('signer_generateWebProxyAccessToken', domain);
4848
}
4949

5050
rejectRequest (requestId) {

Diff for: js/src/jsonrpc/interfaces/signer.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,11 @@ export default {
3030

3131
generateWebProxyAccessToken: {
3232
desc: 'Generates a new web proxy access token.',
33-
params: [],
33+
params: [{
34+
type: String,
35+
desc: 'Domain for which the token is valid. Only requests to this domain will be allowed.',
36+
example: 'https://parity.io'
37+
}],
3438
returns: {
3539
type: String,
3640
desc: 'The new web proxy access token.',

Diff for: js/src/views/Web/store.js

+11-9
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,17 @@ export default class Store {
5959
}
6060

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

65-
if (!hasProtocol.test(url)) {
66-
url = `https://${url}`;
67-
}
64+
if (!hasProtocol.test(url)) {
65+
url = `https://${url}`;
66+
}
6867

69-
this.setNextUrl(url);
70-
this.setCurrentUrl(this.nextUrl);
68+
return this.generateToken(url).then(() => {
69+
transaction(() => {
70+
this.setNextUrl(url);
71+
this.setCurrentUrl(this.nextUrl);
72+
});
7173
});
7274
}
7375

@@ -134,11 +136,11 @@ export default class Store {
134136
this.nextUrl = url;
135137
}
136138

137-
generateToken = () => {
139+
generateToken = (_url) => {
138140
this.setToken(null);
139141

140142
return this._api.signer
141-
.generateWebProxyAccessToken()
143+
.generateWebProxyAccessToken(_url)
142144
.then((token) => {
143145
this.setToken(token);
144146
})

Diff for: js/src/views/Web/store.spec.js

+6-5
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,16 @@ describe('views/Web/Store', () => {
6262
describe('gotoUrl', () => {
6363
it('uses the nextUrl when none specified', () => {
6464
store.setNextUrl('https://parity.io');
65-
store.gotoUrl();
6665

67-
expect(store.currentUrl).to.equal('https://parity.io');
66+
return store.gotoUrl().then(() => {
67+
expect(store.currentUrl).to.equal('https://parity.io');
68+
});
6869
});
6970

7071
it('adds https when no protocol', () => {
71-
store.gotoUrl('google.com');
72-
73-
expect(store.currentUrl).to.equal('https://google.com');
72+
return store.gotoUrl('google.com').then(() => {
73+
expect(store.currentUrl).to.equal('https://google.com');
74+
});
7475
});
7576
});
7677

Diff for: js/src/views/Web/web.js

-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ export default class Web extends Component {
3737

3838
componentDidMount () {
3939
this.store.gotoUrl(this.props.params.url);
40-
return this.store.generateToken();
4140
}
4241

4342
componentWillReceiveProps (props) {

Diff for: parity/dapps.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ mod server {
232232
) -> Result<Middleware, String> {
233233
let signer = deps.signer;
234234
let parity_remote = parity_reactor::Remote::new(deps.remote.clone());
235-
let web_proxy_tokens = Arc::new(move |token| signer.is_valid_web_proxy_access_token(&token));
235+
let web_proxy_tokens = Arc::new(move |token| signer.web_proxy_access_token_domain(&token));
236236

237237
Ok(parity_dapps::Middleware::dapps(
238238
parity_remote,

Diff for: rpc/src/v1/helpers/signer.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
use std::sync::Arc;
1818
use std::ops::Deref;
19+
use http::Origin;
1920
use util::Mutex;
2021
use transient_hashmap::TransientHashMap;
2122

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

@@ -46,16 +47,16 @@ impl SignerService {
4647
}
4748

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

5354
/// Generates a new web proxy access token.
54-
pub fn generate_web_proxy_access_token(&self) -> String {
55+
pub fn generate_web_proxy_access_token(&self, domain: Origin) -> String {
5556
let token = random_string(16);
5657
let mut tokens = self.web_proxy_tokens.lock();
5758
tokens.prune();
58-
tokens.insert(token.clone(), ());
59+
tokens.insert(token.clone(), domain);
5960
token
6061
}
6162

Diff for: rpc/src/v1/impls/signer.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,8 @@ impl<D: Dispatcher + 'static> Signer for SignerClient<D> {
245245
.map_err(|e| errors::token(e))
246246
}
247247

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

252252
fn subscribe_pending(&self, _meta: Self::Metadata, sub: Subscriber<Vec<ConfirmationRequest>>) {

Diff for: rpc/src/v1/traits/signer.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ build_rpc_trait! {
5151
#[rpc(name = "signer_generateAuthorizationToken")]
5252
fn generate_token(&self) -> Result<String, Error>;
5353

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

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

0 commit comments

Comments
 (0)