From 38e047303013c8ad8ce79b8194dbdd21b3812bd2 Mon Sep 17 00:00:00 2001 From: Jacob Rothstein Date: Fri, 13 Oct 2023 17:45:20 -0700 Subject: [PATCH] disallow fetching collector auth token when TokenHash is enabled (#569) --- app/src/ApiClient.ts | 1 + app/src/aggregators/AggregatorDetail.tsx | 8 +++ .../tasks/TaskDetail/CollectorAuthTokens.tsx | 9 +++- app/src/util.tsx | 9 +++- client/tests/tasks.rs | 21 +++++++- src/routes/tasks.rs | 12 +++-- test-support/src/client_logs.rs | 8 +++ tests/tasks.rs | 54 ++++++++++++++++++- 8 files changed, 112 insertions(+), 10 deletions(-) diff --git a/app/src/ApiClient.ts b/app/src/ApiClient.ts index f76c626f..7450fb3c 100644 --- a/app/src/ApiClient.ts +++ b/app/src/ApiClient.ts @@ -129,6 +129,7 @@ export interface Aggregator { is_first_party: boolean; vdafs: string[]; query_types: string[]; + features: string[]; } export interface NewAggregator { diff --git a/app/src/aggregators/AggregatorDetail.tsx b/app/src/aggregators/AggregatorDetail.tsx index fd69ad09..89486aac 100644 --- a/app/src/aggregators/AggregatorDetail.tsx +++ b/app/src/aggregators/AggregatorDetail.tsx @@ -125,6 +125,14 @@ function AggregatorPropertyTable() { .join(", ") } /> + + features + .map((v) => v.replaceAll(/([A-Z])/g, " $1").toLowerCase()) + .join(", ") + } + /> ); diff --git a/app/src/tasks/TaskDetail/CollectorAuthTokens.tsx b/app/src/tasks/TaskDetail/CollectorAuthTokens.tsx index aea85003..fc3a4a9c 100644 --- a/app/src/tasks/TaskDetail/CollectorAuthTokens.tsx +++ b/app/src/tasks/TaskDetail/CollectorAuthTokens.tsx @@ -1,11 +1,11 @@ import { useFetcher } from "react-router-dom"; import Col from "react-bootstrap/Col"; import React from "react"; -import { CollectorAuthToken } from "../../ApiClient"; +import { Aggregator, CollectorAuthToken } from "../../ApiClient"; import Button from "react-bootstrap/Button"; import Card from "react-bootstrap/Card"; import ListGroup from "react-bootstrap/ListGroup"; -import { CopyCode } from "../../util"; +import { CopyCode, useLoaderPromise } from "../../util"; import { Badge } from "react-bootstrap"; export default function CollectorAuthTokens() { @@ -15,6 +15,11 @@ export default function CollectorAuthTokens() { fetcher.load("collector_auth_tokens"); }, [fetcher]); + const leader = useLoaderPromise("leaderAggregator", null); + if (!leader || leader.features.includes("TokenHash")) { + return <>; + } + if (fetcher.data) { const { collectorAuthTokens } = fetcher.data as { collectorAuthTokens: CollectorAuthToken[]; diff --git a/app/src/util.tsx b/app/src/util.tsx index 81fb36b3..3bc5a19c 100644 --- a/app/src/util.tsx +++ b/app/src/util.tsx @@ -3,7 +3,7 @@ import Col from "react-bootstrap/Col"; import Row from "react-bootstrap/Row"; import { LinkContainer } from "react-router-bootstrap"; import React, { Suspense } from "react"; -import { Await, useRouteLoaderData } from "react-router-dom"; +import { Await, useRouteLoaderData, useLoaderData } from "react-router-dom"; import { Account } from "./ApiClient"; import Placeholder from "react-bootstrap/Placeholder"; import { Button, OverlayTrigger, Tooltip } from "react-bootstrap"; @@ -105,6 +105,13 @@ export function CopyCode({ code }: { code: string }) { ); } +export function useLoaderPromise(key: string, initialState: T): T { + return usePromise( + (useLoaderData() as { [k: string]: Promise })[key], + initialState, + ); +} + export function usePromise(promise: PromiseLike, initialState: T): T { const [state, setState] = React.useState(initialState); React.useEffect(() => { diff --git a/client/tests/tasks.rs b/client/tests/tasks.rs index 812abc44..5489365e 100644 --- a/client/tests/tasks.rs +++ b/client/tests/tasks.rs @@ -1,4 +1,5 @@ mod harness; +use divviup_api::entity::aggregator::Features; use divviup_client::{DivviupClient, NewTask, Vdaf}; use harness::with_configured_client; use std::sync::Arc; @@ -53,13 +54,31 @@ async fn rename_task(app: Arc, account: Account, client: DivviupClie } #[test(harness = with_configured_client)] -async fn collector_auth_tokens( +async fn collector_auth_tokens_no_token_hash( app: Arc, account: Account, client: DivviupClient, ) -> TestResult { let task = fixtures::task(&app, &account).await; + + let mut leader = task.leader_aggregator(app.db()).await?.into_active_model(); + leader.features = ActiveValue::Set(Features::default().into()); + leader.update(app.db()).await?; + let tokens = client.task_collector_auth_tokens(&task.id).await?; assert!(!tokens.is_empty()); // we don't have aggregator-api client logs here Ok(()) } + +#[test(harness = with_configured_client)] +async fn collector_auth_tokens_token_hash( + app: Arc, + account: Account, + client: DivviupClient, +) -> TestResult { + let task = fixtures::task(&app, &account).await; + let leader = task.leader_aggregator(app.db()).await?; + assert!(leader.features.token_hash_enabled()); + assert!(client.task_collector_auth_tokens(&task.id).await.is_err()); + Ok(()) +} diff --git a/src/routes/tasks.rs b/src/routes/tasks.rs index 1fd5c45c..2adf5bea 100644 --- a/src/routes/tasks.rs +++ b/src/routes/tasks.rs @@ -108,10 +108,14 @@ pub mod collector_auth_tokens { conn: &mut Conn, (task, db, State(client)): (Task, Db, State), ) -> Result { - let crypter = conn.state().unwrap(); let leader = task.leader_aggregator(&db).await?; - let client = leader.client(client, crypter)?; - let task_response = client.get_task(&task.id).await?; - Ok(Json([task_response.collector_auth_token])) + if leader.features.token_hash_enabled() { + Err(Error::NotFound) + } else { + let crypter = conn.state().unwrap(); + let client = leader.client(client, crypter)?; + let task_response = client.get_task(&task.id).await?; + Ok(Json([task_response.collector_auth_token])) + } } } diff --git a/test-support/src/client_logs.rs b/test-support/src/client_logs.rs index ae1857e6..14bfe09b 100644 --- a/test-support/src/client_logs.rs +++ b/test-support/src/client_logs.rs @@ -70,6 +70,14 @@ pub struct ClientLogs { } impl ClientLogs { + pub fn len(&self) -> usize { + self.logged_conns.read().unwrap().len() + } + + pub fn is_empty(&self) -> bool { + self.logged_conns.read().unwrap().is_empty() + } + pub fn logs(&self) -> Vec { self.logged_conns.read().unwrap().clone() } diff --git a/tests/tasks.rs b/tests/tasks.rs index a7e39bd0..77292491 100644 --- a/tests/tasks.rs +++ b/tests/tasks.rs @@ -723,14 +723,21 @@ mod update { } mod collector_auth_tokens { - use divviup_api::clients::aggregator_client::api_types::AuthenticationToken; + use divviup_api::{ + clients::aggregator_client::api_types::AuthenticationToken, entity::aggregator::Features, + }; use super::{assert_eq, test, *}; #[test(harness = with_client_logs)] - async fn as_member(app: DivviupApi, client_logs: ClientLogs) -> TestResult { + async fn as_member_no_token_hash(app: DivviupApi, client_logs: ClientLogs) -> TestResult { let (user, account, ..) = fixtures::member(&app).await; let task = fixtures::task(&app, &account).await; + + let mut leader = task.leader_aggregator(app.db()).await?.into_active_model(); + leader.features = ActiveValue::Set(Features::default().into()); + leader.update(app.db()).await?; + let mut conn = get(format!("/api/tasks/{}/collector_auth_tokens", task.id)) .with_api_headers() .with_state(user) @@ -749,11 +756,34 @@ mod collector_auth_tokens { Ok(()) } + #[test(harness = with_client_logs)] + async fn as_member_with_token_hash(app: DivviupApi, client_logs: ClientLogs) -> TestResult { + let (user, account, ..) = fixtures::member(&app).await; + let task = fixtures::task(&app, &account).await; + let leader = task.leader_aggregator(app.db()).await?; + assert!(leader.features.token_hash_enabled()); + + let mut conn = get(format!("/api/tasks/{}/collector_auth_tokens", task.id)) + .with_api_headers() + .with_state(user) + .run_async(&app) + .await; + + assert!(client_logs.is_empty()); + assert_not_found!(conn); + Ok(()) + } + #[test(harness = with_client_logs)] async fn as_rando(app: DivviupApi, client_logs: ClientLogs) -> TestResult { let user = fixtures::user(); let account = fixtures::account(&app).await; let task = fixtures::task(&app, &account).await; + + let mut leader = task.leader_aggregator(app.db()).await?.into_active_model(); + leader.features = ActiveValue::Set(Features::default().into()); + leader.update(app.db()).await?; + let mut conn = get(format!("/api/tasks/{}/collector_auth_tokens", task.id)) .with_api_headers() .with_state(user) @@ -769,6 +799,11 @@ mod collector_auth_tokens { let (admin, ..) = fixtures::admin(&app).await; let account = fixtures::account(&app).await; let task = fixtures::task(&app, &account).await; + + let mut leader = task.leader_aggregator(app.db()).await?.into_active_model(); + leader.features = ActiveValue::Set(Features::default().into()); + leader.update(app.db()).await?; + let mut conn = get(format!("/api/tasks/{}/collector_auth_tokens", task.id)) .with_api_headers() .with_state(admin) @@ -791,6 +826,11 @@ mod collector_auth_tokens { let token = fixtures::admin_token(&app).await; let account = fixtures::account(&app).await; let task = fixtures::task(&app, &account).await; + + let mut leader = task.leader_aggregator(app.db()).await?.into_active_model(); + leader.features = ActiveValue::Set(Features::default().into()); + leader.update(app.db()).await?; + let mut conn = get(format!("/api/tasks/{}/collector_auth_tokens", task.id)) .with_api_headers() .with_auth_header(token) @@ -813,6 +853,11 @@ mod collector_auth_tokens { let account = fixtures::account(&app).await; let (_, token) = fixtures::api_token(&app, &account).await; let task = fixtures::task(&app, &account).await; + + let mut leader = task.leader_aggregator(app.db()).await?.into_active_model(); + leader.features = ActiveValue::Set(Features::default().into()); + leader.update(app.db()).await?; + let mut conn = get(format!("/api/tasks/{}/collector_auth_tokens", task.id)) .with_api_headers() .with_auth_header(token) @@ -837,6 +882,11 @@ mod collector_auth_tokens { let account = fixtures::account(&app).await; let task = fixtures::task(&app, &account).await; + + let mut leader = task.leader_aggregator(app.db()).await?.into_active_model(); + leader.features = ActiveValue::Set(Features::default().into()); + leader.update(app.db()).await?; + let mut conn = get(format!("/api/tasks/{}/collector_auth_tokens", task.id)) .with_api_headers() .with_auth_header(token)