hotfix: Add lesmis la is valid url to pdf exporter #4062
Conversation
| @@ -39,7 +39,7 @@ rm -rf lastpass-cli | |||
| mkdir -p /home/ubuntu/.local/share/lpass | |||
|
|
|||
| # install puppeteer dependencies https://pptr.dev/15.3.0/troubleshooting#chrome-headless-doesnt-launch-on-unix | |||
| sudo apt-get -y install \ | |||
| sudo apt-get -yqq install \ | |||
There was a problem hiding this comment.
dependencies didn't install in gold master, see if this will fix.
There was a problem hiding this comment.
Curious to know why adding qq to the args would fix it? Doesn't that just affect logging?
biaoli0
force-pushed
the
add-lesmis-la-is-valid-url-to-pdf-exporter
branch
from
303724e to
dac846b
Compare
biaoli0
force-pushed
the
add-lesmis-la-is-valid-url-to-pdf-exporter
branch
from
dac846b to
0fcee53
Compare
| ca-certificates \ | ||
| fonts-liberation \ | ||
| libappindicator3-1 \ | ||
| libasound2 \ | ||
| libatk-bridge2.0-0 \ | ||
| libatk1.0-0 \ | ||
| libc6 \ | ||
| libcairo2 \ | ||
| libcups2 \ | ||
| libdbus-1-3 \ | ||
| libexpat1 \ | ||
| libfontconfig1 \ | ||
| libgbm1 \ | ||
| libgcc1 \ | ||
| libglib2.0-0 \ |
There was a problem hiding this comment.
@rohan-bes And also one space in front of each dependency for fixing it.
There was a problem hiding this comment.
Ahh whitespace issues, I love em 😂 😭
| @@ -38,12 +38,17 @@ export class PDFExportRoute extends Route<PDFExportRequest> { | |||
| }; | |||
|
|
|||
| private verifyBody = (body: any): Body => { | |||
| const legitLESMISHostnames = ['lesmis.la', 'www.lesmis.la']; | |||
There was a problem hiding this comment.
I'd suggest having an explicit list of the domains that Tupaia supports
| const legitLESMISHostnames = ['lesmis.la', 'www.lesmis.la']; | |
| const validDomains= ['.tupaia.org', '.lesmis.la']; |
And then later do:
if (!validDomains.some(domain=> location.hostname.endsWith(domain)) && !location.hostname.endsWith('localhost')) {
Might even be best to store this list of valid hostnames in a utils file/function somewhere, but I can't see another area in the code where do we such a check, so it seems okay to me for it to be here for now 👍
There was a problem hiding this comment.
Thanks for the renaming suggestion.
This check is for security, because pdf-export-server will access the provided pdfPageUrl from puppeteer headless browser, and without this check, hacker could easily get user's session by a malicious link from the pdfPageUrl.
There was a problem hiding this comment.
Just found that we have to keep lesmis.la because it is a legit DNS for lesmis as well.
|
@billli0 you know the drill when it comes to making changes to the |
|
@rohan-bes Yes I know how to update this file in s3, but I don't have permission to kick off a build. Could you grant me the access? |
|
@billli0 okay have bumped your permissions, wanna give it another shot? |
|
Thanks @rohan-bes, it works! |
Issue #:
pdf export is broken in
lesmis.la