authserver: export the RunConfig→Config construction so embedders can use authserver.New without reimplementing it
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
Summary
pkg/authserver offers two ways to stand up the embedded auth server:
- The runner —
runner.NewEmbeddedAuthServer(ctx, *RunConfig)/NewEmbeddedAuthServerWithStorage(ctx, *RunConfig, storage)— takes the serializableRunConfig, does all theRunConfig → authserver.Configtranslation internally, but does not exposeauthserver.Config.UpstreamFilter(or otherConfig-only fields). authserver.New(ctx, Config, storage)— takes a fully-builtauthserver.Config, which is the only way to setConfig.UpstreamFilter.
An embedder that needs Config.UpstreamFilter (or any Config-only field) must therefore call authserver.New directly — but there is no exported way to derive a Config from a RunConfig. That translation is entirely unexported in pkg/authserver/runner: buildUpstreamConfigs → buildOIDCConfig/buildPureOAuth2Config, resolveSecret, convertUserInfoConfig, the ephemeral key/HMAC defaulting (createKeyProvider/loadHMACSecrets), upstream.RegisterModifiers(), and the DCR body-size cap that EmbeddedAuthServer.Handler() applies.
Problem
So each embedder that wants the Config path re-implements that translation by hand. That is:
- Error-prone. The translation quietly does several things that are easy to miss when copied: the DCR request-body cap (
bodylimit.Middleware(handlers.MaxDCRBodySize)onHandler()), threadingInsecureAllowHTTPonto the top-levelConfig, callingupstream.RegisterModifiers(), client-secret-from-file/env resolution, and scope defaults. Missing any of these silently weakens or breaks auth (e.g. servingauthserver.Server.Handler()directly loses the body cap on the open DCR endpoint). - Drift-prone. A hand copy does not follow changes to the (unexported) source across releases.
- Duplicated. At least two separate downstream consumers have independently re-implemented this same translation (plus a per-user
UpstreamFilteron top). Same logic, multiple copies, each re-discovering the same gaps — a fix in one does not reach the others.
Recommended solution
Export the RunConfig → Config construction from pkg/authserver so embedders that need authserver.New reuse the same reviewed code instead of copying it. Any one of these closes the gap:
- A public constructor —
func (rc *RunConfig) BuildConfig() (Config, keys.KeyProvider, error)(or a freefunc BuildConfig(rc *RunConfig) (...)) — that runs exactly what the runner does today (key/HMAC defaulting, upstream mapping incl. DCR,RegisterModifiers, etc.). The runner constructors then become thin wrappers over it, so there is a single source of truth. - Export the sub-helpers (
BuildUpstreamConfigs, …) if one coarse entry point isn't desirable. - Export a handler-wrapping helper for the DCR body-size cap (e.g.
WrapWithBodyLimit(http.Handler) http.Handler), sinceauthserver.Server.Handler()returns the bare router whileEmbeddedAuthServer.Handler()applies the cap — an easy footgun for anyone servingauthserver.Serverdirectly.
Alternative considered: add a WithUpstreamFilter(...) functional option to the runner constructors so consumers can keep using the runner. Smaller, but it only solves the UpstreamFilter case, not the general "I need a Config-only field" problem — exporting the construction is the more complete fix.
Why it matters
The Config path is (rightly) the intended integration point for UpstreamFilter and other advanced fields. But if building a Config from a RunConfig isn't a first-class exported operation, every embedder re-derives it: the construction gets copy-pasted across codebases, fixes in one don't reach the others, and security-relevant details (the DCR body-size cap, InsecureAllowHTTP, modifier registration) get dropped in the copies.
References (as of v0.34.0)
pkg/authserver/runner/embeddedauthserver.go— the unexported translation (buildUpstreamConfigs,buildOIDCConfig,buildPureOAuth2Config,resolveSecret,convertUserInfoConfig,createKeyProvider,loadHMACSecrets)pkg/authserver/config.go—Config.UpstreamFilterpkg/authserver/server_impl.go—Handler()returns the bare router (no body cap)pkg/authserver/server/handlers/handler.go—UpstreamFilter,MaxDCRBodySize
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in pkg/authserver/runner/embeddedauthserver.go and trace the unexported RunConfig-to-Config helpers, then compare their behavior with pkg/authserver/config.go and pkg/authserver/server_impl.go. Check the handler and DCR references in pkg/authserver/server/handlers/handler.go. Done means embedders can reuse the runner's complete construction and handler safeguards without duplicating translation logic.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- authentication, backend-api-design
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100