Skip to content

feat(providers): add Slack example profile with rotating user tokens - #3730

Open
n1hility wants to merge 3 commits into
NVIDIA:mainfrom
n1hility:3729-slack-rotating-provider/jg
Open

n1hility wants to merge 3 commits into
NVIDIA:mainfrom
n1hility:3729-slack-rotating-provider/jg

Conversation

@n1hility

@n1hility n1hility commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds a Slack example provider profile whose user token is kept fresh by the gateway's oauth2_refresh_token engine, and fixes the one thing that made Slack unusable with that engine: Slack's token endpoint reports failures with HTTP 200 and an error body, which the gateway treated as a malformed success and retried every minute forever instead of parking the grant for reauthorization.

Related Issue

Fixes #3729 (filed alongside this PR; in triage). If maintainers prefer, the first commit (fix(server)) stands on its own as a localized bug fix and can be split out.

Changes

  • fix(server): when a 2xx token response does not parse as a token but does parse as an OAuth error response, route it through the existing error classifier instead of recording oauth_invalid_success_response. Slack's error names are folded onto the existing RFC failure codes with the issuer name preserved as provider_error_subtype, following the invalid_rapt / admin_policy_enforced precedent: invalid_refresh_token, token_revoked, token_expired, account_inactive, invalid_auth map to oauth_invalid_grant (parked for user grants, configuration repair for non-interactive grants); invalid_client_id, bad_client_secret to oauth_invalid_client; invalid_grant_type to oauth_unsupported_grant_type; ratelimited, accesslimited, request_timeout, service_unavailable, internal_error, fatal_error to oauth_token_endpoint_retryable. Of these, invalid_refresh_token, the client errors and the transient names are documented for oauth.v2.access; token_revoked, token_expired, account_inactive and invalid_auth are Slack's general authentication errors and are included defensively.
    • The issuer table is consulted only for errors that arrived in a 2xx envelope. This is deliberate: RFC 6749 error responses (4xx) classify byte-for-byte as before, so the change is confined to a path that today is already a permanent failure. None of the issuer names collide with RFC names, so the gate can be lifted later if an issuer is found to send these names on 4xx.
    • Successful token bodies are handled exactly as before; a 2xx body with neither a token nor an error still reports oauth_invalid_success_response; no provider-controlled text reaches the status message.
  • feat(providers): providers/slack.yaml example profile (bearer SLACK_USER_TOKEN, oauth2_refresh_token against oauth.v2.access, 43200 s lifetime cap matching Slack's fixed token lifetime, 21600 s lead, client_id / client_secret / refresh_token material) with the standard example header, plus docs/how-it-works/providers/slack.mdx and a nav entry. Also tightens ProviderTypeProfile::adc_credential to require the Google token endpoint alongside the three ADC material keys, so --from-gcloud-adc does not accept the Slack profile just because its material has the same shape (happy to split this into its own commit if preferred).
  • fix(providers) (follow-up after self-review): the example grants read-only access. Slack's read methods accept GET and the token travels in the injected header, so the read-only preset covers reads and keeps write methods denied until a policy allows them, matching the GitHub example. Docs offer the raw-URL import form used by the sibling pages.

Out of scope on purpose: openshell-core/src/oauth.rs (extension token-grant / RFC 8693 exchange) has the same status-only success check and is untouched.

Testing

Run on this branch (rebased on main at b52eed7):

  • cargo test -p openshell-server --lib provider_refresh — 60 passed. New tests: issuer error names map by grant kind; client/transient errors keep RFC failure codes; issuer names on a 400 stay unrecognized (4xx behaviour unchanged); a 200 error body parks the state as reauthorization_required; a 200 body with neither token nor error still reports oauth_invalid_success_response; a Slack-shaped success body mints with a 12 h lifetime and persists the rotated refresh token.
  • cargo test -p openshell-server --lib profile — 132 passed (example catalog list updated).
  • cargo test -p openshell-providers — 135 passed (new slack_profile_declares_rotating_user_token; the example catalog validates with the new file).
  • cargo test -p openshell-cli adc — the --from-gcloud-adc tests pass with the tightened detection.
  • cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings, scripts/update_license_headers.py --check (1151 files), and markdownlint-cli2 (152 files, 0 issues) all clean. mise is not installed on the development machine, so these pre-commit sub-tasks were run directly.
  • Verified against a live Slack app with token rotation enabled on a gateway built from this change: connect-time and scheduled refreshes succeed and the stable placeholder swaps under a running process; after uninstalling the app, a forced refresh parked the state as reauthorization_required with subtype invalid_refresh_token.
  • mise run pre-commit equivalents pass (see above)
  • Unit tests added/updated
  • E2E tests added/updated (not applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (not applicable; user docs added under how-it-works/providers)

RFC 6749 section 5.2 puts token endpoint errors on 4xx responses, but
some issuers answer HTTP 200 with an `error` body. Slack's oauth.v2.access
does this for every failure, so a rotated-out or revoked refresh token
left the refresh state in a permanent `oauth_invalid_success_response`
loop: retried every minute, never parked, and never surfaced to the
operator as a reauthorization requirement.

When a 2xx body does not parse as a token response but does parse as an
OAuth error response, route it through the existing error classifier.
Bodies that carry a token are handled exactly as before, and bodies with
neither a token nor an `error` field still report
`oauth_invalid_success_response`.

Fold the Slack token endpoint error vocabulary onto the closest RFC
failure codes so status consumers keep a stable vocabulary, preserving
the issuer's own name as the provider error subtype in the same way the
Google `invalid_rapt` and `admin_policy_enforced` cases already do. The
issuer table is consulted only for errors that arrived in a 2xx
envelope, so RFC 6749 error responses classify exactly as before.
Terminal outcomes stay gated on the grant kind: user refresh grants park
for reauthorization, non-interactive grants fall back to configuration
repair.

Refs NVIDIA#3729

Signed-off-by: Jason T. Greene <jason.greene@redhat.com>
Add `providers/slack.yaml`, an example profile for Slack apps that have
token rotation enabled. Slack issues user access tokens that expire
after 43200 seconds together with a single-use refresh token, and
requires the app client ID and client secret on every refresh, so the
profile declares an `oauth2_refresh_token` credential against
oauth.v2.access with a matching 43200-second lifetime cap, a
21600-second refresh lead, and client_id / client_secret / refresh_token
material. Slack Web API methods are POST even for reads, so the endpoint
uses the read-write method preset; deployers add `rules` for the methods
their workloads need.

Tighten `ProviderTypeProfile::adc_credential` to require the Google
token endpoint in addition to the three ADC material keys, so that
`--from-gcloud-adc` keeps refusing profiles whose refresh material
merely has the same shape.

Document the profile under how-it-works/providers.

Refs NVIDIA#3729

Signed-off-by: Jason T. Greene <jason.greene@redhat.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 26, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Slack's read methods accept GET with query-string arguments, and the
token travels in the injected authorization header rather than as a
query parameter, so the read-only method preset covers reads. Grant
read-only access in the example, matching the posture of the GitHub
example, and state that write methods such as chat.postMessage need an
explicit rule or a read-write copy. Drop the incorrect claim that Slack
reads require POST from the profile header and the docs.

Offer the raw-URL import form used by the sibling provider pages, and
describe where the Slack error name surfaces (the provider_error_subtype
field of the refresh status API, echoed in LAST_ERROR).

Refs NVIDIA#3729

Signed-off-by: Jason T. Greene <jason.greene@redhat.com>
@n1hility
n1hility marked this pull request as draft September 26, 2026 01:21
@n1hility
n1hility marked this pull request as ready for review September 26, 2026 03:07
n1hility added a commit to rh-forge/openshell that referenced this pull request Sep 26, 2026
Slack's read methods accept GET with query-string arguments, and the
token travels in the injected authorization header rather than as a
query parameter, so the read-only method preset covers reads. Grant
read-only access in the built-in profile and state that write methods
such as chat.postMessage need an explicit rule or a read-write copy.
Drop the incorrect claim that Slack reads require POST. Describe where
the Slack error name surfaces (the provider_error_subtype field of the
refresh status API, echoed in LAST_ERROR).

Mirrors the follow-up on the upstream submission (NVIDIA#3730).

Signed-off-by: Jason T. Greene <jason.greene@redhat.com>
n1hility added a commit to rh-forge/openshell that referenced this pull request Sep 26, 2026
Slack's read methods accept GET with query-string arguments, and the
token travels in the injected authorization header rather than as a
query parameter, so the read-only method preset covers reads. Grant
read-only access in the built-in profile and state that write methods
such as chat.postMessage need an explicit rule or a read-write copy.
Drop the incorrect claim that Slack reads require POST. Describe where
the Slack error name surfaces (the provider_error_subtype field of the
refresh status API, echoed in LAST_ERROR).

Mirrors the follow-up on the upstream submission (NVIDIA#3730).

Signed-off-by: Jason T. Greene <jason.greene@redhat.com>

This branch has not been deployed

No deployments
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.

feat(providers): add Slack provider profile with gateway-managed rotating user tokens

1 participant