Add links to uploads to Audit Report

[ajhyndman]

Fixes #330

[ajhyndman]

rather than setting a few more env vars, we can probably just pull the domain out of the req variable used to kick off the audit report gen process. we might not have the website url available in the API, but we do have it available on the client-side when the request is made, maybe it could be part of the request parameters?

Yeah, there are definitely a few approaches here. I don't have a clear preference.

(a) As noted, we can't use the default properties on the express request, because the API host isn't necessarily the same as the website host. (b) We can declare the website host in an environment variable, but this an extra point of configuration to remember. (c) We can derive the host from window.location in the client and pass it to the server in a request parameter. The issue here is that we have to trust the client, and something like a browser extension could hijack either window.location or issue requests with an alternative parameter.

🤷

[igor47]

(a) As noted, we can't use the default properties on the express request, because the API host isn't necessarily the same as the website host.

only in dev, in prod they're the same

(c) We can derive the host from window.location in the client and pass it to the server in a request parameter. The issue here is that we have to trust the client, and something like a browser extension could hijack either window.location or issue requests with an alternative parameter.

i don't think browser extensions hijack window.location, that's a pretty core security feature of browsers. i would prefer this approach, but i can unblock this approach also if you think it's much better, esp. if you also clean up the existing BASE_URL config.

[ajhyndman]

Okay, I learned a few more things

First, BASE_URL is actually an env variable auto-generated by Vue.

https://cli.vuejs.org/guide/mode-and-env.html#using-env-variables-in-client-side-code

Bizarrely, it doesn't seem like variables we declare in .env are actually available to the Vue HTML processory, so I don't think we can remove BASE_URL. Note that the GOST repo also uses BASE_URL for the same purposes. (I'm pretty certain this is part of the boilerplate generated when you bootstrap a new Vue app.

https://github.com/usdigitalresponse/usdr-gost/search?q=BASE_URL

Of course, since this env variable is provided by vue-cli-service, we can't access it from express — especially in production. I can't see a way around keeping both values. 😧

---

Second, I'm pretty sure you're correct: modern browsers do a good job of protecting window.location, so that particular concern probably isn't a real problem.

I admit I might be a little paranoid, but this is still a service that we are deploying into government agencies, and I just don't like the idea of passing around URLs in query parameters where they can be accessed or spoofed. If an attacker did manage to get an arbitrary value into that domain, it could take the user literally anywhere.

[igor47]

path forward: lets set an env var. looks like api_host is not used, just website_host. lets have a function that gets it out of the request host header, and then falls back to the env var if it's not set. we don't have to set it in prod -- the host header always matches for us.

[ajhyndman]

@igor47 Sorry, I don't think I quite follow you actually.

Are you suggesting that we read the default Host header from the request and only falling back to the env variable if that isn't present? I think that header should always be present, it's just incorrect in our dev configuration.

[igor47]

i suggest we try the env var and fall back to the Host header if that's not present. we'll only have to configure the Host header in dev.

[ajhyndman]

Ohh, reversing the direction of the fallback. Sure, that makes more sense. 👍