fix(connectors): normalize shared credential site URLs and report the use rule's refusal

A user who could see a shared credential was refused unless the bound values
equalled a pair's exactly, so https://x.atlassian.net/wiki did not match a pair
saved as https://x.atlassian.net/ although both index the same site. The
binding check also skipped the use rule, so it passed a form that the run then
refused with a 403, and the account list offered accounts the rule refuses
(ENG-4380).

- CredentialBinding.comparison_values gives the values the rule compares.
  site_url_fields compare by site_url_key (scheme and host in lower case, no
  trailing slash); Confluence Cloud also drops a trailing /wiki. Stored
  configs are unchanged.
- credential_use_refusal returns the rule's refusal without raising, and
  assert_user_can_use_credential raises it, so one function decides.
- The binding checks apply the rule before decrypting the credential and
  report a refusal as a shared_credential_settings rejection with its detail.
- /manage/admin/similar-credentials/{source} adds usable and refusal_reason
  per credential, from the rule with no config.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Evan LohnandClaude Opus 5.5 committed 2026-10-22 12:21:40 -07:00
1 parent d440e4ced2
commit 63fcb62f20
12 files changed
+264 -48

No files matched your search

@@ -85,7 +85,7 @@ deletion units.
| DELETE | `/manage/admin/credential/{id}` | `delete_credential_by_id_admin` | |
| DELETE | `/manage/credential/{id}`, `/manage/credential/force/{id}` | | Owner delete, and delete that also removes the pairs that use the credential. |
| PUT | `/manage/admin/credential/swap` | `swap_credentials_for_connector` | Swaps the credential a cc-pair uses. |
| GET | `/manage/admin/credential`, `/manage/admin/similar-credentials/{source}` | | Credentials the caller can use (§4.7), masked. Each snapshot carries `groups` and `is_owner`. |
| GET | `/manage/admin/credential`, `/manage/admin/similar-credentials/{source}` | | Credentials the caller can use (§4.7), masked. Each snapshot carries `groups` and `is_owner`; a similar-credentials snapshot also carries `usages`, `usable` and `refusal_reason` ([[connectors]] §4.9). |
| GET, POST | `/connector/oauth/*` | `server/documents/standard_oauth.py` | Connector-side OAuth authorization flow. |
| PUT | `/manage/connector/{connector_id}/credential/{credential_id}` | `associate_credential_to_connector` | `server/documents/cc_pair.py`. Creates the cc-pair. Takes `ConnectorCredentialPairMetadata`, including manage-access groups and data-access groups. |
| DELETE | `/manage/connector/{connector_id}/credential/{credential_id}` | `dissociate_credential_from_connector` | Hard-deletes the pairing row directly (no background cleanup); used when the pairing has no indexed documents yet. |
+11 -3
View File
@@ -691,9 +691,12 @@ applies it before it builds a connector. A user may use the credential when:
- the credential is visible and its source has no real secret
(`CREDENTIAL_PERMISSIONS_TO_IGNORE`).
The comparison skips a binding's `credential_derived_fields`: values the form
takes from the credential and `validate_credential` pins to it, such as Google
Drive's `credential_kind`.
The comparison uses `CredentialBinding.comparison_values`. It skips a binding's
`credential_derived_fields`: values the form takes from the credential and
`validate_credential` pins to it, such as Google Drive's `credential_kind`. It
compares `site_url_fields` by `connector_config.py:site_url_key` (scheme and
host in lower case, no trailing slash), and Confluence Cloud also drops a
trailing `/wiki`. The stored config keeps the value as entered.
The pairs are the record of the bound values; the credential stores none. The
rule runs in `validate_ccpair_for_user` (its `user` is None only for system
@@ -703,6 +706,11 @@ work), `validate_connector_credential_bindings`,
runs), and `edit_plan/state.py:fetch_proposed_credential` (edit plan and
apply). A refusal is `CREDENTIAL_NOT_FOUND` when the credential is not visible
and `INSUFFICIENT_PERMISSIONS` when the bound values match no pair.
`factory.py:credential_use_refusal` returns the refusal without raising. The
binding-check endpoints report an `INSUFFICIENT_PERMISSIONS` refusal as a
`shared_credential_settings` rejection with the same detail, and
`/manage/admin/similar-credentials/{source}` sets each credential's `usable`
and `refusal_reason` from the rule with no config.
A realm (where an account works: its site, host or subdomain) lives in the
credential only, and the connector reads it from there. The connector config does
+3 -1
View File
@@ -1,4 +1,4 @@
from typing import Annotated
from typing import Annotated, ClassVar
from onyx.configs.app_configs import INDEX_BATCH_SIZE
from onyx.connectors.connector_config import ConnectorConfig, CredentialBinding
@@ -6,6 +6,8 @@ from onyx.connectors.field_policy import FieldClass, FieldPolicy
class CanvasCredentialBinding(CredentialBinding):
site_url_fields: ClassVar[frozenset[str]] = frozenset({"canvas_base_url"})
# Document ids hold only course and item ids, so the URL picks the instance.
canvas_base_url: Annotated[str, FieldPolicy(FieldClass.IDENTITY)]
+12 -1
View File
@@ -1,6 +1,6 @@
import re
from enum import Enum
from typing import Annotated, Any
from typing import Annotated, Any, ClassVar
from urllib.parse import urlparse
from pydantic import field_validator
@@ -35,6 +35,7 @@ from onyx.db.enums import HierarchyNodeType
_OAUTH_WIKI_BASE_KEY = "wiki_base"
_COSMETIC = FieldPolicy(FieldClass.COSMETIC)
_CLOUD_WIKI_PATH = "/wiki"
_SPACE = "space"
_PAGE_ID = "page_id"
_INDEX_RECURSIVELY = "index_recursively"
@@ -103,6 +104,16 @@ class ConfluenceCredentialBinding(CredentialBinding):
# Only selects the API gateway: document ids keep wiki_base.
scoped_token: Annotated[bool, _COSMETIC] = False
site_url_fields: ClassVar[frozenset[str]] = frozenset({"wiki_base"})
def comparison_values(self) -> dict[str, Any]:
# Cloud document ids add "/wiki" when the base lacks it, so a cloud
# base with and without it is one site.
values = super().comparison_values()
if self.is_cloud:
values["wiki_base"] = values["wiki_base"].removesuffix(_CLOUD_WIKI_PATH)
return values
def validate_credential(self, credential_json: dict[str, Any]) -> None:
authorized_wiki_base = credential_json.get(_OAUTH_WIKI_BASE_KEY)
if not authorized_wiki_base:
@@ -1,4 +1,5 @@
from typing import Annotated, Any, ClassVar, LiteralString
from urllib.parse import urlparse, urlunparse
from pydantic import BaseModel, ConfigDict
from pydantic_core import PydanticCustomError
@@ -10,6 +11,20 @@ from onyx.db.enums import HierarchyNodeType
_FIELD_FORMAT_ERROR_TYPE = "connector_config_format"
def site_url_key(url: str) -> str:
"""``url`` as the credential use rule compares it: the scheme and host in
lower case and no trailing slash. A URL with no scheme reads as https."""
url = url.strip()
parsed = urlparse(url if "://" in url else f"https://{url}")
return urlunparse(
parsed._replace(
scheme=parsed.scheme.lower(),
netloc=parsed.netloc.lower(),
path=parsed.path.rstrip("/"),
)
)
def field_format_error(message: LiteralString) -> PydanticCustomError:
"""The error a config field validator raises when a value breaks a format
rule. The form shows ``message`` on the field as-is, with no "Value error"
@@ -29,6 +44,18 @@ class CredentialBinding(BaseModel):
# no place to send the credential, so the credential use rule
# (``factory.assert_user_can_use_credential``) does not compare them.
credential_derived_fields: ClassVar[frozenset[str]] = frozenset()
# URL fields the use rule compares by ``site_url_key``, so spellings of
# one site match.
site_url_fields: ClassVar[frozenset[str]] = frozenset()
def comparison_values(self) -> dict[str, Any]:
"""The values the credential use rule compares with those of a pair
that uses the credential. Spellings that reach the same place compare
equal; the stored config keeps the value as entered."""
values = self.model_dump(exclude=set(self.credential_derived_fields))
for field in self.site_url_fields:
values[field] = site_url_key(values[field])
return values
def validate_credential(
self,
@@ -42,6 +69,7 @@ class CredentialBinding(BaseModel):
class BaseUrlCredentialBinding(CredentialBinding):
"""For sources whose only credential-bound field is the site URL."""
site_url_fields: ClassVar[frozenset[str]] = frozenset({"base_url"})
base_url: Annotated[str, FieldPolicy(FieldClass.IDENTITY)]
+45 -21
View File
@@ -307,20 +307,20 @@ def _bound_values(
binding_class: type[CredentialBinding] | None,
connector_specific_config: dict[str, Any],
) -> dict[str, Any] | None:
"""The config's credential-bound values as the binding model validates
them, so equal settings compare equal, without the fields derived from the
credential. None when they do not validate."""
"""The config's credential-bound values as the binding model compares them
(``CredentialBinding.comparison_values``). None when they do not
validate."""
if binding_class is None:
return {}
try:
return binding_class.model_validate(connector_specific_config).model_dump(
exclude=set(binding_class.credential_derived_fields)
)
return binding_class.model_validate(
connector_specific_config
).comparison_values()
except pydantic.ValidationError:
return None
def assert_user_can_use_credential(
def credential_use_refusal(
db_session: Session,
*,
user: User,
@@ -328,10 +328,12 @@ def assert_user_can_use_credential(
source: DocumentSource,
connector_specific_config: dict[str, Any] | None,
current_pair: ConnectorCredentialPair | None = None,
) -> None:
) -> OnyxError | None:
"""The rule for every use of a credential with a ``source`` config: a
pairing, a credential swap, an edit and every check run. Call it before
anything builds a connector with the credential.
pairing, a credential swap, an edit and every check run. Returns the error
that refuses the use, or None when the use is allowed. Uses go through
``assert_user_can_use_credential``; the binding check and the account list
report the refusal instead.
The use is allowed when one of these is true:
@@ -353,10 +355,9 @@ def assert_user_can_use_credential(
``validate_credential_binding`` is a different check: it compares the
values with what the credential itself records.
Raises:
OnyxError: CREDENTIAL_NOT_FOUND when the user cannot see the
credential. INSUFFICIENT_PERMISSIONS when the bound values match
no pair.
The refusal is CREDENTIAL_NOT_FOUND when the user cannot see the
credential and INSUFFICIENT_PERMISSIONS when the bound values match no
pair.
"""
binding_class = _credential_binding_class(source)
@@ -374,7 +375,7 @@ def assert_user_can_use_credential(
and current_pair.connector.source == source
and _matches(current_pair)
):
return
return None
if (
fetch_credential_by_id_for_user(
@@ -382,7 +383,7 @@ def assert_user_can_use_credential(
)
is None
):
raise OnyxError(
return OnyxError(
OnyxErrorCode.CREDENTIAL_NOT_FOUND,
f"Credential {credential.id} does not exist or is not accessible.",
)
@@ -390,9 +391,9 @@ def assert_user_can_use_credential(
credential.user_id is None
and has_global_permission(user, Permission.MANAGE_CONNECTORS)
):
return
return None
if credential.source in CREDENTIAL_PERMISSIONS_TO_IGNORE:
return
return None
bound_fields = (
sorted(
set(binding_class.model_fields) - binding_class.credential_derived_fields
@@ -401,14 +402,14 @@ def assert_user_can_use_credential(
else []
)
if not bound_fields:
return
return None
if any(
_matches(pair)
for pair in get_cc_pairs_using_credential(db_session, credential.id, source)
):
return
return None
raise OnyxError(
return OnyxError(
OnyxErrorCode.INSUFFICIENT_PERMISSIONS,
f"Credential {credential.id} is shared with you. You can use it only "
"with the settings of a connector that already uses it "
@@ -416,6 +417,29 @@ def assert_user_can_use_credential(
)
def assert_user_can_use_credential(
db_session: Session,
*,
user: User,
credential: Credential,
source: DocumentSource,
connector_specific_config: dict[str, Any] | None,
current_pair: ConnectorCredentialPair | None = None,
) -> None:
"""Raises the refusal of ``credential_use_refusal``. Call it before
anything builds a connector with the credential."""
refusal = credential_use_refusal(
db_session,
user=user,
credential=credential,
source=source,
connector_specific_config=connector_specific_config,
current_pair=current_pair,
)
if refusal is not None:
raise refusal
class CredentialBindingFieldErrorKind(str, Enum):
MISSING = "missing"
INVALID = "invalid"
+3 -1
View File
@@ -1,4 +1,4 @@
from typing import Annotated
from typing import Annotated, ClassVar
from pydantic import field_validator
@@ -28,6 +28,8 @@ JQL_ORDER_BY_ERROR = (
class JiraCredentialBinding(CredentialBinding):
site_url_fields: ClassVar[frozenset[str]] = frozenset({"jira_base_url"})
# Document ids are issue URLs on this base URL.
jira_base_url: Annotated[str, FieldPolicy(FieldClass.IDENTITY)]
# Only selects the API gateway: document ids keep jira_base_url.
+3 -1
View File
@@ -1,4 +1,4 @@
from typing import Annotated
from typing import Annotated, ClassVar
from onyx.configs.app_configs import INDEX_BATCH_SIZE
from onyx.connectors.connector_config import ConnectorConfig, CredentialBinding
@@ -6,6 +6,8 @@ from onyx.connectors.field_policy import FieldClass, FieldPolicy
class ZulipCredentialBinding(CredentialBinding):
site_url_fields: ClassVar[frozenset[str]] = frozenset({"realm_url"})
# The connector stores it but does not use it.
realm_name: Annotated[str, FieldPolicy(FieldClass.COSMETIC)]
# Used only to build message links; the zuliprc credential picks the server.
+32 -16
View File
@@ -7,7 +7,11 @@ from sqlalchemy.orm import Session
from onyx.auth.permissions import require_permission
from onyx.auth.scoped_permissions import assert_within_scope
from onyx.configs.constants import PUBLIC_API_TAGS
from onyx.connectors.factory import parse_credential_binding, validate_ccpair_for_user
from onyx.connectors.factory import (
credential_use_refusal,
parse_credential_binding,
validate_ccpair_for_user,
)
from onyx.db.connector_credential_pair import (
get_manageable_cc_pairs_for_credentials,
verify_user_can_edit_connector,
@@ -139,7 +143,8 @@ def get_cc_source_full_info(
) -> list[SimilarCredentialSnapshot]:
"""The credentials a ``source_type`` connector can use, shown in that
source's keys. Each lists the connectors using it that the user can manage,
with their credential-bound config values, to help pick one."""
with their credential-bound config values, and says whether the user may
use it, to help pick one."""
credentials = fetch_credentials_usable_by_source_for_user(
db_session=db_session,
user=user,
@@ -168,21 +173,32 @@ def get_cc_source_full_info(
db_session, [credential.id for credential in credentials]
)
masked_ids = credential_ids_to_mask(credentials, user, db_session)
return [
SimilarCredentialSnapshot(
**dict(
CredentialSnapshot.from_credential_db_model(
credential,
mask_credential_prefix=credential.id in masked_ids,
view_source=source_type,
viewer_id=user.id,
group_ids=group_ids.get(credential.id),
)
),
usages=usages[credential.id],
snapshots: list[SimilarCredentialSnapshot] = []
for credential in credentials:
refusal = credential_use_refusal(
db_session,
user=user,
credential=credential,
source=source_type,
connector_specific_config=None,
)
for credential in credentials
]
snapshots.append(
SimilarCredentialSnapshot(
**dict(
CredentialSnapshot.from_credential_db_model(
credential,
mask_credential_prefix=credential.id in masked_ids,
view_source=source_type,
viewer_id=user.id,
group_ids=group_ids.get(credential.id),
)
),
usages=usages[credential.id],
usable=refusal is None,
refusal_reason=refusal.detail if refusal else None,
)
)
return snapshots
@router.delete("/admin/credential/{credential_id}")
@@ -46,6 +46,7 @@ from onyx.connectors.exceptions import ConnectorValidationError
from onyx.connectors.factory import (
CredentialBindingFieldError,
credential_binding_field_errors,
credential_use_refusal,
validate_connector_config,
validate_credential_binding,
)
@@ -420,7 +421,11 @@ class CredentialBindingCheckRequest(BaseModel):
class CredentialBindingRejectionCode(str, Enum):
# The credential records a different site than the bound fields.
BINDING_REJECTED = "binding_rejected"
# The credential is shared with the user, and the bound fields match no
# connector that uses it (``credential_use_refusal``).
SHARED_CREDENTIAL_SETTINGS = "shared_credential_settings"
class CredentialBindingRejection(BaseModel):
@@ -467,7 +472,7 @@ def check_credential_binding(
user=user,
db_session=db_session,
)
return _check_binding(request, credential)
return _check_binding(request, credential, user, db_session)
class DraftCredentialBindingCheckRequest(CredentialBindingCheckRequest):
@@ -504,7 +509,7 @@ def check_draft_credential_binding(
user=user,
db_session=db_session,
)
result = _check_binding(request, credential)
result = _check_binding(request, credential, user, db_session)
return DraftCredentialBindingCheckResponse(
field_errors=result.field_errors,
rejection=result.rejection,
@@ -515,12 +520,34 @@ def check_draft_credential_binding(
def _check_binding(
request: CredentialBindingCheckRequest,
credential: Credential,
user: User,
db_session: Session,
) -> CredentialBindingCheckResponse:
"""Applies the checks a run or pairing applies to the bound fields, so a
form that passes here is not refused later for them."""
field_errors = credential_binding_field_errors(
request.source, request.connector_specific_config
)
if field_errors:
return CredentialBindingCheckResponse(field_errors=field_errors, rejection=None)
# Before validate_credential_binding, which decrypts the credential.
refusal = credential_use_refusal(
db_session,
user=user,
credential=credential,
source=request.source,
connector_specific_config=request.connector_specific_config,
)
if refusal is not None:
if refusal.error_code is not OnyxErrorCode.INSUFFICIENT_PERMISSIONS:
raise refusal
return CredentialBindingCheckResponse(
field_errors={},
rejection=CredentialBindingRejection(
code=CredentialBindingRejectionCode.SHARED_CREDENTIAL_SETTINGS,
detail=refusal.detail,
),
)
try:
validate_credential_binding(
request.source, request.connector_specific_config, credential
+6
View File
@@ -273,6 +273,12 @@ class CredentialUsage(BaseModel):
class SimilarCredentialSnapshot(CredentialSnapshot):
# Only connectors the requesting user can manage.
usages: list[CredentialUsage] = Field(default_factory=list)
# Whether the requesting user may use the credential at all
# (``factory.credential_use_refusal`` with no config). A usable shared
# credential may still require the settings of a connector that uses it.
usable: bool
# The refusal's text when ``usable`` is False.
refusal_reason: str | None = None
class IndexAttemptSnapshot(BaseModel):
@@ -3,7 +3,7 @@ they are an admin), or when they can see it and either its source has no
credential-bound fields or the config keeps the credential-bound values of a
pair that already uses it. Covers the rule and
each entry point: pairing, draft runs, capability-check overrides, cc-pair
dry runs and edits."""
dry runs and edits, and the binding check and account list that report it."""
from collections.abc import Generator
from dataclasses import dataclass
@@ -38,9 +38,13 @@ from onyx.error_handling.error_codes import OnyxErrorCode
from onyx.error_handling.exceptions import OnyxError
from onyx.server.documents import capability_check_runs
from onyx.server.documents.capability_check_runs import start_cc_pair_draft_check_run
from onyx.server.documents.credential import get_cc_source_full_info
from onyx.server.documents.credential_capabilities import (
CapabilityCheckRunRequest,
CredentialBindingCheckRequest,
CredentialBindingRejectionCode,
DraftCheckRunRequest,
check_credential_binding,
start_draft_check_run,
trigger_capability_check,
)
@@ -220,6 +224,28 @@ def test_visible_user_cannot_change_the_bound_values(
)
def test_spellings_of_the_pairs_site_match(env: _Env, db_session: Session) -> None:
for wiki_base in (
f"{_SITE}/",
f"{_SITE}/wiki",
f"{_SITE}/wiki/",
"HTTPS://ACME.atlassian.net/wiki",
):
_use(db_session, env.admin, env.shared, _config(wiki_base))
for wiki_base in (
"https://acme.atlassian.net.attacker.example.com/wiki",
"http://acme.atlassian.net",
):
_refused(
OnyxErrorCode.INSUFFICIENT_PERMISSIONS,
_use,
db_session,
env.admin,
env.shared,
_config(wiki_base),
)
def test_bound_credential_without_a_pair_is_for_its_owner(
env: _Env, db_session: Session
) -> None:
@@ -533,3 +559,67 @@ def test_fields_derived_from_the_credential_are_not_compared(
source=DocumentSource.GOOGLE_DRIVE,
connector_specific_config={"credential_kind": kind},
)
# The reports of the rule
def test_binding_check_reports_the_runs_refusal(env: _Env, db_session: Session) -> None:
def check(config: dict[str, Any]) -> CredentialBindingCheckRequest:
return CredentialBindingCheckRequest(
source=_SOURCE, connector_specific_config=config
)
with pytest.raises(OnyxError) as exc:
_use(db_session, env.admin, env.shared, _config(_OTHER_SITE))
result = check_credential_binding(
env.shared.id,
check(_config(_OTHER_SITE)),
user=env.admin,
db_session=db_session,
)
assert result.field_errors == {}
assert result.rejection is not None
assert (
result.rejection.code
== CredentialBindingRejectionCode.SHARED_CREDENTIAL_SETTINGS
)
assert result.rejection.detail == exc.value.detail
for user in (env.admin, env.owner):
assert (
check_credential_binding(
env.shared.id,
check(_config(f"{_SITE}/wiki")),
user=user,
db_session=db_session,
).rejection
is None
)
def test_account_list_marks_the_accounts_the_rule_refuses(
env: _Env, db_session: Session
) -> None:
unpaired = _credential(db_session, env.owner)
db_session.commit()
def listed(user: User) -> dict[int, tuple[bool, str | None]]:
return {
snapshot.id: (snapshot.usable, snapshot.refusal_reason)
for snapshot in get_cc_source_full_info(
_SOURCE, user=user, db_session=db_session
)
if snapshot.id in (env.shared.id, unpaired.id)
}
admin_view = listed(env.admin)
assert admin_view[env.shared.id] == (True, None)
usable, reason = admin_view[unpaired.id]
assert not usable
assert reason is not None and "shared with you" in reason
assert listed(env.owner) == {
env.shared.id: (True, None),
unpaired.id: (True, None),
}