Slack provider with rotating user tokens - #1
Merged
Merged
Conversation
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. Terminal outcomes stay gated on the grant kind: user refresh grants park for reauthorization, non-interactive grants fall back to configuration repair. Signed-off-by: Jason T. Greene <jason.greene@redhat.com>
Add a built-in `slack` provider 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. Signed-off-by: Jason T. Greene <jason.greene@redhat.com>
Consult the issuer-specific error table only when the error arrived in a 2xx envelope, so RFC 6749 error responses keep the RFC-only classification they had before and the change stays confined to the branch that previously reported `oauth_invalid_success_response`. Add a regression test for an issuer-named error on a 400 response. Drop the manual `refresh rotate` step from the Slack provider docs: the worker may mint concurrently and Slack refresh tokens are single use. State that a used refresh token is invalidated without asserting a grace period. Signed-off-by: Jason T. Greene <jason.greene@redhat.com>
Signed-off-by: Jason T. Greene <jason.greene@redhat.com>
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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Carried patch for Slack token rotation on the Forge OpenShell image. Mirrors upstream NVIDIA#3730 (issue NVIDIA#3729); the fork keeps its built-in profile catalog, so the profile lands as a built-in here rather than an example.
Commits:
fix(server)classify OAuth token errors returned with HTTP 2xx (Slack'soauth.v2.accessreports failures as 200 +error; previously a permanentoauth_invalid_success_responseloop, now parked asreauthorization_requiredwith the Slack name as the subtype; issuer table gated to 2xx envelopes so 4xx classification is unchanged) ·feat(providers)built-inslackprofile (43200 s cap, 21600 s lead, client_id/client_secret/refresh_token material) + tighteradc_credential·fix(providers)profile is read-only (Slack reads accept GET) with corrected docs.Verification
Live on saw-jasongreene with a gateway built from this branch (tag v0.0.116-rhaiv.0-forge.2 was cut at 8037354; the later commits are the 2xx gate, a clippy fix and the read-only profile, so re-cut forge.3 after merge): connect-time, manual and worker-driven rotations succeeded with the read proxy serving throughout; after uninstalling the Slack app a forced refresh parked as
reauthorization_required/oauth_invalid_grant/invalid_refresh_token.Tests:
provider_refresh57,profilehandlers 118,openshell-providers131, CLI--from-gcloud-adctests; fmt and clippy clean on the touched crates.