From a5271ba650c342c104fbec6f423989ecc434c47e Mon Sep 17 00:00:00 2001 From: Valtora Date: Fri, 23 Oct 2026 19:35:04 +0000 Subject: [PATCH] fix(calendar): log and show why connecting a calendar failed The OAuth callback redirected to /settings?tab=account, whose legacy redirect drops the query string, so no outcome reached the Calendars card. It now lands on /settings/integrations. A failure at the token exchange, account lookup or calendar list step is logged with the step, the HTTP status and the provider's error code and message, and the code travels to the card as `reason`. The card shows the failure inline, with hints for the common Google Workspace and Microsoft 365 causes. The provider-error helpers move from the push service into calendar_service so both paths share them. Fixes: #668 --- backend/api/v1/endpoints/calendar.py | 62 ++++- backend/services/calendar_push_service.py | 71 +---- backend/services/calendar_service/__init__.py | 12 + .../services/calendar_service/constants.py | 5 +- .../calendar_service/provider_errors.py | 140 ++++++++++ backend/services/calendar_service/sync.py | 57 +++-- backend/services/calendar_service/urls.py | 18 +- backend/tests/test_calendar_and_task_api.py | 242 +++++++++++++++++- .../tests/test_calendar_provider_errors.py | 99 +++++++ backend/tests/test_calendar_push_service.py | 29 ++- docs/CALENDAR.md | 13 + .../CalendarConnectionsSettings.test.tsx | 63 ++++- .../settings/CalendarConnectionsSettings.tsx | 72 ++++-- .../settings/calendarConnectResult.test.ts | 82 ++++++ .../settings/calendarConnectResult.ts | 175 +++++++++++++ pyproject.toml | 1 + 16 files changed, 1009 insertions(+), 132 deletions(-) create mode 100644 backend/services/calendar_service/provider_errors.py create mode 100644 backend/tests/test_calendar_provider_errors.py create mode 100644 frontend/src/components/settings/calendarConnectResult.test.ts create mode 100644 frontend/src/components/settings/calendarConnectResult.ts diff --git a/backend/api/v1/endpoints/calendar.py b/backend/api/v1/endpoints/calendar.py index 82e78dde..4dad31bd 100644 --- a/backend/api/v1/endpoints/calendar.py +++ b/backend/api/v1/endpoints/calendar.py @@ -1,5 +1,6 @@ import json import logging +import re from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response from fastapi.responses import PlainTextResponse, RedirectResponse @@ -24,7 +25,10 @@ handle_microsoft_notification, ) from backend.services.calendar_service import ( + PROVIDER_REASON_PATTERN, + CalendarConnectFailed, _build_account_redirect, + _describe_failure_for_log, disconnect_connection, get_dashboard_summary, get_overview, @@ -46,6 +50,8 @@ # notification batches are small, so anything larger is rejected before parsing # to stop an attacker exhausting worker memory with a huge JSON body. MICROSOFT_WEBHOOK_MAX_BODY_BYTES = 1024 * 1024 +# An Entra sign-in error number, such as AADSTS50194, inside an error description. +AADSTS_CODE_PATTERN = re.compile(r"AADSTS\d{1,8}") def _validate_provider(provider: str) -> str: @@ -69,6 +75,27 @@ def _map_callback_status( return "error" +def _log_refused_sign_in( + provider: str, error: str | None, error_description: str | None +) -> None: + """Log why the provider sent the user back without a code. + + Keeps the OAuth error code and any Entra ``AADSTS`` number, never the + description itself, which can name the account. Both come from the query + string, so only values shaped like a provider code are written. + """ + if not error: + logger.warning("%s calendar sign-in came back without a code", provider) + return + aadsts = AADSTS_CODE_PATTERN.search(error_description or "") + logger.warning( + "%s calendar sign-in came back with error %s%s", + provider, + error if PROVIDER_REASON_PATTERN.fullmatch(error) else "(unrecognised)", + f" ({aadsts.group(0)})" if aadsts else "", + ) + + @router.get("", response_model=CalendarOverviewRead) @router.get("/", response_model=CalendarOverviewRead) async def get_calendar_overview( @@ -132,7 +159,12 @@ async def start_calendar_authorisation_redirect( return RedirectResponse( _build_account_redirect(status_value, provider), status_code=303 ) - except Exception: # noqa: BLE001 -- boundary: OAuth start failures are shown on the account page, not as a 500 + except Exception as exc: # noqa: BLE001 -- boundary: OAuth start failures are logged and shown on the settings page, not as a 500 + logger.warning( + "Starting a %s calendar sign-in failed: %s", + provider, + _describe_failure_for_log(exc), + ) return RedirectResponse( _build_account_redirect("error", provider), status_code=303 ) @@ -163,13 +195,37 @@ async def calendar_authorisation_callback( if error or not code or not state: status_value = _map_callback_status(provider, error, error_description) + if status_value != "cancelled": + _log_refused_sign_in(provider, error, error_description) return RedirectResponse( - _build_account_redirect(status_value, provider), status_code=303 + _build_account_redirect(status_value, provider, error), status_code=303 ) try: await handle_callback(db, provider, current_user, code, state) - except Exception: # noqa: BLE001 -- boundary: callback failures are shown on the account page, not as a 500 + except CalendarConnectFailed as exc: + logger.warning( + "Connecting a %s calendar failed at the %s step: %s", + provider, + exc.step, + exc.detail, + ) + return RedirectResponse( + _build_account_redirect("error", provider, exc.reason), status_code=303 + ) + except HTTPException as exc: + # The integration is not configured, or the sign-in session expired or + # belongs to someone else: fixed messages with nothing of the account. + logger.warning("Connecting a %s calendar failed: %s", provider, exc.detail) + return RedirectResponse( + _build_account_redirect("error", provider), status_code=303 + ) + except Exception as exc: # noqa: BLE001 -- boundary: callback failures are logged and shown on the settings page, not as a 500 + logger.warning( + "Connecting a %s calendar failed: %s", + provider, + _describe_failure_for_log(exc), + ) return RedirectResponse( _build_account_redirect("error", provider), status_code=303 ) diff --git a/backend/services/calendar_push_service.py b/backend/services/calendar_push_service.py index 1447264c..13562ee2 100644 --- a/backend/services/calendar_push_service.py +++ b/backend/services/calendar_push_service.py @@ -44,8 +44,10 @@ from backend.services.calendar_service import ( MICROSOFT_GRAPH_URL, _build_push_notification_url, + _describe_provider_failure, _get_access_token_for_connection, _parse_iso_datetime, + _truncate_error, get_provider_runtime_config, ) from backend.utils.time import utc_now @@ -68,9 +70,7 @@ # blocked, domain unverified, permission missing) and needs a change here first. PUSH_FAILURE_BACKOFF = timedelta(minutes=30) PUSH_REJECTED_BACKOFF = timedelta(hours=6) -# Cap on the provider's error code and message kept in logs and ``last_error``. -PUSH_ERROR_DETAIL_LIMIT = 200 -# ``_describe_push_failure`` starts ``last_error`` with the HTTP status, and +# ``_describe_provider_failure`` starts ``last_error`` with the HTTP status, and # ``_push_retry_backoff`` reads it back from there. RECORDED_HTTP_STATUS = re.compile(r"^HTTP (\d{3})\b") HTTP_CLIENT_ERRORS = range(400, 500) @@ -120,69 +120,6 @@ def _format_graph_datetime(value: datetime) -> str: return value.replace(microsecond=0).isoformat() + "Z" -def _truncate_error(value: str | None, limit: int = 500) -> str | None: - if not value: - return None - cleaned = " ".join(str(value).split()) - return cleaned[:limit] if cleaned else None - - -def _google_error_reason(error: dict[str, Any]) -> str | None: - """Google's specific reason (``pushWebhookBadDomain``), else its status.""" - for item in error.get("errors") or []: - if isinstance(item, dict) and isinstance(item.get("reason"), str): - return item["reason"] - status = error.get("status") - return status if isinstance(status, str) else None - - -def _provider_error_fields(payload: Any) -> tuple[str | None, str | None]: - """Pick the error code and message out of a provider's JSON error body. - - Graph sends ``{"error": {"code": "InvalidRequest", "message": ...}}``, Google - ``{"error": {"code": 400, "message": ..., "errors": [{"reason": ...}]}}``, - and a token endpoint ``{"error": "invalid_grant", "error_description": ...}``. - """ - if not isinstance(payload, dict): - return None, None - error = payload.get("error") - if isinstance(error, str): - description = payload.get("error_description") - return error, description if isinstance(description, str) else None - if not isinstance(error, dict): - return None, None - code = error.get("code") - if not isinstance(code, str): - code = _google_error_reason(error) - message = error.get("message") - return code, message if isinstance(message, str) else None - - -def _describe_push_failure(exc: Exception) -> str: - """Summarise a failed provisioning call for the log and ``last_error``. - - Keeps the HTTP status and the provider's own error code and message, which - say why a subscription was refused where httpx's status line does not. It - reads only the error fields of the response body, never the request, so no - token or calendar address reaches it, and it caps the provider's text. - """ - if isinstance(exc, httpx.HTTPStatusError): - try: - payload = exc.response.json() - except ValueError: - payload = None - code, message = _provider_error_fields(payload) - detail = _truncate_error( - ": ".join(part for part in (code, message) if part), - PUSH_ERROR_DETAIL_LIMIT, - ) - status_line = f"HTTP {exc.response.status_code}" - return f"{status_line} {detail}" if detail else status_line - detail = _truncate_error(str(exc), PUSH_ERROR_DETAIL_LIMIT) - name = type(exc).__name__ - return f"{name}: {detail}" if detail else name - - def _push_retry_backoff(last_error: str | None) -> timedelta: """How long a failed channel waits before its next provisioning attempt. @@ -626,7 +563,7 @@ async def _provision_selected_calendars( try: await _ensure_channel(ctx, calendar, channel) except Exception as exc: # noqa: BLE001 -- boundary: one calendar's channel failing must not stop the others - detail = _describe_push_failure(exc) + detail = _describe_provider_failure(exc) logger.warning( "Failed to provision %s push channel for calendar %s: %s " "(next attempt in %d minutes)", diff --git a/backend/services/calendar_service/__init__.py b/backend/services/calendar_service/__init__.py index 796ded00..1735096a 100644 --- a/backend/services/calendar_service/__init__.py +++ b/backend/services/calendar_service/__init__.py @@ -93,6 +93,18 @@ _serialise_source, _upsert_connection, ) +from .provider_errors import ( + PROVIDER_ERROR_DETAIL_LIMIT, + PROVIDER_REASON_PATTERN, + CalendarConnectFailed, + _describe_failure_for_log, + _describe_provider_failure, + _google_error_reason, + _provider_error_fields, + _provider_error_reason, + _truncate_error, + _usable_reason, +) from .providers import ( _build_google_events_query_params, _build_microsoft_calendar_view_url, diff --git a/backend/services/calendar_service/constants.py b/backend/services/calendar_service/constants.py index aec9324e..6a7922b2 100644 --- a/backend/services/calendar_service/constants.py +++ b/backend/services/calendar_service/constants.py @@ -63,9 +63,12 @@ "cancelled", "tenant-config-error", ) +# The callback lands on the page that holds the Calendars card. The settings +# index turns a legacy ``?tab=`` link into a category route and drops every other +# query parameter, so pointing at ``/settings?tab=...`` loses the result. ACCOUNT_REDIRECT_PATHS = { provider: { - status_value: f"/settings?{urlencode({'tab': 'account', 'calendar': status_value, 'provider': provider})}" + status_value: f"/settings/integrations?{urlencode({'calendar': status_value, 'provider': provider})}" for status_value in ACCOUNT_REDIRECT_STATUSES } for provider in PROVIDER_DISPLAY_NAMES diff --git a/backend/services/calendar_service/provider_errors.py b/backend/services/calendar_service/provider_errors.py new file mode 100644 index 00000000..93b50aca --- /dev/null +++ b/backend/services/calendar_service/provider_errors.py @@ -0,0 +1,140 @@ +"""Reading a provider's error response for the log and the user. + +Shared by connecting an account (the OAuth callback) and by push-channel +provisioning. Each helper reads only the error fields of a response body, never +the request, so no token, authorisation code, state value or calendar address +reaches the output, and any email address a provider echoes back is masked. + +It imports only the ``models_dto`` leaf from the calendar package. +""" + +from __future__ import annotations + +import re +from typing import Any + +import httpx + +from .models_dto import CalendarReauthorisationRequired + +# Cap on the provider's error code and message kept in logs and ``last_error``. +PROVIDER_ERROR_DETAIL_LIMIT = 200 +# A provider error code as it may travel in the settings redirect: Google's +# ``notACalendarUser``, Graph's ``MailboxNotEnabledForRESTAPI``, OAuth's +# ``invalid_client``. Anything else is dropped rather than echoed to the page. +PROVIDER_REASON_PATTERN = re.compile(r"[A-Za-z][A-Za-z0-9_.-]{0,63}") +EMAIL_ADDRESS_PATTERN = re.compile(r"[\w.+-]+@[\w-]+(?:\.[\w-]+)+") + + +class CalendarConnectFailed(Exception): + """A provider refused or failed a step of connecting an account. + + ``step`` names where it happened, ``reason`` is the provider's own error code + when it gave a usable one, and ``detail`` is the described failure the + callback logs. + """ + + def __init__(self, step: str, exc: Exception) -> None: + self.step = step + self.reason = _provider_error_reason(exc) + self.detail = _describe_provider_failure(exc) + super().__init__(f"{step}: {self.detail}") + + +def _truncate_error(value: str | None, limit: int = 500) -> str | None: + if not value: + return None + cleaned = " ".join(str(value).split()) + return cleaned[:limit] if cleaned else None + + +def _google_error_reason(error: dict[str, Any]) -> str | None: + """Google's specific reason (``pushWebhookBadDomain``), else its status.""" + for item in error.get("errors") or []: + if isinstance(item, dict) and isinstance(item.get("reason"), str): + return item["reason"] + status = error.get("status") + return status if isinstance(status, str) else None + + +def _provider_error_fields(payload: Any) -> tuple[str | None, str | None]: + """Pick the error code and message out of a provider's JSON error body. + + Graph sends ``{"error": {"code": "InvalidRequest", "message": ...}}``, Google + ``{"error": {"code": 400, "message": ..., "errors": [{"reason": ...}]}}``, + and a token endpoint ``{"error": "invalid_grant", "error_description": ...}``. + Google's numeric ``code`` repeats the HTTP status, so its reason stands in. + """ + if not isinstance(payload, dict): + return None, None + error = payload.get("error") + if isinstance(error, str): + description = payload.get("error_description") + return error, description if isinstance(description, str) else None + if not isinstance(error, dict): + return None, None + code = error.get("code") + if not isinstance(code, str): + code = _google_error_reason(error) + message = error.get("message") + return code, message if isinstance(message, str) else None + + +def _response_error_fields( + exc: httpx.HTTPStatusError, +) -> tuple[str | None, str | None]: + try: + payload = exc.response.json() + except ValueError: + payload = None + return _provider_error_fields(payload) + + +def _mask_addresses(value: str) -> str: + return EMAIL_ADDRESS_PATTERN.sub("
", value) + + +def _describe_provider_failure(exc: Exception) -> str: + """Summarise a failed provider call for the log and ``last_error``. + + Keeps the HTTP status and the provider's own error code and message, which + say why a request was refused where httpx's status line does not, and caps + the provider's text. Any other exception keeps its type and message. + """ + if isinstance(exc, httpx.HTTPStatusError): + code, message = _response_error_fields(exc) + detail = _truncate_error( + _mask_addresses(": ".join(part for part in (code, message) if part)), + PROVIDER_ERROR_DETAIL_LIMIT, + ) + status_line = f"HTTP {exc.response.status_code}" + return f"{status_line} {detail}" if detail else status_line + detail = _truncate_error(_mask_addresses(str(exc)), PROVIDER_ERROR_DETAIL_LIMIT) + name = type(exc).__name__ + return f"{name}: {detail}" if detail else name + + +def _describe_failure_for_log(exc: Exception) -> str: + """Describe a failure at a boundary that catches everything. + + A provider's answer, a network error and a refused grant are described in + full. Anything else keeps only its type: a database error's message carries + the statement's parameters, which can hold calendar names and event text. + """ + if isinstance(exc, (httpx.HTTPError, CalendarReauthorisationRequired)): + return _describe_provider_failure(exc) + return type(exc).__name__ + + +def _usable_reason(value: str | None) -> str | None: + if value and PROVIDER_REASON_PATTERN.fullmatch(value): + return value + return None + + +def _provider_error_reason(exc: Exception) -> str | None: + """The provider's error code, when it is safe to carry to the settings page.""" + if not isinstance(exc, httpx.HTTPStatusError): + return None + code, _message = _response_error_fields(exc) + return _usable_reason(code) diff --git a/backend/services/calendar_service/sync.py b/backend/services/calendar_service/sync.py index 23672ba5..c3d54480 100644 --- a/backend/services/calendar_service/sync.py +++ b/backend/services/calendar_service/sync.py @@ -12,6 +12,7 @@ import logging +import httpx from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload from sqlmodel import select @@ -39,7 +40,11 @@ from .constants import PROVIDER_DISPLAY_NAMES from .models_dto import ( IncrementalSyncResetRequired, + ProviderCalendarRecord, ProviderEventSyncResult, + ProviderIdentity, + ProviderRuntimeConfig, + TokenBundle, UnreadableCalendarConnectionState, ) from .oauth import ( @@ -61,6 +66,7 @@ _serialise_connection_with_push, _upsert_connection, ) +from .provider_errors import CalendarConnectFailed, _describe_failure_for_log from .providers import ( _get_microsoft_delta_cursor, _list_google_calendars, @@ -135,6 +141,34 @@ async def get_overview(db: AsyncSession, user: User) -> CalendarOverviewRead: ) +async def _read_connecting_account( + provider: str, + runtime_config: ProviderRuntimeConfig, + code: str, + code_verifier: str, +) -> tuple[TokenBundle, ProviderIdentity, list[ProviderCalendarRecord]]: + """Exchange the code, then read the account and its calendars. + + A failure at any step is raised as ``CalendarConnectFailed``, naming the step + and keeping the provider's own error code and message for the callback to + log and show. Nothing is stored until all three succeed. + """ + google = provider == CalendarProvider.GOOGLE.value + step = "token exchange" + try: + exchange_code = _exchange_google_code if google else _exchange_microsoft_code + token_bundle = await exchange_code(runtime_config, code, code_verifier) + step = "account lookup" + fetch_identity = _fetch_google_identity if google else _fetch_microsoft_identity + identity = await fetch_identity(token_bundle.access_token) + step = "calendar list" + list_calendars = _list_google_calendars if google else _list_microsoft_calendars + provider_calendars = await list_calendars(token_bundle.access_token) + except (httpx.HTTPError, KeyError, ValueError) as exc: + raise CalendarConnectFailed(step, exc) from exc + return token_bundle, identity, provider_calendars + + async def handle_callback( db: AsyncSession, provider: str, user: User, code: str, state: str ) -> CalendarConnection: @@ -157,18 +191,9 @@ async def handle_callback( status_code=400, detail="The calendar connection session is invalid" ) - if provider == CalendarProvider.GOOGLE.value: - token_bundle = await _exchange_google_code( - runtime_config, code, str(state_payload["code_verifier"]) - ) - identity = await _fetch_google_identity(token_bundle.access_token) - provider_calendars = await _list_google_calendars(token_bundle.access_token) - else: - token_bundle = await _exchange_microsoft_code( - runtime_config, code, str(state_payload["code_verifier"]) - ) - identity = await _fetch_microsoft_identity(token_bundle.access_token) - provider_calendars = await _list_microsoft_calendars(token_bundle.access_token) + token_bundle, identity, provider_calendars = await _read_connecting_account( + provider, runtime_config, code, str(state_payload["code_verifier"]) + ) connection = await _upsert_connection( db, @@ -180,11 +205,12 @@ async def handle_callback( await _refresh_connection_calendars(db, connection, provider_calendars) try: await sync_connection_in_session(db, connection.id) - except Exception: # noqa: BLE001 -- boundary: the connection is saved; the periodic sync retries + except Exception as exc: # noqa: BLE001 -- boundary: the connection is saved; the periodic sync retries logger.warning( - "Initial calendar sync failed for connection %s (%s)", + "Initial calendar sync failed for connection %s (%s): %s", connection.id, provider, + _describe_failure_for_log(exc), ) await db.refresh(connection) await _enqueue_push_channel_refresh(connection.id) @@ -540,9 +566,10 @@ async def sync_connection_in_session(db: AsyncSession, connection_id: int) -> No return except Exception as exc: # noqa: BLE001 -- boundary: provider and network errors are recorded on the connection, not raised logger.warning( - "Calendar sync failed for connection %s (%s)", + "Calendar sync failed for connection %s (%s): %s", connection.id, connection.provider, + _describe_failure_for_log(exc), ) failure_status, failure_message = _classify_sync_failure(exc) connection.sync_status = failure_status diff --git a/backend/services/calendar_service/urls.py b/backend/services/calendar_service/urls.py index 7d70b9f3..8260a30f 100644 --- a/backend/services/calendar_service/urls.py +++ b/backend/services/calendar_service/urls.py @@ -7,10 +7,13 @@ from __future__ import annotations +from urllib.parse import urlencode + from backend.models.calendar import CalendarProvider from backend.utils.config_manager import get_trusted_web_origin from .constants import ACCOUNT_REDIRECT_PATHS +from .provider_errors import _usable_reason def _build_redirect_uri(provider: str) -> str: @@ -21,8 +24,19 @@ def _build_push_notification_url(provider: str) -> str: return f"{get_trusted_web_origin()}/api/v1/calendar/webhooks/{provider}" -def _build_account_redirect(status_value: str, provider: str) -> str: +def _build_account_redirect( + status_value: str, provider: str, reason: str | None = None +) -> str: + """The settings page a calendar sign-in returns to, as a relative path. + + ``reason`` is the provider's error code for a failed connection. It is added + only to an ``error`` redirect and only when it looks like a provider code, so + the page never echoes free text from a query string or a response body. + """ provider_redirects = ACCOUNT_REDIRECT_PATHS.get(provider) if provider_redirects is None: provider_redirects = ACCOUNT_REDIRECT_PATHS[CalendarProvider.GOOGLE.value] - return provider_redirects.get(status_value, provider_redirects["error"]) + path = provider_redirects.get(status_value, provider_redirects["error"]) + if path == provider_redirects["error"] and _usable_reason(reason): + path = f"{path}&{urlencode({'reason': reason})}" + return path diff --git a/backend/tests/test_calendar_and_task_api.py b/backend/tests/test_calendar_and_task_api.py index ae122e5d..a72b8493 100644 --- a/backend/tests/test_calendar_and_task_api.py +++ b/backend/tests/test_calendar_and_task_api.py @@ -6,6 +6,7 @@ import pkgutil from datetime import UTC, datetime +import httpx import pytest from fastapi import FastAPI, HTTPException from httpx import ASGITransport, AsyncClient @@ -1229,11 +1230,11 @@ async def test_calendar_dashboard_surfaces_recordings_without_calendar_connectio [ ( HTTPException(status_code=400, detail="Provider is not configured"), - "/settings?tab=account&calendar=config-error&provider=google", + "/settings/integrations?calendar=config-error&provider=google", ), ( RuntimeError("boom"), - "/settings?tab=account&calendar=error&provider=google", + "/settings/integrations?calendar=error&provider=google", ), ], ) @@ -1267,7 +1268,7 @@ async def failing_start_authorisation(*args, **kwargs): ( "google", {"error": "access_denied"}, - "/settings?tab=account&calendar=cancelled&provider=google", + "/settings/integrations?calendar=cancelled&provider=google", ), ( "microsoft", @@ -1275,7 +1276,7 @@ async def failing_start_authorisation(*args, **kwargs): "error": "server_error", "error_description": "AADSTS50194: application is not configured as multi-tenant", }, - "/settings?tab=account&calendar=tenant-config-error&provider=microsoft", + "/settings/integrations?calendar=tenant-config-error&provider=microsoft", ), ], ) @@ -1320,7 +1321,7 @@ async def failing_handle_callback(*args, **kwargs): assert response.status_code == 303 assert ( response.headers["location"] - == "/settings?tab=account&calendar=error&provider=google" + == "/settings/integrations?calendar=error&provider=google" ) @@ -1348,8 +1349,237 @@ async def fake_handle_callback(*args, **kwargs): assert response.status_code == 303 assert ( response.headers["location"] - == "/settings?tab=account&calendar=success&provider=google" + == "/settings/integrations?calendar=success&provider=google" + ) + + +# What a provider hands back during a connection, for the tests below. None of +# these may reach the log: they are the credentials and the account's address. +CONNECT_ACCESS_TOKEN = "provider-access-token-1234" +CONNECT_REFRESH_TOKEN = "provider-refresh-token-5678" +CONNECT_CODE = "authorisation-code-9012" +CONNECT_STATE = "oauth-state-3456" +CONNECT_EMAIL = "alice@example.com" +GOOGLE_CALENDAR_LIST_FORBIDDEN = { + "error": { + "code": 403, + "message": "The caller does not have permission", + "errors": [ + { + "message": "The caller does not have permission", + "domain": "global", + "reason": "forbidden", + } + ], + "status": "PERMISSION_DENIED", + } +} +GRAPH_CALENDAR_LIST_DENIED = { + "error": { + "code": "ErrorAccessDenied", + "message": "Access is denied. Check credentials and try again.", + "innerError": {"request-id": "00000000-0000-0000-0000-000000000004"}, + } +} + + +def _backend_log(caplog: pytest.LogCaptureFixture) -> str: + """What this codebase logged, without httpx's request lines, which carry + the test client's own callback URL with its code and state.""" + return "\n".join( + record.getMessage() + for record in caplog.records + if record.name.startswith("backend") + ) + + +def _answer_provider(calendar_list_refusal: dict) -> httpx.MockTransport: + """Google or Microsoft accepting the code and the account lookup, then + refusing the calendar list with 403.""" + + def answer(request: httpx.Request) -> httpx.Response: + path = request.url.path + if request.method == "POST" and path.endswith("/token"): + return httpx.Response( + 200, + json={ + "access_token": CONNECT_ACCESS_TOKEN, + "refresh_token": CONNECT_REFRESH_TOKEN, + "expires_in": 3599, + "scope": "openid email profile calendar.readonly", + }, + ) + if path == "/v1/userinfo": + return httpx.Response( + 200, + json={"sub": "account-1", "email": CONNECT_EMAIL, "name": "Alice"}, + ) + if path == "/v1.0/me": + return httpx.Response( + 200, + json={"id": "account-1", "mail": CONNECT_EMAIL, "displayName": "Alice"}, + ) + if path in ("/calendar/v3/users/me/calendarList", "/v1.0/me/calendars"): + return httpx.Response(403, json=calendar_list_refusal) + raise AssertionError(f"unexpected provider request {request.method} {path}") + + return httpx.MockTransport(answer) + + +@pytest.fixture +def stub_provider(monkeypatch: pytest.MonkeyPatch): + """Stand in for the provider and the sign-in state a callback needs. + + The service opens its own ``httpx.AsyncClient`` per request, so the class + is swapped for one that defaults to the stub transport; the test client + passes its own transport and is unaffected. The state lookup and the rate + limit would otherwise reach for Redis. + """ + from backend.api.v1.endpoints import calendar as calendar_endpoint + from backend.services.calendar_service import sync as calendar_sync + + real_client = httpx.AsyncClient + + async def _no_rate_limit(*_args, **_kwargs) -> None: + return None + + def _stub(provider: str, calendar_list_refusal: dict) -> None: + transport = _answer_provider(calendar_list_refusal) + + class StubProviderClient(real_client): + def __init__(self, *args, **kwargs) -> None: + kwargs.setdefault("transport", transport) + super().__init__(*args, **kwargs) + + async def _pop_state(state: str) -> dict[str, object] | None: + if state != CONNECT_STATE: + return None + return {"user_id": 1, "provider": provider, "code_verifier": "verifier"} + + monkeypatch.setattr(httpx, "AsyncClient", StubProviderClient) + monkeypatch.setattr(calendar_sync, "_pop_oauth_state", _pop_state) + monkeypatch.setattr(calendar_endpoint, "enforce_rate_limit", _no_rate_limit) + + return _stub + + +@pytest.mark.anyio +@pytest.mark.parametrize( + ("provider", "refusal", "reason", "logged"), + [ + ( + "google", + GOOGLE_CALENDAR_LIST_FORBIDDEN, + "forbidden", + "HTTP 403 forbidden: The caller does not have permission", + ), + ( + "microsoft", + GRAPH_CALENDAR_LIST_DENIED, + "ErrorAccessDenied", + "HTTP 403 ErrorAccessDenied: Access is denied. Check credentials and " + "try again.", + ), + ], +) +async def test_oauth_callback_keeps_the_reason_a_provider_refused_the_calendars( + client: AsyncClient, + override_current_user, + test_session_maker: sessionmaker, + stub_provider, + caplog: pytest.LogCaptureFixture, + provider: str, + refusal: dict, + reason: str, + logged: str, +) -> None: + stub_provider(provider, refusal) + await seed_provider_config(test_session_maker, provider=provider) + override_current_user(1) + caplog.set_level(logging.DEBUG, logger="backend") + + response = await client.get( + f"/api/v1/calendar/oauth/{provider}/callback", + params={"code": CONNECT_CODE, "state": CONNECT_STATE}, + ) + + assert response.status_code == 303 + assert response.headers["location"] == ( + f"/settings/integrations?calendar=error&provider={provider}&reason={reason}" + ) + logged_text = _backend_log(caplog) + assert ( + f"Connecting a {provider} calendar failed at the calendar list step: {logged}" + in logged_text + ) + for secret in ( + CONNECT_ACCESS_TOKEN, + CONNECT_REFRESH_TOKEN, + CONNECT_CODE, + CONNECT_STATE, + CONNECT_EMAIL, + ): + assert secret not in logged_text + assert ( + await fetch_scalar( + test_session_maker, "SELECT COUNT(*) FROM calendar_connections" + ) + == 0 + ) + + +@pytest.mark.anyio +async def test_oauth_callback_logs_and_carries_a_refused_sign_in( + client: AsyncClient, + override_current_user, + caplog: pytest.LogCaptureFixture, +) -> None: + override_current_user(1) + caplog.set_level(logging.DEBUG, logger="backend") + + response = await client.get( + "/api/v1/calendar/oauth/microsoft/callback", + params={ + "error": "unauthorized_client", + "error_description": ( + f"AADSTS700016: Application not found for {CONNECT_EMAIL}." + ), + }, + ) + + assert response.status_code == 303 + assert response.headers["location"] == ( + "/settings/integrations?calendar=error&provider=microsoft" + "&reason=unauthorized_client" + ) + logged_text = _backend_log(caplog) + assert ( + "microsoft calendar sign-in came back with error unauthorized_client " + "(AADSTS700016)" + ) in logged_text + assert CONNECT_EMAIL not in logged_text + + +@pytest.mark.anyio +async def test_oauth_callback_drops_a_reason_that_is_not_a_provider_code( + client: AsyncClient, + override_current_user, + caplog: pytest.LogCaptureFixture, +) -> None: + override_current_user(1) + caplog.set_level(logging.DEBUG, logger="backend") + + response = await client.get( + "/api/v1/calendar/oauth/google/callback", + params={"error": "Call +44 20 7946 0000 to restore access"}, + ) + + assert response.headers["location"] == ( + "/settings/integrations?calendar=error&provider=google" ) + logged_text = _backend_log(caplog) + assert "google calendar sign-in came back with error (unrecognised)" in logged_text + assert "7946" not in logged_text @pytest.mark.anyio diff --git a/backend/tests/test_calendar_provider_errors.py b/backend/tests/test_calendar_provider_errors.py new file mode 100644 index 00000000..54bc3734 --- /dev/null +++ b/backend/tests/test_calendar_provider_errors.py @@ -0,0 +1,99 @@ +"""How a provider's refusal is described for the log and the settings page. + +Connecting an account (#668) and provisioning a push channel (#662) share these +helpers. Their output reaches the API log and the redirect to the settings page, +so it must keep the provider's reason and nothing of the request or account. +""" + +from __future__ import annotations + +import httpx +import pytest +from sqlalchemy.exc import IntegrityError + +from backend.services.calendar_service import ( + CalendarConnectFailed, + _build_account_redirect, + _describe_failure_for_log, + _describe_provider_failure, +) + +TOKEN_URL = "https://login.microsoftonline.com/common/oauth2/v2.0/token" + + +def _refusal(status_code: int, body: dict) -> httpx.HTTPStatusError: + request = httpx.Request("POST", TOKEN_URL, data={"code": "authorisation-code"}) + response = httpx.Response(status_code, json=body, request=request) + return httpx.HTTPStatusError("refused", request=request, response=response) + + +def test_a_token_endpoint_refusal_keeps_its_oauth_code() -> None: + failure = CalendarConnectFailed( + "token exchange", + _refusal( + 401, + { + "error": "invalid_client", + "error_description": "AADSTS7000215: Invalid client secret provided.", + }, + ), + ) + + assert failure.step == "token exchange" + assert failure.reason == "invalid_client" + assert failure.detail == ( + "HTTP 401 invalid_client: AADSTS7000215: Invalid client secret provided." + ) + assert "authorisation-code" not in failure.detail + + +def test_a_provider_message_never_carries_an_address() -> None: + detail = _describe_provider_failure( + _refusal( + 403, + { + "error": { + "code": "ErrorAccessDenied", + "message": "Access to alice.example@example.com is denied.", + } + }, + ) + ) + + assert detail == "HTTP 403 ErrorAccessDenied: Access to
is denied." + + +def test_a_reason_that_is_not_a_provider_code_stays_off_the_page() -> None: + failure = CalendarConnectFailed( + "calendar list", + _refusal(403, {"error": {"code": "Call us on 0000", "message": "Denied"}}), + ) + + assert failure.reason is None + assert _build_account_redirect("error", "google", "Call us on 0000") == ( + "/settings/integrations?calendar=error&provider=google" + ) + + +@pytest.mark.parametrize("status_value", ["success", "cancelled", "config-error"]) +def test_only_a_failed_connection_carries_a_reason(status_value: str) -> None: + assert _build_account_redirect(status_value, "microsoft", "forbidden") == ( + f"/settings/integrations?calendar={status_value}&provider=microsoft" + ) + + +def test_a_database_error_is_logged_by_type_alone() -> None: + # The statement's parameters are part of the message, and can hold calendar + # names and event text. + error = IntegrityError( + "INSERT INTO calendar_sources (name) VALUES (?)", ("Alice's calendar",), None + ) + + assert _describe_failure_for_log(error) == "IntegrityError" + + +def test_a_network_error_is_logged_in_full() -> None: + assert ( + _describe_failure_for_log(httpx.ConnectError("All connection attempts failed")) + == "ConnectError: All connection attempts failed" + ) diff --git a/backend/tests/test_calendar_push_service.py b/backend/tests/test_calendar_push_service.py index 978c4f02..eb7cc113 100644 --- a/backend/tests/test_calendar_push_service.py +++ b/backend/tests/test_calendar_push_service.py @@ -16,6 +16,7 @@ from backend.core.encryption import encrypt_secret from backend.models.calendar import CalendarProvider, CalendarPushChannelStatus from backend.services import calendar_push_service as push +from backend.services.calendar_service import PROVIDER_ERROR_DETAIL_LIMIT NOW = datetime(2026, 7, 2, 12, 0, 0) @@ -167,8 +168,10 @@ def _provider_error(status_code: int, body: dict | str) -> httpx.HTTPStatusError } -def test_describe_push_failure_keeps_graph_code_and_message() -> None: - detail = push._describe_push_failure(_provider_error(400, GRAPH_VALIDATION_FAILED)) +def test_describe_provider_failure_keeps_graph_code_and_message() -> None: + detail = push._describe_provider_failure( + _provider_error(400, GRAPH_VALIDATION_FAILED) + ) assert detail == ( "HTTP 400 InvalidRequest: Subscription validation request failed. " @@ -176,8 +179,8 @@ def test_describe_push_failure_keeps_graph_code_and_message() -> None: ) -def test_describe_push_failure_keeps_google_reason_and_message() -> None: - detail = push._describe_push_failure( +def test_describe_provider_failure_keeps_google_reason_and_message() -> None: + detail = push._describe_provider_failure( _provider_error(400, GOOGLE_WEBHOOK_UNVERIFIED) ) @@ -191,29 +194,29 @@ def test_describe_push_failure_keeps_google_reason_and_message() -> None: "body", [GRAPH_VALIDATION_FAILED, GOOGLE_WEBHOOK_UNVERIFIED, "blocked", {}], ) -def test_describe_push_failure_never_carries_the_request(body) -> None: - detail = push._describe_push_failure(_provider_error(400, body)) +def test_describe_provider_failure_never_carries_the_request(body) -> None: + detail = push._describe_provider_failure(_provider_error(400, body)) assert ACCESS_TOKEN not in detail assert GOOGLE_CALENDAR_ADDRESS not in detail assert "alice" not in detail.lower() -def test_describe_push_failure_without_a_json_body_keeps_the_status() -> None: - assert push._describe_push_failure(_provider_error(502, "")) == "HTTP 502" +def test_describe_provider_failure_without_a_json_body_keeps_the_status() -> None: + assert push._describe_provider_failure(_provider_error(502, "")) == "HTTP 502" -def test_describe_push_failure_caps_the_provider_text() -> None: +def test_describe_provider_failure_caps_the_provider_text() -> None: body = {"error": {"code": "InvalidRequest", "message": "x" * 1000}} - detail = push._describe_push_failure(_provider_error(400, body)) + detail = push._describe_provider_failure(_provider_error(400, body)) assert detail.startswith("HTTP 400 InvalidRequest: xxx") - assert len(detail) == len("HTTP 400 ") + push.PUSH_ERROR_DETAIL_LIMIT + assert len(detail) == len("HTTP 400 ") + PROVIDER_ERROR_DETAIL_LIMIT -def test_describe_push_failure_names_a_network_error() -> None: - detail = push._describe_push_failure( +def test_describe_provider_failure_names_a_network_error() -> None: + detail = push._describe_provider_failure( httpx.ConnectError("All connection attempts failed") ) diff --git a/docs/CALENDAR.md b/docs/CALENDAR.md index 219813fb..d4b8ee80 100644 --- a/docs/CALENDAR.md +++ b/docs/CALENDAR.md @@ -75,6 +75,8 @@ Live sync needs no extra Microsoft setup: `Calendars.Read` covers change-notific Each connected account has **Sync now** and **Disconnect** buttons, and shows its status: **Up to date**, **Syncing**, **Needs attention** or **Reconnect required**. +If the connection fails after you approve access, the **Calendars** card says why, with the provider's own reason, and no account is added. [Connecting Fails After Sign-In](#connecting-fails-after-sign-in) lists the common causes. + To check a new setup, connect an account as a normal user, select a calendar, and confirm the dashboard shows its events. ## Dashboard and Linked Recordings @@ -125,6 +127,17 @@ Nojoin holds the **Secret ID** instead of the secret **Value**. Enter the **Valu The tenant blocks user consent for the delegated permissions. A tenant administrator must grant consent. +### Connecting Fails After Sign-In + +The **Calendars** card shows what the provider said, and the API log (`docker logs nojoin-api`) records the step that failed with the HTTP status and the provider's error code and message, for example `Connecting a google calendar failed at the calendar list step: HTTP 403 forbidden: ...`. The log never holds the account's tokens or address. Common reasons: + +- **`notACalendarUser`, or `forbidden` from Google for a Google Workspace account.** Calendar is turned off for that user, or the Workspace administrator restricts third-party apps' access to the Calendar API. In the Google Admin console, turn Calendar on for the user under **Apps > Google Workspace > Calendar**, or mark Nojoin's OAuth app as trusted under **Security > Access and data control > API controls**. +- **`admin_policy_enforced` from Google.** The Workspace administrator blocks the app. Trust it under **API controls** as above. +- **`accessNotConfigured` from Google.** The Google Calendar API is not enabled in the Google Cloud project behind the client ID. Enable it, as in [Google OAuth Setup](#google-oauth-setup). +- **`ErrorAccessDenied` from Microsoft.** The tenant blocks the app or has not consented to its permissions. See [Admin Approval Message in Microsoft](#admin-approval-message-in-microsoft). +- **`MailboxNotEnabledForRESTAPI` from Microsoft.** The account has no Exchange Online mailbox, for example because it is unlicensed or its mailbox is hosted on-premises, so it has no calendar Nojoin can read. +- **`invalid_client`.** The provider refused the app credentials. For Microsoft, see [`invalid_client` During Token Exchange](#invalid_client-during-token-exchange). + ### Google Testing Mode Blocks a User While the Google app is in testing mode, only listed test users can connect. Add the account as a test user or publish the consent screen. diff --git a/frontend/src/components/settings/CalendarConnectionsSettings.test.tsx b/frontend/src/components/settings/CalendarConnectionsSettings.test.tsx index 7ce8e8cc..efe7bfcd 100644 --- a/frontend/src/components/settings/CalendarConnectionsSettings.test.tsx +++ b/frontend/src/components/settings/CalendarConnectionsSettings.test.tsx @@ -8,10 +8,13 @@ vi.mock("./SettingsProvider", () => ({ useSettingsContext: () => context, })); +// The address the card renders at; a sign-in comes back with its outcome in the +// query string. +const navigation = { search: new URLSearchParams(), replace: vi.fn() }; vi.mock("next/navigation", () => ({ - useRouter: () => ({ replace: vi.fn(), push: vi.fn() }), + useRouter: () => ({ replace: navigation.replace, push: vi.fn() }), usePathname: () => "/settings/integrations", - useSearchParams: () => new URLSearchParams(), + useSearchParams: () => navigation.search, })); const unconfigured: CalendarOverview = { @@ -36,6 +39,8 @@ import CalendarConnectionsSettings from "./CalendarConnectionsSettings"; beforeEach(() => { context.isAdmin = false; + navigation.search = new URLSearchParams(); + navigation.replace.mockClear(); }); describe("CalendarConnectionsSettings with no providers set up", () => { @@ -96,3 +101,57 @@ describe("CalendarConnectionsSettings with a connection", () => { expect(sync).toHaveFocus(); }); }); + +describe("CalendarConnectionsSettings after a sign-in fails", () => { + it("says why Google refused the calendars, with the Workspace hint", async () => { + navigation.search = new URLSearchParams( + "calendar=error&provider=google&reason=forbidden", + ); + render(); + + const alert = await screen.findByRole("alert"); + expect(alert).toHaveTextContent("Could not connect Google Calendar"); + expect(alert).toHaveTextContent( + "Google refused Nojoin access to this account's calendars.", + ); + expect(alert).toHaveTextContent( + "an administrator may have turned Calendar off for you, or restricted third-party apps' access to the Calendar API, in the Google Admin console.", + ); + expect(alert).toHaveTextContent("Reason given by Google: forbidden"); + expect(navigation.replace).toHaveBeenCalledWith("/settings/integrations", { + scroll: false, + }); + }); + + it("says why Microsoft refused the calendars", async () => { + navigation.search = new URLSearchParams( + "calendar=error&provider=microsoft&reason=ErrorAccessDenied", + ); + render(); + + const alert = await screen.findByRole("alert"); + expect(alert).toHaveTextContent("Could not connect Outlook Calendar"); + expect(alert).toHaveTextContent( + "Microsoft refused Nojoin access to this account's calendars.", + ); + expect(alert).toHaveTextContent("Reason given by Microsoft: ErrorAccessDenied"); + }); + + it("stays until dismissed", async () => { + navigation.search = new URLSearchParams("calendar=error&provider=google"); + render(); + + const alert = await screen.findByRole("alert"); + expect(alert).toHaveTextContent("Something went wrong while connecting."); + fireEvent.click(screen.getByRole("button", { name: "Dismiss" })); + expect(screen.queryByRole("alert")).toBeNull(); + }); + + it("shows nothing after a successful sign-in", async () => { + navigation.search = new URLSearchParams("calendar=success&provider=google"); + render(); + + await screen.findByText(/Ask an administrator to add it/); + expect(screen.queryByRole("alert")).toBeNull(); + }); +}); diff --git a/frontend/src/components/settings/CalendarConnectionsSettings.tsx b/frontend/src/components/settings/CalendarConnectionsSettings.tsx index 524a948d..7cfc6c16 100644 --- a/frontend/src/components/settings/CalendarConnectionsSettings.tsx +++ b/frontend/src/components/settings/CalendarConnectionsSettings.tsx @@ -30,6 +30,10 @@ import { Trash2, Zap, } from "lucide-react"; +import { + CALENDAR_CONNECT_LABELS as CONNECT_LABELS, + describeCalendarConnectFailure, +} from "./calendarConnectResult"; import SettingsBlock from "./SettingsBlock"; import SettingsCallout from "./SettingsCallout"; import SettingsCard from "./SettingsCard"; @@ -50,11 +54,6 @@ const PROVIDER_LABELS: Record = { microsoft: "Microsoft", }; -const CONNECT_LABELS: Record = { - google: "Google Calendar", - microsoft: "Outlook Calendar", -}; - const STATUS_LABELS: Record = { idle: "Idle", syncing: "Syncing", @@ -108,6 +107,16 @@ export default function CalendarConnectionsSettings() { const [busyKey, setBusyKey] = useState(null); const { addNotification } = useNotificationStore(); const handledCallbackRef = useRef(false); + // A sign-in arrives by a full page load, so the outcome it carries in the + // query string is read once, when the card first renders. The effect below + // then strips it from the address bar. + const [connectFailure, setConnectFailure] = useState(() => + describeCalendarConnectFailure( + searchParams.get("calendar"), + searchParams.get("provider"), + searchParams.get("reason"), + ), + ); const loadOverview = useCallback(async () => { setLoading(true); @@ -144,26 +153,21 @@ export default function CalendarConnectionsSettings() { const nextSearchParams = new URLSearchParams(searchParams.toString()); nextSearchParams.delete("calendar"); nextSearchParams.delete("provider"); + nextSearchParams.delete("reason"); const nextQuery = nextSearchParams.toString(); const providerLabel = CONNECT_LABELS[provider as CalendarProvider] || "calendar"; - addNotification({ - type: calendarStatus === "success" ? "success" : "error", - message: (() => { - if (calendarStatus === "success") { - return `${providerLabel} connected successfully`; - } - if (calendarStatus === "config-error") { - return `${providerLabel} sign-in is not configured yet. An administrator must add the installation OAuth app credentials first.`; - } - if (calendarStatus === "tenant-config-error") { - return `${providerLabel} sign-in is configured to use tenant common, but the Microsoft app registration is still single-tenant. In Entra, change Supported account types to include organisational directories and personal Microsoft accounts, or enter a specific tenant ID instead of common.`; - } - if (calendarStatus === "cancelled") { - return `${providerLabel} connection was cancelled.`; - } - return `Failed to connect ${providerLabel}`; - })(), - }); + // A failure is shown in the card, where it stays until dismissed. + if (calendarStatus === "success") { + addNotification({ + type: "success", + message: `${providerLabel} connected successfully`, + }); + } else if (calendarStatus === "cancelled") { + addNotification({ + type: "error", + message: `${providerLabel} connection was cancelled.`, + }); + } router.replace(nextQuery ? `${pathname}?${nextQuery}` : pathname, { scroll: false, }); @@ -176,6 +180,7 @@ export default function CalendarConnectionsSettings() { ); const handleConnect = async (provider: CalendarProvider) => { + setConnectFailure(null); setBusyKey(`connect:${provider}`); try { const { authorisation_url } = await startCalendarAuthorisation(provider); @@ -313,6 +318,27 @@ export default function CalendarConnectionsSettings() { title="Calendars" description="Connect a Google or Outlook account, approve access on its own sign-in page, then choose which calendars Nojoin shows you." > + {connectFailure && ( + +
+ +

{connectFailure.message}

+ {connectFailure.hint &&

{connectFailure.hint}

} + {connectFailure.reasonText && ( +

{connectFailure.reasonText}

+ )} + + +
+ + )} + {loading ? (
diff --git a/frontend/src/components/settings/calendarConnectResult.test.ts b/frontend/src/components/settings/calendarConnectResult.test.ts new file mode 100644 index 00000000..a8400bee --- /dev/null +++ b/frontend/src/components/settings/calendarConnectResult.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from "vitest"; + +import { describeCalendarConnectFailure } from "./calendarConnectResult"; + +describe("describeCalendarConnectFailure", () => { + it("has nothing to say about a success or a cancelled sign-in", () => { + expect(describeCalendarConnectFailure("success", "google", null)).toBeNull(); + expect(describeCalendarConnectFailure("cancelled", "microsoft", null)).toBeNull(); + expect(describeCalendarConnectFailure(null, null, null)).toBeNull(); + }); + + it("explains a Workspace account with Calendar turned off", () => { + expect( + describeCalendarConnectFailure("error", "google", "notACalendarUser"), + ).toEqual({ + title: "Could not connect Google Calendar", + message: "Google says this account is not set up for Google Calendar.", + hint: "If this is a Google Workspace account, an administrator can turn Calendar on for it in the Google Admin console.", + reasonText: "Reason given by Google: notACalendarUser", + }); + }); + + it("explains a Calendar API the Cloud project has not enabled", () => { + const failure = describeCalendarConnectFailure( + "error", + "google", + "accessNotConfigured", + ); + + expect(failure?.message).toBe( + "The Google Calendar API is turned off in the Google Cloud project this installation signs in with.", + ); + }); + + it("explains a Microsoft account without a mailbox", () => { + const failure = describeCalendarConnectFailure( + "error", + "microsoft", + "MailboxNotEnabledForRESTAPI", + ); + + expect(failure?.message).toBe( + "Microsoft says this account has no active Exchange Online mailbox, so it has no calendar Nojoin can read.", + ); + }); + + it("shows a code it does not know as the provider gave it", () => { + expect( + describeCalendarConnectFailure("error", "google", "somethingNew"), + ).toEqual({ + title: "Could not connect Google Calendar", + message: "Google refused the connection.", + reasonText: "Reason given by Google: somethingNew", + }); + }); + + it("never shows text from the link that is not shaped like a code", () => { + const failure = describeCalendarConnectFailure( + "error", + "google", + "Call 0000 to restore access", + ); + + expect(failure?.reasonText).toBeUndefined(); + expect(failure?.message).toBe( + "Something went wrong while connecting. Try again, and if it keeps failing, an administrator can find the cause in Nojoin's logs.", + ); + }); + + it("keeps the Entra fix for a single-tenant app registration", () => { + const failure = describeCalendarConnectFailure( + "tenant-config-error", + "microsoft", + null, + ); + + expect(failure?.message).toBe( + "Outlook Calendar sign-in uses the tenant common, but the Microsoft app registration is still single-tenant.", + ); + expect(failure?.hint).toContain("Supported account types"); + }); +}); diff --git a/frontend/src/components/settings/calendarConnectResult.ts b/frontend/src/components/settings/calendarConnectResult.ts new file mode 100644 index 00000000..8f9579c0 --- /dev/null +++ b/frontend/src/components/settings/calendarConnectResult.ts @@ -0,0 +1,175 @@ +/** + * What the Calendars card says when a calendar sign-in comes back. + * + * The OAuth callback redirects to Settings > Integrations with `calendar` (the + * outcome), `provider` and, for a failure the provider explained, `reason`: the + * provider's own error code, such as Google's `notACalendarUser` or Graph's + * `ErrorAccessDenied`. Known codes get a plain-words explanation; any code is + * also shown as given, so an administrator can look it up. + */ + +import type { CalendarProvider } from "@/types"; + +export interface CalendarConnectFailure { + title: string; + message: string; + hint?: string; + /** "Reason given by Google: forbidden", when the provider gave a code. */ + reasonText?: string; +} + +type ReasonKind = + | "calendar-off" + | "api-disabled" + | "scope-missing" + | "refused" + | "admin-blocked" + | "no-mailbox" + | "credentials" + | "expired"; + +const PROVIDER_NAMES: Record = { + google: "Google", + microsoft: "Microsoft", +}; + +export const CALENDAR_CONNECT_LABELS: Record = { + google: "Google Calendar", + microsoft: "Outlook Calendar", +}; + +function isCalendarProvider(value: string): value is CalendarProvider { + return value === "google" || value === "microsoft"; +} + +// The shape the API lets through (PROVIDER_REASON_PATTERN in +// backend/services/calendar_service/provider_errors.py). The page is reachable +// by any link, so a value of another shape is not shown. +const PROVIDER_REASON = /^[A-Za-z][A-Za-z0-9_.-]{0,63}$/; + +// Keyed in lower case: Google's reasons are camelCase, Graph's codes PascalCase +// and OAuth's snake_case, and a provider is not always consistent about it. +const REASON_KINDS: Partial> = { + notacalendaruser: "calendar-off", + accessnotconfigured: "api-disabled", + service_disabled: "api-disabled", + insufficientpermissions: "scope-missing", + access_token_scope_insufficient: "scope-missing", + forbidden: "refused", + permission_denied: "refused", + accessdenied: "refused", + erroraccessdenied: "refused", + authorization_requestdenied: "refused", + admin_policy_enforced: "admin-blocked", + mailboxnotenabledforrestapi: "no-mailbox", + invalid_client: "credentials", + unauthorized_client: "credentials", + invalid_grant: "expired", +}; + +const GOOGLE_WORKSPACE_HINT = + "If this is a Google Workspace account, an administrator may have turned Calendar off for you, or restricted third-party apps' access to the Calendar API, in the Google Admin console."; + +const MICROSOFT_ORGANISATION_HINT = + "In a Microsoft 365 organisation, an administrator may need to approve the app, or a policy may be blocking it."; + +function explainReason( + kind: ReasonKind | undefined, + provider: CalendarProvider, +): Pick { + const google = provider === "google"; + const providerName = PROVIDER_NAMES[provider]; + switch (kind) { + case "calendar-off": + return { + message: "Google says this account is not set up for Google Calendar.", + hint: "If this is a Google Workspace account, an administrator can turn Calendar on for it in the Google Admin console.", + }; + case "api-disabled": + return { + message: + "The Google Calendar API is turned off in the Google Cloud project this installation signs in with.", + hint: "An administrator can enable it in Google Cloud Console, under APIs and services.", + }; + case "scope-missing": + return { + message: `${providerName} did not give Nojoin permission to read your calendars. Connect again and allow calendar access when ${providerName} asks.`, + hint: google + ? "If it keeps happening on a Google Workspace account, an administrator may be restricting third-party apps' access to the Calendar API in the Google Admin console." + : undefined, + }; + case "refused": + return { + message: `${providerName} refused Nojoin access to this account's calendars.`, + hint: google ? GOOGLE_WORKSPACE_HINT : MICROSOFT_ORGANISATION_HINT, + }; + case "admin-blocked": + return { + message: + "Your Google Workspace administrator does not allow this app to access your account.", + hint: "An administrator can allow it in the Google Admin console, under API controls.", + }; + case "no-mailbox": + return { + message: + "Microsoft says this account has no active Exchange Online mailbox, so it has no calendar Nojoin can read.", + hint: "A Microsoft 365 administrator can check the account's Exchange Online licence.", + }; + case "credentials": + return { + message: `${providerName} did not accept this installation's sign-in app credentials.`, + hint: "An administrator can check the client ID and secret in Settings > Integrations, under Advanced.", + }; + case "expired": + return { + message: "The sign-in expired before Nojoin could finish it. Connect again.", + }; + default: + return { message: `${providerName} refused the connection.` }; + } +} + +/** + * The failure to show for a callback outcome, or null for success, a + * cancelled sign-in, or no outcome at all. + */ +export function describeCalendarConnectFailure( + status: string | null, + provider: string | null, + reason: string | null, +): CalendarConnectFailure | null { + if (!status || !provider || status === "success" || status === "cancelled") { + return null; + } + + const knownProvider = isCalendarProvider(provider) ? provider : null; + const label = knownProvider ? CALENDAR_CONNECT_LABELS[knownProvider] : "your calendar"; + const title = `Could not connect ${label}`; + + if (status === "config-error") { + return { + title, + message: `${label} sign-in is not set up yet. An administrator must add this installation's app credentials first.`, + }; + } + if (status === "tenant-config-error") { + return { + title, + message: `${label} sign-in uses the tenant common, but the Microsoft app registration is still single-tenant.`, + hint: "In Entra, change Supported account types to include organisational directories and personal Microsoft accounts, or enter a specific tenant ID instead of common.", + }; + } + if (!reason || !PROVIDER_REASON.test(reason) || !knownProvider) { + return { + title, + message: + "Something went wrong while connecting. Try again, and if it keeps failing, an administrator can find the cause in Nojoin's logs.", + }; + } + + return { + title, + ...explainReason(REASON_KINDS[reason.toLowerCase()], knownProvider), + reasonText: `Reason given by ${PROVIDER_NAMES[knownProvider]}: ${reason}`, + }; +} diff --git a/pyproject.toml b/pyproject.toml index 1269ae53..72959b6a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,6 +55,7 @@ files = [ "backend/storage_hygiene.py", "backend/core/backup/restore/legacy_owners.py", "backend/services/exports", + "backend/services/calendar_service/provider_errors.py", # API-facing schema and ORM models (public/cross-module surface). "backend/models/base.py", "backend/models/recording.py",