refactor(server): slim the Server and dedupe issuer-URL helpers

[nabokihms]

Overview

Housekeeping on the server package after the handler-extraction refactor: the Server struct had accumulated fields and methods that no longer earn their place, the issuer-URL path helpers were copy-pasted across every handler, and the gRPC API reached back into the Server for discovery.

What this PR does / why we need it

• One issuer-URL helper. New oauth2.IssuerURL wraps url.URL with AbsPath/AbsURL. Handlers hold it instead of a bare url.URL, replacing four identical absPath and three absURL implementations (authflow, consent, mfa, logout, and Server) with a single one.
• Drop dead connector methods. Server.CloseConnector had no callers left — the connector cache was extracted into connectors.Cache, and the gRPC API invalidates it directly (d.connectors.Close). Server.OpenConnector was a thin pass-through; its callers now use s.connectors.Open. Both removed.
• Build discovery once, hand it to the API. The discovery handler was reconstructed from scattered Server fields on every call; it is now built once and stored. apiserver.NewAPI takes the *discovery.Handler directly (symmetric with the *connectors.Cache it already holds) instead of a func(ctx) Document that called back into the Server, and Server.ConstructDiscovery becomes a plain Discovery() accessor.
• Slim the Server struct. Six construction-only fields (alwaysShowLogin, passwordConnector, auth/deviceRequestsValidFor, mfaProviders, defaultMFAChain) only ever fed a handler at wiring time and are now locals / direct config reads. Together with the discovery change, the struct drops from 20 fields to 14 — the ones actually read after construction, plus a few the in-package tests use.
• Concrete routeMux. It was a bundle of four closures with four delegating methods over a fifth header-wrapping closure. It is now a real router.Mux implementation: issuer-path prefixing, header/context wrapping, and CORS live in its methods, and the direct routes (/healthz, /static, /robots.txt) go through it too.

No behavior change; existing tests cover the affected paths.

Special notes for your reviewer

Tests that constructed handlers with a raw url.URL were updated to oauth2.IssuerURL{...}.

[Copilot]

Pull request overview

Refactors the server package to reduce Server surface area and centralize issuer-relative URL/path construction, while keeping discovery generation consistent across HTTP and gRPC.

Changes:

• Introduces server/oauth2.IssuerURL (wrapping url.URL) to provide shared AbsPath/AbsURL helpers used by handlers and tests.
• Removes unused Server.OpenConnector/Server.CloseConnector methods and updates call sites to use the connector cache directly.
• Builds the OIDC discovery handler once during server construction and reuses it for both HTTP routing and gRPC discovery construction.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
server/session/session.go	Switches session manager issuer field to oauth2.IssuerURL.
server/server.go	Stores issuer as oauth2.IssuerURL, removes dead connector methods, and builds/caches discovery handler once.
server/server_test.go	Updates tests to use IssuerURL.AbsPath/AbsURL and validates grants via srv.discovery.
server/server_oauth2_test.go	Updates server test setup to wrap issuer URL and pass raw url.URL to token issuer.
server/server_login_test.go	Updates tests to open connectors via s.connectors.Open.
server/server_introspection_test.go	Updates introspection helpers to use the embedded url.URL where needed.
server/server_grant_password_test.go	Updates tests to open connectors via s.connectors.Open.
server/server_authorize_test.go	Updates tests to open connectors via s.connectors.Open.
server/oauth2/issuer.go	Adds IssuerURL helper type with AbsPath/AbsURL.
server/mfa/handler.go	Replaces per-handler absPath helper with IssuerURL.AbsPath.
server/mfa/handler_test.go	Updates MFA handler tests to construct oauth2.IssuerURL.
server/logout/logout.go	Replaces per-handler absURL helper with IssuerURL.AbsURL.
server/home/home.go	Uses IssuerURL.AbsURL for logout link construction.
server/device/device.go	Uses IssuerURL.AbsPath for device verification and callback path building.
server/device/device_test.go	Updates device handler tests to construct oauth2.IssuerURL.
server/consent/consent.go	Replaces per-handler absPath helper with IssuerURL.AbsPath.
server/authflow/urls.go	Switches flow redirect construction to IssuerURL.AbsPath.
server/authflow/sessionlogin_test.go	Updates session test server setup to use oauth2.IssuerURL for handlers and sessions.
server/authflow/request.go	Updates device callback redirect handling to use IssuerURL.AbsPath.
server/authflow/request_test.go	Updates authflow handler setup to construct oauth2.IssuerURL.
server/authflow/render.go	Removes per-handler absPath/absURL helpers in favor of IssuerURL.
server/authflow/login.go	Updates connector/login URL construction to use IssuerURL.AbsPath/AbsURL.
server/authflow/handler.go	Changes handler issuer field type to oauth2.IssuerURL.
server/authflow/handler_test.go	Updates flow assembly tests to construct oauth2.IssuerURL for all handlers.
server/authflow/authorize.go	Updates connector selection redirects to use IssuerURL.AbsPath.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

server/server.go:219

• The routeMux doc comment says every route is "wrapped" with common headers/context/instrumentation, but HandlePrefix mounts prefix handlers without calling wrap (so it only prefixes/strips). This makes the comment misleading for static/theme prefix routes.

// routeMux implements router.Mux over a gorilla mux.Router: every route is
// prefixed with the issuer path and wrapped with the common response headers,
// request-id/real-ip context, and instrumentation; HandleCORS additionally adds
// CORS for the public endpoints. instrument and realIP are the server's
// (prometheus- and config-bound) closures.

server/server.go:398

• This comment still references the removed ConstructDiscovery method (it now passes the shared discovery.Handler to the gRPC API). Updating it avoids confusion about how discovery is served over gRPC.

	// Build the discovery handler once from config; both the mounted HTTP route
	// and the gRPC API's ConstructDiscovery serve this same handler.

[Copilot]

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

server/router/router.go:75

• mountMux.wrap always calls Config.Instrument; if Instrument is nil, this will panic. Since router.New is exported and Config doesn’t document Instrument as required, it’s safer to fall back to calling the handler directly when Instrument isn’t provided.

		m.c.Instrument(name, h)(w, r.WithContext(rCtx))

[Copilot]

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated no new comments.