Webapp changes to support asking for a PIN

remoting/webapp/client_session.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview
  * Class handling creation and teardown of a remoting client session.
  *
  * The ClientSession class controls lifetime of the client plugin
  * object and provides the plugin with the functionality it needs to
  * establish connection. Specifically it:
  *  - Delivers incoming/outgoing signaling messages,
  *  - Adjusts plugin size and position when destop resolution changes,
  *
  * This class should not access the plugin directly, instead it should
  * do it through ClientPlugin class which abstracts plugin version
  * differences.
  */
 
 'use strict';
 
 /** @suppress {duplicate} */
 var remoting = remoting || {};
 
 /**
  * @param {string} hostJid The jid of the host to connect to.
  * @param {string} clientJid The jid of the WCS client.
  * @param {string} hostPublicKey The base64 encoded version of the host's
  *     public key.
- * @param {string} sharedSecret The access code for IT2Me or the PIN
- *     for Me2Me.
+ * @param {string} passPhrase The pre-determined pass phrase for connection.

[Jamie, 2013/03/18 20:28:23]

I think it's worth keeping the style of the original comment here. In other
words, explain what this parameter is for IT2Me and for Me2Me.

[rmsousa, 2013/03/18 21:58:01]

Done.

+ * @param {function(function(string): void): void} fetchPin Called
+ *     when a PIN needs to be obtained interactively.

[Jamie, 2013/03/18 20:28:23]

I think we should be consistent with referring to PIN vs. pass phrase.
Reintroducing the term PIN in the previous parameter description is probably
sufficient

[rmsousa, 2013/03/18 21:58:01]

I tried to differentiate between a user-generated PIN (Me2Me), and a non-user
pass phrase (It2Me). ut since the latter is only used for IT2Me anyway, we might
as well call it accessCode explicitly.

  * @param {string} authenticationMethods Comma-separated list of
  *     authentication methods the client should attempt to use.
  * @param {string} hostId The host identifier for Me2Me, or empty for IT2Me.
  *     Mixed into authentication hashes for some authentication methods.
  * @param {remoting.ClientSession.Mode} mode The mode of this connection.
  * @constructor
  */
 remoting.ClientSession = function(hostJid, clientJid,
-                                  hostPublicKey, sharedSecret,
+                                  hostPublicKey, passPhrase,
+                                  fetchPin,

[Jamie, 2013/03/18 20:28:23]

Nit: I'm not sure what the intended line-break scheme was here. Either
one-per-line or only as needed would be fine, but there's no need for fetchPin
to have a line to itself (hostJid and clientJid are sufficiently closely-related
that they can stay as they are if you prefer).

[rmsousa, 2013/03/18 21:58:01]

Done.

                                   authenticationMethods, hostId, mode) {
   this.state = remoting.ClientSession.State.CREATED;
 
   this.hostJid = hostJid;
   this.clientJid = clientJid;
   this.hostPublicKey = hostPublicKey;
-  this.sharedSecret = sharedSecret;
+  this.passPhrase_ = passPhrase;
+  this.fetchPin_ = fetchPin;

[Jamie, 2013/03/18 20:28:23]

Please add @private annotations to these members.

[rmsousa, 2013/03/18 21:58:01]

Done.

   this.authenticationMethods = authenticationMethods;
   this.hostId = hostId;
   /** @type {remoting.ClientSession.Mode} */
   this.mode = mode;
   this.sessionId = '';
   /** @type {remoting.ClientPlugin} */
   this.plugin = null;
   /** @private */
   this.shrinkToFit_ = true;
   /** @private */
@@ skipping 225 matching lines @@
     this.resizeToClient_ = /** @type {boolean} */
         options[remoting.ClientSession.KEY_RESIZE_TO_CLIENT];
   }
   if (remoting.ClientSession.KEY_SHRINK_TO_FIT in options &&
       typeof(options[remoting.ClientSession.KEY_SHRINK_TO_FIT]) ==
           'boolean') {
     this.shrinkToFit_ = /** @type {boolean} */
         options[remoting.ClientSession.KEY_SHRINK_TO_FIT];
   }
 
-  this.plugin.element().focus();
-
   /** @param {boolean} result */
   this.plugin.initialize(this.onPluginInitialized_.bind(this));
+};
+
+/**
+ * Constrains the focus to the plugin element. 

[Jamie, 2013/03/18 20:28:23]

Add @private

[rmsousa, 2013/03/18 21:58:01]

Done.

+ */
+remoting.ClientSession.prototype.setFocusHandlers_ = function() {
   this.plugin.element().addEventListener(
       'focus', this.callPluginGotFocus_, false);
   this.plugin.element().addEventListener(
       'blur', this.callPluginLostFocus_, false);
+  this.plugin.element().focus();
 };
 
 /**
  * @param {boolean} initialized
  */
 remoting.ClientSession.prototype.onPluginInitialized_ = function(initialized) {
   if (!initialized) {
     console.error('ERROR: remoting plugin not loaded');
     this.plugin.cleanup();
     delete this.plugin;
@@ skipping 270 matching lines @@
 };
 
 /**
  * Connects the plugin to WCS.
  *
  * @private
  * @return {void} Nothing.
  */
 remoting.ClientSession.prototype.connectPluginToWcs_ = function() {
   remoting.formatIq.setJids(this.clientJid, this.hostJid);
+  /** @type remoting.ClientPlugin */

[Jamie, 2013/03/18 20:28:23]

Brackets around the type, please.

[rmsousa, 2013/03/18 21:58:01]

Done.

   var plugin = this.plugin;
   var forwardIq = plugin.onIncomingIq.bind(plugin);
   /** @param {string} stanza The IQ stanza received. */
   var onIncomingIq = function(stanza) {
     // HACK: Remove 'x' prefix added to the id in sendIq_().
     try {
       var parser = new DOMParser();
       var iqNode = parser.parseFromString(stanza, 'text/xml').firstChild;
       var type = iqNode.getAttribute('type');
       var id = iqNode.getAttribute('id');
       if (type != 'set' && id.charAt(0) == 'x') {
         iqNode.setAttribute('id', id.substr(1));
         stanza = (new XMLSerializer()).serializeToString(iqNode);
       }
     } catch (err) {
       // Pass message as is when it is malformed.
     }
 
     console.log(remoting.timestamp(),
                 remoting.formatIq.prettifyReceiveIq(stanza));
     forwardIq(stanza);
   }
   remoting.wcsSandbox.setOnIq(onIncomingIq);

[Jamie, 2013/03/18 20:28:23]

Blank line here, please.

[rmsousa, 2013/03/18 21:58:01]

Done.

+  // Pass phrase was already supplied before connecting (It2Me case).

[Jamie, 2013/03/18 20:28:23]

Can you move this comment inside the if, for consistency with the else clauses?
Also consider blank lines before the elses for readability.

[rmsousa, 2013/03/18 21:58:01]

Done.

+  if (this.passPhrase_) {
+    this.connectToHost_(this.passPhrase_);
+  } else if (plugin.hasFeature(
+      remoting.ClientPlugin.Feature.USE_ASYNC_PIN_DIALOG)) {
+    // Plugin supports asynchronously asking for passphrase.
+    plugin.useAsyncPinDialog();
+    /** @type remoting.ClientSession */
+    var that = this;
+    var fetchPin = function() {
+      that.fetchPin_(plugin.onPinFetched.bind(plugin));
+    };
+    plugin.fetchPinHandler = fetchPin;
+    this.connectToHost_('');
+  } else {
+    // Plugin doesn't support asynchronously asking for passphrase, ask now.
+    this.fetchPin_(this.connectToHost_.bind(this));
+  }
+};
+
+/**
+ * Connects to the host.
+ *
+ * @private

[Jamie, 2013/03/18 20:28:23]

Nit: We're pretty consistent about @private being at the end of the annotations.

[rmsousa, 2013/03/18 21:58:01]

Done.

+ * @param {string} sharedSecret Shared secret for SPAKE negotiation.
+ * @return {void} Nothing.
+ */
+remoting.ClientSession.prototype.connectToHost_ = function(sharedSecret) {
   this.plugin.connect(this.hostJid, this.hostPublicKey, this.clientJid,
-                      this.sharedSecret, this.authenticationMethods,
+                      sharedSecret, this.authenticationMethods,
                       this.hostId);
 };
 
 /**
  * Callback that the plugin invokes to indicate that the connection
  * status has changed.
  *
  * @private
  * @param {number} status The plugin's status.
  * @param {number} error The plugin's error state, if any.
  */
 remoting.ClientSession.prototype.onConnectionStatusUpdate_ =
     function(status, error) {
   if (status == remoting.ClientSession.State.CONNECTED) {
+    this.setFocusHandlers_();
     this.onDesktopSizeChanged_();
     if (this.resizeToClient_) {
       this.plugin.notifyClientResolution(window.innerWidth,
                                          window.innerHeight,
                                          window.devicePixelRatio);
     }
   } else if (status == remoting.ClientSession.State.FAILED) {
     this.error_ = /** @type {remoting.ClientSession.ConnectionError} */ (error);
   }
   this.setState_(/** @type {remoting.ClientSession.State} */ (status));
@@ skipping 381 matching lines @@
       var lateAdjustment = 1 + (now - expected) / timeout;
       if (!that.scroll_(lateAdjustment * dx, lateAdjustment * dy)) {
         that.bumpScrollTimer_ = window.setTimeout(
             function() { repeatScroll(now + timeout); },
             timeout);
       }
     };
     repeatScroll(new Date().getTime());
   }
 };