Only login if not already authenticated

Calling login here calls identity.request to be called, which in turn
eventually results in the onlogin callback running.

The onlogin callback is not currently idempotent (and it isn't trivial
to make it so) so this results in errors being thrown by Annotator.

Fixes #1611.

h/static/scripts/controllers.coffee

@@ -37,6 +37,7 @@ class AppController
   ) ->
     {plugins, host, providers} = annotator
 
+    checkingToken = false
     isFirstRun = $location.search().hasOwnProperty('firstrun')
 
     applyUpdates = (action, data) ->
@@ -158,6 +159,8 @@ class AppController
       _dfdSock.promise
 
     onlogin = (assertion) ->
+      checkingToken = true
+
       # Configure the Auth plugin with the issued assertion as refresh token.
       annotator.addPlugin 'Auth',
         tokenUrl: documentHelpers.absoluteURI(
@@ -165,6 +168,7 @@ class AppController
 
       # Set the user from the token.
       plugins.Auth.withToken (token) ->
+        checkingToken = false
         annotator.addPlugin 'Permissions',
           user: token.userId
           userAuthorize: authorizeAction
@@ -187,14 +191,18 @@ class AppController
       delete plugins.Permissions
 
       $scope.persona = null
+      checkingToken = false
       reset()
 
     onready = ->
-      if plugins.Auth is undefined
+      if not checkingToken and typeof $scope.persona == 'undefined'
+        # If we're not checking the token and persona is undefined, onlogin
+        # hasn't run, which means we aren't authenticated.
         $scope.persona = null
         reset()
 
-      $scope.login() if isFirstRun
+        if isFirstRun
+          $scope.login()
 
     oncancel = ->
       $scope.dialog.visible = false

tests/js/controllers-test.coffee

 assert = chai.assert
+sinon.assert.expose assert, prefix: null
 
 describe 'h.controllers', ->
+  $scope = null
+  fakeIdentity = null
+  fakeLocation = null
   fakeParams = null
+  fakeSocket = null
+  sandbox = null
 
   beforeEach module('h')
 
   beforeEach module ($provide) ->
+    sandbox = sinon.sandbox.create()
+
+    fakeAnnotator = {
+      plugins: {
+        Auth: {withToken: sandbox.spy()}
+      }
+      options: {}
+      socialView: {name: 'none'}
+      addPlugin: sandbox.spy()
+    }
+    fakeIdentity = {
+      watch: sandbox.spy()
+      request: sandbox.spy()
+    }
+    fakeLocation = {
+      search: sandbox.stub().returns({})
+    }
     fakeParams = {id: 'test'}
+    fakeSocket = sandbox.stub().returns({
+      onclose: null
+      close: sandbox.spy()
+    })
+
+    $provide.value 'annotator', fakeAnnotator
+    $provide.value 'identity', fakeIdentity
+    $provide.value 'socket', fakeSocket
+    $provide.value '$location', fakeLocation
     $provide.value '$routeParams', fakeParams
     return
 
+  afterEach ->
+    sandbox.restore()
+
+  describe 'AppController', ->
+    createController = null
+
+    beforeEach inject ($controller, $rootScope) ->
+      $scope = $rootScope.$new()
+
+      createController = ->
+        $controller('AppController', {$scope: $scope})
+
+    it 'watches the identity service for identity change events', ->
+      app = createController()
+      assert.calledOnce(fakeIdentity.watch)
+
+    it 'sets the persona to null when the identity has been checked', ->
+      app = createController()
+      {onlogin, onlogout, onready} = fakeIdentity.watch.args[0][0]
+      onready()
+      assert.isNull($scope.persona)
+
+    it 'does not set the persona to null while token is still being checked', ->
+      app = createController()
+      {onlogin, onlogout, onready} = fakeIdentity.watch.args[0][0]
+      onlogin()
+      onready()
+      assert.isNotNull($scope.persona)
+
+    it 'shows login form for logged out users on first run', ->
+      fakeLocation.search.returns({'firstrun': ''})
+      app = createController()
+      {onlogin, onlogout, onready} = fakeIdentity.watch.args[0][0]
+      onready()
+      assert.isTrue($scope.dialog.visible)
+
+    it 'does not show login form for logged out users if not first run', ->
+      app = createController()
+      {onlogin, onlogout, onready} = fakeIdentity.watch.args[0][0]
+      onready()
+      assert.isFalse($scope.dialog.visible)
+
+    it 'does not show login form for logged in users', ->
+      app = createController()
+      {onlogin, onlogout, onready} = fakeIdentity.watch.args[0][0]
+      onlogin('abcdef123')
+      onready()
+      assert.isFalse($scope.dialog.visible)
+
   describe 'AnnotationViewer', ->
-    $scope = null
     annotationViewer = null
 
     beforeEach inject ($controller, $rootScope) ->