Env: HTTPS Support

[bgoewert]

What?

Provides HTTPS support for wp-env. Also allows to define the ports used for HTTPS.

Why?

Addresses #8211

For another use case, I'm currently developing a plugin for CSP management, and Chromium browsers seemingly require HTTPS for the report-to directive. I'm sure there are other scenarios that require the use of HTTPS.

This will allow you to provide a certificate and key file that you create to enable HTTPS. There are many methods of creating an SSL, everyone has there preferences, and each system is different. By providing a means of passing a certificate and key, it allows the individual to use their own method.

How?

Creates Docker configurable ports for development (8883:443) and/or tests (8884:443). Copies the given certificate and key files into the container. Enables Apache mod_ssl. Modifies the default-ssl.conf to use the given certificate and key, and the nenables the default-ssl site config.

Testing Instructions

1. Create a certificate and key if none exist. There are multiple methods to do this.

2. Create a .wp-env.override.json file and add a path to valid certificate and key files.

   {
       "ssl": {
           "cert": "/path/to/crt",
           "key": "/path/to/key"
       }
   }

3. Optionally set the WP_ENV_SSL_PORT and WP_ENV_SSL_TESTS_PORT environment variables.

4. Start wp-env

5. Navigate to https://localhost:8883 (or whatever you set the development/tests port to)

Additional Comments

I'm a new contributor to Gutenberg and fairly new to Node.js in general, not to mention Jest. I was not able to figure out what was required to add to the tests. Any help there would be great. If there are any other issues, let me know.

[github-actions]

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

• Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
• Labels found: First-time Contributor, [Package] Env.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

[github-actions]

Warning: Type of PR label error

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

• Type-related labels to choose from: [Type] Accessibility (a11y), [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
• Labels found: .

Read more about Type labels in Gutenberg.

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @bgoewert! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[ObliviousHarmony]

I have a few concerns about this feature:

• SSL certificates for local websites are inherently problematic because you really shouldn't be registering a domain that maps to a local IP.
• Requiring the user to generate and pass an SSL certificate is a power user feature. This isn't a common task for most developers and they're going to have to follow platform-specific guides to generate and trust a self-signed certificate.
• Supporting https:// URLs is important for some use-cases but I don't think those are very common.

Given the above, I personally don't think this specific feature belongs in wp-env. It seems pretty project-specific and should likely be implemented accordingly. A lot of the features in this pull request are already provided. You can map the SSL certificates using the mappings option and you can use lifecycleScripts.afterStart to update the web server config with wp-env run commands.

The real problem is that we aren't exposing the port necessary for this to work. Even if the web server is configured correctly, you can't access it from the host. What if, instead of this PR, we added a feature to provide custom port mappings to the generated docker-compose.yml file? That would support a more general use-case while letting you implement this in your project. For instance, it would let people easily support mapping a different XDebug port to avoid collisions. I would also suggest supporting the remapping of the MySQL port for the same reason. I'm not totally sure what that configuration might look like because the mysql container is a separate service though.

With the features mentioned above you would only need to run wp-env start and it would set everything up correctly.

[bgoewert]

use lifecycleScripts.afterStart to update the web server config with wp-env run commands

I was unaware of this. Looking back I should have reviewed the readme.md further to see what exactly was possible already.

The real problem is that we aren't exposing the port necessary for this to work. What if, instead of this PR, we added a feature to provide custom port mappings to the generated docker-compose.yml file?

I agree, allowing for custom Docker port mappings would be great.

Before closing this PR to work on Docker port mappings...

Requiring the user to generate and pass an SSL certificate is a power user feature. This isn't a common task for most developers and they're going to have to follow platform-specific guides to generate and trust a self-signed certificate.

If we were to add functionality to create an SSL for non power users that simply want to enable HTTPS, would this be considered a value adding feature?

By potentially using something like devcert, we could generate a certificate, key, and a CA. There are security concerns with creating a CA as well. All that would be needed is setting "https": true before running wp-env start. The certificate and key could maybe be saved under ~/.wp-env/{env}/ssl. If a CA is generated, unsure where this could be safely saved for different systems, it could be used to sign new key pairs.

Any thoughts on this @ObliviousHarmony? Or should we just close this in favor of custom Docker port mappings?

[ObliviousHarmony]

By potentially using something like devcert, we could generate a certificate, key, and a CA.

This is a super cool package! If we went this route I think we would want to make opting-in a bit more explicit though. Adding "https": true should auto-generate the certificates and set up the mapping. For trusting the certificate though I think it makes sense to have a separate command that they explicitly run and it can warn them of any implications. This seems pretty standard with these kinds of tools that heavy-handedly trust a local CA.

Given the more general use-case of port mapping, I think I'd prefer if we did that and revisited the need for including an SSL certificate in the future. Regardless of what we do in terms of SSL I think there's a benefit to the port mapping. It would be great to get some second opinions though such as from @noahtallen!

[noahtallen]

I definitely like the autogenerated SSL concept, which is more user-friendly! Mapping more ports is also a good idea; I think people have wanted a stable mysql port as an example. I'm not sure how to get that working though

[ObliviousHarmony]

I think people have wanted a stable mysql port as an example. I'm not sure how to get that working though

My suggestion @noahtallen would be that we add a new root-only (ports, portMapping, exposedPorts, ???) config option:

{
  "portMapping": {
    "database": {
      "3303": 33003
    },
    "development": {
      "4621": 44621
    },
    "tests": {
      "4621": 44622
    }
  }
}

One of the nice things about this approach is that, internally, we could actually just have port, testsPort, and environment.{env}.port map into the port map during config loading. Then when we build the Compose config we can just pass in the port map to each container rather than generating it there. This simplifies the code there and lets folks expose any ports they want to any other port they want.

We would just need validation to prevent any reserved ports from being mapped since they're configurable already. If we did an HTTPS feature for instance we might have an httpsPort option that should reserve 443.

To be honest this actually doesn't seem like very much work!

[noahtallen]

What's the purpose of the mapping -- could we just say "xdebug: 12345" and handle the internal port internally?

[ObliviousHarmony]

What's the purpose of the mapping -- could we just say "xdebug: 12345" and handle the internal port internally?

I suppose you're right 😄 Assuming that we move forward with an HTTP feature, perhaps some kind of config for mapping the ports of these individual services would be good?

{
  "ports": {
    "http": 80,
    "https": 443,
    "database": 33006
    "xdebug": 12345
  }
}

This would be an environment-specific config. I would say it gives meaningful control over the ports used by the services without supporting or encouraging custom ports. My thinking was that people might want to run commands to install other services within the environment. In this case it would be valuable to support custom ports. If that's something we'd want to support maybe we can take a hybrid approach? Support both keywords and a container port? Just food for thought!

[bgoewert]

If we were to have custom port mappings like this, and if at some point we support an "easy" HTTPS feature, would we just use the HTTP ports for HTTPS?

{
  "development": {
    "https": true,
    "ports": {
      "http": 8888
    }
  }
}

Or would that be too confusing and should we just allow both?

{
  "development": {
    "https": true,
    "ports": {
      "http": 8888,
      "https": 8443
    }
  }
}

Or if the root port is defined.

{
  "port": 8888,
  "development": {
    "https": true
  }
}

Also, the env http port should override the root port.

[ObliviousHarmony]

Or would that be too confusing and should we just allow both?

I would say supporting both is ideal. As for naming, I'd probably suggest httpsPort or something of the sort.

[ObliviousHarmony]

Hi @bgoewert,

I am currently going through all of my open review requests and I wanted to see if this feature is still something you want to pursue?

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @bgoewert@goewert.me.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: bgoewert@goewert.me.

Co-authored-by: bgoewert <bgoewert@git.wordpress.org>
Co-authored-by: ObliviousHarmony <obliviousharmony@git.wordpress.org>
Co-authored-by: noahtallen <noahtallen@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[bgoewert]

@ObliviousHarmony It is a feature that I was interested in. I think I got a bit overwhelmed. I can't say I am super interested these days. Last thing I was working on I think was generic port mapping, which I seem to have lost that branch. I'm fine if we close this PR. I may try to tackle port mapping again separately, and come back to the auto-SSL feature some other time.