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

feat(cubejs-server): Integrated support for TLS #213

Merged
merged 5 commits into from
Sep 27, 2019
Merged

feat(cubejs-server): Integrated support for TLS #213

merged 5 commits into from
Sep 27, 2019

Conversation

philippefutureboy
Copy link
Contributor

@philippefutureboy philippefutureboy commented Sep 21, 2019

@cubejs-backend/server listen supports receiving an option object.
Given env CUBEJS_ENABLE_TLS=true, the CubejsServer will use the option object in order to setup https connection.
Upon resolution, the listen promise will provide an access to the server object (http.Server or https.Server) for easy access would the user need to access it. For instance, the main reason for this PR is that I want to be able to use self-signed certificates for internal data transfers (from EC2 to EC2) and be able to swap them dynamically when they are about to expire using tls.setSecureContext).

Check List

  • Tests has been run in packages where changes made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

@cubejs-backend/server listen supports receiving an option object.
Given env CUBEJS_ENABLE_TLS=true, the CubejsServer will use the option object in order to setup https connection.
@philippefutureboy
Copy link
Contributor Author

How can I update the docs?

@paveltiunov
Copy link
Member

@philippefutureboy Hey Philippe! Thanks for contributing this! Could you please provide some code examples on how it can be used?

There's GitHub edit link for each page in Docs BTW. You can use it to locate every page inside repository. Probably we need to make it a little bit bigger.

@philippefutureboy
Copy link
Contributor Author

philippefutureboy commented Sep 24, 2019

@paveltiunov Sup :)

So examples:

/**
 * Example 1: Certificate from filesystem (e.g. certbot)
 * You could add a filesystem watch for the certificate folder and update the
 * keys automatically based on change in files
 * Assuming CUBEJS_ENABLE_TLS=true
 */
const fs = require("fs-extra");
const CubejsServer = require("@cubejs-backend/server");
const cubejsOptions = require("./cubejsOptions");

var tlsOptions = {
  key: fs.readFileSync(process.env.CUBEJS_TLS_PRIVATE_KEY_FILE),
  cert: fs.readFileSync(process.env.CUBEJS_TLS_PRIVATE_FULLCHAIN_FILE),
};

const cubejsServer = cubejsOptions
  ? new CubejsServer(cubejsOptions)
  : new CubejsServer();

cubejsServer.listen(tlsOptions).then(({ tlsPort, server }) => {
  console.log(`🚀 Cube.js server is listening securely on ${tlsPort}`);
});
/**
 * Example 2: Self-signed, self-renewed certificate renewal
 * Assuming CUBEJS_ENABLE_TLS=true
 */
const CubejsServer = require("@cubejs-backend/server");
const cubejsOptions = require("./cubejsOptions");
const {
  createCertificate,
  scheduleCertificateRenewal,
} = require("./certificate");

async function main() {
  const cubejsServer = cubejsOptions
    ? new CubejsServer(cubejsOptions)
    : new CubejsServer();

  const certOptions = { days: 2, selfSigned: true };
  const tlsOptions = await createCertificate(certOptions);

  const ({ tlsPort, server }) = await cubejsServer.listen(tlsOptions);
  
  console.log(`🚀 Cube.js server is listening securely on ${tlsPort}`);
  
  scheduleCertificateRenewal(server, certOptions, (err, result) => {
    if (err !== null) {
      console.error(
        `🚨 Certificate renewal failed with error "${error.message}"`
      );
      // take some action here to notify the DevOps
      return;
    }
    console.log(`🔐 Certificate renewal successful`);
  });
}

main();

As for the docs, I think it would be better if the Docs have been added / updated if required checkbox links to a doc explaining how to go about modifying the docs. Can modifying the doc on the website be done on the PR?

And for the last bit, the FIXME: Does this work if the port is not 443? My take is that it would be better to replace https://${req.headers.host}${req.url} with https://${req.headers.host}:${TLS_PORT}${req.url} cause otherwise I think https will automatically reroute to 443 which is not necessarily what we want.

@paveltiunov
Copy link
Member

@philippefutureboy Nice! Makes sense. Could you please share how this redirect to TLS port is used in your case?

@philippefutureboy
Copy link
Contributor Author

@paveltiunov Yes! It's simply standard practice when someone attempts to use a secured service through an insecure channel. Rather than denying the client, the client is redirected to the secure channel.

Also, can you let me know how I update the docs as part of this PR?

@paveltiunov
Copy link
Member

@philippefutureboy Ok. Sounds good. Docs can be updated here: https://github.com/cube-js/cube.js/tree/master/docs.

Could you please elaborate how do you use ./config/env in tests? I can't see any part of it is used.

@philippefutureboy
Copy link
Contributor Author

@paveltiunov Nice, thanks for the info!

As for the ./config/env, it is a copy of create-react-app's way of handling environment variables, and I have found it to be really valuable in every project that I have used it since it grants you the ability to run separate configs for each environments, and have separate files for secret overrides and env to be committed to version control.

It is currently used in scripts/test.js at line 16 to load configuration in the test runner (which again is straight from create-react-app). As you may have noticed, I didn't commit any .env and .env.test file, so it is not currently doing any useful job. If you wish I can remove the reference on line 16 and remove the config/env.js file.

Cheers!

@paveltiunov
Copy link
Member

@philippefutureboy Makes sense. Yeah. If it's not used let's remove it for now and we're good to merge.

@philippefutureboy
Copy link
Contributor Author

philippefutureboy commented Sep 26, 2019

@paveltiunov Should be good to go! Make sure to give a review to the docs.

Now the quick question is when will this be published to the npm package?

@paveltiunov
Copy link
Member

@philippefutureboy Thanks! We're going to ship this week or early next week.

@paveltiunov paveltiunov merged commit 66fe156 into cube-js:master Sep 27, 2019
@philippefutureboy philippefutureboy deleted the feat/tls branch September 27, 2019 18:38
@rpaik rpaik added the pr:community Contribution from Cube.js community members. label Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:community Contribution from Cube.js community members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants