diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d96fb64..e2832ca 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -11,5 +11,5 @@ jobs: ci: uses: Sea-Haven-Industries/.github/.github/workflows/ci-python-sam.yaml@4a6cbfd362140a68810f0f46d338026863b8e827 # v1.0.10 with: - source-dirs: "src/slack-bot src/weekly-post src/roster-sync src/ring-scheduler src/shared/shared tests" + source-dirs: "src/slack-bot src/weekly-post src/roster-sync src/roster-api src/ring-scheduler src/shared/shared tests" run-tests: true diff --git a/README.md b/README.md index 62b665c..8e7888f 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,7 @@ Example: `/oncall admin holiday add 2026-07-04 2 x2 Independence Day` schedules | `afterhours-shift-manager` | API Gateway (POST /slack/events) | Slack bot — handles `/oncall` commands and interactive buttons | | `afterhours-weekly-post` | EventBridge (Monday 7am ET) | Posts weekly schedule to Slack, sends pay report email | | `afterhours-roster-sync` | EventBridge (daily 6am ET) | Syncs employee roster from 3CX | +| `afterhours-roster-api` | API Gateway (PUT /roster, DELETE /roster/{extension}) | Bearer-authenticated roster upsert/delete for the identity processor | | `afterhours-ring-scheduler` | EventBridge (daily 8am ET + weekend 5pm ET) | Updates 3CX queue routing based on who's on shift | | `afterhours-holiday-router` | EventBridge Scheduler (per-holiday one-off: 8am activate / 5pm deactivate ET) | Repoints the IVR to the holiday queue and sets queue agents for a holiday day shift; reverts at 5pm (see [Holidays](#holidays)) | | `afterhours-release-notifier` | Invoked by the Deploy workflow's release job on minor/major releases | Posts a "What's New" announcement to the shift channel | @@ -81,6 +82,7 @@ src/ slack-bot/ Slack Bolt Lambda (handler + app); ships CHANGELOG.md for App Home weekly-post/ Monday schedule + pay post roster-sync/ Daily 3CX roster sync + roster-api/ HTTP PUT/DELETE /roster for identity hire/offboard ring-scheduler/ 3CX queue routing updates holiday-router/ 3CX IVR/queue repoint for holiday day shifts (activate/deactivate) release-notifier/ Posts release announcements to Slack @@ -177,6 +179,30 @@ A slot claimed after the shift has started always needs an admin to approve it. | `afterhours-shift-manager/3cx-domain` | 3CX FQDN (e.g. `company.3cx.us`) | | `afterhours-shift-manager/3cx-client-id` | 3CX OAuth2 client ID | | `afterhours-shift-manager/3cx-client-secret` | 3CX OAuth2 client secret | +| `afterhours-shift-manager/roster-api-token` | Bearer token for PUT/DELETE `/roster`. Duplicate the same value into the seahaven-prod secret `paychex-integrations/afterhours-roster-token`. | + +### Roster HTTP API + +Identity hire/offboard in `paychex-integrations` calls this API. It is a separate Lambda on the same implicit HTTP API as Slack (`POST /slack/events` is unchanged). + +| Method | Path | Body | Success | +|---|---|---|---| +| PUT | `/roster` | `{"name","extension","slack_user_id"}` (all required strings) | 200 `{"ok":true}` | +| DELETE | `/roster/{extension}` | none | 204 empty body, including when the row is already gone | + +Header: `Authorization: Bearer {token}`. Missing or wrong token is 401. Invalid JSON or fields is 400. A secret-read failure is 503. + +Set processor `AFTERHOURS_BASE_URL` to the stack output `AfterhoursApiBaseUrl`: + +``` +https://${ServerlessHttpApi}.execute-api.us-east-1.amazonaws.com +``` + +That value is the API origin only. Do not append `/mgmt` or `/roster`. + +Daily `afterhours-roster-sync` still owns the 3CX `DEFAULT` group at 6am ET: rows absent from that group are deleted. Hire is safe because 3CX create (into `DEFAULT`) happens before the roster PUT. An HTTP-only row that is not in that group will be removed on the next sync. Sync preserves `slack_user_id` on existing rows and does not overwrite a just-created API row's Slack id. + +Token rotation is a maintenance-window action. The Lambda caches the token per execution environment. Update the mgmt secret and the prod copy together, then recycle `afterhours-roster-api`. Updating only one copy, or recycling environments out of order, causes 401s until both sides match. ## Documentation @@ -202,9 +228,10 @@ All CloudWatch alarms are defined in `template.yaml` and notify the shared is not paged. Alarm names follow the in-template convention `Lambda--` (e.g. `Lambda-Errors-afterhours-ring-scheduler`). -**Lambda alarms** (all six functions: `afterhours-shift-manager`, -`afterhours-weekly-post`, `afterhours-roster-sync`, `afterhours-ring-scheduler`, -`afterhours-holiday-router`, `afterhours-release-notifier`): +**Lambda alarms** (all seven functions: `afterhours-shift-manager`, +`afterhours-weekly-post`, `afterhours-roster-sync`, `afterhours-roster-api`, +`afterhours-ring-scheduler`, `afterhours-holiday-router`, +`afterhours-release-notifier`): | Alarm | Metric | Condition | Notes | |---|---|---|---| diff --git a/SETUP.md b/SETUP.md index bb3acc0..f56c7d5 100644 --- a/SETUP.md +++ b/SETUP.md @@ -45,8 +45,30 @@ aws secretsmanager create-secret \ aws secretsmanager create-secret \ --name afterhours-shift-manager/3cx-client-secret \ --secret-string "YOUR-3CX-CLIENT-SECRET" + +# Roster HTTP API bearer token (plain string). Duplicate the same value into +# seahaven-prod as paychex-integrations/afterhours-roster-token. Generate the +# token into a temp file, pass --secret-string file://..., then delete the file. +# Never paste the value into chat, Terraform, or a PR. +aws secretsmanager create-secret \ + --name afterhours-shift-manager/roster-api-token \ + --secret-string file://./roster-api-token.tmp ``` +Create `afterhours-shift-manager/roster-api-token` **before** the first deploy that +includes `afterhours-roster-api`, or live PUT/DELETE calls return 503. + +Rotation is coordinated: write the new value to both the mgmt secret and the +prod copy, then recycle `afterhours-roster-api` so cached execution environments +pick it up. Updating only one copy causes 401s. The identity processor +`AFTERHOURS_BASE_URL` is the stack output `AfterhoursApiBaseUrl` (origin only, +no `/mgmt` or `/roster` suffix). Leave that URL empty until the API is live +and smoke-tested. + +Daily roster-sync still removes DynamoDB rows that are not in the 3CX `DEFAULT` +group. Hire stays safe because 3CX create lands the extension in that group +before the identity processor PUTs `/roster`. + > The Slack **channel ID** is not a secret — it's passed as the `ShiftChannel` > deploy parameter in step 3, not stored in Secrets Manager or SSM. @@ -61,7 +83,8 @@ sam deploy --guided \ --region us-east-1 \ --parameter-overrides ShiftChannel=C0XXXXXXX QueueNumber=801 -# Note the SlackBotApiUrl output — you'll need it for step 4 +# Note SlackBotApiUrl (Slack Request URL) and AfterhoursApiBaseUrl +# (paychex AFTERHOURS_BASE_URL origin). ``` ## 4. Set the Slack Request URL diff --git a/src/roster-api/app.py b/src/roster-api/app.py new file mode 100644 index 0000000..ba7502c --- /dev/null +++ b/src/roster-api/app.py @@ -0,0 +1,235 @@ +"""HTTP API v2 handler — Bearer-authenticated roster PUT/DELETE. + +PUT /roster upsert name, extension, slack_user_id +DELETE /roster/{extension} delete the row (204 even if it was already gone) + +Auth is an app-level Bearer token stored in Secrets Manager. Do not log the +Authorization header, the token, or the request body. +""" + +from __future__ import annotations + +import base64 +import hmac +import json +import logging +import os +import unicodedata +from typing import Any + +import shared.sentry_init # noqa: F401 +from shared.schedule import ShiftSchedule +from shared.secrets import get_secret + +logger = logging.getLogger() +logger.setLevel(logging.INFO) + +PUT_FIELDS = ("name", "extension", "slack_user_id") +MAX_BODY_BYTES = 4096 +MAX_NAME_LEN = 128 +MAX_EXTENSION_LEN = 16 +MAX_SLACK_ID_LEN = 64 + +_cached_token: str | None = None + + +class AuthError(Exception): + """Missing or wrong Bearer token.""" + + +class SecretUnavailable(Exception): + """Token secret could not be read.""" + + +def _json_response(status: int, body: dict[str, Any] | None = None) -> dict: + if status == 204: + return {"statusCode": 204, "headers": {}, "body": ""} + payload = {} if body is None else body + return { + "statusCode": status, + "headers": {"Content-Type": "application/json"}, + "body": json.dumps(payload, separators=(",", ":")), + } + + +def _header(event: dict, name: str) -> str: + headers = event.get("headers") or {} + if not isinstance(headers, dict): + return "" + target = name.lower() + for key, value in headers.items(): + if str(key).lower() == target: + return "" if value is None else str(value) + return "" + + +def _route(event: dict) -> tuple[str, str]: + request_context = event.get("requestContext") or {} + http = request_context.get("http") or {} + method = str(http.get("method") or event.get("httpMethod") or "").upper() + path = str(event.get("rawPath") or http.get("path") or event.get("path") or "") + route_key = event.get("routeKey") + if isinstance(route_key, str) and " " in route_key: + rk_method, rk_path = route_key.split(" ", 1) + method = method or rk_method.upper() + path = path or rk_path + return method, path + + +def _raw_body(event: dict) -> bytes | None: + body = event.get("body") + if body is None: + return b"" + try: + if event.get("isBase64Encoded"): + if isinstance(body, bytes): + body = body.decode("ascii") + return base64.b64decode(body, validate=True) + if isinstance(body, bytes): + return body + return str(body).encode("utf-8") + except (ValueError, UnicodeError): + return None + + +def _has_disallowed_chars(value: str, *, allow_space: bool) -> bool: + for char in value: + if char == " " and allow_space: + continue + if char.isspace() or unicodedata.category(char).startswith("C"): + return True + return False + + +def _expected_token() -> str: + global _cached_token + if _cached_token: + return _cached_token + secret_id = os.environ["ROSTER_API_TOKEN_SECRET"] + try: + token = get_secret(secret_id) + except Exception: + logger.exception("roster api token secret read failed") + raise SecretUnavailable from None + if not isinstance(token, str): + raise SecretUnavailable + token = token.strip() + if not token: + raise SecretUnavailable + _cached_token = token + return token + + +def _authorize(event: dict) -> None: + presented = _header(event, "authorization") + if not presented: + raise AuthError + parts = presented.split(None, 1) + if len(parts) != 2 or parts[0].lower() != "bearer" or not parts[1].strip(): + raise AuthError + token = parts[1].strip() + expected = _expected_token() + try: + matched = hmac.compare_digest(token, expected) + except (TypeError, ValueError): + raise AuthError from None + if not matched: + raise AuthError + + +def _validate_put(raw: bytes) -> tuple[str, str, str]: + if len(raw) > MAX_BODY_BYTES: + raise ValueError("oversized") + try: + parsed = json.loads(raw.decode("utf-8")) + except (UnicodeDecodeError, json.JSONDecodeError): + raise ValueError("invalid json") from None + if not isinstance(parsed, dict): + raise TypeError("invalid json") + if set(parsed) != set(PUT_FIELDS): + raise ValueError("fields") + values: dict[str, str] = {} + for field in PUT_FIELDS: + value = parsed[field] + if not isinstance(value, str): + raise TypeError("fields") + trimmed = value.strip() + if not trimmed: + raise ValueError("fields") + values[field] = trimmed + + name = values["name"] + extension = values["extension"] + slack_user_id = values["slack_user_id"] + + if len(name) > MAX_NAME_LEN or _has_disallowed_chars(name, allow_space=True): + raise ValueError("fields") + if ( + len(extension) > MAX_EXTENSION_LEN + or not extension.isdigit() + or _has_disallowed_chars(extension, allow_space=False) + ): + raise ValueError("fields") + if ( + len(slack_user_id) > MAX_SLACK_ID_LEN + or not slack_user_id.isalnum() + or _has_disallowed_chars(slack_user_id, allow_space=False) + ): + raise ValueError("fields") + return name, extension, slack_user_id + + +def _delete_extension(event: dict) -> str: + params = event.get("pathParameters") or {} + if not isinstance(params, dict): + raise TypeError("extension") + raw = params.get("extension") + if not isinstance(raw, str): + raise TypeError("extension") + extension = raw.strip() + if ( + not extension + or len(extension) > MAX_EXTENSION_LEN + or not extension.isdigit() + or _has_disallowed_chars(extension, allow_space=False) + ): + raise ValueError("extension") + return extension + + +def handler(event, context): + try: + method, path = _route(event) + logger.info("roster api %s %s", method, path) + try: + _authorize(event) + except AuthError: + return _json_response(401, {"error": "unauthorized"}) + except SecretUnavailable: + return _json_response(503, {"error": "service unavailable"}) + + if method == "PUT" and (path == "/roster" or path.rstrip("/") == "/roster"): + raw = _raw_body(event) + if raw is None: + return _json_response(400, {"error": "invalid request"}) + try: + name, extension, slack_user_id = _validate_put(raw) + except (TypeError, ValueError): + return _json_response(400, {"error": "invalid request"}) + ShiftSchedule().upsert_roster_entry(extension, name, slack_user_id) + logger.info("roster upserted extension=%s", extension) + return _json_response(200, {"ok": True}) + + if method == "DELETE" and path.startswith("/roster/"): + try: + extension = _delete_extension(event) + except (TypeError, ValueError): + return _json_response(400, {"error": "invalid request"}) + ShiftSchedule().remove_roster_entry(extension) + logger.info("roster deleted extension=%s", extension) + return _json_response(204) + + return _json_response(405, {"error": "method not allowed"}) + except Exception: + logger.exception("roster api unexpected failure") + return _json_response(500, {"error": "internal error"}) diff --git a/src/roster-api/requirements.txt b/src/roster-api/requirements.txt new file mode 100644 index 0000000..9f1628c --- /dev/null +++ b/src/roster-api/requirements.txt @@ -0,0 +1 @@ +boto3>=1.43.82 diff --git a/src/roster-sync/app.py b/src/roster-sync/app.py index 86f0e8f..5461bf2 100644 --- a/src/roster-sync/app.py +++ b/src/roster-sync/app.py @@ -89,14 +89,20 @@ def handler(event, context): ) updated.append(f"Ext {number}: {existing['name']} -> {name}") else: - schedule.table.put_item( - Item={ - "PK": "ROSTER", - "SK": number, - "name": name, - "extension": number, - "slack_user_id": "", - } + # UpdateItem so a stale get_roster snapshot cannot PutItem-overwrite + # a roster-API row that landed after the read and wipe its Slack id. + schedule.table.update_item( + Key={"PK": "ROSTER", "SK": number}, + UpdateExpression=( + "SET #n = :name, extension = :ext, " + "slack_user_id = if_not_exists(slack_user_id, :empty)" + ), + ExpressionAttributeNames={"#n": "name"}, + ExpressionAttributeValues={ + ":name": name, + ":ext": number, + ":empty": "", + }, ) added.append(f"Ext {number}: {name}") diff --git a/src/shared/shared/schedule.py b/src/shared/shared/schedule.py index a219c0c..be19042 100644 --- a/src/shared/shared/schedule.py +++ b/src/shared/shared/schedule.py @@ -700,6 +700,26 @@ class ShiftSchedule: except self.table.meta.client.exceptions.ConditionalCheckFailedException: return False + def upsert_roster_entry( + self, extension: str, name: str, slack_user_id: str + ) -> None: + """Create or update a roster row without clobbering unrelated attributes. + + Always writes ``name``, ``extension``, and ``slack_user_id``. An existing + ``shift_rate`` (and any other attributes) survive. Unlike + :meth:`add_roster_entry`, this is an upsert and writes the Slack id. + """ + self.table.update_item( + Key={"PK": "ROSTER", "SK": extension}, + UpdateExpression="SET #n = :name, extension = :ext, slack_user_id = :sid", + ExpressionAttributeNames={"#n": "name"}, + ExpressionAttributeValues={ + ":name": name, + ":ext": extension, + ":sid": slack_user_id, + }, + ) + def remove_roster_entry(self, extension: str) -> None: self.table.delete_item(Key={"PK": "ROSTER", "SK": extension}) diff --git a/template.yaml b/template.yaml index 13d62a9..c8266f1 100644 --- a/template.yaml +++ b/template.yaml @@ -268,6 +268,44 @@ Resources: Description: "Sync roster from 3CX at 6am EDT" Enabled: true + # --- Roster API (HTTP PUT /roster and DELETE /roster/{extension}) --- + RosterApiFunction: + Type: AWS::Serverless::Function + Properties: + FunctionName: afterhours-roster-api + Handler: app.handler + CodeUri: src/roster-api/ + Layers: + - !Ref SharedLayer + Environment: + Variables: + SHIFT_TABLE: !Ref ShiftTable + ROSTER_API_TOKEN_SECRET: afterhours-shift-manager/roster-api-token + TZ: !Ref Timezone + Policies: + - Statement: + - Effect: Allow + Action: + - dynamodb:UpdateItem + - dynamodb:DeleteItem + Resource: !GetAtt ShiftTable.Arn + - Effect: Allow + Action: + - secretsmanager:GetSecretValue + Resource: + - !Sub "arn:aws:secretsmanager:${AWS::Region}:${AWS::AccountId}:secret:afterhours-shift-manager/roster-api-token-*" + Events: + PutRoster: + Type: HttpApi + Properties: + Path: /roster + Method: PUT + DeleteRoster: + Type: HttpApi + Properties: + Path: /roster/{extension} + Method: DELETE + # --- Ring Scheduler (daily 3CX queue routing updates) --- RingSchedulerFunction: Type: AWS::Serverless::Function @@ -626,8 +664,8 @@ Resources: # --- Lambda Duration alarms (Maximum, ms; 2-of-3 evaluation) --- # Thresholds set to ~80% of each function's timeout, with 2-of-3 evaluation. - # Timeouts: slack-bot/weekly-post/release-notifier = 30s (global default); - # roster-sync/ring-scheduler/holiday-router = 60s. + # Timeouts: slack-bot/weekly-post/release-notifier/roster-api = 30s (global + # default); roster-sync/ring-scheduler/holiday-router = 60s. SlackBotDurationAlarm: Type: AWS::CloudWatch::Alarm Properties: @@ -844,6 +882,64 @@ Resources: AlarmActions: - !Sub "arn:aws:sns:${AWS::Region}:${AWS::AccountId}:site-alerts" + RosterApiErrorAlarm: + Type: AWS::CloudWatch::Alarm + Properties: + AlarmName: !Sub "Lambda-Errors-${RosterApiFunction}" + AlarmDescription: "Roster API Lambda reported one or more errors" + Namespace: AWS/Lambda + MetricName: Errors + Dimensions: + - Name: FunctionName + Value: !Ref RosterApiFunction + Statistic: Sum + Period: 300 + EvaluationPeriods: 1 + Threshold: 1 + ComparisonOperator: GreaterThanOrEqualToThreshold + TreatMissingData: notBreaching + AlarmActions: + - !Sub "arn:aws:sns:${AWS::Region}:${AWS::AccountId}:site-alerts" + + RosterApiDurationAlarm: + Type: AWS::CloudWatch::Alarm + Properties: + AlarmName: !Sub "Lambda-Duration-${RosterApiFunction}" + AlarmDescription: "Roster API Lambda duration approaching its 30s timeout (>=24s)" + Namespace: AWS/Lambda + MetricName: Duration + Dimensions: + - Name: FunctionName + Value: !Ref RosterApiFunction + Statistic: Maximum + Period: 300 + EvaluationPeriods: 3 + DatapointsToAlarm: 2 + Threshold: 24000 + ComparisonOperator: GreaterThanOrEqualToThreshold + TreatMissingData: notBreaching + AlarmActions: + - !Sub "arn:aws:sns:${AWS::Region}:${AWS::AccountId}:site-alerts" + + RosterApiThrottlesAlarm: + Type: AWS::CloudWatch::Alarm + Properties: + AlarmName: !Sub "Lambda-Throttles-${RosterApiFunction}" + AlarmDescription: "Roster API Lambda was throttled (concurrency limit hit)" + Namespace: AWS/Lambda + MetricName: Throttles + Dimensions: + - Name: FunctionName + Value: !Ref RosterApiFunction + Statistic: Sum + Period: 300 + EvaluationPeriods: 1 + Threshold: 1 + ComparisonOperator: GreaterThanOrEqualToThreshold + TreatMissingData: notBreaching + AlarmActions: + - !Sub "arn:aws:sns:${AWS::Region}:${AWS::AccountId}:site-alerts" + ReleaseNotifierThrottlesAlarm: Type: AWS::CloudWatch::Alarm Properties: @@ -913,7 +1009,7 @@ Resources: Type: AWS::CloudWatch::Alarm Properties: AlarmName: !Sub "ApiGateway-4xx-${ServerlessHttpApi}" - AlarmDescription: "Elevated 4xx responses on the Slack events HTTP API" + AlarmDescription: "Elevated 4xx responses on the afterhours HTTP API" Namespace: AWS/ApiGateway MetricName: 4xx Dimensions: @@ -932,7 +1028,7 @@ Resources: Type: AWS::CloudWatch::Alarm Properties: AlarmName: !Sub "ApiGateway-5xx-${ServerlessHttpApi}" - AlarmDescription: "5xx responses on the Slack events HTTP API" + AlarmDescription: "5xx responses on the afterhours HTTP API" Namespace: AWS/ApiGateway MetricName: 5xx Dimensions: @@ -953,7 +1049,7 @@ Resources: Type: AWS::CloudWatch::Alarm Properties: AlarmName: !Sub "ApiGateway-Latency-${ServerlessHttpApi}" - AlarmDescription: "p99 latency on the Slack events HTTP API exceeded 3s" + AlarmDescription: "p99 latency on the afterhours HTTP API exceeded 3s" Namespace: AWS/ApiGateway MetricName: Latency Dimensions: @@ -988,6 +1084,12 @@ Resources: LogGroupName: !Sub "/aws/lambda/${RosterSyncFunction}" RetentionInDays: 60 + RosterApiLogGroup: + Type: AWS::Logs::LogGroup + Properties: + LogGroupName: !Sub "/aws/lambda/${RosterApiFunction}" + RetentionInDays: 60 + RingSchedulerLogGroup: Type: AWS::Logs::LogGroup Properties: @@ -1010,6 +1112,9 @@ Outputs: SlackBotApiUrl: Description: URL for Slack app Request URL configuration Value: !Sub "https://${ServerlessHttpApi}.execute-api.${AWS::Region}.amazonaws.com/slack/events" + AfterhoursApiBaseUrl: + Description: Origin for AFTERHOURS_BASE_URL (no /mgmt or /roster suffix) + Value: !Sub "https://${ServerlessHttpApi}.execute-api.${AWS::Region}.amazonaws.com" ShiftTableName: Value: !Ref ShiftTable SlackBotFunctionArn: diff --git a/tests/roster_api/conftest.py b/tests/roster_api/conftest.py new file mode 100644 index 0000000..e35823d --- /dev/null +++ b/tests/roster_api/conftest.py @@ -0,0 +1,24 @@ +"""Load src/roster-api/app.py under a unique module name.""" + +import importlib.util +import pathlib +import sys + +import pytest + +_ROOT = pathlib.Path(__file__).resolve().parents[2] + + +def _load(name, relpath): + spec = importlib.util.spec_from_file_location(name, _ROOT / relpath) + mod = importlib.util.module_from_spec(spec) + sys.modules[name] = mod + spec.loader.exec_module(mod) + return mod + + +@pytest.fixture +def rosterapi_app(): + mod = _load("rosterapi_app", "src/roster-api/app.py") + yield mod + mod._cached_token = None diff --git a/tests/roster_api/test_handler.py b/tests/roster_api/test_handler.py new file mode 100644 index 0000000..ae67e25 --- /dev/null +++ b/tests/roster_api/test_handler.py @@ -0,0 +1,282 @@ +"""Tests for the roster-api Lambda handler.""" + +import base64 +import json +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +TOKEN = "roster-test-token" +SECRET_NAME = "afterhours-shift-manager/roster-api-token" + + +@pytest.fixture +def env(monkeypatch): + monkeypatch.setenv("ROSTER_API_TOKEN_SECRET", SECRET_NAME) + + +@pytest.fixture +def secrets(rosterapi_app, monkeypatch, env): + monkeypatch.setattr(rosterapi_app, "get_secret", lambda _id: TOKEN) + rosterapi_app._cached_token = None + + +def _event( + method="PUT", + path="/roster", + body=None, + token=TOKEN, + headers=None, + is_base64=False, + extension=None, + include_auth=True, +): + hdrs = {} + if headers: + hdrs.update(headers) + if include_auth and token is not None: + hdrs.setdefault("authorization", f"Bearer {token}") + payload = None + if body is not None: + raw = ( + json.dumps(body).encode("utf-8") + if not isinstance(body, (bytes, str)) + else body + ) + if isinstance(raw, str): + raw = raw.encode("utf-8") + if is_base64: + payload = base64.b64encode(raw).decode("ascii") + else: + payload = raw.decode("utf-8") + event = { + "version": "2.0", + "routeKey": f"{method} {path}", + "rawPath": path, + "headers": hdrs, + "requestContext": {"http": {"method": method, "path": path}}, + "body": payload, + "isBase64Encoded": is_base64, + } + if extension is not None: + event["pathParameters"] = {"extension": extension} + return event + + +def _put_body(**overrides): + data = { + "name": "Pat Smith", + "extension": "110", + "slack_user_id": "U123ABCDE", + } + data.update(overrides) + return data + + +def test_put_writes_all_three_fields(rosterapi_app, schedule, secrets): + result = rosterapi_app.handler(_event(body=_put_body()), None) + assert result["statusCode"] == 200 + assert json.loads(result["body"]) == {"ok": True} + emp = schedule.get_employee_by_extension("110") + assert emp["name"] == "Pat Smith" + assert emp["extension"] == "110" + assert emp["slack_user_id"] == "U123ABCDE" + + +def test_put_updates_existing_and_preserves_shift_rate( + rosterapi_app, schedule, seed, secrets +): + seed.roster("110", "Old Name", slack_user_id="", shift_rate="80") + result = rosterapi_app.handler(_event(body=_put_body()), None) + assert result["statusCode"] == 200 + emp = schedule.get_employee_by_extension("110") + assert emp["name"] == "Pat Smith" + assert emp["slack_user_id"] == "U123ABCDE" + assert emp["shift_rate"] == "80" + + +def test_put_trims_surrounding_whitespace(rosterapi_app, schedule, secrets): + result = rosterapi_app.handler( + _event( + body={ + "name": " Pat Smith ", + "extension": " 110 ", + "slack_user_id": " U123ABCDE ", + } + ), + None, + ) + assert result["statusCode"] == 200 + emp = schedule.get_employee_by_extension("110") + assert emp["name"] == "Pat Smith" + assert emp["slack_user_id"] == "U123ABCDE" + + +def test_put_accepts_base64_body(rosterapi_app, schedule, secrets): + result = rosterapi_app.handler(_event(body=_put_body(), is_base64=True), None) + assert result["statusCode"] == 200 + assert schedule.get_employee_by_extension("110")["slack_user_id"] == "U123ABCDE" + + +def test_put_accepts_case_insensitive_authorization_header( + rosterapi_app, schedule, secrets +): + result = rosterapi_app.handler( + _event( + body=_put_body(), + include_auth=False, + headers={"Authorization": f"Bearer {TOKEN}"}, + ), + None, + ) + assert result["statusCode"] == 200 + + +def test_401_missing_token(rosterapi_app, schedule, secrets): + result = rosterapi_app.handler(_event(body=_put_body(), include_auth=False), None) + assert result["statusCode"] == 401 + assert schedule.get_employee_by_extension("110") is None + + +def test_401_wrong_token(rosterapi_app, schedule, secrets): + result = rosterapi_app.handler(_event(body=_put_body(), token="nope"), None) + assert result["statusCode"] == 401 + assert schedule.get_employee_by_extension("110") is None + + +def test_authorize_strips_secret_trailing_newline( + rosterapi_app, schedule, env, monkeypatch +): + monkeypatch.setattr(rosterapi_app, "get_secret", lambda _id: TOKEN + "\n") + rosterapi_app._cached_token = None + result = rosterapi_app.handler(_event(body=_put_body()), None) + assert result["statusCode"] == 200 + assert schedule.get_employee_by_extension("110")["slack_user_id"] == "U123ABCDE" + + +def test_503_when_secret_is_whitespace_only(rosterapi_app, schedule, env, monkeypatch): + monkeypatch.setattr(rosterapi_app, "get_secret", lambda _id: "\n") + rosterapi_app._cached_token = None + result = rosterapi_app.handler(_event(body=_put_body()), None) + assert result["statusCode"] == 503 + assert schedule.get_employee_by_extension("110") is None + + +def test_503_when_secret_read_fails(rosterapi_app, schedule, env, monkeypatch): + def boom(_id): + raise RuntimeError("secrets down") + + monkeypatch.setattr(rosterapi_app, "get_secret", boom) + rosterapi_app._cached_token = None + result = rosterapi_app.handler(_event(body=_put_body()), None) + assert result["statusCode"] == 503 + assert schedule.get_employee_by_extension("110") is None + + +def test_400_missing_field(rosterapi_app, schedule, secrets): + body = _put_body() + del body["slack_user_id"] + result = rosterapi_app.handler(_event(body=body), None) + assert result["statusCode"] == 400 + assert schedule.get_employee_by_extension("110") is None + + +def test_400_extra_field(rosterapi_app, schedule, secrets): + body = _put_body(extra="nope") + result = rosterapi_app.handler(_event(body=body), None) + assert result["statusCode"] == 400 + assert schedule.get_employee_by_extension("110") is None + + +def test_400_blank_and_control_values(rosterapi_app, schedule, secrets): + for body in ( + _put_body(name=" "), + _put_body(name="Pat\nSmith"), + _put_body(extension="11a"), + _put_body(slack_user_id="U123 AB"), + _put_body(extension=""), + ): + result = rosterapi_app.handler(_event(body=body), None) + assert result["statusCode"] == 400, body + assert schedule.get_roster() == [] + + +def test_400_oversized_body(rosterapi_app, schedule, secrets): + huge = _put_body(name="P" * 5000) + result = rosterapi_app.handler(_event(body=huge), None) + assert result["statusCode"] == 400 + assert schedule.get_employee_by_extension("110") is None + + +def test_400_invalid_json(rosterapi_app, schedule, secrets): + result = rosterapi_app.handler(_event(body="{not json"), None) + assert result["statusCode"] == 400 + + +def test_delete_missing_row_is_204(rosterapi_app, schedule, secrets): + result = rosterapi_app.handler( + _event(method="DELETE", path="/roster/999", extension="999", body=None), + None, + ) + assert result["statusCode"] == 204 + assert result["body"] == "" + assert schedule.get_employee_by_extension("999") is None + + +def test_delete_existing_row_is_204(rosterapi_app, schedule, seed, secrets): + seed.roster("110", "Pat Smith", slack_user_id="U123ABCDE") + result = rosterapi_app.handler( + _event(method="DELETE", path="/roster/110", extension="110", body=None), + None, + ) + assert result["statusCode"] == 204 + assert schedule.get_employee_by_extension("110") is None + + +def test_405_other_methods(rosterapi_app, secrets): + result = rosterapi_app.handler( + _event(method="GET", path="/roster", body=None), None + ) + assert result["statusCode"] == 405 + + +def test_never_logs_authorization_or_body(rosterapi_app, schedule, secrets, caplog): + import logging + + caplog.set_level(logging.DEBUG) + body = _put_body(name="SecretName") + rosterapi_app.handler( + _event(body=body, token=TOKEN, headers={"authorization": f"Bearer {TOKEN}"}), + None, + ) + ours = "\n".join( + rec.getMessage() for rec in caplog.records if "roster-api" in rec.pathname + ) + assert TOKEN not in ours + assert "SecretName" not in ours + assert "Bearer" not in ours + + +def test_never_calls_roster_sync(rosterapi_app): + source = ( + Path(__file__) + .resolve() + .parents[2] + .joinpath("src/roster-api/app.py") + .read_text() + ) + assert "roster-sync" not in source + assert "roster_sync" not in source + assert "InvokeFunction" not in source + assert rosterapi_app.handler.__module__ == "rosterapi_app" + + +def test_500_on_unexpected_failure(rosterapi_app, secrets, monkeypatch): + monkeypatch.setattr( + rosterapi_app, + "ShiftSchedule", + MagicMock(side_effect=RuntimeError("ddb down")), + ) + result = rosterapi_app.handler(_event(body=_put_body()), None) + assert result["statusCode"] == 500 diff --git a/tests/roster_sync/test_handler.py b/tests/roster_sync/test_handler.py index 443368c..fd9b013 100644 --- a/tests/roster_sync/test_handler.py +++ b/tests/roster_sync/test_handler.py @@ -89,3 +89,34 @@ def test_skips_when_wrong_hour_and_not_forced(rostersync_app, env, fake_3cx): # 12:00 UTC = 08:00 ET, not 6am → skip. with freezegun.freeze_time("2026-06-01 12:00:00"): assert rostersync_app.handler({}, None) == {"skipped": True} + + +def test_stale_read_does_not_clear_api_written_slack_id( + rostersync_app, schedule, env, fake_3cx, monkeypatch +): + """A roster-API row that lands after get_roster must keep its Slack id.""" + from shared.schedule import ShiftSchedule + + schedule.upsert_roster_entry("116", "New Person", "U_NEW") + monkeypatch.setattr(ShiftSchedule, "get_roster", lambda self: []) + fake_3cx([{"Number": "116", "MemberName": "New Person", "Type": "Extension"}]) + + result = rostersync_app.handler({"force": True}, None) + + assert any("116" in a for a in result["added"]) + emp = schedule.get_employee_by_extension("116") + assert emp["name"] == "New Person" + assert emp["slack_user_id"] == "U_NEW" + assert emp["extension"] == "116" + + +def test_new_extension_still_starts_with_empty_slack_id( + rostersync_app, schedule, env, fake_3cx +): + fake_3cx([{"Number": "116", "MemberName": "New Person", "Type": "Extension"}]) + + rostersync_app.handler({"force": True}, None) + + emp = schedule.get_employee_by_extension("116") + assert emp["name"] == "New Person" + assert emp["slack_user_id"] == "" diff --git a/tests/shared/test_schedule.py b/tests/shared/test_schedule.py index 1214cd2..56b774d 100644 --- a/tests/shared/test_schedule.py +++ b/tests/shared/test_schedule.py @@ -195,6 +195,35 @@ class TestRoster: assert schedule.add_roster_entry("114", "Alice") is True assert schedule.add_roster_entry("114", "Alice Again") is False assert schedule.get_employee_by_extension("114")["name"] == "Alice" + assert schedule.get_employee_by_extension("114")["slack_user_id"] == "" + + def test_upsert_roster_entry_creates_row_with_slack_id(self, schedule): + schedule.upsert_roster_entry("114", "Alice", "U_ALICE") + emp = schedule.get_employee_by_extension("114") + assert emp["PK"] == "ROSTER" and emp["SK"] == "114" + assert emp["name"] == "Alice" + assert emp["extension"] == "114" + assert emp["slack_user_id"] == "U_ALICE" + assert "shift_rate" not in emp + + def test_upsert_roster_entry_updates_slack_id_and_preserves_shift_rate( + self, schedule, seed + ): + seed.roster("114", "Alice", slack_user_id="", shift_rate="75") + schedule.upsert_roster_entry("114", "Alicia", "U_ALICE") + emp = schedule.get_employee_by_extension("114") + assert emp["name"] == "Alicia" + assert emp["extension"] == "114" + assert emp["slack_user_id"] == "U_ALICE" + assert emp["shift_rate"] == "75" + + def test_add_roster_entry_stays_create_only(self, schedule, seed): + seed.roster("114", "Alice", slack_user_id="U_ALICE", shift_rate="75") + assert schedule.add_roster_entry("114", "Overwrite") is False + emp = schedule.get_employee_by_extension("114") + assert emp["name"] == "Alice" + assert emp["slack_user_id"] == "U_ALICE" + assert emp["shift_rate"] == "75" def test_remove_roster_entry(self, schedule, seed): seed.roster("114", "Alice")