mirror of
https://github.com/onyx-dot-app/onyx.git
synced 2026-10-23 11:53:08 -05:00
Evan LohnandClaude Opus 5.5
d400ba5637
## Description
Eleventh backend PR for the connector checks redesign, stacked on
#16201. SharePoint follows the design's rows, copy and states. It uses
the shared Microsoft sign-in and Graph error rows from #16201.
**SharePoint**
- **Rows:**
- The 15 checks are the design's 15 rows, with its names: "Check
selected content", "List sites", "Find selected sites", "Find selected
libraries", "Sample documents", "Sign in with certificate", "Read site
permissions", "Read nested groups" and so on.
- Dependencies follow the design's run order. Two changes from the
design:
- "Sample documents" also runs after "Find selected sites". "Find
selected libraries" does not apply when no site URL names a library, so
without this a missing site would not stop the sample.
- "Read group members" and "Read nested groups" also run after "Sign in
with certificate". Both start from a site's SharePoint REST permissions,
which a client secret cannot read.
- **Copy:**
- Every failure gives the design's line and description. Lines name the
site, library, folder or group, cut to fit 40 characters. Descriptions
name the grant, for example "Grant `Sites.FullControl.All` (SharePoint
API, application) to the app registration and admin-consent it."
- The URL rules give their own rows: "Not a site URL", "Site URL not
allowed", "URL names no site" and "Site is on another host".
- Passing checks give what they found: "Indexing documents, site pages",
"{n} sites visible", "{n} selected sites found", "Read a file in
{library}", "Read a page on {site}", "Certificate valid until
YYYY/MM/DD", "Read members of {group}", "Read groups inside {group}".
- **Partial finds:** "Find selected sites" and "Find selected
libraries".
- Some missing or refused: Incomplete, for example "Site not found: ops"
or "2 of 3 sites not found". The indexing checks after them run on the
found sites and libraries.
- None found: Failed ("No selected site found", "No selected library
found").
- More than 5: Successful with a caveat ("First 5 of 7 sites found").
- **Honest passes.** These checks passed while testing nothing:
- No file in the sampled libraries: Incomplete ("No files found to
test"). Library permissions read but no file: Incomplete.
- No library, site page, site group or Entra group in scope: Skipped.
- A sample file that went away during the permission read: Successful
with a caveat.
- A sample file or page that went away during the read: Incomplete.
- **Empty scope.** Every selected site excluded, or every listed site
excluded, fails the site check.
**Fixes from the design's review**
- **REST token.** A SharePoint REST 401 gives "No token for the
SharePoint host".
- **Certificate.** "Sign in with certificate" gives the certificate's
end date, read from the .pfx with no call (a new gateway operation,
`get_certificate_not_after`).
- **Typed URL errors.** `connector_utils.validate_site_url` raises three
subclasses of `ConnectorValidationError` (format, blocked, host), so the
check can give each its row. Indexing sees the same base class.
- **Library metadata.** A library without traversal metadata raises
`DriveMetadataError` (a `ValueError`), so it is not taken for an
ambiguous name.
**Not in this PR:**
- "Tenant host unknown": the gateway raises the root-site failure as a
plain `ConnectorValidationError`, with no row.
- The permission checks do not run after "Find selected sites" (the
design's order). They read the configured sites themselves, so a missing
site fails their own read.
## How Has This Been Tested?
- New unit tests (`test_sharepoint_check_row_text.py`): the 15 names,
the pass lines, the URL rule rows, the empty scope, the partial site
find (and that the sample after it reads only the found site), none
found, the 5-site caveat, the sample states, the grant lines, the REST
401, the client-secret stop and the rate-limit stop.
- Existing SharePoint tests updated to the new states.
- Connector, server and EE unit tests (top of the Microsoft stack): 5599
passed, 1 failed (`web/test_browser_lifecycle.py`, a web connector test
that also fails without this change), 25 skipped. `ty` and pre-commit
are clean.
- The live tests (`test_sharepoint_capability_checks_live.py`) are
updated: a missing library now fails "Find selected libraries". I did
not run them: the test tenant's secrets are in AWS Secrets Manager, and
this session has no AWS access.
## Additional Options
- [ ] [Optional] Please cherry-pick this PR to the latest release
version.
- [x] [Optional] Override Linear Check
---
_Generated by [Claude
Code](https://claude.ai/code/session_01KFMazQbuG8ui3jxBxm47sC)_
<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
SharePoint's connector checks now follow the design's row names, copy,
and states, reusing the shared Microsoft sign-in and Graph error rows.
Inconclusive samples now pass perm-sync validation, and a locked site
gets its own row. Not included: "Tenant host unknown" still surfaces as
a plain validation error, and the permission checks read the configured
sites themselves rather than running after "Find selected sites".
**Behavior changes**
- The 15 checks match the design's names and run order, with two
deviations: "Sample documents" also runs after "Find selected sites",
and the Entra group checks also run after "Sign in with certificate"
since both read SharePoint REST permissions a client secret cannot.
- "Find selected sites" and "Find selected libraries" are partial finds:
some missing reports INCOMPLETE (later checks run on what was found),
none found fails, and more than 5 found passes with a caveat.
- Checks that tested nothing now say so: no file in scope is INCOMPLETE
(such a sample passes perm-sync validation), nothing in scope is
SKIPPED, a file that vanished passes with a caveat, and an empty scope
fails when "List sites" reads the whole listing (a page-cap-stopped
listing is inconclusive).
- Every failure names the site, library, folder, or group (trimmed to 40
characters) and the grant needed.
- URL rules each get their own row, a locked site gets a "Site is
locked" row, and passing checks report what they found.
**Underlying changes**
- New gateway operation `get_certificate_not_after` reads the
certificate's end date from the .pfx without calling Graph.
- `validate_site_url` now raises typed subclasses of
`ConnectorValidationError` (format, blocked, host) so each URL rule maps
to its own row.
- A library without traversal metadata raises `DriveMetadataError`, no
longer mistaken for an ambiguous name, and the site listing now reports
whether a page cap stopped it early.
<sup>Written for commit 9623124bc3.
Summary will update on new commits.</sup>
<a
href="https://cubic.dev/pr/onyx-dot-app/onyx/pull/16202?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="View guided diff"
src="https://www.cubic.dev/buttons/review-in-cubic-light.svg"></picture></a>
<a
href="https://www.cubic.dev/action/auto-fix/pr/onyx-dot-app/onyx/16202?returnTo=https%3A%2F%2Fgithub.com%2Fonyx-dot-app%2Fonyx%2Fpull%2F16202&source=description"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"><img
alt="Turn on auto-fix"
src="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- SharePoint connection checks now provide clearer results for partial,
missing, inaccessible, invalid, or unavailable sites, libraries, files,
pages, folders, and groups.
- Added targeted guidance for missing Microsoft Entra group permissions.
- SharePoint site discovery now reports whether results are complete.
- Certificate-based authentication checks now display certificate
expiration information.
- **Bug Fixes**
- Improved handling of incomplete permission checks and missing Graph
metadata.
- SharePoint URL validation now reports more specific errors for
malformed, blocked, or incorrect tenant URLs.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
