From a43282fab606ba48cc2a13d9977b97d6f749fe0b Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Mon, 6 Feb 2017 13:26:46 -0700 Subject: [PATCH] secure state, api fix for discover(), url trailing slash fix --- oauth3.core.js | 63 ++++++++++++++++++++++++++++++++++++++------------ oauth3.js | 2 +- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/oauth3.core.js b/oauth3.core.js index cbc0879..729e1b9 100644 --- a/oauth3.core.js +++ b/oauth3.core.js @@ -73,20 +73,25 @@ if (!providerUri) { throw new Error("cannot discover without providerUri"); } - if (!opts.state) { - throw new Error("cannot discover without opts.state"); - } if (!opts.appUrl) { throw new Error("cannot discover without opts.appUrl"); } var params = { action: 'directives' - , state: opts.state + , state: core.utils.randomState() , redirect_uri: opts.appUrl + (opts.appCallbackPath || '/.well-known/oauth3/callback.html') + , debug: opts.debug || undefined }; - return providerUri + '/.well-known/oauth3/directives.html#' + core.querystringify(params); + var result = { + url: providerUri + '/.well-known/oauth3/directives.html#' + core.querystringify(params) + , state: params.state + , method: 'GET' + , query: params + }; + + return result; }; // these might not really belong in core... not sure @@ -104,6 +109,29 @@ b64 = b64.replace(/=+/g, ''); return b64; } + , randomState: function () { + var i; + var ch; + var str; + + // TODO put in different file for browser vs node + try { + return Array.prototype.slice.call(window.crypto.getRandomValues(new Uint8Array(16))).map(function (ch) { return (ch).toString(16); }).join(''); + } catch(e) { + // TODO use fisher-yates on 0..255 and select [0] 16 times + // [security] https://medium.com/@betable/tifu-by-using-math-random-f1c308c4fd9d#.5qx0bf95a + // https://github.com/v8/v8/blob/b0e4dce6091a8777bda80d962df76525dc6c5ea9/src/js/math.js#L135-L144 + // Note: newer versions of v8 do not have this bug, but other engines may still + console.warn('[security] crypto.getRandomValues() failed, falling back to Math.random()'); + str = ''; + for (i = 0; i < 32; i += 1) { + ch = Math.round(Math.random() * 255).toString(16); + if (ch.length < 2) { ch = '0' + ch; } + str += ch; + } + return str; + } + } }; core.jwt = { // decode only (no verification) @@ -149,7 +177,7 @@ // GET https://example.com/api/org.oauth3.provider/authorization_dialog // ?response_type=code // &scope=`encodeURIComponent('profile.login profile.email')` - // &state=`Math.random()` + // &state=`cryptoutil.random().toString('hex')` // &client_id=xxxxxxxxxxx // &redirect_uri=`encodeURIComponent('https://myapp.com/oauth3.html')` // @@ -175,7 +203,7 @@ // &scope=`encodeURIComponent('profile.login profile.email')` // // (optional) - // &state=`Math.random()` + // &state=`cryptoutil.random().toString('hex')` // &redirect_uri=`encodeURIComponent('https://myapp.com/oauth3.html')` // // NOTE: This is not a request sent to the provider, but rather a request sent to the @@ -186,12 +214,12 @@ var scope = opts.scope || directive.authn_scope; var providerUri = directive.provider_uri; - - var state = Math.random().toString().replace(/^0\./, ''); - var params = {}; + var params = { + state: core.utils.randomState() + , debug: opts.debug || undefined + }; var slimProviderUri = encodeURIComponent(providerUri.replace(/^(https?|spdy):\/\//, '')); - params.state = state; if (scope) { params.scope = scope; } @@ -216,7 +244,7 @@ return { url: authorizationRedirect + '?' + core.querystringify(params) , method: 'GET' - , state: state // this becomes browser_state + , state: params.state // this becomes browser_state , params: params // includes scope, final redirect_uri? }; }; @@ -230,7 +258,7 @@ // GET https://example.com/api/org.oauth3.provider/authorization_dialog // ?response_type=token // &scope=`encodeURIComponent('profile.login profile.email')` - // &state=`Math.random()` + // &state=`cryptoutil.random().toString('hex')` // &client_id=xxxxxxxxxxx // &redirect_uri=`encodeURIComponent('https://myapp.com/oauth3.html')` // @@ -246,8 +274,10 @@ var clientId = opts.appId || opts.clientId; var args = directive[type]; var uri = args.url; - var state = Math.random().toString().replace(/^0\./, ''); - var params = {}; + var state = core.utils.randomState(); + var params = { + debug: opts.debug || undefined + }; var loc; var result; @@ -303,6 +333,7 @@ "username": opts.id || opts.username , "request_otp": true // opts.requestOtp || undefined //, "jwt": opts.jwt // TODO sign a proof + , debug: opts.debug || undefined }; var uri = args.url; var body; @@ -365,6 +396,7 @@ //, "public_key": opts.rememberDevice && opts.publicKey || undefined //, "public_key_type": opts.rememberDevice && opts.publicKeyType || undefined // RSA/ECDSA //, "jwt": opts.jwt // TODO sign a proof with a previously loaded public_key + , debug: opts.debug || undefined }; var uri = args.url; var body; @@ -426,6 +458,7 @@ //, "client_uri": undefined //, "scope": undefined //, "client_secret": undefined + , debug: opts.debug || undefined }; var uri = args.url; var body; diff --git a/oauth3.js b/oauth3.js index e6d9d2a..379ece2 100644 --- a/oauth3.js +++ b/oauth3.js @@ -11,7 +11,7 @@ console.warn('[deprecated] using window.location.{protocol, host, pathname} when opts.appUrl should be used'); return window.location.protocol + '//' + window.location.host - + (window.location.pathname).replace(/\/?$/, '/') + + (window.location.pathname).replace(/\/?$/, '') ; }