Skip to content

fix(source-salesloft): use native OAuth authenticator so token refresh works - #84300

Draft
devin-ai-integration[bot] wants to merge 2 commits into
masterfrom
devin/1786545486-salesloft-native-oauth
Draft

fix(source-salesloft): use native OAuth authenticator so token refresh works#84300
devin-ai-integration[bot] wants to merge 2 commits into
masterfrom
devin/1786545486-salesloft-native-oauth

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

What

OAuth-configured source-salesloft sources work while the configured access token is valid and fail on the first token refresh (both check and long syncs) with:

AttributeError: 'SingleUseOauth2Authenticator' object has no attribute '_token_refresh_endpoint'
Exception: Error while refreshing access token: 'SingleUseOauth2Authenticator' object has no attribute '_token_refresh_endpoint'

Resolves https://github.com/airbytehq/oncall/issues/13289:

Community report: #84297

Requested by aaronsteers via the /ai-fix workflow.

How

The connector shipped a custom component:

@dataclass
class SingleUseOauth2Authenticator(DeclarativeSingleUseRefreshTokenOauth2Authenticator):
    config: Config

    def __post_init__(self):
        self._connector_config = self.config
        self._token_expiry_date_config_path = "credentials/token_expiry_date"
        self.token_refresh_endpoint = "https://accounts.salesloft.com/oauth/token"
        self._access_token_config_path = "credentials/access_token"

The CDK parent is not a dataclass, so the dataclass-generated __init__(self, config) shadows the CDK constructor and super().__init__() is never called — _token_refresh_endpoint, _client_id, _client_secret, _refresh_token and the expiry/grant-type attributes never exist on the instance. ModelToComponentFactory.create_custom_component filters manifest kwargs through get_type_hints, so the manifest's token_refresh_endpoint/client_id/client_secret/refresh_token/grant_type were silently dropped and the component was built with config only. While the configured access token is unexpired, access_token is read straight from config and the refresh path is never entered — hence "works, then breaks". (The two config paths were also "a/b" strings where the CDK expects a Sequence[str].)

Rather than hand-setting more private CDK attributes, the oauth2.0 branch of the existing SelectiveAuthenticator now uses the CDK's native OAuthAuthenticator with a refresh_token_updater, and components.py is deleted:

oauth2.0:
  type: OAuthAuthenticator
  token_refresh_endpoint: https://accounts.salesloft.com/oauth/token
  grant_type: refresh_token
  refresh_token_updater:
    refresh_token_name: refresh_token
    access_token_config_path: [credentials, access_token]
    refresh_token_config_path: [credentials, refresh_token]
    token_expiry_date_config_path: [credentials, token_expiry_date]

Salesloft revokes all previous refresh tokens on each refresh (docs), so the write-back behaviour of refresh_token_updater is required, not optional — a plain OAuthAuthenticator would break on the second refresh. refresh_token_updater is available in the CDK version behind the connector's base image (5.15.0), which the manifest already declares.

The api_key (BearerAuthenticator) branch is untouched. The spec already contains credentials.access_token, credentials.refresh_token and credentials.token_expiry_date, so no spec change was needed and this is not a breaking change (no schema, PK, cursor, stream or state change) — patch bump to 1.5.3.

Not in scope: the base image is still source-declarative-manifest:5.15.0 (Oct 2024). Bumping it is independent of this fix; one upside would be that current CDK reports this refresh failure class as a classified AirbyteTracedException instead of a bare Exception.

Declarative-First Evaluation

Used the fully declarative path: the CDK's built-in OAuthAuthenticator + RefreshTokenUpdater covers everything the custom component was trying to do (refresh against Salesloft's token endpoint and persist the rotated access/refresh token and expiry back into the config), so no custom Python component is needed. Same pattern as source-gong, source-airtable, source-gitlab, source-pinterest and source-quickbooks.

Test Coverage

Added unit_tests/ (the connector had none), modelled on source-gong's manifest-only test setup. unit_tests/test_oauth_refresh.py builds the source from manifest.yaml with an OAuth config whose token_expiry_date is in the past, mocks the token endpoint, and asserts that the refresh succeeds, that the outgoing request hits https://accounts.salesloft.com/oauth/token with the exact expected form body (client_id, client_secret, refresh_token, grant_type=refresh_token), and that the rotated access token, refresh token and expiry are written back into the config. A second test asserts the api_key branch still resolves to the configured bearer token.

Verified the behavioral test fails on the pre-fix manifest with the reported AttributeError, and passes after. The failure was also reproduced standalone on airbyte-cdk==5.15.0 by instantiating the shipped component the way the factory does (config only) — it raises before any HTTP call, so no credentials are required.

test_oauth_refresh.py ..                                                 [100%]
2 passed

Not verified: an end-to-end live refresh against Salesloft (needs the OAuth test credentials in GSM plus waiting out the 2-hour access-token lifetime).

Review guide

  1. airbyte-integrations/connectors/source-salesloft/manifest.yaml — the authenticator swap
  2. airbyte-integrations/connectors/source-salesloft/components.py — deleted
  3. airbyte-integrations/connectors/source-salesloft/unit_tests/test_oauth_refresh.py
  4. metadata.yaml / docs/integrations/sources/salesloft.md — version + changelog

User Impact

OAuth-configured Salesloft sources can survive access-token expiry: check and syncs no longer fail once the access token needs refreshing, and the rotated refresh token is persisted. No user action or reconfiguration required. API-key users are unaffected.

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Link to Devin session: https://app.devin.ai/sessions/5c1434fece8b479497fe709c8c0547ce

Co-Authored-By: bot_apk <apk@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

Copy link
Copy Markdown
Contributor

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

💡 Show Tips and Tricks

PR Slash Commands

Airbyte Maintainers (that's you!) can execute the following slash commands on your PR:

  • 🛠️ Quick Fixes
    • /format-fix - Fixes most formatting issues.
    • /bump-version - Bumps connector versions, scraping changelog description from the PR title.
      • Bump types: patch (default), minor, major, major_rc, rc, promote.
      • The rc type is a smart default: applies minor_rc if stable, or bumps the RC number if already RC.
      • The promote type strips the RC suffix to finalize a release.
      • Example: /bump-version type=rc or /bump-version type=minor
    • /bump-progressive-rollout-version - Alias for /bump-version type=rc. Bumps with an RC suffix and enables progressive rollout.
  • ❇️ AI Testing and Review (internal link: AI-SDLC Docs):
    • /ai-prove-fix - Runs prerelease readiness checks, including testing against customer connections.
    • /ai-canary-prerelease - Rolls out prerelease to 5-10 connections for canary testing.
    • /ai-review - AI-powered PR review for connector safety and quality gates.
  • 📝 AI Documentation:
    • /ai-docs-review - AI-powered documentation review for PRs with connector changes.
    • /ai-create-docs-pr - Creates a documentation PR for connector changes, stacked on the current PR.
  • 🚀 Connector Releases:
    • /publish-connectors-prerelease - Publishes pre-release connector builds (tagged as {version}-preview.{git-sha}) for all modified connectors in the PR.
    • /enable-autopilot-rollouts - Enables autopilot progressive rollouts for the modified connector(s) in the PR, remediating "autopilot rollouts not enabled for {connector-name}" auto-merge blockers. Sets defaultRolloutMode: autopilot and enableProgressiveRollout: true, preserving any existing autopilotConfig.
      • Optional args: connector=<CONNECTOR_NAME> (defaults to the modified connectors in the PR), strategy=fast|slow|default (defaults to fast).
      • Example: /enable-autopilot-rollouts or /enable-autopilot-rollouts connector=source-faker strategy=slow
  • ☕️ JVM connectors:
    • /update-connector-cdk-version connector=<CONNECTOR_NAME> - Updates the specified connector to the latest CDK version.
      Example: /update-connector-cdk-version connector=destination-bigquery
  • 🐍 Python connectors:
    • /poe connector source-example lock - Run the Poe lock task on the source-example connector, committing the results back to the branch.
    • /poe source example lock - Alias for /poe connector source-example lock.
    • /poe source example use-cdk-branch my/branch - Pin the source-example CDK reference to the branch name specified.
    • /poe source example use-cdk-latest - Update the source-example CDK dependency to the latest available version.
  • ⚙️ Admin commands:
    • /force-merge reason="<REASON>" - Force merges the PR using admin privileges, bypassing CI checks. Requires a reason.
      Example: /force-merge reason="CI is flaky, tests pass locally"
📚 Show Repo Guidance

Helpful Resources

📝 Edit this welcome message.

@github-actions

Copy link
Copy Markdown
Contributor

Note

Autopilot progressive rollouts are not enabled for the following modified connector(s):

  • source-salesloft

This is a courtesy heads-up only — it does not block merge or fail any check.
To enable automatic progressive rollouts for the connector(s) above, comment
/enable-autopilot-rollouts on this PR. This sets defaultRolloutMode: autopilot
and enableProgressiveRollout: true in each connector's metadata.yaml,
preserving any existing autopilotConfig.

Co-Authored-By: bot_apk <apk@cognition.ai>
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

source-salesloft Connector Test Results

16 tests   9 ✅  1m 4s ⏱️
 2 suites  3 💤
 2 files    4 ❌

For more details on these failures, see this check.

Results for commit a3ff3d8.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Deploy preview for airbyte-docs ready!

Project:airbyte-docs
Status: ✅  Deploy successful!
Preview URL:https://airbyte-docs-jup0qks3c-airbyte-growth.vercel.app
Latest Commit:a3ff3d8

Deployed with vercel-action

@airbyte-support-bot

Copy link
Copy Markdown
Contributor

CI status: the two red connector checks look independent of this change

New Salesloft unit tests pass in CI (2 passed), along with lint, format, docs, metadata/changelog QA and the manifest spec test. The Test source-salesloft Connector failures break down into two pre-existing problems, neither of which this diff touches:

1. 401 Invalid Bearer token on the live/acceptance reads — stale test credentials in GSM. Fetched both secrets directly and checked them against the API outside of any connector code:

  • SECRET_SOURCE-SALESLOFT__CREDS (the api_key config the acceptance tests use, and the auth branch this PR does not modify): GET https://api.salesloft.com/v2/accounts401 {"error":"Invalid Bearer token"}
  • SECRET_SOURCE-SALESLOFT_OAUTH__CREDS: token_expiry_date is 2024-06-04, and its stored access token also returns 401 Invalid Bearer token

No request to https://accounts.salesloft.com/oauth/token appears anywhere in the CI log, so the OAuth refresh path was never exercised. These credentials need rotating before the connector's live tests can pass, on this branch or on master. I deliberately did not exercise the stored refresh token: Salesloft revokes the old refresh token on every refresh and I only have read access to GSM, so consuming it would invalidate the stored secret.

2. docker run ... spec exit 125 — the pinned base image has no airbyte user. Reproduced locally on this branch and on a clean origin/master worktree of the connector, with identical generated Dockerfiles:

$ docker run --rm airbyte/source-salesloft:<tag> spec
docker: Error response from daemon: unable to find user airbyte: no matching entries in passwd file.
exit status 125

The generated Dockerfile ends with USER airbyte, but the pinned base image source-declarative-manifest:5.15.0@sha256:09a84e06… has no such passwd entry (docker inspect shows user="", and id inside the image as root shows no airbyte user). It fails before Python starts, and --user 0 runs fine on both images:

$ docker run --rm --user 0 airbyte/source-salesloft:<tag> spec
{"type":"SPEC","spec":{...}}

Presence or absence of components.py makes no difference — the master image still contains it and fails the same way. This is a good argument for bumping the connector off the Oct-2024 5.15.0 base image, but I've kept that out of this PR since it is independent of the refresh fix; happy to do it here or in a follow-up if a maintainer prefers.


Devin session

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants