Skip to content

Commit

Permalink
disallow fetching collector auth token when TokenHash is enabled (#569)
Browse files Browse the repository at this point in the history
  • Loading branch information
jbr authored Oct 14, 2023
1 parent 8571ab6 commit 38e0473
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 10 deletions.
1 change: 1 addition & 0 deletions app/src/ApiClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export interface Aggregator {
is_first_party: boolean;
vdafs: string[];
query_types: string[];
features: string[];
}

export interface NewAggregator {
Expand Down
8 changes: 8 additions & 0 deletions app/src/aggregators/AggregatorDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ function AggregatorPropertyTable() {
.join(", ")
}
/>
<TableRow
label="Supported features"
value={({ features }) =>
features
.map((v) => v.replaceAll(/([A-Z])/g, " $1").toLowerCase())
.join(", ")
}
/>
</tbody>
</Table>
);
Expand Down
9 changes: 7 additions & 2 deletions app/src/tasks/TaskDetail/CollectorAuthTokens.tsx
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -15,6 +15,11 @@ export default function CollectorAuthTokens() {
fetcher.load("collector_auth_tokens");
}, [fetcher]);

const leader = useLoaderPromise<Aggregator | null>("leaderAggregator", null);
if (!leader || leader.features.includes("TokenHash")) {
return <></>;
}

if (fetcher.data) {
const { collectorAuthTokens } = fetcher.data as {
collectorAuthTokens: CollectorAuthToken[];
Expand Down
9 changes: 8 additions & 1 deletion app/src/util.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -105,6 +105,13 @@ export function CopyCode({ code }: { code: string }) {
);
}

export function useLoaderPromise<T>(key: string, initialState: T): T {
return usePromise(
(useLoaderData() as { [k: string]: Promise<T> })[key],
initialState,
);
}

export function usePromise<T>(promise: PromiseLike<T>, initialState: T): T {
const [state, setState] = React.useState<T>(initialState);
React.useEffect(() => {
Expand Down
21 changes: 20 additions & 1 deletion client/tests/tasks.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -53,13 +54,31 @@ async fn rename_task(app: Arc<DivviupApi>, account: Account, client: DivviupClie
}

#[test(harness = with_configured_client)]
async fn collector_auth_tokens(
async fn collector_auth_tokens_no_token_hash(
app: Arc<DivviupApi>,
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<DivviupApi>,
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(())
}
12 changes: 8 additions & 4 deletions src/routes/tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,14 @@ pub mod collector_auth_tokens {
conn: &mut Conn,
(task, db, State(client)): (Task, Db, State<Client>),
) -> Result<impl Handler, Error> {
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]))
}
}
}
8 changes: 8 additions & 0 deletions test-support/src/client_logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<LoggedConn> {
self.logged_conns.read().unwrap().clone()
}
Expand Down
54 changes: 52 additions & 2 deletions tests/tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit 38e0473

Please sign in to comment.