cleanup

divviup · Jul 26, 2023 · 8b4e9ea · 8b4e9ea
1 parent b631af6
commit 8b4e9ea
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 100 deletions.
diff --git a/aggregator/src/aggregator/aggregate_init_tests.rs b/aggregator/src/aggregator/aggregate_init_tests.rs
@@ -199,7 +199,7 @@ async fn aggregation_job_init_authorization_dap_auth_token() {
         .aggregation_job_uri(&test_case.aggregation_job_id)
         .unwrap()
         .path())
-    .with_request_header(auth_header, auth_value.to_vec())
+    .with_request_header(auth_header, auth_value)
     .with_request_header(
         KnownHeaderName::ContentType,
         AggregationJobInitializeReq::<TimeInterval>::MEDIA_TYPE,

diff --git a/aggregator/src/aggregator/collection_job_tests.rs b/aggregator/src/aggregator/collection_job_tests.rs
@@ -63,7 +63,7 @@ impl CollectionJobTestCase {
             .path());
         if let Some(auth) = auth_token {
             let (header, value) = auth.request_authentication();
-            test_conn = test_conn.with_request_header(header, value.to_vec());
+            test_conn = test_conn.with_request_header(header, value);
         }
 
         test_conn
@@ -102,7 +102,7 @@ impl CollectionJobTestCase {
         );
         if let Some(auth) = auth_token {
             let (header, value) = auth.request_authentication();
-            test_conn = test_conn.with_request_header(header, value.to_vec());
+            test_conn = test_conn.with_request_header(header, value);
         }
         test_conn.run_async(&self.handler).await
     }

diff --git a/aggregator/src/aggregator/http_handlers.rs b/aggregator/src/aggregator/http_handlers.rs
@@ -1376,7 +1376,7 @@ mod tests {
 
             if let Some(auth_token) = auth_token {
                 let (auth_header, auth_value) = auth_token.request_authentication();
-                test_conn = test_conn.with_request_header(auth_header, auth_value.to_vec());
+                test_conn = test_conn.with_request_header(auth_header, auth_value);
             }
 
             let mut test_conn = test_conn.run_async(&handler).await;
@@ -4050,8 +4050,8 @@ mod tests {
             take_problem_details(&mut test_conn).await,
             json!({
                 "status": Status::BadRequest as u16,
-                "type": "urn:ietf:params:ppm:dap:error:unauthorizedRequest",
-                "title": "The request's authorization is not valid.",
+                "type": "urn:ietf:params:ppm:dap:error:unrecognizedTask",
+                "title": "An endpoint received a message with an unknown task ID.",
                 "taskid": format!("{}", test_case.task.id()),
             })
         );

diff --git a/client/Cargo.toml b/client/Cargo.toml
@@ -20,7 +20,7 @@ prio.workspace = true
 rand = "0.8"
 reqwest = { version = "0.11.18", default-features = false, features = ["rustls-tls", "json"] }
 thiserror.workspace = true
-tokio = { version = "1.29", features = ["full"] }
+tokio.workspace = true
 tracing = "0.1.37"
 url = "2.4.0"
 

diff --git a/collector/src/lib.rs b/collector/src/lib.rs
@@ -704,7 +704,7 @@ mod tests {
         let parameters = CollectorParameters::new(
             random(),
             server_url,
-            AuthenticationToken::new_bearer_token_from_bytes(b"token").unwrap(),
+            AuthenticationToken::new_bearer_token_from_string("Y29sbGVjdG9yIHRva2Vu").unwrap(),
             hpke_keypair.config().clone(),
             hpke_keypair.private_key().clone(),
         )
@@ -805,7 +805,7 @@ mod tests {
         let collector_parameters = CollectorParameters::new(
             random(),
             "http://example.com/dap".parse().unwrap(),
-            AuthenticationToken::new_bearer_token_from_bytes(b"token").unwrap(),
+            AuthenticationToken::new_bearer_token_from_string("Y29sbGVjdG9yIHRva2Vu").unwrap(),
             hpke_keypair.config().clone(),
             hpke_keypair.private_key().clone(),
         );
@@ -818,7 +818,7 @@ mod tests {
         let collector_parameters = CollectorParameters::new(
             random(),
             "http://example.com".parse().unwrap(),
-            AuthenticationToken::new_bearer_token_from_bytes(b"token").unwrap(),
+            AuthenticationToken::new_bearer_token_from_string("Y29sbGVjdG9yIHRva2Vu").unwrap(),
             hpke_keypair.config().clone(),
             hpke_keypair.private_key().clone(),
         );
@@ -838,7 +838,6 @@ mod tests {
         let collector = setup_collector(&mut server, vdaf);
         let (auth_header, auth_value) =
             collector.parameters.authentication.request_authentication();
-        let auth_value = String::from_utf8(auth_value.to_vec()).unwrap();
 
         let batch_interval = Interval::new(
             Time::from_seconds_since_epoch(1_000_000),
@@ -1247,23 +1246,23 @@ mod tests {
                 CONTENT_TYPE.as_str(),
                 CollectionReq::<TimeInterval>::MEDIA_TYPE,
             )
-            .match_header(AUTHORIZATION.as_str(), "Bearer QUFBQUFBQUFBQUFBQUFBQQ==")
+            .match_header(AUTHORIZATION.as_str(), "Bearer AAAAAAAAAAAAAAAA")
             .with_status(201)
             .expect(1)
             .create_async()
             .await;
 
         let job = collector
             .start_collection(Query::new_time_interval(batch_interval), &())
-            .await
-            .unwrap();
-        assert_eq!(job.query.batch_interval(), &batch_interval);
+            .await;
 
         mocked_collect_start_success.assert_async().await;
+        let job = job.unwrap();
+        assert_eq!(job.query.batch_interval(), &batch_interval);
 
         let mocked_collect_complete = server
             .mock("POST", job.collection_job_url.path())
-            .match_header(AUTHORIZATION.as_str(), "Bearer QUFBQUFBQUFBQUFBQUFBQQ==")
+            .match_header(AUTHORIZATION.as_str(), "Bearer AAAAAAAAAAAAAAAA")
             .with_status(200)
             .with_header(
                 CONTENT_TYPE.as_str(),

diff --git a/core/src/task.rs b/core/src/task.rs
@@ -549,17 +549,14 @@ macro_rules! vdaf_dispatch {
 #[serde(tag = "type", content = "token")]
 #[non_exhaustive]
 pub enum AuthenticationToken {
-    /// TODO(timg) revisit this doccomment
-    /// A bearer token. The value is an opaque byte string. Its Base64 encoding is inserted into
-    /// HTTP requests as specified in [RFC 6750 section 2.1][1], but in configuration files or
-    /// aggregator API requests, it is encoded in unpadded, URL-safe Base64.
+    /// A bearer token, presented as the value of the "Authorization" HTTP header as specified in
+    /// [RFC 6750 section 2.1][1].
     ///
-    ///The token is not necessarily an OAuth token.
+    /// The token is not necessarily an OAuth token.
     ///
     /// [1]: https://datatracker.ietf.org/doc/html/rfc6750#section-2.1
     Bearer(TokenInner),
 
-    /// TODO(timg) revisit this doccomment
     /// Token presented as the value of the "DAP-Auth-Token" HTTP header. Conforms to
     /// [draft-dcook-ppm-dap-interop-test-design-03][1], sections [4.3.3][2] and [4.4.2][3], and
     /// [draft-ietf-dap-ppm-01 section 3.2][4].
@@ -596,10 +593,10 @@ impl AuthenticationToken {
 
     /// Returns an HTTP header and value that should be used to authenticate an HTTP request with
     /// this credential.
-    pub fn request_authentication(&self) -> (&'static str, &[u8]) {
+    pub fn request_authentication(&self) -> (&'static str, String) {
         match self {
-            Self::Bearer(token) => (AUTHORIZATION.as_str(), token.as_ref()),
-            Self::DapAuth(token) => (DAP_AUTH_HEADER, token.as_ref()),
+            Self::Bearer(token) => (AUTHORIZATION.as_str(), format!("Bearer {}", token.as_str())),
+            Self::DapAuth(token) => (DAP_AUTH_HEADER, token.as_str().to_string()),
         }
     }
 
@@ -667,24 +664,6 @@ impl AsRef<[u8]> for TokenInner {
     }
 }
 
-// impl TryFrom<String> for TokenInner {
-//     type Error = anyhow::Error;
-
-//     fn try_from(value: String) -> Result<Self, Self::Error> {
-//         // Verify that the string is legal unpadded, URL-safe Base64
-//         URL_SAFE_NO_PAD.decode(&value)?;
-//         Ok(Self(value))
-//     }
-// }
-
-// impl TryFrom<Vec<u8>> for TokenInner {
-//     type Error = anyhow::Error;
-
-//     fn try_from(value: Vec<u8>) -> Result<Self, Self::Error> {
-//         Self::try_from(String::from_utf8(value)?)
-//     }
-// }
-
 impl<'de> Deserialize<'de> for TokenInner {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
@@ -713,62 +692,6 @@ impl Distribution<TokenInner> for Standard {
     }
 }
 
-/// Token presented as the value of the "DAP-Auth-Token" HTTP header. The token is used directly in
-/// the HTTP request without further encoding and so must be a legal HTTP header value. Conforms to
-/// [draft-ietf-dap-ppm-01 section 3.2][1].
-///
-/// This opaque type ensures it's impossible to construct an [`AuthenticationToken::DapAuth`] whose
-/// contents are invalid.
-///
-/// [1]: https://datatracker.ietf.org/doc/html/draft-ietf-ppm-dap-01#name-https-sender-authentication
-// #[derive(Clone, Derivative)]
-// #[derivative(Debug)]
-// pub struct DapAuthToken(#[derivative(Debug = "ignore")] Vec<u8>);
-
-// impl DapAuthToken {}
-
-// impl AsRef<[u8]> for DapAuthToken {
-//     fn as_ref(&self) -> &[u8] {
-//         &self.0
-//     }
-// }
-
-// impl TryFrom<Vec<u8>> for DapAuthToken {
-//     type Error = anyhow::Error;
-
-//     fn try_from(token: Vec<u8>) -> Result<Self, Self::Error> {
-//         HeaderValue::try_from(token.as_slice())?;
-//         Ok(Self(token))
-//     }
-// }
-
-// impl Serialize for DapAuthToken {
-//     fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
-//         serializer.serialize_str(&URL_SAFE_NO_PAD.encode(self.as_ref()))
-//     }
-// }
-
-// impl<'de> Deserialize<'de> for DapAuthToken {
-//     fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
-//         // Verify that the string is a safe HTTP header value
-//         String::deserialize(deserializer)
-//             .and_then(|string| {
-//                 URL_SAFE_NO_PAD.decode(string).map_err(|e| {
-//                     D::Error::custom(format!(
-//                         "cannot decode value from unpadded Base64URL: {e:?}"
-//                     ))
-//                 })
-//             })
-//             .and_then(|bytes| Self::try_from(bytes).map_err(D::Error::custom))
-//     }
-// }
-
-// impl Distribution<DapAuthToken> for Standard {
-//     fn sample<R: rand::Rng + ?Sized>(&self, rng: &mut R) -> DapAuthToken {
-//         DapAuthToken(Vec::from(hex::encode(rng.gen::<[u8; 16]>())))
-//     }
-// }
-
 /// Modifies a [`Url`] in place to ensure it ends with a slash.
 ///
 /// Aggregator endpoint URLs should end with a slash if they will be used with [`Url::join`],

diff --git a/integration_tests/tests/in_cluster.rs b/integration_tests/tests/in_cluster.rs
@@ -168,7 +168,6 @@ impl InClusterJanusPair {
         // Update the task parameters with the ID and collector auth token from divviup-api.
         task_parameters.task_id = TaskId::from_str(provisioned_task.id.as_ref()).unwrap();
         task_parameters.collector_auth_token =
-
             AuthenticationToken::new_bearer_token_from_string(collector_auth_tokens[0].clone())
                 .unwrap();