Commit baf9906d authored by Randall Leeds's avatar Randall Leeds

Fix regressions in auth errors and state resume

Fix two recent regressions with authentication. The first is the lack
of form error reporting. The second is the issue that ongoing highlight
mode switches and edits don't proceed after login.

- Unify all the interceptors related to sessioning. The interceptor for
  flash messages, csrf, and extracting the model from the session view
  responses is now all in session.coffee. This is better because it
  means other requests that don't return data in the same format aren't
  processed by these interceptors. It also makes it clearer which data
  the interceptors are processing; the CSRF interceptor may have been
  broken because the flash interceptor was discarding the data outside
  the model object.
- The CSRF interceptor was also not returning a rejected promise on
  errors. This mistake caused the errors not to be propagated to the
  Auth form controller. Fix #1266.
- The interaction between the Auth controller, session model changes,
  and scope resets is improved. The timeouts on the auth sheet are
  fixed so that they don't lose the ongoing mode switches and edits
  when the sheet is closed. Fix #1214.
parent bf74c788
...@@ -2,7 +2,6 @@ imports = [ ...@@ -2,7 +2,6 @@ imports = [
'bootstrap' 'bootstrap'
'ngAnimate' 'ngAnimate'
'ngRoute' 'ngRoute'
'h.csrf'
'h.controllers' 'h.controllers'
'h.directives' 'h.directives'
'h.app_directives' 'h.app_directives'
......
...@@ -16,6 +16,7 @@ class App ...@@ -16,6 +16,7 @@ class App
scope: scope:
frame: frame:
visible: false visible: false
model: {}
sheet: sheet:
collapsed: true collapsed: true
tab: null tab: null
...@@ -41,18 +42,21 @@ class App ...@@ -41,18 +42,21 @@ class App
) -> ) ->
{plugins, host, providers} = annotator {plugins, host, providers} = annotator
$scope.$watch 'auth.personas', (newValue, oldValue) => session.$promise.then (data) ->
angular.extend $scope.model, data
$scope.$watch 'model.personas', (newValue, oldValue) =>
if newValue?.length if newValue?.length
unless $scope.auth.persona and $scope.auth.persona in newValue unless $scope.model.persona and $scope.model.persona in newValue
$scope.auth.persona = newValue[0] $scope.model.persona = newValue[0]
else 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 $scope.sheet.collapsed = true
unless annotator.discardDrafts() unless annotator.discardDrafts()
$scope.auth.persona = oldValue $scope.model.persona = oldValue
return return
plugins.Auth?.element.removeData('annotator:headers') plugins.Auth?.element.removeData('annotator:headers')
...@@ -110,15 +114,20 @@ class App ...@@ -110,15 +114,20 @@ class App
p.channel.notify method: 'setActiveHighlights' p.channel.notify method: 'setActiveHighlights'
$scope.$watch 'sheet.collapsed', (hidden) -> $scope.$watch 'sheet.collapsed', (hidden) ->
if hidden $scope.sheet.tab = if hidden then null else 'login'
$element.find('.sheet').scope().$broadcast('$reset')
else
$scope.sheet.tab = 'login'
authTimeout = null
$scope.$watch 'sheet.tab', (tab) -> $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 $element
.find('form') .find('form')
.filter(-> this.name is tab) .filter(-> this.name is tab)
...@@ -128,16 +137,6 @@ class App ...@@ -128,16 +137,6 @@ class App
.focus() .focus()
, 10 , 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', -> $scope.$on 'back', ->
return unless annotator.discardDrafts() return unless annotator.discardDrafts()
if $location.path() == '/viewer' and $location.search()?.id? if $location.path() == '/viewer' and $location.search()?.id?
...@@ -146,23 +145,20 @@ class App ...@@ -146,23 +145,20 @@ class App
annotator.hide() annotator.hide()
$scope.$on 'showAuth', (event, show=true) -> $scope.$on 'showAuth', (event, show=true) ->
angular.extend $scope.sheet, $scope.sheet.collapsed = !show
collapsed: !show
tab: 'login'
$scope.$on '$reset', => $scope.$on '$reset', =>
delete annotator.ongoing_edit delete annotator.ongoing_edit
base = angular.copy @scope base = angular.copy @scope
angular.extend $scope, base, angular.extend $scope, base,
auth: session
frame: $scope.frame or @scope.frame frame: $scope.frame or @scope.frame
socialView: annotator.socialView socialView: annotator.socialView
$scope.$on 'success', (event, action) -> $scope.$on 'success', (event, action) ->
angular.extend $scope.model, session.model
if action == 'forgot' if action == 'forgot'
$scope.sheet.tab = 'activate' $scope.sheet.tab = 'activate'
else else
$scope.sheet.tab = 'login'
$scope.sheet.collapsed = true $scope.sheet.collapsed = true
$scope.$broadcast '$reset' $scope.$broadcast '$reset'
...@@ -363,7 +359,7 @@ class App ...@@ -363,7 +359,7 @@ class App
unless data instanceof Array then data = [data] 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 '' user = if p? then "acct:" + p.username + "@" + p.provider else ''
unless data instanceof Array then data = [data] unless data instanceof Array then data = [data]
...@@ -618,40 +614,37 @@ class Annotation ...@@ -618,40 +614,37 @@ class Annotation
class Auth class Auth
scope:
username: null
email: null
password: null
code: null
this.$inject = [ this.$inject = [
'$scope', 'session', 'flash' '$scope', 'session', 'flash'
] ]
constructor: ( constructor: (
$scope, session, flash $scope, session, flash
) -> ) ->
_reset = => base =
angular.copy @scope, $scope.model 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' for own _, ctrl of $scope when typeof ctrl?.$setPristine is 'function'
ctrl.$setPristine() ctrl.$setPristine()
_success = (form) -> _error = (errors={}) ->
_reset() # TODO: show these messages inline with the form
$scope.$emit 'success', form.$name for field, error of errors
console.log(field, error)
_error = (response) -> flash('error', error)
if response.errors
# TODO: show these messages inline with the form
for field, error of response.errors
console.log(field, error)
flash('error', error)
$scope.$on '$reset', _reset $scope.$on '$reset', _reset
$scope.submit = (form) -> $scope.submit = (form) ->
angular.extend session, $scope.model angular.extend session, $scope.model
return unless form.$valid 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 class Editor
......
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)
...@@ -44,33 +44,5 @@ class FlashProvider ...@@ -44,33 +44,5 @@ class FlashProvider
this._process() 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']) angular.module('h.flash', ['ngResource'])
.provider('flash', FlashProvider) .provider('flash', FlashProvider)
.factory('flashInterceptor', flashInterceptor)
.config(['$httpProvider', ($httpProvider) ->
$httpProvider.interceptors.push 'flashInterceptor'
])
imports = [ imports = [
'h.filters' 'h.filters'
'h.session'
] ]
...@@ -59,18 +58,15 @@ class Hypothesis extends Annotator ...@@ -59,18 +58,15 @@ class Hypothesis extends Annotator
this.$inject = [ this.$inject = [
'$document', '$location', '$rootScope', '$route', '$window', '$document', '$location', '$rootScope', '$route', '$window',
'session'
] ]
constructor: ( constructor: (
$document, $location, $rootScope, $route, $window, $document, $location, $rootScope, $route, $window,
session
) -> ) ->
Gettext.prototype.parse_locale_data annotator_locale_data Gettext.prototype.parse_locale_data annotator_locale_data
super ($document.find 'body') super ($document.find 'body')
window.annotator = this window.annotator = this
@auth = session
@providers = [] @providers = []
@socialView = @socialView =
name: "none" # "single-player" name: "none" # "single-player"
...@@ -561,9 +557,9 @@ class Hypothesis extends Annotator ...@@ -561,9 +557,9 @@ class Hypothesis extends Annotator
console.log "Not applying any Social View filters." console.log "Not applying any Social View filters."
delete query.user delete query.user
when "single-player" when "single-player"
if @auth.persona if @plugins.Permissions?.user
console.log "Social View filter: single player mode." console.log "Social View filter: single player mode."
query.user = @auth.persona query.user = @plugins.Permissions.user
else else
console.log "Social View: single-player mode, but ignoring it, since not logged in." console.log "Social View: single-player mode, but ignoring it, since not logged in."
delete query.user delete query.user
...@@ -597,7 +593,6 @@ class Hypothesis extends Annotator ...@@ -597,7 +593,6 @@ class Hypothesis extends Annotator
# If we are not logged in, start the auth process # If we are not logged in, start the auth process
scope.ongoingHighlightSwitch = true scope.ongoingHighlightSwitch = true
scope.sheet.collapsed = false scope.sheet.collapsed = false
scope.sheet.tab = 'login'
this.show() this.show()
return return
......
imports = [ imports = [
'ngResource' 'ngResource'
'h.flash'
'h.helpers' '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 = [ ACTION = [
'login' 'login'
'logout' 'logout'
...@@ -26,8 +17,6 @@ ACTION_OPTION = ...@@ -26,8 +17,6 @@ ACTION_OPTION =
load: load:
method: 'GET' method: 'GET'
withCredentials: true withCredentials: true
interceptor:
response: sessionPersonaInterceptor
for action in ACTION for action in ACTION
ACTION_OPTION[action] = ACTION_OPTION[action] =
...@@ -35,8 +24,12 @@ for action in ACTION ...@@ -35,8 +24,12 @@ for action in ACTION
params: params:
__formid__: action __formid__: action
withCredentials: true 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. # Class providing a server-side session resource.
...@@ -67,16 +60,67 @@ class SessionProvider ...@@ -67,16 +60,67 @@ class SessionProvider
@options = {} @options = {}
$get: [ $get: [
'$resource', 'baseURI' '$q', '$resource', 'baseURI', 'flash',
($resource, baseURI) -> ($q, $resource, baseURI, flash) ->
actions = {} 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 for name, options of ACTION_OPTION
actions[name] = angular.extend {}, options, @options 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) .provider('session', SessionProvider)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment