[PureGo] Add custom auth-header hook to transport#494
Open
zlata-stefanovic-db wants to merge 6 commits into
Open
[PureGo] Add custom auth-header hook to transport#494zlata-stefanovic-db wants to merge 6 commits into
zlata-stefanovic-db wants to merge 6 commits into
Conversation
This was referenced Jul 13, 2026
zlata-stefanovic-db
force-pushed
the
purego-transport-headers
branch
from
9a37c80 to
dd85ce6
Compare
zlata-stefanovic-db
force-pushed
the
purego-transport-headers
branch
from
dd85ce6 to
9bd111b
Compare
zlata-stefanovic-db
force-pushed
the
purego-transport-headers
branch
2 times, most recently
from
ac84862 to
a8ccc95
Compare
Add HeadersProvider support to the ephemeral stream: StreamParams gains an optional HeadersProvider used in place of Token, withStreamMetadataHeaders merges provider headers (table name stays authoritative from stream-open params), and an auth rejection on open invalidates the provider. Reject non-ASCII/control chars at the wire boundary for every token source. When a HeadersProvider is set, Token is ignored per its documented contract: Open passes an empty token so a provider that omits an authorization header cannot silently fall back to sending Token. Signed-off-by: Zlata Stefanovic <zlata.stefanovic@databricks.com>
Add control-char rejection tests for provider header values and document that GetHeaders runs before the handshake timeout is applied. Signed-off-by: Zlata Stefanovic <zlata.stefanovic@databricks.com>
Broaden isUsableAsHeader godoc to cover all metadata values and parameterize auth-rejection invalidation tests for PermissionDenied. Signed-off-by: Zlata Stefanovic <zlata.stefanovic@databricks.com>
zlata-stefanovic-db
force-pushed
the
purego-transport-headers
branch
from
cf75cbf to
b22dc27
Compare
| return nil, fmt.Errorf("transport: open %q: header %q contains invalid value characters", p.TableName, strings.TrimSpace(k)) | ||
| } | ||
| } | ||
| } else if !isUsableAsHeader(p.Token) { |
Collaborator
There was a problem hiding this comment.
You might have this planned out already, but if HeadersProvider is not set, I think we should ideally default to OAuth currently and the other mandatory header(s).
Contributor
Author
There was a problem hiding this comment.
This will land in one of the stacked PRs (#496), but noted.
| // authHeaderValue normalizes a token into an authorization header value: a value | ||
| // already carrying a known scheme (Bearer/Basic/DPoP) is returned verbatim, a | ||
| // bare token is prefixed with "Bearer ", and an empty token yields "". | ||
| func authHeaderValue(token string) string { |
Collaborator
There was a problem hiding this comment.
Predates this PR but are we sure it is okay for the authorization header to not start with anything other than "Bearer..."?
Contributor
Author
There was a problem hiding this comment.
Not introduced here, Rust core & other SDKs always format it as "Bearer "
Signed-off-by: Zlata Stefanovic <zlata.stefanovic@databricks.com>
Signed-off-by: Zlata Stefanovic <zlata.stefanovic@databricks.com>
zlata-stefanovic-db
requested review from
a team,
elenagaljak-db and
teodordelibasic-db
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
2 participants
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
First of a 3-PR stack splitting the original purego OAuth branch. Base of the stack — targets
main.Adds
HeadersProvider-based auth to the ephemeral stream transport layer as the single source of stream credentials:StreamParamscarries an optionalHeadersProvider. When set,GetHeaderssupplies the auth (and any other) metadata headers; when nil, the stream is opened without an auth header.withStreamMetadataHeadersmerges provider headers into stream metadata; the table-name header stays authoritative from stream-open params.Unauthenticated/PermissionDenied) on open invalidates the provider so the next open re-mints.GetHeadersruns under the open deadline (the caller'sctx, defaulted todefaultHandshakeTimeoutwhen it has none), so a blocking token mint can't hang Open indefinitely.No dependency on the
authpackage — transport declares its own localHeadersProviderinterface and consumes it duck-typed.There is no
StreamParams.Token: the raw-token field was dropped in favor of the provider hook, matching the auth model used by the other SDKs. Token normalization (bare token →"Bearer <token>", known schemes —Bearer/Basic/DPoP— sent verbatim) still happens in transport when it sets the authorization header.Stack
Test plan
go build ./...andgo test ./internal/transport/pass.