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.

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

Both Google Chrome and Safari have begun to block GRNsight's iframe #688

Closed
mihirsamdarshi opened this issue Oct 11, 2018 · 42 comments
Closed

Comments

@mihirsamdarshi
Copy link
Collaborator

At first it was just Safari, but now Google Chrome has started blocking the iframe for "attempting to load unsafe scripts." This needs to be fixed fairly soon.
screen shot 2018-10-11 at 1 11 43 pm

@kdahlquist
Copy link
Collaborator

@kdahlquist doesn't see this with Chrome on her machine (version is up to date, Version 69.0.3497.100 (Official Build) (64-bit)). Need to check with other people to see if this is happening to them.

@mihirsamdarshi
Copy link
Collaborator Author

The console shows the exact source of the problem:
screen shot 2018-10-25 at 2 31 22 pm

@jlopez616
Copy link
Collaborator

According to this link, this might require serving the pages using HTTPS

@mihirsamdarshi
Copy link
Collaborator Author

There are 2 parts to solving this:

  1. adding a package so that Express can serve the iframe over HTTPS
  1. we need a certificate for that

@dondi
Copy link
Owner

dondi commented May 30, 2019

It looks like #327 may be a duplicate of this issue.

@kdahlquist
Copy link
Collaborator

Yes, I'll close #327--it's been duplicated a few times actually.

@mihirsamdarshi
Copy link
Collaborator Author

@dondi, I believe that I set up all the requirements for the server to be delivered over https. The webpage at https://localhost:5001 is delivered over HTTPS, however any requests are not delivered via SSL, because apparently http://localhost:5000 is not over HTTP, despite me setting it up so

@dondi
Copy link
Owner

dondi commented May 31, 2019

@mihirsamdarshi There are still a lot of specific accesses of plain HTTP in the source code. For example, here: https://github.com/dondi/GRNsight/blob/beta/server/config/config.js#L5-L14

Try transitioning all of those to https:// and see if that makes a difference.

@mihirsamdarshi
Copy link
Collaborator Author

@dondi I figured it out, just waiting on certificates from Masao

@dondi
Copy link
Owner

dondi commented May 31, 2019

OK sounds good, I will check with him.

Of note, certificates are domain-specific. So, whatever you did to get things to work on your specific computer will remain true for other developers. The certificate we acquire will pertain only too grnsight.cs.lmu.edu and will require specific installation on that host. This certificate will not be of use for developers running a local version.

Thus, plan on documenting what you did so that future developers will know how to set up their computers for GRNsight development. It will also be a good idea to look ahead and also document how to set up a permanent server like grnsight.cs.lmu.edu. (I presume that it will be very similar to local computer setup, but just using a different certificate)

@kdahlquist
Copy link
Collaborator

How about creating a wiki page (or adding to one of the ones we have)...

@mihirsamdarshi
Copy link
Collaborator Author

Will do am mocking it up in the setting up GRNsight wiki page right now @dondi @kdahlquist

@mihirsamdarshi
Copy link
Collaborator Author

Both have been added to the wiki, although I am not sure yet about the protocol for the production and beta servers yet.

@dondi
Copy link
Owner

dondi commented Jun 2, 2019

The write-ups generally look right, though I have one question: Does every developer need their own self-signed .key and .cert files, or can one fixed set be created then committed, for all developers to use? The files are known to be for developer use only anyway, so I don't have a problem with putting them on the repository since they'll only work with local machine setups.

That is, presuming that it works. See what you can find out in that department.

@mihirsamdarshi
Copy link
Collaborator Author

@dondi it seems that each developer needs their own self-signed certificate, as it seems that there are security concerns with pre-shared self-signed certificates, although I'm not sure what those concerns are

@dondi
Copy link
Owner

dondi commented Jun 4, 2019

Masao has gotten back to @mihirsamdarshi and me with the certificate info and upon looking at what he did, it appears that he was able to do this at a level above our actual software; i.e., the grnsight.cs.lmu.edu host is already serving HTTPS without any software-level changes (with respect to security):

image

The benefit of this is that it looks like we won’t need to do any further server configuration when deploying GRNsight. Masao has done the work.

The downside is that with the new HTTPS-at-the-level-of-grnsight.cs.lmu.edu setup already having been done, both our release and beta pages are broken (also visible in the screenshot above):

http://dondi.github.io/GRNsight/
http://dondi.github.io/GRNsight/beta.html

The reason for the breakage is the same (attempt to access HTTP from an HTTPS resource), but the source of the HTTP has propagated to a different place. (i.e., whereas the original error was due to grnsight.cs.lmu.edu being HTTP, this time, that site is now HTTPS and it is complaining about resources being accessed via HTTP)

@mihirsamdarshi The task now shifts to looking at what the above sites are complaining about then tracking down the places where those requests are being made. Most of the time, the change will be a matter of shifting URLs from http:// to https:// (or possibly just // as mentioned earlier).

There may be some instances where it won’t be that simple. Alert me if you encounter those.

As a starter, attached is the error log that I’m currently getting for http://dondi.github.io/GRNsight/beta.html —I would track down each of those errors to the source of the erroneous request in our code, then try to change that to // first, and if that doesn’t work, change to to an explicit https://.

Hope these instructions make sense. The trial I’m on isn’t over so I will only be accessible during recesses and breaks.

image

Because the sites are broken now, there may be a need for a “hot fix” where we deploy just HTTPS-related changes. Either that or @mihirsamdarshi you can ask Masao to temporarily back out these server changes until such time that the GRNsight software itself is ready for full HTTPS. The final course of action here will, I think, depend on what you find regarding the error messages that are showing up right now.

@kdahlquist
Copy link
Collaborator

How about putting an "under construction" type message at the top of the GRNsight master and beta pages explaining that the sites are unavailable due to conversion to https? It seems that would be preferable than having Masao turn it off again. Assuming that it will be fixed in the next couple of days.

@mihirsamdarshi
Copy link
Collaborator Author

mihirsamdarshi commented Jun 4, 2019

@kdahlquist I just added that

@dondi does this mean I should remove the Express changes from my PR?

@dondi
Copy link
Owner

dondi commented Jun 4, 2019

@mihirsamdarshi I’m not sure what will stay or go. The problem with removing the https server for local development is that, if we change all other links to https, then I don’t know if a local server instance will work if it can’t do https.

Then again, I don’t think having a local Express HTTPS server will work well with the server-level certificate and HTTPS that Masao has set up.

My guess as to what will happen: (a) yes we will no longer need the Express https server, but (b) we will need a configuration change where, when running locally, we use http links but running in production or beta uses https links.

This would be exactly the situation where just plain // URLs (i.e., without either http: nor https: preceding) would be useful, if they worked. There might be some trial and error needed (sorry, no pun intended regarding my current situation).

I think you’ll have to do the following:

  • Test a local server to make sure it works without errors
  • Upload that to beta to see if that works (if we’re under construction, then having up-and-down functionality will be understandable)
  • Rinse and repeat until you have the right combination of software, configuration, and links that will work correctly in both situations

@dondi
Copy link
Owner

dondi commented Jun 4, 2019

Another possibility: set the server up so that it responds to both http and https. That way, the https “wrapper” takes over on the server for production and beta while talking to that server in http, but for local development, we access the https on the Express server directly.

There is some open-endedness here (i.e., I don’t know the answer offhand myself, and would be doing the exact same thing I’m suggesting in these comments if I were tasked directly with resolving this issue). The key is to be aware of what we want to accomplish and to systematically work our way to that situation.

@mihirsamdarshi
Copy link
Collaborator Author

understood, thanks

@mihirsamdarshi
Copy link
Collaborator Author

mihirsamdarshi commented Jun 5, 2019

I just added the fixes for jQuery not delivering over HTTPS, however I still get the error Mixed Content: The page at 'https://dondi.github.io/GRNsight/beta.html' was loaded over HTTPS, but requested an insecure resource 'http://grnsight.cs.lmu.edu/beta/client'. This request has been blocked; the content must be served over HTTPS'. This request has been blocked; the content must be served over HTTPS. when loading the page, meaning that the iframe is still not being delivered over HTTPS. I think that the server level changes, creating a certificate must be done.

@mihirsamdarshi
Copy link
Collaborator Author

this is fixed, and the iframe now delivers over HTTPS

@dondi
Copy link
Owner

dondi commented Jun 5, 2019

When the https transition is complete, how should we handle master/production? We can either wait until we have a release, or we “peel off” just the https changes into an interim hotfix on the master branch.

@mihirsamdarshi
Copy link
Collaborator Author

Actually, all it took was changing gh-pages, so Master is already fixed as well

@mihirsamdarshi
Copy link
Collaborator Author

I have also configured it so that localhost does not need to be served over HTTPS, thus eliminating a need for developers to install their own certificates.

@dondi
Copy link
Owner

dondi commented Jun 5, 2019

I’m still seeing errors in both production and beta; looks the same for production as yesterday, but down to just the /server/ga one in beta (see screenshots). (I disabled to cache to make sure I got the latest files)

It looks like the production files haven’t been updated yet? Let me know when the HTTPS transition is complete so we can close this issue. Thanks!

  • Production:

image

  • Beta:

image

@kdahlquist
Copy link
Collaborator

I'm going to leave this one open--on my machine, beta is functional in Firefox and Chrome, but master is not. I'll let @dondi chime in as to when it is appropriate to close.

@dondi
Copy link
Owner

dondi commented Jun 6, 2019

I’m seeing the same thing as @kdahlquist: production isn’t loading right but beta is. I’m currently not in a situation where I can check out the console log so can’t provide further details for now, so @mihirsamdarshi please verify whether master/production actually needs a re-release. You had mentioned in #688 (comment) that it was not needed, but based on what we’re seeing this might not be the case.

@kdahlquist
Copy link
Collaborator

@mihirsamdarshi told me that master won't be updated until we do a release.

@dondi
Copy link
Owner

dondi commented Jun 6, 2019

OK, so perhaps we can leave the under construction sign up for Home until then? @mihirsamdarshi let me know which open or new issues need my intervention.

@mihirsamdarshi
Copy link
Collaborator Author

@dondi, hopefully this will be gone after the new release, because it is simply that jQuery and Bootstrap are not being delivered over HTTPS, which has already been fixed in beta

@dondi
Copy link
Owner

dondi commented Jun 7, 2019

This does appear to be the case, so let’s note this as something to monitor whenever the release to production has taken place.

@dondi
Copy link
Owner

dondi commented Jun 7, 2019

Possible regression noted in beta 3.1.24:

#406 (comment)

@mihirsamdarshi
Copy link
Collaborator Author

@dondi not sure what is going on since that error does not occur when hosted locally

@mihirsamdarshi
Copy link
Collaborator Author

Never mind, I figured it out, it is Google Analytics code that triggers that request, however I am not sure how to edit it since it isn't hosted in the GRNsight folder.

@dondi
Copy link
Owner

dondi commented Jun 7, 2019

Try changing this one to https:// or just plain //:

https://github.com/dondi/GRNsight/blob/gh-pages/assets/js/ga-report.js#L3

@mihirsamdarshi
Copy link
Collaborator Author

I didn't realize ga-report was hosted in gh-pages, it is edited and fixed.

@dondi
Copy link
Owner

dondi commented Jun 7, 2019

Neither did I, until I started to look for it.

Always take into account the entire website when triaging requests that have issues, and not just the specific web app code that you’re working with.

@dondi
Copy link
Owner

dondi commented Jun 7, 2019

You need one more change; now that the requesting URL is https, then the CORS header should also be changed to accept https:

https://github.com/dondi/GRNsight/blob/master/server/controllers/ga-controller.js#L46

Change the http:// on that line to https://

@dondi
Copy link
Owner

dondi commented Jun 7, 2019

(not sure if you weren’t seeing that CORS error due to the presence of a CORS-disabling extension that’s installed in the browser; if you do have such an extension, I recommend generally keeping it turned off unless there is really no other way to get a CORS issue to work—otherwise, you won’t see errors that most other users will see)

@kdahlquist
Copy link
Collaborator

production is fixed upon release of 4.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants