[services] Remove requirement to specify port for identity service co…

…nfig

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](https://linear.app/comm/issue/ENG-9865/fix-issue-in-identity-service-logic-when-using-default-port).

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

services/identity/src/config.rs

@@ -9,8 +9,8 @@ use tracing::{error, info};
 use url::Url;
 
 use crate::constants::{
-  cors::ALLOW_ORIGIN_LIST, cors::PROD_ORIGIN_HOST_STR, BACKUP_SERVICE_URL,
-  BLOB_SERVICE_URL, DEFAULT_BACKUP_SERVICE_URL, DEFAULT_BLOB_SERVICE_URL,
+  cors::ALLOW_ORIGIN_LIST, BACKUP_SERVICE_URL, BLOB_SERVICE_URL,
+  DEFAULT_BACKUP_SERVICE_URL, DEFAULT_BLOB_SERVICE_URL,
   DEFAULT_OPENSEARCH_ENDPOINT, DEFAULT_TUNNELBROKER_ENDPOINT,
   KEYSERVER_PUBLIC_KEY, LOCALSTACK_ENDPOINT, OPAQUE_SERVER_SETUP,
   OPENSEARCH_ENDPOINT, REDACT_SENSITIVE_DATA, SECRETS_DIRECTORY,
@@ -180,7 +180,6 @@ pub enum Error {
 pub enum InvalidOriginError {
   InvalidScheme,
   MissingHost,
-  MissingPort,
   ParseError,
 }
 
@@ -231,15 +230,9 @@ fn validate_origin(origin_str: &str) -> Result<(), Error> {
   if !matches!(url.scheme(), "http" | "https") {
     return Err(Error::InvalidOrigin(InvalidOriginError::InvalidScheme));
   };
-  let Some(host_str) = url.host_str() else {
+  if url.host_str().is_none() {
     return Err(Error::InvalidOrigin(InvalidOriginError::MissingHost));
   };
-  if host_str == PROD_ORIGIN_HOST_STR {
-    return Ok(());
-  }
-  if url.port().is_none() {
-    return Err(Error::InvalidOrigin(InvalidOriginError::MissingPort));
-  };
   Ok(())
 }
 
@@ -276,24 +269,13 @@ mod tests {
 
   #[test]
   fn test_valid_origin_missing_port() {
-    // If the host is web.comm.app, we do not require a port
     let valid_origin = "https://web.comm.app";
     assert!(
       validate_origin(valid_origin).is_ok(),
       "Expected origin missing port to be valid"
     );
   }
 
-  #[test]
-  fn test_invalid_origin_missing_port() {
-    // If the host is not web.comm.app, we require a port
-    let invalid_origin = "http://localhost";
-    assert!(
-      validate_origin(invalid_origin).is_err(),
-      "Expected an invalid origin (missing port), but got a valid one"
-    );
-  }
-
   #[test]
   fn test_invalid_origin_invalid_scheme() {
     // We only allow http and https origins

services/identity/src/constants.rs

@@ -337,7 +337,6 @@ pub mod cors {
     super::request_metadata::ACCESS_TOKEN,
   ];
   pub const ALLOW_ORIGIN_LIST: &str = "ALLOW_ORIGIN_LIST";
-  pub const PROD_ORIGIN_HOST_STR: &str = "web.comm.app";
 }
 
 // Tracing