diff --git a/api/database_upgrade_schema.py b/api/database_upgrade_schema.py index 2bf218cd2..509e39eab 100755 --- a/api/database_upgrade_schema.py +++ b/api/database_upgrade_schema.py @@ -119,16 +119,6 @@ def setup_db(): `role` String ) ENGINE = EmbeddedRocksDB -PRIMARY KEY account_id""" - ) - run( - """ -CREATE TABLE IF NOT EXISTS session_expunge -( - `account_id` FixedString(32), - `threshold` DateTime DEFAULT now() -) -ENGINE = EmbeddedRocksDB PRIMARY KEY account_id""" ) diff --git a/api/ooniapi/auth.py b/api/ooniapi/auth.py index 883d49f93..49fee9d37 100644 --- a/api/ooniapi/auth.py +++ b/api/ooniapi/auth.py @@ -112,19 +112,7 @@ def wrapper(*args, **kwargs): # catches ExpiredSignatureError return jerror("Authentication required", 401) - # check for session expunge - # TODO: cache query? - query = """SELECT threshold - FROM session_expunge - WHERE account_id = :account_id """ account_id = tok["account_id"] - query_params = dict(account_id=account_id) - row = query_click_one_row(sql.text(query), query_params) - if row: - threshold = row["threshold"] - iat = datetime.utcfromtimestamp(tok["iat"]) - if iat < threshold: - return jerror("Authentication token expired", 401) # If needed we can add here a 2-tier expiration time: long for # /api/v1/user_refresh_token and short for everything else @@ -395,7 +383,6 @@ def user_refresh_token() -> Response: """ log = current_app.logger tok = get_client_token() - # @role_required already checked for expunged tokens if not tok: return jerror("Invalid credentials", code=401) newtoken = _create_session_token(tok["account_id"], tok["role"], tok["login_time"]) @@ -413,13 +400,6 @@ def user_refresh_token() -> Response: GRANT SELECT ON TABLE accounts TO amsapi; GRANT SELECT ON TABLE accounts TO readonly; - -CREATE TABLE IF NOT EXISTS session_expunge ( - account_id text PRIMARY KEY, - threshold timestamp without time zone NOT NULL -); -GRANT SELECT ON TABLE public.session_expunge TO amsapi; -GRANT SELECT ON TABLE public.session_expunge TO readonly; """ @@ -551,66 +531,3 @@ def get_account_role(email_address) -> Response: log.info(f"Getting account {account_id} role: {role}") return nocachejson(role=role) - - -@auth_blueprint.route("/api/v1/set_session_expunge", methods=["POST"]) -@role_required("admin") -def set_session_expunge() -> Response: - """Force refreshing all session tokens for a given account. - Only for admins. - --- - security: - cookieAuth: - type: JWT - in: cookie - name: ooni - parameters: - - in: body - name: email address - description: data as HTML form or JSON - required: true - schema: - type: object - properties: - email_address: - type: string - responses: - 200: - description: Confirmation - """ - log = current_app.logger - req = request.json if request.is_json else request.form - assert req - email_address = req.get("email_address", "").strip().lower() - if EMAIL_RE.fullmatch(email_address) is None: - return jerror("Invalid email address") - account_id = hash_email_address(email_address) - log.info(f"Setting expunge for account {account_id}") - # If an entry is already in place update the threshold as the new - # value is going to be safer - # 'session_expunge' is on RocksDB (ACID key-value database) - log.info("Inserting into Clickhouse session_expunge") - query = "INSERT INTO session_expunge (account_id) VALUES" - query_params: Any = dict(account_id=account_id) - # the `threshold` column defaults to the current time - insert_click(query, [query_params]) - return nocachejson() - - -def _remove_from_session_expunge(email_address: str) -> None: - # Used by integ test - log = current_app.logger - account_id = hash_email_address(email_address) - query_params: Dict[str, Any] = dict(account_id=account_id) - # 'session_expunge' is on RocksDB (ACID key-value database) - q1 = "SELECT * FROM session_expunge WHERE account_id = :account_id" - row = query_click_one_row(sql.text(q1), query_params) - # https://github.com/ClickHouse/ClickHouse/issues/20546 - if row: - log.info("Resetting expunge in Clickhouse session_expunge") - query = "INSERT INTO session_expunge (account_id, threshold) VALUES" - query_params["threshold"] = 0 - insert_click(query, [query_params]) - - -# TODO: purge session_expunge diff --git a/api/tests/integ/clickhouse_1_schema.sql b/api/tests/integ/clickhouse_1_schema.sql index b3081dd47..ed7b26694 100644 --- a/api/tests/integ/clickhouse_1_schema.sql +++ b/api/tests/integ/clickhouse_1_schema.sql @@ -98,14 +98,6 @@ CREATE TABLE accounts ENGINE = EmbeddedRocksDB PRIMARY KEY account_id; -CREATE TABLE session_expunge -( - `account_id` FixedString(32), - `threshold` DateTime DEFAULT now() -) -ENGINE = EmbeddedRocksDB -PRIMARY KEY account_id; - -- Materialized views CREATE MATERIALIZED VIEW default.counters_test_list diff --git a/api/tests/integ/test_integration_auth.py b/api/tests/integ/test_integration_auth.py index a2fcb7368..bbae3c13e 100644 --- a/api/tests/integ/test_integration_auth.py +++ b/api/tests/integ/test_integration_auth.py @@ -140,7 +140,6 @@ def test_user_register_non_valid_redirect(client, mocksmtp): def _register_and_login(client, email_address): - ooniapi.auth._remove_from_session_expunge(email_address) # # return cookie header for further use d = dict(email_address=email_address, redirect_to="https://explorer.ooni.org") r = client.post("/api/v1/user_register", json=d) @@ -247,23 +246,6 @@ def test_role_set_multiple(client, mocksmtp, integtest_admin): assert r.status_code == 200 -@pytest.mark.skip("FIXME not deterministic, see auth.py _delete_account_data") -def test_role_set_with_expunged_token(client, mocksmtp, integtest_admin): - h = _register_and_login(client, admin_e) - - d = dict(email_address="BOGUS_EMAIL_ADDR", role="admin") - r = client.post("/api/v1/set_session_expunge", json=d) - assert r.status_code == 400, r.json - - # As admin, I expunge my own session token - d = dict(email_address=admin_e, role="admin") - r = client.post("/api/v1/set_session_expunge", json=d) - assert r.status_code == 200 - - r = client.get("/api/v1/get_account_role/" + admin_e) - assert r.status_code == 401 - - def decode_token(header): # mimics auth.py:get_client_token() assert isinstance(header, dict) @@ -279,7 +261,6 @@ def test_session_refresh_and_expire(client, mocksmtp, integtest_admin): # SESSION_EXPIRY_DAYS = 2 # LOGIN_EXPIRY_DAYS = 7 with freeze_time("2012-01-14"): - ooniapi.auth._remove_from_session_expunge(admin_e) h = _register_and_login(client, admin_e) tok = decode_token(h) assert tok == { diff --git a/docs/API.md b/docs/API.md index 365e1574d..7cf9a2ac1 100644 --- a/docs/API.md +++ b/docs/API.md @@ -127,9 +127,6 @@ The API als provides entry points to: - Set account roles -- Expunge sessions (see below) - - Browsers sessions can be expunged to require users to log in again. This can be used if an account role needs to be downgraded or terminated urgently. diff --git a/docs/Database.md b/docs/Database.md index 20ea5be73..b71992499 100644 --- a/docs/Database.md +++ b/docs/Database.md @@ -70,8 +70,6 @@ An overview of the more important tables: - [oonirun table](#oonirun-table) ⛁ ReplacingMergeTree -- [session_expunge table](#session_expunge-table) ⛁ EmbeddedRocksDB - - [test_groups table](#test_groups-table) ⛁ Join - [url_priorities table](#url_priorities-table) ⛁ CollapsingMergeTree @@ -512,22 +510,6 @@ ORDER BY (ooni_run_link_id, descriptor_creation_time) SETTINGS index_granularity = 1 ``` -#### session_expunge table - -Used for authentication. It stores - -Schema: - -```sql -CREATE TABLE default.session_expunge -( - `account_id` FixedString(32), - `threshold` DateTime DEFAULT now() -) -ENGINE = EmbeddedRocksDB -PRIMARY KEY account_id -``` - #### obs_openvpn table Table used by OpenVPN tests. Written by the [Fastpath](#fastpath) ⚙ diff --git a/fastpath/clickhouse_init.sql b/fastpath/clickhouse_init.sql index da49958ce..9708a3eca 100644 --- a/fastpath/clickhouse_init.sql +++ b/fastpath/clickhouse_init.sql @@ -98,14 +98,6 @@ CREATE TABLE IF NOT EXISTS accounts ENGINE = EmbeddedRocksDB PRIMARY KEY account_id; -CREATE TABLE IF NOT EXISTS session_expunge -( - `account_id` FixedString(32), - `threshold` DateTime DEFAULT now() -) -ENGINE = EmbeddedRocksDB -PRIMARY KEY account_id; - -- Materialized views CREATE MATERIALIZED VIEW IF NOT EXISTS default.counters_test_list diff --git a/ooniapi/common/src/common/alembic/versions/c9119c05cf42_ooniprobe_services.py b/ooniapi/common/src/common/alembic/versions/c9119c05cf42_ooniprobe_services.py index af86a9895..d63000f49 100644 --- a/ooniapi/common/src/common/alembic/versions/c9119c05cf42_ooniprobe_services.py +++ b/ooniapi/common/src/common/alembic/versions/c9119c05cf42_ooniprobe_services.py @@ -40,7 +40,9 @@ def upgrade() -> None: sa.Column("openvpn_key", sa.String(), nullable=False), ) - ooniprobe_vpn_provider_endpoint_id_seq = Sequence("ooniprobe_vpn_provider_endpoint_id_seq", start=1) + ooniprobe_vpn_provider_endpoint_id_seq = Sequence( + "ooniprobe_vpn_provider_endpoint_id_seq", start=1 + ) op.execute(CreateSequence(ooniprobe_vpn_provider_endpoint_id_seq)) op.create_table( @@ -64,6 +66,7 @@ def upgrade() -> None: ), ) + def downgrade() -> None: op.drop_table("ooniprobe_vpn_provider_endpoint") op.drop_table("ooniprobe_vpn_provider") diff --git a/ooniapi/common/src/common/auth.py b/ooniapi/common/src/common/auth.py index e6740bf79..ee2c11209 100644 --- a/ooniapi/common/src/common/auth.py +++ b/ooniapi/common/src/common/auth.py @@ -1,5 +1,5 @@ import hashlib -from typing import Optional, Dict, Any +from typing import Optional, Dict, Any import jwt @@ -9,12 +9,11 @@ def hash_email_address(email_address: str, key: str) -> str: def check_email_address( - authorization: str, - jwt_encryption_key: str, - email_address: str, - key: str + authorization: str, jwt_encryption_key: str, email_address: str, key: str ) -> bool: - account_id = get_account_id_or_raise(authorization, jwt_encryption_key=jwt_encryption_key) + account_id = get_account_id_or_raise( + authorization, jwt_encryption_key=jwt_encryption_key + ) hashed = hash_email_address(email_address, key=key) if account_id == hashed: return True @@ -52,6 +51,7 @@ def get_client_role(authorization: str, jwt_encryption_key: str) -> str: except: return None + def get_account_id_or_none( authorization: str, jwt_encryption_key: str ) -> Optional[str]: diff --git a/ooniapi/common/src/common/clickhouse.py b/ooniapi/common/src/common/clickhouse.py index bd9e7872a..e9ccbc021 100644 --- a/ooniapi/common/src/common/clickhouse.py +++ b/ooniapi/common/src/common/clickhouse.py @@ -1,4 +1,4 @@ from clickhouse_sqlalchemy import get_declarative_base -Base = get_declarative_base() \ No newline at end of file +Base = get_declarative_base() diff --git a/ooniapi/common/src/common/dependencies.py b/ooniapi/common/src/common/dependencies.py index 22beafa49..f00d5588a 100644 --- a/ooniapi/common/src/common/dependencies.py +++ b/ooniapi/common/src/common/dependencies.py @@ -29,27 +29,12 @@ async def verify_jwt( tok = get_client_token(authorization, settings.jwt_encryption_key) except: raise HTTPException(detail="Authentication required", status_code=401) - + if not tok: raise HTTPException(detail="Authentication required", status_code=401) if tok["role"] not in roles: raise HTTPException(detail="Role not authorized", status_code=401) return tok - # TODO(art): we don't check for the session_expunge table yet. It's empty so the impact is none - # query = """SELECT threshold - # FROM session_expunge - # WHERE account_id = :account_id """ - # account_id = tok["account_id"] - # query_params = dict(account_id=account_id) - # row = query_click_one_row(sql.text(query), query_params) - # if row: - # threshold = row["threshold"] - # iat = datetime.utcfromtimestamp(tok["iat"]) - # if iat < threshold: - # return jerror("Authentication token expired", 401) - - # If needed we can add here a 2-tier expiration time: long for - # /api/v1/user_refresh_token and short for everything else - + return verify_jwt diff --git a/ooniapi/services/ooniprobe/tests/fixtures/initdb/01-scheme.sql b/ooniapi/services/ooniprobe/tests/fixtures/initdb/01-scheme.sql index b3081dd47..ed7b26694 100644 --- a/ooniapi/services/ooniprobe/tests/fixtures/initdb/01-scheme.sql +++ b/ooniapi/services/ooniprobe/tests/fixtures/initdb/01-scheme.sql @@ -98,14 +98,6 @@ CREATE TABLE accounts ENGINE = EmbeddedRocksDB PRIMARY KEY account_id; -CREATE TABLE session_expunge -( - `account_id` FixedString(32), - `threshold` DateTime DEFAULT now() -) -ENGINE = EmbeddedRocksDB -PRIMARY KEY account_id; - -- Materialized views CREATE MATERIALIZED VIEW default.counters_test_list