ohmage / gwt-front-end

The javascript/GWT code behind the front end.
6 stars 2 forks source link

unified auth page: redirect issue #287

Closed stevenolen closed 11 years ago

stevenolen commented 11 years ago

I think the frontend handles the logic here, but could be wrong. Since the unified login page redirects back to the redirect url, this can sometimes result in very weird results. Steps to reproduce:

Perhaps, we can resolve this by preventing redirects outside the frontend's domain, and then offer an optional "redirect url" parameter for when an external app visits our site for auth, and wants to send authenticated users to a specific location.

jshslsky commented 11 years ago

@jojenki can you check this out?

jojenki commented 11 years ago

The server's auth_token API only takes a path, meaning it won't allow the above scenario to happen. Therefore, this is a server front end issue.

jshslsky commented 11 years ago

Because IE7 and IE8 return the empty string for document.referrer when the connection is https, I am thinking about supporting a redirect parameter. I think we initially wanted the front end to use a redirect parameter, but then dropped it in favor of the Referrer header. I want to add a restriction that will only allow the redirect parameter to contain relative URIs. Can you guys think of anything that would break if I do this? @jeroenooms

jeroen commented 11 years ago

Couple of notes:

On Thu, Jun 20, 2013 at 7:47 AM, Joshua Selsky notifications@github.comwrote:

Because IE7 and IE8 return the empty string for document.referrer when the connection is https, I am thinking about supporting a redirect parameter. I think we initially wanted the front end to use a redirect parameter, but then dropped it in favor of the Referrer header. I want to add a restriction that will only allow the redirect parameter to contain relative URIs. Can you guys think of anything that would break if I do this? @jeroenooms https://github.com/jeroenooms

— Reply to this email directly or view it on GitHubhttps://github.com/cens/ohmageFrontEnd/issues/287#issuecomment-19757950 .

stevenolen commented 11 years ago

When creating tests for selenium/sauce labs some selenium web drivers use the redirect and prevent automated testing from taking place on a plethora of platforms.

jshslsky commented 11 years ago

Can you just test the redirect manually? I'm not sure I understand the problem.

stevenolen commented 11 years ago

the selenium actions are pre-defined actions which navigate through the frontend and very elements exist properly, etc.

the first of these actions is: visit https://test.ohmage.org/web. This works just fine on firefox, chrome and ie...but on safari and any mobile devices, the webdriver (powering the tests) does some sort of redirect from the webdriver start page, which then invokes this bug. At that point with the web driver, I have no control over the driver or the actual test at that point, though.

hopefully that's explaining a bit better. I'm not actually testing the redirect, but the implementation of the redirect (which we already noted in this issue has some flaws) is preventing my automated tests from being useful.

jshslsky commented 11 years ago

That is very odd. If you are going straight to /web there should not be a redirect at all.

There are three different ways a redirect happens.

  1. You navigate to / and Tomcat sends a 302 to go directly to /web.
  2. You want to use the unified login page with another browser-based ohmage app so you add your URI to a referrer header and redirect to / where (1) happens. After login, the front end examines the redirect header and uses document.location.replace() to send the browser to the other ohmage app.
  3. Like (2) but uses a cookie for the redirect for older versions of IE. (Specific to Trialist.)
stevenolen commented 11 years ago

in (2) the URI for referrer header is set and followed regardless of whether you direct someone to / or /web.

you can take a look at what is happening using a test with safari below, or the steps above (googling test.ohmage.org and following the link).
https://saucelabs.com/tests/8e38857d516a4fa4b018cafe4bbb9465 It's a bit odd that the web driver is considering the visit to test.ohmage.org/web/#login a referral, but that's far beyond the scope, I think...