Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for hasher role in /lookup endpoint #1729

Merged
merged 4 commits into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hasher-matcher-actioner/.devcontainer/postcreate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ set -e

pip install --editable .[all]

# Find Python packages in opt and install them
# Find Python packages in extensions and install them
for setup_script in "$(find /workspace/extensions -name setup.py)"
do
module_dir="$(dirname "$setup_script")"
Expand Down
2 changes: 1 addition & 1 deletion hasher-matcher-actioner/src/OpenMediaMatch/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
with warnings.catch_warnings():
warnings.simplefilter("ignore")
from threatexchange.signal_type.pdq import signal as _
## Resume regularly scheduled imports
# Resume regularly scheduled imports

import logging
import os
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def query_index(
try:
signal = signal_type.validate_signal_str(signal)
except Exception as e:
abort(400, f"invalid signal type: {e}")
abort(400, f"invalid signal: {e}")

index = _get_index(signal_type)

Expand Down Expand Up @@ -203,8 +203,10 @@ def lookup_get():
Output:
* List of matching banks
"""
# Url was passed as a query param?
if request.args.get("url", None):
if not current_app.config.get("ROLE_HASHER", False):
abort(403, "Hashing is disabled, missing role")

hashes = hashing.hash_media()
resp = {}
for signal_type in hashes.keys():
Expand All @@ -230,6 +232,9 @@ def lookup_post():
Output:
* List of matching banks
"""
if not current_app.config.get("ROLE_HASHER", False):
abort(403, "Hashing is disabled, missing role")

hashes = hashing.hash_media_post()

resp = {}
Expand Down
24 changes: 23 additions & 1 deletion hasher-matcher-actioner/src/OpenMediaMatch/tests/test_api.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.

import tempfile
import typing as t

from flask.testing import FlaskClient
Expand Down Expand Up @@ -112,7 +113,8 @@ def test_banks_update(client: FlaskClient):
assert get_response.status_code == 200
json = get_response.get_json()
assert len(json) == 1
assert json[0] == {"name": "MY_TEST_BANK_RENAMED", "matching_enabled_ratio": 0.5}
assert json[0] == {"name": "MY_TEST_BANK_RENAMED",
"matching_enabled_ratio": 0.5}


def test_banks_delete(client: FlaskClient):
Expand Down Expand Up @@ -221,6 +223,26 @@ def test_banks_add_hash_index(app: Flask, client: FlaskClient):
assert post_response.json == {"matches": [2]}


def test_lookup_add_hash_without_role(app: Flask, client: FlaskClient):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't think we're "adding" a hash in this unittest. In other tests, this is referring to banking functionality.

# role resets to True in the next test
client.application.config["ROLE_HASHER"] = False

# test GET
image_url = "https://github.com/facebook/ThreatExchange/blob/main/pdq/data/bridge-mods/aaa-orig.jpg?raw=true"
get_resp = client.get(f"/m/lookup?url={image_url}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: I don't see any existing unittests in this file that check against this endpoint at all (the others are using "raw_lookup". Can you also test a lookup is accepted?

This file is getting big enough that it may make sense to break out unittests for the different APIs.

assert get_resp.status_code == 403

# test POST with temp file
with tempfile.NamedTemporaryFile(suffix='.jpg') as f:
# Write a minimal valid JPEG file header
f.write(
b'\xff\xd8\xff\xe0\x00\x10\x4a\x46\x49\x46\x00\x01\x01\x00\x00\x01\x00\x01\x00\x00\xff\xd9')
f.flush()
files = {"file": (f.name, f.name, "image/jpeg")}
resp = client.post("/m/lookup", data=files)
assert resp.status_code == 403


def test_exchange_api_set_auth(app: Flask, client: FlaskClient):
storage = get_storage()
sample_name = StaticSampleSignalExchangeAPI.get_name()
Expand Down
Loading