Always require auth header in VC (#2517)

## Issue Addressed

- Resolves #2512 

## Proposed Changes

Enforces that all routes require an auth token for the VC.

## TODO

- [x] Tests

common/eth2/src/lighthouse_vc/http_client.rs

@@ -22,6 +22,7 @@ pub struct ValidatorClientHttpClient {
     server: SensitiveUrl,
     secret: ZeroizeString,
     server_pubkey: PublicKey,
+    send_authorization_header: bool,
 }
 
 /// Parse an API token and return a secp256k1 public key.
@@ -60,6 +61,7 @@ impl ValidatorClientHttpClient {
             server,
             server_pubkey: parse_pubkey(&secret)?,
             secret: secret.into(),
+            send_authorization_header: true,
         })
     }
 
@@ -73,9 +75,18 @@ impl ValidatorClientHttpClient {
             server,
             server_pubkey: parse_pubkey(&secret)?,
             secret: secret.into(),
+            send_authorization_header: true,
         })
     }
 
+    /// Set to `false` to disable sending the `Authorization` header on requests.
+    ///
+    /// Failing to send the `Authorization` header will cause the VC to reject requests with a 403.
+    /// This function is intended only for testing purposes.
+    pub fn send_authorization_header(&mut self, should_send: bool) {
+        self.send_authorization_header = should_send;
+    }
+
     async fn signed_body(&self, response: Response) -> Result<Bytes, Error> {
         let sig = response
             .headers()
@@ -108,13 +119,16 @@ impl ValidatorClientHttpClient {
     }
 
     fn headers(&self) -> Result<HeaderMap, Error> {
-        let header_value = HeaderValue::from_str(&format!("Basic {}", self.secret.as_str()))
-            .map_err(|e| {
-                Error::InvalidSecret(format!("secret is invalid as a header value: {}", e))
-            })?;
-
         let mut headers = HeaderMap::new();
-        headers.insert("Authorization", header_value);
+
+        if self.send_authorization_header {
+            let header_value = HeaderValue::from_str(&format!("Basic {}", self.secret.as_str()))
+                .map_err(|e| {
+                    Error::InvalidSecret(format!("secret is invalid as a header value: {}", e))
+                })?;
+
+            headers.insert("Authorization", header_value);
+        }
 
         Ok(headers)
     }

validator_client/src/http_api/mod.rs

@@ -468,21 +468,26 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
 
     let routes = warp::any()
         .and(authorization_header_filter)
+        // Note: it is critical that the `authorization_header_filter` is applied to all routes.
+        // Keeping all the routes inside the following `and` is a reliable way to achieve this.
+        //
+        // When adding a route, don't forget to add it to the `routes_with_invalid_auth` tests!
         .and(
-            warp::get().and(
-                get_node_version
-                    .or(get_lighthouse_health)
-                    .or(get_lighthouse_spec)
-                    .or(get_lighthouse_validators)
-                    .or(get_lighthouse_validators_pubkey),
-            ),
+            warp::get()
+                .and(
+                    get_node_version
+                        .or(get_lighthouse_health)
+                        .or(get_lighthouse_spec)
+                        .or(get_lighthouse_validators)
+                        .or(get_lighthouse_validators_pubkey),
+                )
+                .or(warp::post().and(
+                    post_validators
+                        .or(post_validators_keystore)
+                        .or(post_validators_mnemonic),
+                ))
+                .or(warp::patch().and(patch_validators)),
         )
-        .or(warp::post().and(
-            post_validators
-                .or(post_validators_keystore)
-                .or(post_validators_mnemonic),
-        ))
-        .or(warp::patch().and(patch_validators))
         // Maps errors into HTTP responses.
         .recover(warp_utils::reject::handle_rejection)
         // Add a `Server` header.

validator_client/src/http_api/tests.rs

@@ -12,12 +12,17 @@ use account_utils::{
 };
 use deposit_contract::decode_eth1_tx_data;
 use environment::null_logger;
-use eth2::lighthouse_vc::{http_client::ValidatorClientHttpClient, types::*};
+use eth2::{
+    lighthouse_vc::{http_client::ValidatorClientHttpClient, types::*},
+    types::ErrorMessage as ApiErrorMessage,
+    Error as ApiError,
+};
 use eth2_keystore::KeystoreBuilder;
 use parking_lot::RwLock;
 use sensitive_url::SensitiveUrl;
 use slashing_protection::{SlashingDatabase, SLASHING_PROTECTION_FILENAME};
 use slot_clock::{SlotClock, TestingSlotClock};
+use std::future::Future;
 use std::marker::PhantomData;
 use std::net::Ipv4Addr;
 use std::sync::Arc;
@@ -139,12 +144,45 @@ impl ApiTester {
         }
     }
 
-    pub fn invalidate_api_token(mut self) -> Self {
+    pub fn invalid_token_client(&self) -> ValidatorClientHttpClient {
         let tmp = tempdir().unwrap();
         let api_secret = ApiSecret::create_or_open(tmp.path()).unwrap();
         let invalid_pubkey = api_secret.api_token();
+        ValidatorClientHttpClient::new(self.url.clone(), invalid_pubkey.clone()).unwrap()
+    }
+
+    pub async fn test_with_invalid_auth<F, A, T>(self, func: F) -> Self
+    where
+        F: Fn(ValidatorClientHttpClient) -> A,
+        A: Future<Output = Result<T, ApiError>>,
+    {
+        /*
+         * Test with an invalid Authorization header.
+         */
+        match func(self.invalid_token_client()).await {
+            Err(ApiError::ServerMessage(ApiErrorMessage { code: 403, .. })) => (),
+            Err(other) => panic!("expected authorized error, got {:?}", other),
+            Ok(_) => panic!("expected authorized error, got Ok"),
+        }
+
+        /*
+         * Test with a missing Authorization header.
+         */
+        let mut missing_token_client = self.client.clone();
+        missing_token_client.send_authorization_header(false);
+        match func(missing_token_client).await {
+            Err(ApiError::ServerMessage(ApiErrorMessage {
+                code: 400, message, ..
+            })) if message.contains("missing Authorization header") => (),
+            Err(other) => panic!("expected missing header error, got {:?}", other),
+            Ok(_) => panic!("expected missing header error, got Ok"),
+        }
 
-        self.client = ValidatorClientHttpClient::new(self.url.clone(), invalid_pubkey).unwrap();
+        self
+    }
+
+    pub fn invalidate_api_token(mut self) -> Self {
+        self.client = self.invalid_token_client();
         self
     }
 
@@ -455,6 +493,76 @@ fn invalid_pubkey() {
     });
 }
 
+#[test]
+fn routes_with_invalid_auth() {
+    let runtime = build_runtime();
+    let weak_runtime = Arc::downgrade(&runtime);
+    runtime.block_on(async {
+        ApiTester::new(weak_runtime)
+            .await
+            .test_with_invalid_auth(|client| async move { client.get_lighthouse_version().await })
+            .await
+            .test_with_invalid_auth(|client| async move { client.get_lighthouse_health().await })
+            .await
+            .test_with_invalid_auth(|client| async move { client.get_lighthouse_spec().await })
+            .await
+            .test_with_invalid_auth(
+                |client| async move { client.get_lighthouse_validators().await },
+            )
+            .await
+            .test_with_invalid_auth(|client| async move {
+                client
+                    .get_lighthouse_validators_pubkey(&PublicKeyBytes::empty())
+                    .await
+            })
+            .await
+            .test_with_invalid_auth(|client| async move {
+                client
+                    .post_lighthouse_validators(vec![ValidatorRequest {
+                        enable: <_>::default(),
+                        description: <_>::default(),
+                        graffiti: <_>::default(),
+                        deposit_gwei: <_>::default(),
+                    }])
+                    .await
+            })
+            .await
+            .test_with_invalid_auth(|client| async move {
+                client
+                    .post_lighthouse_validators_mnemonic(&CreateValidatorsMnemonicRequest {
+                        mnemonic: String::default().into(),
+                        key_derivation_path_offset: <_>::default(),
+                        validators: <_>::default(),
+                    })
+                    .await
+            })
+            .await
+            .test_with_invalid_auth(|client| async move {
+                let password = random_password();
+                let keypair = Keypair::random();
+                let keystore = KeystoreBuilder::new(&keypair, password.as_bytes(), String::new())
+                    .unwrap()
+                    .build()
+                    .unwrap();
+                client
+                    .post_lighthouse_validators_keystore(&KeystoreValidatorsPostRequest {
+                        password: String::default().into(),
+                        enable: <_>::default(),
+                        keystore,
+                        graffiti: <_>::default(),
+                    })
+                    .await
+            })
+            .await
+            .test_with_invalid_auth(|client| async move {
+                client
+                    .patch_lighthouse_validators(&PublicKeyBytes::empty(), false)
+                    .await
+            })
+            .await
+    });
+}
+
 #[test]
 fn simple_getters() {
     let runtime = build_runtime();