zooniverse / AnnoTate

Full text transcription app for the Tate Britain
http://anno.tate.org.uk
Other
12 stars 2 forks source link

Implement OAuth module #206

Closed rogerhutchings closed 8 years ago

rogerhutchings commented 8 years ago

Counterpart to https://github.com/zooniverse/shakespeares_world/pull/288

shaunanoordin commented 8 years ago

I've deployed this branch to https://preview.zooniverse.org/annotate/ for testing but have been unable to perform the login tests.

Issue: Attempting to log in to Panoptes from https://preview.zooniverse.org/annotate/ will result in a The redirect uri included is not valid. error. (Redirect URI is https://preview.zooniverse.org/annotate/) Solution: Staging URL (notably with the HTTPS) added onto the whitelist of the production app

Status: Solved Actions: Tests resuming

shaunanoordin commented 8 years ago

We have an issue, but we can figure out what it is.

Issue: After logging in to https://preview.zooniverse.org/annotate/ via Panoptes, the app does not apparently recognise that the user has logged in; the 'Signed In' button remains.

Analysis: The Panoptes JS client correctly detects that the user is logged in, but the Angular framework isn't notified of this. The problem can be traced to this change in app/modules/auth/auth.factory.js, _setUserData():

.then(function (response) {
  var response = response[0];
  _user.avatar = (response.src) ? response.src : null;
  //$rootScope.$broadcast('auth:loginChange', _user);  //This was removed, but not replaced
  return _user;
},

The lack of a .$broadcast prevented the Angular framework from realising the user has logged in on page load.

Recommendation:

function _setUserData() {
  return zooAPI.type('me').get()
    .then(function (response) {
      var response = response[0];
      _user.id = response.id;
      _user.display_name = response.display_name;
      return response.get('avatar');
    })
    .then(function (response) {
      var response = response[0];
      _user.avatar = (response.src) ? response.src : null;
      $rootScope.$broadcast('auth:loginChange', _user);  //ADD
      return _user;
    }, function(error) {
      console.info('No avatar found for', _user.id);
      $rootScope.$broadcast('auth:loginChange', _user);  //ADD
      return _user;
    })
    .catch(function (error) {
      console.error('Error setting user data', error);
    });
}

Actions: Awaiting fix, although I'm quite happy to apply the fix myself then re-test/merge, if that's OK with the Zoo's code review etiquette.

rogerhutchings commented 8 years ago

D'oh. Pushed a fix and deployed it to staging

shaunanoordin commented 8 years ago

Nice one! The website works great now.

Tested on Chrome50/Firefox45/Edge25 on Win10+SurfacePro3, Firefox45 on OSX+MacbookPro

Status: Verified Action: Merging and deploying master

shaunanoordin commented 8 years ago

Oh yes, I learnt a little lesson from Shakespeare's World. Before deploying, I checked https://anno.tate.org.uk and made the following observations of the current baseline behaviour:

XMLHttpRequest cannot load https://panoptes.zooniverse.org/oauth/token. Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'https://anno.tate.org.uk' is therefore not allowed access. The response had HTTP status code 404. and XMLHttpRequest cannot load https://panoptes.zooniverse.org/?now=1461020251076. No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'https://anno.tate.org.uk' is therefore not allowed access.

After I deploy the updated master, I'll compare this baseline with the latest behaviour.