Skip to content

Conversation

evan-onyx
Copy link
Contributor

@evan-onyx evan-onyx commented Jun 27, 2025

Description

Addresses https://linear.app/danswer/issue/DAN-2149/permission-sync-validation-framework
Adds a new validation function to connectors that connector implementers can implement. The validation function is called for all CC pairs set up to use perm sync

How Has This Been Tested?

n/a

Backporting (check the box to trigger backport action)

Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.

  • This PR should be backported (make sure to check that the backport attempt succeeds)
  • [Optional] Override Linear Check

Copy link

vercel bot commented Jun 27, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
internal-search ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 28, 2025 1:01am

@evan-onyx evan-onyx marked this pull request as ready for review June 27, 2025 20:02
@evan-onyx evan-onyx requested a review from a team as a code owner June 27, 2025 20:02
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

Implements a new permission sync validation framework that enables connectors to validate their permission sync capabilities during CC pair setup and credential swapping.

  • Added validate_perm_sync method to BaseConnector in backend/onyx/connectors/interfaces.py allowing connectors to implement custom validation logic
  • Added access_type parameter to credential-related functions across the stack to enforce proper permission validation during CC pair creation and modification
  • Modified CredentialSwapRequest model in backend/onyx/server/documents/models.py to include required access_type field for validation
  • Updated frontend components to properly pass and handle accessType during credential operations, maintaining type safety
  • Standardized permission validation across credential creation, modification, and swapping flows

11 files reviewed, 2 comments
Edit PR Review Bot Settings | Greptile

Comment on lines 953 to 959
validate_ccpair_for_user(
connector_id=connector_id,
credential_id=credential_id,
access_type=connector_data.access_type,
db_session=db_session,
)
response = add_credential_to_connector(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: New access_type validation might throw ConnectorValidationError - add error handling here similar to line 990

Comment on lines 278 to 280
if (onSwap && attachedConnector) {
onSwap(selectedCredential!, attachedConnector.id);
onSwap(selectedCredential!, attachedConnector.id, accessType);
if (close) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: non-null assertion (!) used here could be replaced with a proper type guard to ensure type safety

Copy link
Contributor

@Weves Weves left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me!

@Weves Weves added this pull request to the merge queue Jun 28, 2025
Merged via the queue into main with commit be25b1e Jun 28, 2025
13 of 14 checks passed
@Weves Weves deleted the feat/perm-sync-valid-framework branch June 28, 2025 21:27
AnkitTukatek pushed a commit to TukaTek/onyx that referenced this pull request Sep 23, 2025
* perm synce validation framework

* frontend fixes

* validate perm sync when getting runner

* attempt to fix integration tests

* added new file

* oops

* skipping salesforce test due to creds

* add todo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants