Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

?? SOLUTION to 122 (IE problem) ?? #123

Closed
dickschrauwen opened this issue Nov 30, 2016 · 14 comments
Closed

?? SOLUTION to 122 (IE problem) ?? #123

dickschrauwen opened this issue Nov 30, 2016 · 14 comments

Comments

@dickschrauwen
Copy link

Modified forward.mustache

<!DOCTYPE html>
<html>
  <head>
    <title>Portier &ndash; Forwarding...</title>
    <script src="/static/forward.js"></script>
  </head>
  <body>
    <form id="form" action="{{ redirect_uri }}" method="post">
      {{# params }}
        <input type="hidden" name="{{ name }}" value="{{ value }}">
      {{/ params }}
    </form>
	<!-- a very very simple solution to IE-problem (https://github.com/portier/portier-broker/issues/122) -->
	<!-- NOT tested -->
	<SCRIPT>
	  document.getElementById('form').submit();
	  // a (to my taste to complicated) alternative @ https://github.com/jfriend00/docReady 
	</SCRIPT>
	
  </body>
</html>
@dickschrauwen dickschrauwen changed the title SOLUTION to 122 (IE problem) ?? SOLUTION to 122 (IE problem) ?? Nov 30, 2016
@onli
Copy link
Member

onli commented Nov 30, 2016 •
edited
Loading

(reference to #122)

To just call it inline would most likely solve this, but I think inline JS is forbidden by our security headers.

But we can also solve it in /static/forward.js. One of two solution should work:

  1. Move the script element to the bottom and call the submit() directly, without wrapping it in an event for DomContentLoaded
  2. Check whether that event already fired, by checking document.readyState == 'complete', as in https://github.com/onli/feedtragon/blob/11c5fc582f015f56f0bc8775dc3b40085dbb2929/public/main.js#L428

This assumes the problem is that DomContentLoaded was already executed when that JS is interpreted which is a likely problem with IE. But that is what needs to be tested.

Regardless, we can't rely on the current foward.js, as it is bound to happen sometimes that the event was fired before. I got burned by that browser behaviour in two different projects already, should've noticed it before :/

@dickschrauwen
Copy link
Author

dickschrauwen commented Dec 1, 2016 •
edited
Loading

Javascript is not my mother tongue nor is RUST. Why don't POST the 'form' directly to the receiving url?Would be simple in Python using http://docs.python-requests.org/en/master/ | $ pip install requests

@onli
Copy link
Member

onli commented Dec 1, 2016

Because you want the user to deliver the token via the browser, so that in the end he is directly on the page he tried to login to.

I think you got the issue identified, and I'm pretty sure one of my two approaches will fix this. We just need someone to test it, which means running a patched broker and IE 11, if necessary in a VM. @stephank, might that maybe be easiest for you?

@dickschrauwen
Copy link
Author

Okay, I see. I'll be happy to test it (running several winboxes 7, 8, 10, can also test it on OSX, Tails/Tor (if I'm able to install Python3), Deb8-Jessy

@stephank
Copy link
Member

stephank commented Dec 2, 2016

I just tried this on a Windows 7 VM with IE 11, downloaded from modern.ie, and it seems to work fine. Tried with the current public broker at broker.portier.io, and with current master branch.

I also tested with a real, up-to-date Windows 10 machine we have at the office, and it works in both IE 11 and Edge there.

So something else is up, but I'm not sure what. :/

@stephank
Copy link
Member

stephank commented Dec 2, 2016

@dickschrauwen Can you check if the public demo works for you, on the setup where you see the issue? It's at: https://portier-demo.herokuapp.com/

@dickschrauwen
Copy link
Author

dickschrauwen commented Dec 2, 2016 •
edited
Loading

@stephank

Tested  https://portier-demo.herokuapp.com/ 
chrome54 win7  :: You are now logged in as [email protected]  (via link)
ie11 win7      :: You are now logged in as [email protected]. (automatic)  how/why?
ie11 win7      :: You are now logged in as [email protected].  (copy/paste 	code)
ie11 win7      :: You are now logged in as [email protected] (via email link) 
Tor? tails2.4  :: You are now logged in as [email protected] (via email link) 
Tor? tails2.4  :: You are now logged in as [email protected]. (automatic / logged in in gmail)
edge? win10    :: You are now logged in as [email protected] (via google account)
ffox47 win10   :: You are now logged in as [email protected] (via email link)
ie11 win10     :: You are now logged in as [email protected]  (via email link)
ffox50 win10 :: You are now logged in as [email protected] (via g-account)

@stephank
Copy link
Member

stephank commented Dec 2, 2016

@dickschrauwen So the redirect only fails when you try to run the demo locally?

I suppose both automatic logins are because you used a @gmail.com address which happened to be logged in at Google in that browser.

@dickschrauwen
Copy link
Author

@stephank Yes, locally the redirect stil fails when i run it locally in IE / chrome is OK

@dickschrauwen
Copy link
Author

So something else is up, but I'm not sure what. :/ , strange ....

@onli
Copy link
Member

onli commented Dec 2, 2016 •
edited
Loading

Okay, at the very least we know the event timing is not the problem.

Maybe IE11 is blocking the redirect to localhost only? https://stackoverflow.com/questions/17415111/127-0-0-1-will-not-work-in-ie-11 suggests that there might be issues with the security model. If it were that, it'd be out of our hands.

@dickschrauwen
Copy link
Author

Anyway, i'm just going to migrate from Persona to Portier. Checked out Janrain and Stormpath but i dont like it

@dickschrauwen
Copy link
Author

I tried all 'solutions' in https://stackoverflow.com/questions/17415111/127-0-0-1-will-not-work-in-ie-11 . Didn't help.; forward/redirtect still fails in IE

@stephank
Copy link
Member

Hope you don't mind me closing this. See #122 and #204.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@stephank @onli @dickschrauwen