Merge pull request #1357 from hypothesis/identity-refactor

Refactoring identity and auth

h/static/scripts/app.coffee

 imports = [
-  'bootstrap'
   'ngAnimate'
   'ngRoute'
+  'h.app_directives'
   'h.auth'
   'h.controllers'
   'h.directives'
-  'h.app_directives'
-  'h.helpers'
-  'h.flash'
   'h.filters'
-  'h.session'
-  'h.services'
-  'h.socket'
+  'h.identity'
   'h.streamsearch'
 ]
 

h/static/scripts/auth.coffee → h/static/scripts/auth/auth.coffee

@@ -9,6 +9,7 @@ class AuthController
     timeout = null
 
     success = ->
+      $scope.tab = if $scope.tab is 'forgot' then 'activate' else null
       $scope.model = null
       $scope.$broadcast 'success'
 
@@ -36,11 +37,10 @@ class AuthController
       data = {}
       method = '$' + form.$name
 
-      for own key of session
-        delete session[key]
-
-      angular.extend session, $scope.model
-      session[method] success, angular.bind(this, failure, form)
+      angular.copy $scope.model, session
+      session.$promise = session[method] success,
+        angular.bind(this, failure, form)
+      session.$resolved = false
 
     $scope.$on '$destroy', ->
       if timeout
@@ -59,36 +59,58 @@ class AuthController
         , 300000
 
 
-authDirective = ->
+authDirective = ['$timeout', ($timeout) ->
   controller: 'AuthController'
-  link: (scope, elem, attrs, [form, auth]) ->
+  link: (scope, elem, attrs, [auth, form]) ->
     elem.on 'submit', (event) ->
       scope.$apply ->
         $target = angular.element event.target
         $form = $target.controller('form')
 
-        $form.responseErrorMessage = null
+        delete $form.responseErrorMessage
 
         for ctrl in $form.$error.response?.slice?() or []
           ctrl.$setValidity('response', true)
 
         auth.submit($form)
 
+    scope.model = {}
+
+    scope.$on 'authorize', ->
+      scope.tab = 'login'
+
     scope.$on 'error', (event) ->
-      scope[attrs.onError]?(event)
+      scope.onError()
 
     scope.$on 'success', (event) ->
-      scope[attrs.onSuccess]?(event)
+      scope.onSuccess()
 
     scope.$on 'timeout', (event) ->
-      scope[attrs.onTimeout]?(event)
+      scope.onTimeout()
 
     scope.$watch 'model', (value) ->
       if value is null
         form.$setPristine()
-  require: ['form', 'auth']
+
+    scope.$watch 'tab', (name) ->
+      $timeout ->
+        elem
+          .find('form')
+          .filter(-> this.name is name)
+          .find('input')
+          .filter(-> this.type isnt 'hidden')
+          .first()
+          .focus()
+  require: ['auth', 'form']
   restrict: 'C'
-  scope: true
+  scope:
+    onError: '&'
+    onSuccess: '&'
+    onTimeout: '&'
+    session: '='
+    tab: '=ngModel'
+  templateUrl: 'auth.html'
+]
 
 
 angular.module('h.auth', imports)

h/static/scripts/auth/identity.coffee

+imports = [
+  'h.helpers'
+  'h.session'
+]
+
+
+identityFactory = [
+  '$rootScope', 'baseURI', 'session',
+  ($rootScope,   baseURI,   session) ->
+    loggedInUser = undefined
+
+    onlogin = null
+    onlogout = null
+    onmatch = null
+
+    $rootScope.session = session
+
+    $rootScope.$watch 'session.$promise', (promise) ->
+      # Wait for any pending action to resolve.
+      promise.finally ->
+        # Get the userid and convert it to the persona format.
+        persona = session.userid?.replace(/^acct:/, '') or null
+
+        # Fire callbacks as appropriate.
+        # Consult the state matrix in the `navigator.id.watch` documentation.
+        # https://developer.mozilla.org/en-US/docs/Web/API/navigator.id.watch
+        if loggedInUser is null
+          if persona
+            loggedInUser = persona
+            onlogin?(session.csrf_token)
+          else
+            onmatch?()
+        else if loggedInUser
+          if persona
+            if loggedInUser is persona
+              onmatch?()
+            else
+              loggedInUser = persona
+              onlogin?(session.csrf_token)
+          else
+            loggedInUser = null
+            onlogout?()
+        else
+          if persona
+            loggedInUser = persona
+            onlogin?(session.csrf_token)
+          else
+            loggedInUser = null
+            onlogout?()
+
+    logout: ->
+      # Clear the session but preserve its identity and give it a new promise.
+      $promise = session.$logout()
+      $resolved = false
+      angular.copy({$promise, $resolved}, session)
+
+    request: ->
+      $rootScope.$broadcast 'authorize'
+
+    watch: (options) ->
+      {loggedInUser, onlogin, onlogout, onmatch} = options
+]
+
+
+angular.module('h.identity', imports)
+.factory('identity', identityFactory)

h/static/scripts/session.coffee → h/static/scripts/auth/session.coffee

@@ -73,14 +73,6 @@ class SessionProvider
         model.errors = data.errors
         model.reason = data.reason
 
-        # bw compat
-        if angular.isObject(data.persona)
-          persona = data.persona
-          data.persona = "acct:#{persona.username}@#{persona.provider}"
-          data.personas = for persona in data.personas
-            "acct:#{persona.username}@#{persona.provider}"
-        # end bw compat
-
         # Fire flash messages.
         for q, msgs of data.flash
           flash q, msgs

h/static/scripts/controllers.coffee

 imports = [
   'bootstrap'
+  'h.flash'
   'h.helpers'
+  'h.identity'
+  'h.services'
   'h.socket'
   'h.searchfilters'
 ]
 
 class App
-  scope:
-    frame:
-      visible: false
-    ongoingHighlightSwitch: false
-    search: {}
-    sheet: {}
-    sorts: [
-      'Newest'
-      'Oldest'
-      'Location'
-    ]
-    views: [
-      'Screen'
-      'Document'
-      'Comments'
-    ]
-
   this.$inject = [
     '$element', '$location', '$q', '$rootScope', '$route', '$scope', '$timeout',
-    'annotator', 'flash', 'session', 'socket', 'streamfilter', 'viewFilter'
+    'annotator', 'flash', 'identity', 'queryparser', 'socket',
+    'streamfilter', 'viewFilter'
   ]
   constructor: (
      $element,   $location,   $q,   $rootScope,   $route,   $scope,   $timeout
-     annotator,   flash,   session,   socket,   streamfilter,   viewFilter
+     annotator,   flash,   identity,   queryparser,   socket,
+     streamfilter,   viewFilter
   ) ->
     {plugins, host, providers} = annotator
 
-    _reset = =>
-      delete annotator.ongoing_edit
-      base = angular.copy @scope
-      angular.extend $scope, base,
-        frame: $scope.frame or @scope.frame
-        socialView: annotator.socialView
-        ongoingHighlightSwitch: false
-        search:
-          query: $location.search()['q']
-        session: session
-
-    _reset()
-
-    annotator.subscribe 'serviceDiscovery', (options) ->
-      annotator.options.Store ?= {}
-      angular.extend annotator.options.Store, options
+    # Verified user id.
+    # Undefined means we don't track the session, but the identity module will
+    # tell us the state of the session. A null value means that the session
+    # has been checked and it was found that there is no user logged in.
+    loggedInUser = undefined
+
+    # Resolved once the API service has been discovered.
+    storeReady = $q.defer()
+
+    configureIdentity = (persona) ->
+      # Store the argument as the claimed user id.
+      claimedUser = persona
+
+      # Convert it to the format used by persona.
+      if claimedUser then claimedUser = claimedUser.replace(/^acct:/, '')
+
+      if claimedUser is loggedInUser
+        if loggedInUser is undefined
+          # This is the first execution.
+          # Configure the identity callbacks and the initial user id claim.
+          identity.watch
+            loggedInUser: claimedUser
+            onlogin: (assertion) ->
+              onlogin(assertion)
+            onlogout: ->
+              onlogout()
+      else if annotator.discardDrafts()
+        if claimedUser
+          identity.request()
+        else
+          identity.logout()
 
-      session.$promise.then (data) ->
-        unless data.personas?.length
-          $scope.initUpdater()
-          $scope.reloadAnnotations()
+    onlogin = (assertion) ->
+      # Delete any old Auth plugin.
+      delete plugins.Auth
 
-    $scope.$watch 'session.personas', (newValue, oldValue) =>
-      if newValue?.length
-        unless $scope.session.persona and $scope.session.persona in newValue
-          $scope.session.persona = newValue[0]
-      else
-        $scope.session.persona = null
+      # Configure the Auth plugin with the issued assertion as refresh token.
+      annotator.addPlugin 'Auth',
+        tokenUrl: "#{baseURI}api/token?assertion=#{assertion}"
 
-    $scope.$watch 'session.persona', (newValue, oldValue) =>
-      unless annotator.discardDrafts()
-        $scope.session.persona = oldValue
-        return
+      # Set the user from the token.
+      plugins.Auth.withToken (token) ->
+        plugins.Permissions._setAuthFromToken(token)
+        loggedInUser = plugins.Permissions.user.replace /^acct:/, ''
+        reset()
 
+    onlogout = ->
       plugins.Auth?.element.removeData('annotator:headers')
       delete plugins.Auth
 
-      plugins.Permissions?.setUser(null)
+      plugins.Permissions.setUser(null)
+
       # XXX: Temporary workaround until Annotator v2.0 or v1.2.10
-      plugins.Permissions?.options.permissions =
+      plugins.Permissions.options.permissions =
         read: []
         update: []
         delete: []
         admin: []
 
-      if newValue?
-        annotator.addPlugin 'Auth', tokenUrl: "/api/token?persona=#{newValue}"
+      loggedInUser = null
+      reset()
 
-        plugins.Auth.withToken (token) =>
-          plugins.Permissions._setAuthFromToken token
+    reset = ->
+      # Do not rely on the identity service to invoke callbacks within an
+      # angular digest cycle.
+      $scope.$evalAsync ->
+        if annotator.ongoing_edit
+          annotator.clickAdder()
 
-          if annotator.ongoing_edit
-            $timeout ->
-              annotator.clickAdder()
-            , 1000
+        if $scope.ongoingHighlightSwitch
+          $scope.ongoingHighlightSwitch = false
+          annotator.setTool 'highlight'
+        else
+          annotator.setTool 'comment'
 
-          if $scope.ongoingHighlightSwitch
-            $scope.ongoingHighlightSwitch = false
-            annotator.setTool 'highlight'
-          else
-            $scope.reloadAnnotations()
-            $scope.initUpdater()
-      else if oldValue?
-        session.$logout =>
-          $scope.$broadcast 'reset'
-
-          if annotator.tool isnt 'comment'
-            annotator.setTool 'comment'
-          else
-            $scope.reloadAnnotations()
-            $scope.initUpdater()
+        # Convert the verified user id to the format used by the API.
+        persona = loggedInUser
+        if persona then persona = "acct:#{persona}"
+
+        # Ensure it is synchronized on the scope.
+        # Without this, failed identity changes will remain on the scope.
+        $scope.persona = persona
+
+        # Reload services
+        storeReady.promise.then ->
+          initStore()
+          initUpdater()
+
+    annotator.subscribe 'serviceDiscovery', (options) ->
+      annotator.options.Store ?= {}
+      angular.extend annotator.options.Store, options
+      storeReady.resolve()
+
+    $scope.$watch 'persona', configureIdentity
 
     $scope.$watch 'socialView.name', (newValue, oldValue) ->
       return if newValue is oldValue
-      console.log "Social View changed to '" + newValue + "'. Reloading annotations."
-      $scope.reloadAnnotations()
+
+      if $scope.persona
+        initStore()
+      else if newValue is 'single-player'
+        identity.request()
 
     $scope.$watch 'frame.visible', (newValue, oldValue) ->
       routeName = $location.path().replace /^\//, ''
@@ -121,20 +135,6 @@ class App
         for p in annotator.providers
           p.channel.notify method: 'setActiveHighlights'
 
-    $scope.$watch 'sheet.show', (visible) ->
-      $scope.sheet.tab = if visible then 'login' else null
-
-    $scope.$watch 'sheet.tab', (tab) ->
-      $timeout ->
-        $element
-        .find('form')
-        .filter(-> this.name is tab)
-        .find('input')
-        .filter(-> this.type isnt 'hidden')
-        .first()
-        .focus()
-      , 10
-
     $scope.$watch 'store.entities', (entities, oldEntities) ->
       return if entities is oldEntities
 
@@ -232,15 +232,7 @@ class App
       $scope.updater.then (sock) ->
         sock.send(JSON.stringify(sockmsg))
 
-    $scope.search.clear = ->
-      $location.search('q', null)
-
-    $scope.search.update = (query) ->
-      unless angular.equals $location.search()['q'], query
-        if annotator.discardDrafts()
-          $location.search('q', query or null)
-
-    $scope.reloadAnnotations = ->
+    initStore = ->
       Store = plugins.Store
 
       delete plugins.Store
@@ -323,15 +315,29 @@ class App
       cleanup (a for a in annotations when a.thread)
       annotator.subscribe 'annotationsLoaded', cleanup
 
-    $scope.authSuccess = ->
-      $scope.sheet.show = false
-
     $scope.authTimeout = ->
+      delete annotator.ongoing_edit
+      $scope.ongoingHighlightSwitch = false
       flash 'info',
         'For your security, the forms have been reset due to inactivity.'
-      _reset()
 
-    $scope.initUpdater = (failureCount=0) ->
+    $scope.frame = visible: false
+    $scope.id = identity
+
+    $scope.model = persona: undefined
+
+    $scope.search =
+      clear: ->
+        $location.search('q', null)
+
+      update: (query) ->
+        unless angular.equals $location.search()['q'], query
+          if annotator.discardDrafts()
+            $location.search('q', query or null)
+
+    $scope.socialView = annotator.socialView
+
+    initUpdater = (failureCount=0) ->
       _dfdSock = $q.defer()
       _sock = socket()
 
@@ -350,7 +356,7 @@ class App
         failureCount = Math.min(10, ++failureCount)
         slots = Math.random() * (Math.pow(2, failureCount) - 1)
         $timeout ->
-          _retry = $scope.initUpdater(failureCount)
+          _retry = initUpdater(failureCount)
           _dfdSock?.resolve(_retry)
         , slots * 500
 
@@ -363,7 +369,7 @@ class App
 
         unless data instanceof Array then data = [data]
 
-        p = $scope.session.persona
+        p = $scope.persona
         user = if p? then "acct:" + p.username + "@" + p.provider else ''
         unless data instanceof Array then data = [data]
 

h/static/scripts/directives.coffee

@@ -235,13 +235,6 @@ thread = ['$rootScope', '$window', ($rootScope, $window) ->
 ]
 
 
-userPicker = ->
-  restrict: 'ACE'
-  scope:
-    model: '=userPickerModel'
-    options: '=userPickerOptions'
-  templateUrl: 'userPicker.html'
-
 repeatAnim = ->
   restrict: 'A'
   scope:
@@ -446,7 +439,6 @@ angular.module('h.directives', ['ngSanitize'])
 .directive('tags', tags)
 .directive('thread', thread)
 .directive('username', username)
-.directive('userPicker', userPicker)
 .directive('repeatAnim', repeatAnim)
 .directive('simpleSearch', simpleSearch)
 .directive('whenscrolled', whenscrolled)

h/static/scripts/services.coffee

@@ -435,13 +435,13 @@ class Hypothesis extends Annotator
   showEditor: (annotation) =>
     this.show()
     @element.injector().invoke [
-      '$location', '$rootScope', 'drafts'
-      ($location,   $rootScope,   drafts) =>
+      '$location', '$rootScope', 'drafts', 'identity',
+      ($location,   $rootScope,   drafts,   identity) =>
         @ongoing_edit = annotation
 
         unless this.plugins.Auth? and this.plugins.Auth.haveValidToken()
           $rootScope.$apply ->
-            $rootScope.$broadcast 'showAuth', true
+            identity.request()
           for p in @providers
             p.channel.notify method: 'onEditorHide'
           return
@@ -566,7 +566,7 @@ class Hypothesis extends Annotator
         scope = @element.scope()
         # If we are not logged in, start the auth process
         scope.ongoingHighlightSwitch = true
-        scope.sheet.collapsed = false
+        @element.injector().get('identity').request()
         this.show()
         return
 

h/static/scripts/streamsearch.coffee

@@ -5,18 +5,17 @@ imports = [
   'h.filters'
   'h.flash'
   'h.helpers'
-  'h.session'
   'h.searchfilters'
 ]
 
 class StreamSearch
   this.inject = [
     '$scope', '$rootScope',
-    'queryparser', 'session', 'searchfilter', 'streamfilter'
+    'queryparser', 'searchfilter', 'streamfilter'
   ]
   constructor: (
      $scope,   $rootScope,
-     queryparser,   session,   searchfilter,   streamfilter
+     queryparser,   searchfilter,   streamfilter
   ) ->
     # Initialize the base filter
     streamfilter

karma.config.js

@@ -53,9 +53,11 @@ module.exports = function(config) {
       'h/static/scripts/vendor/jquery.ui.effect-highlight.js',
       'h/static/scripts/vendor/tag-it.js',
       'h/static/scripts/vendor/uuid.js',
+      'h/static/scripts/hypothesis-auth.js',
       'h/static/scripts/hypothesis.js',
       'h/static/scripts/vendor/sinon.js',
       'h/static/scripts/vendor/chai.js',
+      'h/templates/*.html',
       'tests/js/**/*-test.coffee'
     ],
 
@@ -64,11 +66,16 @@ module.exports = function(config) {
     exclude: [
     ],
 
+    // strip templates of leading path
+    ngHtml2JsPreprocessor: {
+      stripPrefix: 'h/templates/'
+    },
 
     // preprocess matching files before serving them to the browser
     // available preprocessors: https://npmjs.org/browse/keyword/karma-preprocessor
     preprocessors: {
-      '**/*.coffee': ['coffee']
+      '**/*.coffee': ['coffee'],
+      '**/*.html': ['html2js']
     },
 
 

package.json

@@ -13,6 +13,7 @@
     "karma-cli": "0.0.4",
     "karma-coffee-preprocessor": "^0.2.1",
     "karma-mocha": "^0.1.4",
+    "karma-ng-html2js-preprocessor": "^0.1.0",
     "karma-phantomjs-launcher": "^0.1.4",
     "mocha": "^1.20.1",
     "phantomjs": "1.9.7-10"

tests/js/auth-test.coffee

@@ -14,6 +14,7 @@ class MockSession
 
 describe 'h.auth', ->
   beforeEach module('h.auth')
+  beforeEach module('auth.html')
 
   beforeEach module ($provide) ->
     $provide.value '$timeout', sinon.spy()
@@ -97,39 +98,41 @@ describe 'h.auth', ->
       elem = angular.element(
         '''
         <div class="auth" ng-form="form"
-             on-error="stub" on-success="stub" on-timeout="stub">
-          <form name="login">
-            <input type="text" name="username" ng-model="username"></input>
-          </form>
+             on-error="stub()" on-success="stub()" on-timeout="stub()">
         </div>
         '''
       )
       session = _session_
       $rootScope = _$rootScope_
-      $scope = $compile(elem)($rootScope).scope()
-      $scope.$digest()
+
+      $compile(elem)($rootScope)
+      $rootScope.$digest()
+
+      $scope = elem.isolateScope()
 
     it 'should reset response errors before submit', ->
-      $scope.form.login.responseErrorMessage = 'test'
-      $scope.form.login.username.$setValidity('response', false)
-      assert.isFalse $scope.form.login.$valid
+      $scope.login.username.$setViewValue('test')
+      $scope.login.password.$setViewValue('1234')
+      $scope.login.responseErrorMessage = 'test'
+      $scope.login.username.$setValidity('response', false)
+      assert.isFalse $scope.login.$valid
 
       elem.find('input').trigger('submit')
-      assert.isTrue $scope.form.login.$valid
-      assert.isNull $scope.form.login.responseErrorMessage
+      assert.isTrue $scope.login.$valid
+      assert.isUndefined $scope.login.responseErrorMessage
 
     it 'should reset to pristine state when the model is reset', ->
-      $scope.form.$setDirty()
-      $scope.$digest()
-      assert.isFalse $scope.form.$pristine
+      $rootScope.form.$setDirty()
+      $rootScope.$digest()
+      assert.isFalse $rootScope.form.$pristine
 
       $scope.model = null
       $scope.$digest()
-      assert.isTrue $scope.form.$pristine
+      assert.isTrue $rootScope.form.$pristine
 
     it 'should invoke handlers set by attributes', ->
-      $scope.stub = sinon.stub()
+      $rootScope.stub = sinon.stub()
       for event in ['error', 'success', 'timeout']
-        $scope.stub.reset()
+        $rootScope.stub.reset()
         $scope.$broadcast(event)
-        assert.called $scope.stub
+        assert.called $rootScope.stub