Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added
- `core/resource/verifier`: `ValidateIssuer(issuer string) error` — the RFC 8414 §2 issuer-shape rule, exported so every construction boundary applies one implementation rather than a copy. Rejects a query or fragment component, and requires an absolute URL with a scheme and host. `NewTokenVerifier`, `resource.New` and `authplane.NewClient` all route through it.
- `core/resource/verifier`: `ErrInvalidIssuer` sentinel, returned by everything that validates an issuer identifier. Match it with `errors.Is`.

### Fixed
- `core/resource/verifier`, `core/authplane`: an issuer rejected at construction is no longer echoed verbatim into the error. The query/fragment branch fires for exactly the shape that can carry a credential (`https://as.example.com?access_token=…`), and `net/url.Error` prints its URL field without redacting, so the raw identifier — query, fragment and any userinfo — reached whatever log the construction error landed in. Messages now carry scheme and host only. Parse failures are still wrapped with `%w`, so `errors.As(err, new(*url.Error))` keeps working; only the URL the error prints is substituted.
- `http`: the RFC 9728 PRM discovery bypass in the `net/http` adapter now compares `r.URL.EscapedPath()` against the escaped well-known path instead of the decoded `r.URL.Path`. A resource identifier carrying a percent-encoded octet (e.g. `%2F`) yields an escaped well-known path; comparing the decoded path let `%2F` collapse to `/`, the two sides disagreed, and the discovery endpoint stopped being bypassed and returned 401 even though RFC 9728 §3.2 requires it publicly reachable. The check is deliberately stricter than RFC 3986 §6.2.2.1 (a percent-encoded *unreserved* octet won't match its decoded form), an accepted trade-off since a conformant client signs the same octets the operator configured.

### Changed
- **BREAKING** `core/resource/verifier`, `core/resource`: `NewTokenVerifier` and `resource.New` now reject an issuer carrying a query or fragment component, and require the identifier to be an absolute URL with a scheme and host (RFC 8414 §2). Construction that succeeded in 0.2.0 — a relative reference such as `/tenant`, or an issuer with `?x=1` — now fails. `url.ParseRequestURI` alone accepted both: it takes a path-only reference, and it folds a fragment into `Path` rather than splitting it. **Migration:** pass the authorization server's issuer identifier exactly as published — absolute, `https`, no query, no fragment.
- **BREAKING** `core/authplane`: `NewClient` additionally requires the issuer to be absolute with a scheme and host, beyond the query/fragment rule below. This gate is not redundant with the verifier's: a `*Client` used only for token, introspection and revocation calls never constructs a `TokenVerifier`, so it is the only thing keeping a relative reference out of eager discovery. **Migration:** as above.
- **BREAKING** `core/authplane`: `ErrInvalidIssuer` is now an alias of `verifier.ErrInvalidIssuer` rather than its own sentinel. Two consequences for code that inspects it: the message changes from `authplane: invalid issuer` to `verifier: invalid issuer`, and `errors.Is(err, authplane.ErrInvalidIssuer)` now returns true for a rejection raised by the verifier, where it previously returned false. **Migration:** if you relied on the two sentinels being distinct to tell which layer rejected an identifier, that distinction is gone — both boundaries now apply the same rule, so match on the single sentinel and read the message for the specific violation. Code that only did `errors.Is(err, authplane.ErrInvalidIssuer)` on a `NewClient` error is unaffected.
- **BREAKING** `core/authplane`: `NewClient` now rejects an issuer containing a query or fragment component (RFC 8414 §2 forbids both) instead of passing it straight into metadata discovery. Previously the resource side rejected a fragment but the issuer had no such check, and the two discovery-URL builders diverged when either was present — the RFC 8414 builder silently dropped the issuer's query/fragment while the OIDC builder carried them along, so the two discovery attempts targeted different identities. Construction now fails immediately with a clear error. **Migration:** strip any query or fragment from the issuer you pass to `NewClient`; an issuer identifier never carries one.
- **BREAKING** `core/resource`: `resource.New` now rejects a resource URI containing a `#` (RFC 8707 §2 forbids a fragment in a resource indicator). `url.ParseRequestURI` does not split the fragment, so `https://api.example.com/mcp#frag` previously passed the scheme/host check and leaked the fragment into the derived PRM URL. This is a construction-time change on the exported constructor. **Migration:** remove any fragment from the resource URI you pass to `resource.New`.
- **BREAKING** `core/resource`: the RFC 9728 §3.1 PRM well-known URL now strips any terminating slash following the host component before inserting the well-known path suffix, so a resource identifier ending in `/mcp/` is served at (and derived by a conformant client as) `/.well-known/oauth-protected-resource/mcp` rather than `.../mcp/`. The resource identifier itself is unchanged — only the derived publication URL loses the slash. **Migration:** if you currently serve your PRM document at a trailing-slash well-known path, move it to the slash-stripped path (or route both) so RFC 9728 clients stop 404ing.
- **BREAKING** `core/resource`: `WellKnownPRMPath()` and `PRMURL()` now derive from the resource identifier's escaped path, so a percent-encoded octet (RFC 3986 §3.3 path data, e.g. `%2F`) is carried through verbatim instead of being decoded to `/`. A resource identifier such as `https://api.example.com/mcp%2Fx` therefore yields `.../oauth-protected-resource/mcp%2Fx` where 0.2.0 returned `.../mcp/x` — a visible output change on both exported methods. **Migration:** if you consume these values (routing the PRM handler, advertising `resource_metadata`), ensure your router matches the escaped path.
- **BREAKING** `core/internal/metadata`: the RFC 8414 §3.3 issuer check now compares the configured issuer and the metadata document's `issuer` byte-for-byte (§4: code-point-for-code-point, no normalization) instead of trailing-slash-insensitively. A document whose issuer differs from the configured issuer only by a trailing slash is now rejected as a mismatch. Because discovery is eager, this surfaces at `NewClient` as `metadata: issuer mismatch` — construction fails immediately, not at the first token verification. **Migration:** If your configured issuer differs from your authorization server's actual identifier by a trailing slash, correct the config — the SDK no longer silently reconciles them.
- **BREAKING** `core/resource/verifier`: the token verifier stores the issuer passed to `NewTokenVerifier` verbatim and matches a token's `iss` claim byte-for-byte (RFC 8414 §4: code-point-for-code-point, no normalization) instead of trailing-slash-insensitively. A token whose `iss` differs from the configured issuer only by a trailing slash is now an `ErrIssuerMismatch`. **Migration:** If the issuer you pass to `NewTokenVerifier` differs from your authorization server's actual identifier by a trailing slash, correct it — the SDK no longer silently reconciles them.

## [0.2.0] - 2026-07-21

### Added
Expand Down
26 changes: 26 additions & 0 deletions core/authplane/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,32 @@ func NewClient(ctx context.Context, issuer string, opts ...Option) (*Client, err
opt(cfg)
}

// RFC 8414 §2 forbids both a query and a fragment component in an issuer
// identifier. The resource side already rejects a fragment (resource.New),
// but the issuer flowed straight into metadata.Config with no such check —
// and the two discovery-URL builders disagree when either is present.
// buildOAuthMetadataURL now resolves a well-known reference against the
// issuer, silently dropping the issuer's query and fragment, while
// buildOIDCDiscoveryURL still trims only a trailing slash and concatenates
// the well-known suffix onto the whole string, carrying the query/fragment
// along. For "https://as.example.com/tenant?x=1" the RFC 8414 and OIDC
// discovery attempts would therefore target two different identities.
// Rejecting a query- or fragment-bearing issuer here makes the two helpers
// agree by construction.
//
// This gate is load-bearing on its own, not merely an earlier copy of the
// verifier's. A *Client used only for token, introspection and revocation
// calls never constructs a TokenVerifier, so verifier.NewTokenVerifier is
// never reached and this is the only thing keeping a relative or
// query-bearing issuer out of eager discovery.
//
// It calls the same exported rule rather than restating it, so the two
// boundaries cannot drift — and both reject with the same redacted message
// and the same ErrInvalidIssuer sentinel.
if err := verifier.ValidateIssuer(issuer); err != nil {
return nil, err
}

// Fetch settings precedence: explicit WithFetchSettings > AUTHPLANE_DEV_MODE env > defaults.
var fetchSettings ssrf.FetchSettings
switch {
Expand Down
82 changes: 82 additions & 0 deletions core/authplane/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"net/http"
"net/http/httptest"
"strings"
"sync/atomic"
"testing"
"time"
Expand Down Expand Up @@ -85,6 +86,87 @@ func TestNewClient_Success(t *testing.T) {
defer client.Close()
}

func TestNewClient_RejectsIssuerWithQueryOrFragment(t *testing.T) {
// RFC 8414 §2 forbids both a query and a fragment in an issuer identifier.
// NewClient must reject them at construction — before discovery — so the two
// discovery-URL builders cannot diverge on a query/fragment-bearing issuer.
cases := []struct {
name string
issuer string
// wantMsg is the substring the rejection message must carry. The two
// rules produce different wording, so asserting the shared sentinel
// alone would not tell them apart.
wantMsg string
}{
{"query", "https://as.example.com/tenant?x=1", "query or fragment"},
{"fragment", "https://as.example.com/tenant#frag", "query or fragment"},
{"both", "https://as.example.com/tenant?x=1#frag", "query or fragment"},
// The scheme/host rule is not redundant with the verifier's gate: a
// *Client used only for token, introspection and revocation calls never
// constructs a TokenVerifier, so NewClient is the only boundary that
// keeps a relative reference out of eager discovery. Without these rows
// the branch could be deleted and no test would go red.
{"no scheme or host", "/tenant", "scheme and host"},
{"scheme only", "https://", "scheme and host"},
// A bare authority fails earlier, in url.ParseRequestURI, so it takes
// the wrapped-parse-error branch rather than the scheme/host one. It is
// still rejected, and its message is still redacted.
{"host only", "as.example.com/tenant", "unparseable issuer"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
client, err := authplane.NewClient(context.Background(), tc.issuer,
authplane.WithFetchSettings(authplane.DevModeFetchSettings()))
if err == nil {
if client != nil {
client.Close()
}
t.Fatalf("expected error for issuer %q, got nil", tc.issuer)
}
if !errors.Is(err, authplane.ErrInvalidIssuer) {
t.Fatalf("expected error to wrap ErrInvalidIssuer, got %v", err)
}
if !strings.Contains(err.Error(), tc.wantMsg) {
t.Fatalf("expected %q in rejection message, got %v", tc.wantMsg, err)
}
})
}
}

func TestNewClient_RejectionDoesNotEchoIssuerSecrets(t *testing.T) {
// The query/fragment branch fires for exactly the shape that carries a
// secret. Construction errors land in startup logs, so the message must not
// reproduce the query, the fragment or any userinfo.
// The needles must not be substrings of the rejection wording itself —
// "frag" would match the word "fragment" in the message and report a leak
// that is not one.
const (
secret = "s3cr3t-token-value"
password = "hunter2-not-a-word"
fragNeed = "zz-fragment-needle"
)
issuer := "https://admin:" + password + "@as.example.com/tenant?access_token=" + secret + "#" + fragNeed

client, err := authplane.NewClient(context.Background(), issuer,
authplane.WithFetchSettings(authplane.DevModeFetchSettings()))
if err == nil {
if client != nil {
client.Close()
}
t.Fatal("expected error for issuer carrying a query and fragment, got nil")
}
msg := err.Error()
for _, leaked := range []string{secret, password, "access_token", fragNeed} {
if strings.Contains(msg, leaked) {
t.Fatalf("rejection message leaked %q: %s", leaked, msg)
}
}
// The host is deliberately kept — without it the error is unactionable.
if !strings.Contains(msg, "as.example.com") {
t.Fatalf("expected the host to survive redaction, got %s", msg)
}
}

func TestNewClient_NoCredentials(t *testing.T) {
server, serverURL := mockAS(t)
defer server.Close()
Expand Down
12 changes: 12 additions & 0 deletions core/authplane/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package authplane

import "github.com/authplane/go-sdk/core/resource/verifier"

// ErrInvalidIssuer is returned when the issuer identifier is not the shape RFC
// 8414 requires: §2 forbids a query and a fragment component, and the
// identifier must be an absolute URL with a scheme and host.
//
// It is the same sentinel value verifier.ErrInvalidIssuer names, so errors.Is
// matches whether the rejection came from NewClient or from the authoritative
// gate in verifier.NewTokenVerifier.
var ErrInvalidIssuer = verifier.ErrInvalidIssuer
26 changes: 26 additions & 0 deletions core/conformancetests/rfc8414_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,32 @@ func TestRFC8414MetadataIssuerMustMatchConfiguredIssuer(t *testing.T) {
if !strings.Contains(err.Error(), "issuer mismatch") {
t.Errorf("expected issuer mismatch error, got: %v", err)
}

// Catalog variant: §3.3 requires the advertised issuer to be *identical*,
// and §4 spells the comparison out as code-point-for-code-point. A metadata
// issuer differing from the configured one only by a terminating slash is
// therefore also a mismatch — this is the case a normalizing comparison
// would silently accept, binding the client to a different identity.
slashTS := metadataServerDynamic(t, func(issuer string) map[string]any {
return map[string]any{
"issuer": issuer + "/",
"jwks_uri": issuer + "/jwks",
}
})

slashMC := metadata.New(metadata.Config{
IssuerURL: slashTS.URL,
FetchSettings: ssrf.DevModeFetchSettings(),
})
defer slashMC.Close()

_, err = slashMC.Get(ctx)
if err == nil {
t.Fatal("expected error when the metadata issuer differs only by a terminating slash")
}
if !strings.Contains(err.Error(), "issuer mismatch") {
t.Errorf("expected issuer mismatch error, got: %v", err)
}
}

func TestRFC8414JWKSURIRequiredForJWTValidation(t *testing.T) {
Expand Down
11 changes: 11 additions & 0 deletions core/conformancetests/rfc9728_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ func TestRFC9728WellKnownPathMustDeriveFromResourceURI(t *testing.T) {
{"https://api.example.com", "/.well-known/oauth-protected-resource"},
{"https://api.example.com/mcp", "/.well-known/oauth-protected-resource/mcp"},
{"https://api.example.com/v2/mcp", "/.well-known/oauth-protected-resource/v2/mcp"},
// Catalog row: a resource published with a terminating slash serves its
// metadata at the slash-less well-known path, so identifiers differing
// only by that slash resolve to the same document (RFC 9728 §3.1).
{"https://api.example.com/mcp/", "/.well-known/oauth-protected-resource/mcp"},
// Every terminating slash is stripped, not one — pinned so the choice
// cannot silently drift back to a single-character trim.
{"https://api.example.com/mcp//", "/.well-known/oauth-protected-resource/mcp"},
// A percent-encoded octet is path data (RFC 3986 §3.3), not the "/"
// delimiter, so it survives the derivation verbatim rather than
// decoding into a separator and naming a different resource.
{"https://api.example.com/mcp%2Fx", "/.well-known/oauth-protected-resource/mcp%2Fx"},
}

for _, tc := range cases {
Expand Down
4 changes: 4 additions & 0 deletions core/docs/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ func main() {

`authplane.NewClient` is the top-level entry point. It owns AS metadata discovery, JWKS caching, token caching, DPoP configuration, and the circuit breaker.

The issuer must be the authorization server's identifier exactly as published: an absolute `https` URL with a host, carrying **no query and no fragment** (RFC 8414 §2 forbids both). Anything else is rejected at construction with an error wrapping `verifier.ErrInvalidIssuer` — match it with `errors.Is`. The same rule is applied by `verifier.NewTokenVerifier` and `resource.New`, all three through the exported `verifier.ValidateIssuer`.

The identifier is stored verbatim: a trailing slash is significant. If your AS publishes `https://auth.example.com/`, configure that, including the slash — the SDK compares the token's `iss` byte-for-byte (RFC 8414 §4) and no longer reconciles the two forms. Deriving the `.well-known` discovery URL still drops the terminating slash, but that is derivation, not identity.

```go
import "github.com/authplane/go-sdk/core/authplane"

Expand Down
Loading
Loading