HomePhabricator
Diffusion Comm 4bc7a9275edf

[services] Remove requirement to specify port for identity service config

Description

[services] Remove requirement to specify port for identity service config

Summary:
Currently, the identity service requires a port to be specified for all hosts that aren't web.comm.app.

However, when a port is specified that happens to be the default port for a given scheme (eg. 443 for https or 80 for http), the url crate ignores the port, and url.port().is_none() will return true.

As a result, there's no valid way to specify a host with a default port. This led to ENG-9865.

My assessment here is that requiring a port isn't super necessary... if it's skipped, then it's reasonable for us to just use the default port for the given scheme.

Test Plan:
This diff is primarily removing a check, so there's not much to test beyond confirming that the things still work without the check.

  1. I ran cargo test to confirm the unit tests still pass, especially test_valid_origin_missing_port now that we're not hardcoding an exception for https://web.comm.app
  2. I'll make sure to deploy the changes to staging before deploying to prod

Reviewers: varun, will, bartek

Reviewed By: varun

Subscribers: tomek

Differential Revision: https://phab.comm.dev/D13883