Merge pull request #1278 from hypothesis/1214-and-1266-auth-regressions

Fix regressions in auth errors and state resume

h/js/app.coffee

@@ -2,7 +2,6 @@ imports = [
   'bootstrap'
   'ngAnimate'
   'ngRoute'
-  'h.csrf'
   'h.controllers'
   'h.directives'
   'h.app_directives'

h/js/controllers.coffee

@@ -16,6 +16,7 @@ class App
   scope:
     frame:
       visible: false
+    model: {}
     sheet:
       collapsed: true
       tab: null
@@ -41,18 +42,30 @@ class App
   ) ->
     {plugins, host, providers} = annotator
 
-    $scope.$watch 'auth.personas', (newValue, oldValue) =>
+    _reset = =>
+      delete annotator.ongoing_edit
+      base = angular.copy @scope
+      angular.extend $scope, base,
+        frame: $scope.frame or @scope.frame
+        socialView: annotator.socialView
+
+    _reset()
+
+    session.$promise.then (data) ->
+      angular.extend $scope.model, data
+
+    $scope.$watch 'model.personas', (newValue, oldValue) =>
       if newValue?.length
-        unless $scope.auth.persona and $scope.auth.persona in newValue
-          $scope.auth.persona = newValue[0]
+        unless $scope.model.persona and $scope.model.persona in newValue
+          $scope.model.persona = newValue[0]
       else
-        $scope.auth.persona = null
+        $scope.model.persona = null
 
-    $scope.$watch 'auth.persona', (newValue, oldValue) =>
+    $scope.$watch 'model.persona', (newValue, oldValue) =>
       $scope.sheet.collapsed = true
 
       unless annotator.discardDrafts()
-        $scope.auth.persona = oldValue
+        $scope.model.persona = oldValue
         return
 
       plugins.Auth?.element.removeData('annotator:headers')
@@ -84,7 +97,7 @@ class App
             $scope.reloadAnnotations()
       else if oldValue?
         session.$logout =>
-          $scope.$broadcast '$reset'
+          $scope.$broadcast 'reset'
 
           if annotator.tool isnt 'comment'
             annotator.setTool 'comment'
@@ -110,15 +123,20 @@ class App
           p.channel.notify method: 'setActiveHighlights'
 
     $scope.$watch 'sheet.collapsed', (hidden) ->
-      if hidden
-        $element.find('.sheet').scope().$broadcast('$reset')
-      else
-        $scope.sheet.tab = 'login'
+      $scope.sheet.tab = if hidden then null else 'login'
 
+    authTimeout = null
     $scope.$watch 'sheet.tab', (tab) ->
-      return unless tab
+      if authTimeout
+        $timeout.cancel authTimeout
 
-      $timeout =>
+      unless $scope.model.persona
+        authTimeout = $timeout (-> $scope.$broadcast 'reset'), 60000
+        unless tab
+          $scope.ongoingHighlightSwitch = false
+          delete annotator.ongoing_edit
+
+      $timeout ->
         $element
         .find('form')
         .filter(-> this.name is tab)
@@ -128,16 +146,6 @@ class App
         .focus()
       , 10
 
-      reset = $timeout (-> $scope.$broadcast '$reset'), 60000
-      unwatch = $scope.$watch 'sheet.tab', (newTab) ->
-        $timeout.cancel reset
-        if newTab
-          reset = $timeout (-> $scope.$broadcast '$reset'), 60000
-        else
-          $scope.ongoingHighlightSwitch = false
-          delete annotator.ongoing_edit
-          unwatch()
-
     $scope.$on 'back', ->
       return unless annotator.discardDrafts()
       if $location.path() == '/viewer' and $location.search()?.id?
@@ -146,27 +154,17 @@ class App
         annotator.hide()
 
     $scope.$on 'showAuth', (event, show=true) ->
-      angular.extend $scope.sheet,
-        collapsed: !show
-        tab: 'login'
+      $scope.sheet.collapsed = !show
 
-    $scope.$on '$reset', =>
-      delete annotator.ongoing_edit
-      base = angular.copy @scope
-      angular.extend $scope, base,
-        auth: session
-        frame: $scope.frame or @scope.frame
-        socialView: annotator.socialView
+    $scope.$on 'reset', _reset
 
     $scope.$on 'success', (event, action) ->
+      angular.extend $scope.model, session.model
       if action == 'forgot'
         $scope.sheet.tab = 'activate'
       else
-        $scope.sheet.tab = 'login'
         $scope.sheet.collapsed = true
 
-    $scope.$broadcast '$reset'
-
     $rootScope.viewState =
       sort: ''
       view: 'Document'
@@ -363,7 +361,7 @@ class App
 
         unless data instanceof Array then data = [data]
 
-        p = $scope.auth.persona
+        p = $scope.model.persona
         user = if p? then "acct:" + p.username + "@" + p.provider else ''
         unless data instanceof Array then data = [data]
 
@@ -618,40 +616,37 @@ class Annotation
 
 
 class Auth
-  scope:
-    username: null
-    email: null
-    password: null
-    code: null
-
   this.$inject = [
     '$scope', 'session', 'flash'
   ]
   constructor: (
      $scope,   session,   flash
   ) ->
-    _reset = =>
-      angular.copy @scope, $scope.model
+    base =
+      username: null
+      email: null
+      password: null
+      code: null
+
+    _reset = ->
+      angular.copy base, $scope.model
       for own _, ctrl of $scope when typeof ctrl?.$setPristine is 'function'
         ctrl.$setPristine()
 
-    _success = (form) ->
-      _reset()
-      $scope.$emit 'success', form.$name
-
-    _error = (response) ->
-      if response.errors
-        # TODO: show these messages inline with the form
-        for field, error of response.errors
-          console.log(field, error)
-          flash('error', error)
+    _error = (errors={}) ->
+      # TODO: show these messages inline with the form
+      for field, error of errors
+        console.log(field, error)
+        flash('error', error)
 
-    $scope.$on '$reset', _reset
+    $scope.$on 'reset', _reset
 
     $scope.submit = (form) ->
       angular.extend session, $scope.model
       return unless form.$valid
-      session["$#{form.$name}"] (-> _success form), _error
+      promise = session["$#{form.$name}"] ->
+        $scope.$emit 'success', form.$name
+      promise.then(angular.noop, _error)
 
 
 class Editor

h/js/csrf.coffee

-imports = ['h.helpers']
-
-
-configure = ['$httpProvider', ($httpProvider) ->
-  # Use the Pyramid XSRF header name
-  $httpProvider.defaults.xsrfHeaderName = 'X-CSRF-Token'
-
-  # Track the token with an interceptor because the cookies will not be
-  # available on extension requests due to cross-origin restrictions.
-  $httpProvider.interceptors.push ['baseURI', (baseURI) ->
-    defaults = $httpProvider.defaults
-    token = null
-
-    _getToken = (response) ->
-      data = response.data
-      format = response.headers 'content-type'
-      if format?.match /^application\/json/
-        if data.csrf?
-          token = data.csrf
-          delete data.csrf
-      response
-
-    _setToken = (config) ->
-      if config.url.match(baseURI)?.index == 0
-        cookieName = config.xsrfCookieName || defaults.xsrfCookieName
-        headerName = config.xsrfHeaderName || defaults.xsrfHeaderName
-        config.headers[headerName] ?= token
-      config
-
-    request: _setToken
-    response: _getToken
-    responseError: _getToken
-  ]
-]
-
-
-angular.module('h.csrf', imports, configure)

h/js/flash.coffee

@@ -44,33 +44,5 @@ class FlashProvider
       this._process()
 
 
-flashInterceptor = ['$q', 'flash', ($q, flash) ->
-  _intercept = (response) ->
-    data = response.data
-    format = response.headers 'content-type'
-    if format?.match /^application\/json/
-      if data.flash?
-        for q, msgs of data.flash
-          flash q, msgs
-
-      if data.status is 'failure'
-        flash 'error', data.reason
-        $q.reject(data)
-      else if data.status is 'okay' and data.model
-        response.data = data.model
-        response
-      else
-        response
-    else
-      response
-  response: _intercept
-  responseError: _intercept
-]
-
-
 angular.module('h.flash', ['ngResource'])
 .provider('flash', FlashProvider)
-.factory('flashInterceptor', flashInterceptor)
-.config(['$httpProvider', ($httpProvider) ->
-  $httpProvider.interceptors.push 'flashInterceptor'
-])

h/js/services.coffee

 imports = [
   'h.filters'
-  'h.session'
 ]
 
 
@@ -59,18 +58,15 @@ class Hypothesis extends Annotator
 
   this.$inject = [
     '$document', '$location', '$rootScope', '$route', '$window',
-    'session'
   ]
   constructor: (
      $document,   $location,   $rootScope,   $route,   $window,
-     session
   ) ->
     Gettext.prototype.parse_locale_data annotator_locale_data
     super ($document.find 'body')
 
     window.annotator = this
 
-    @auth = session
     @providers = []
     @socialView =
       name: "none" # "single-player"
@@ -561,9 +557,9 @@ class Hypothesis extends Annotator
         console.log "Not applying any Social View filters."
         delete query.user
       when "single-player"
-        if @auth.persona
+        if @plugins.Permissions?.user
           console.log "Social View filter: single player mode."
-          query.user = @auth.persona
+          query.user = @plugins.Permissions.user
         else
           console.log "Social View: single-player mode, but ignoring it, since not logged in."
           delete query.user
@@ -597,7 +593,6 @@ class Hypothesis extends Annotator
         # If we are not logged in, start the auth process
         scope.ongoingHighlightSwitch = true
         scope.sheet.collapsed = false
-        scope.sheet.tab = 'login'
         this.show()
         return
 

h/js/session.coffee

 imports = [
   'ngResource'
+  'h.flash'
   'h.helpers'
 ]
 
 
-# bw compat
-sessionPersonaInterceptor = (response) ->
-  data = response.data
-  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}"
-  response
-
 ACTION = [
   'login'
   'logout'
@@ -26,8 +17,6 @@ ACTION_OPTION =
   load:
     method: 'GET'
     withCredentials: true
-    interceptor:
-      response: sessionPersonaInterceptor
 
 for action in ACTION
   ACTION_OPTION[action] =
@@ -35,8 +24,12 @@ for action in ACTION
     params:
       __formid__: action
     withCredentials: true
-    interceptor:
-      response: sessionPersonaInterceptor
+
+
+# Global because $resource doesn't support request interceptors, so a
+# the default http request interceptor and the session resource interceptor
+# need to share it.
+csrfToken = null
 
 
 # Class providing a server-side session resource.
@@ -67,16 +60,67 @@ class SessionProvider
     @options = {}
 
   $get: [
-    '$resource', 'baseURI'
-    ($resource,   baseURI) ->
+    '$q', '$resource', 'baseURI', 'flash',
+    ($q,   $resource,   baseURI,   flash) ->
       actions = {}
 
+      _process = (response) ->
+        data = response.data
+        model = data.model
+
+        # 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
+
+        # Capture the cross site request forgery token without cookies.
+        # If cookies are blocked this is our only way to get it.
+        csrfToken = model.csrf
+        delete model.csrf
+
+        # Lift the model object so it becomes the response data.
+
+        # Return the response or a rejected response.
+        if data.status is 'failure'
+          flash 'error', data.reason
+          $q.reject(data.errors)
+        else
+          model
+
       for name, options of ACTION_OPTION
         actions[name] = angular.extend {}, options, @options
+        actions[name].interceptor =
+          response: _process
+          responseError: _process
 
-      model = $resource("#{baseURI}app", {}, actions).load()
+      $resource("#{baseURI}app", {}, actions).load()
   ]
 
 
-angular.module('h.session', imports)
+configure = ['$httpProvider', ($httpProvider) ->
+  defaults = $httpProvider.defaults
+
+  # Use the Pyramid XSRF header name
+  defaults.xsrfHeaderName = 'X-CSRF-Token'
+
+  $httpProvider.interceptors.push ['baseURI', (baseURI) ->
+    request: (config) ->
+      if config.url.match("#{baseURI}app")?.index == 0
+        # Set the cross site request forgery token
+        cookieName = config.xsrfCookieName || defaults.xsrfCookieName
+        headerName = config.xsrfHeaderName || defaults.xsrfHeaderName
+        config.headers[headerName] ?= csrfToken
+      config
+  ]
+]
+
+
+angular.module('h.session', imports, configure)
 .provider('session', SessionProvider)