From 777905126f2d183ff7c2f9b68e406e64f395e684 Mon Sep 17 00:00:00 2001 From: Axel Etcheverry Date: Sat, 10 Oct 2026 12:56:22 +0200 Subject: [PATCH] fix(oauth2): refuse client authentication with an empty secret MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit client_secret_basic decoded "Basic base64(:)" into an empty secret and handed it to the client's SecretMatcher, and DefaultClient.SecretMatches matched it against an empty Secret. Anyone who knew the client_id of a client registered without a secret — a public client, or a confidential or untyped one left without one — authenticated as it at /token and /revoke, and at /introspect when it was confidential, which the confidential-only check of #81 let through. client_secret_post never accepted an empty client_secret, so the two secret methods disagreed. An empty secret is no credential. RFC 6749 §2.3 lets the authorization server accept the client authentication meeting its security requirements and forbids relying on a public client's authentication to identify it, and OAuth 2.1 §2.1 calls clients without credentials "public clients". RFC 6749 §2.3.1 lets a client omit an empty client_secret, but requires Basic only for clients "that were issued a client password". client_secret_basic now refuses a missing client_id or an empty password with 401 invalid_client before loading the client, with the guard client_secret_post's Authenticate already has: no store lookup, one answer for every client. DefaultClient.SecretMatches never matches an empty Secret, and the SecretMatcher contract says implementations MUST NOT match an empty secret: callers of DefaultClient are covered, custom matchers are told to follow suit. Public clients that sent an empty Basic password must send their client_id in the form body (none); confidential or untyped clients registered without a secret need one to use client_secret_basic or client_secret_post. CHANGELOG and MIGRATION.md describe the change. Refs #94. --- CHANGELOG.md | 23 +++++ LIMITATIONS.md | 5 + MIGRATION.md | 52 ++++++++++ docs/security-considerations.md | 44 +++++--- oauth2/client.go | 23 ++++- oauth2/clientauth/basic.go | 14 +++ oauth2/clientauth/clientauth_test.go | 86 ++++++++++++++++ oauth2/empty_client_secret_test.go | 146 +++++++++++++++++++++++++++ oauth2/introspect_endpoint_test.go | 8 +- oauth2/values_test.go | 6 ++ 10 files changed, 385 insertions(+), 22 deletions(-) create mode 100644 oauth2/empty_client_secret_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 51eb34e..27ca677 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -312,3 +312,26 @@ legacy packages (`authentication/`, `authorization/`, the in-tree `grant.Config.OnError`. JWT access tokens validated offline stay valid until they expire. Previously the tokens issued from a replayed code stayed valid. +- OAuth2 `client_secret_basic` no longer authenticates a client with an + empty password (RFC 6749 §2.3, §10.1). It handed the empty secret of + `Authorization: Basic base64(":")` to the client's + `SecretMatcher`, and `DefaultClient.SecretMatches` matched it against an + empty `Secret`: anyone knowing the `client_id` of a client registered + without a secret authenticated as it at `/token` — obtaining tokens, + `client_credentials` included — and at `/revoke`, whatever the client's + type, and at `/introspect` when the client was confidential, reading the + validity and claims of the tokens they held. `client_secret_basic` now + answers a missing `client_id` or an empty password with + `401 invalid_client` (`missing client_id or client_secret`) before + loading the client, with the guard `client_secret_post`'s `Authenticate` + already has: no store lookup, one answer for every client. + `DefaultClient.SecretMatches` never matches an empty `Secret`, and the + `oauth2.SecretMatcher` contract now says implementations MUST NOT match + an empty secret: callers of `DefaultClient` are covered, custom matchers + are told to follow suit. Public clients still authenticate with `none`, + by design; through it, a public client can still use any grant its + `GrantTypeValues` allow, `client_credentials` included (RFC 6749 §4.4 is + not enforced yet; see [MIGRATION.md](MIGRATION.md)). This breaks public + clients that sent an empty Basic password — they must use `none` — and + clients registered as confidential or untyped that authenticated with an + empty Basic password. See [MIGRATION.md](MIGRATION.md). diff --git a/LIMITATIONS.md b/LIMITATIONS.md index 75cf81a..ecb4fe9 100644 --- a/LIMITATIONS.md +++ b/LIMITATIONS.md @@ -10,6 +10,11 @@ than future refactor phases. - **`private_key_jwt` client authentication (RFC 7523)** — not implemented. `client_secret_basic`, `client_secret_post`, and `none` are. +- **RFC 6749 §4.4 is not enforced yet** — the `client_credentials` grant + "MUST only be used by confidential clients", but it is not refused to + public clients (authenticated with `none`), and an empty + `GrantTypeValues` allows every grant: restrict `GrantTypeValues` for + public clients. - **`/.well-known/jwks.json` endpoint** — not exposed. JWKS publication depends on a server-side public-key store; the `jwtsec` module already provides the building blocks (`NewStaticJWKS`). diff --git a/MIGRATION.md b/MIGRATION.md index 76ccf99..eb3e1cc 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -295,3 +295,55 @@ Unknown tokens and other clients' tokens still answer `200 OK`. Every underlying failure still reaches `ServerConfig.OnError` exactly once, as a `server_error` carrying its cause (a failed lookup is now reported too, as `revoke: token lookup failed`); the 503 itself is not reported again. + +### `oauth2`: `client_secret_basic` refuses an empty secret (#94) + +`client_secret_basic` decoded `Authorization: Basic base64(":")` +into an empty secret and handed it to the client's `SecretMatcher`, and +`DefaultClient.SecretMatches` matched it against an empty `Secret`: anyone +knowing the `client_id` of a client registered without a secret +authenticated as it at `/token` and `/revoke`, and at `/introspect` when the +client was confidential. An empty secret is no credential. RFC 6749 §2.3 +lets the authorization server accept the client authentication "meeting its +security requirements" and forbids relying on a public client's +authentication to identify it, and §10.1 forbids issuing client passwords +to native or user-agent-based clients. RFC 6749 §2.3.1 and Appendix A.2 +(`client-secret = *VSCHAR`) allow an empty secret on the wire, but they do +not oblige the server to accept one: Basic is required "for authenticating +clients that were issued a client password", and a client registered +without a secret was issued none. + +`client_secret_basic` now refuses a missing `client_id` or an empty +password with `401 invalid_client` (`missing client_id or client_secret`) +before loading the client, as `client_secret_post`'s `Authenticate` +already did (a form with an empty `client_secret` is a `none` request), +and `DefaultClient.SecretMatches` never matches an empty `Secret`: + +| Request | Before | After | +| -------------------------------------------------------------- | -------------------------------------------------------- | ------------------------------- | +| Basic, empty password, client registered without a secret | authenticated (`/introspect`: confidential clients only) | `401 invalid_client` | +| Basic, empty password, client with a secret, or unknown client | `401`, description depends on the client | `401`, same description for all | +| Basic/Post with a real secret; `none` for public clients | authenticated | unchanged | +| `DefaultClient{Secret: ""}.SecretMatches("")` | `true` | `false` | + +To migrate: + +1. **Public clients that sent an empty Basic password** use `none` + instead: `client_id` in the form body, no `Authorization` header and no + `client_secret` (RFC 6749 §3.2.1). Register them with + `TypeValue: oauth2.ClientPublic`, add `clientauth.NewNone()` to + `ServerConfig.ClientAuth` (`examples/oauth2` does not wire it) and, if + the client sets `AuthMethodValues`, include `"none"`. +2. **Confidential or untyped clients registered without a secret** can no + longer authenticate through `client_secret_basic` or + `client_secret_post` (and `none` refuses non-public clients). Give each + one a high-entropy secret, or authenticate it with a custom + `ClientAuthenticator` using another credential (e.g. `private_key_jwt`, + mTLS). Register a client as public only if it cannot keep a secret — a + browser or native app using `authorization_code` + PKCE — and then set + its `GrantTypeValues`: an empty list allows every grant, and the server + does not yet refuse `client_credentials` to public clients (RFC 6749 + §4.4). +3. **Custom `SecretMatcher`s** must return `false` when no secret is + registered or the presented one is empty, whatever they compare + against: a bcrypt hash of `""` matches `""`. diff --git a/docs/security-considerations.md b/docs/security-considerations.md index 0cd2f79..ba920cc 100644 --- a/docs/security-considerations.md +++ b/docs/security-considerations.md @@ -123,8 +123,16 @@ configurable clock skew, and resolves keys by `kid` against a JWKS provider additionally refuses the `password` grant. Legacy grants are opt-in and refused outright under the stricter profile. - **Client authentication** — `client_secret_basic` / `client_secret_post` - compare secrets in constant time. Public clients use `none` and MUST use - PKCE. `/introspect` answers `ClientConfidential` callers only: a public + compare secrets in constant time and never accept an empty one. Basic + answers an empty password or a missing `client_id` with + `401 invalid_client` before loading the client, the same answer whether + or not the client exists; a form with an empty or absent `client_secret` + is a `none` request, which only public clients may make. Custom + `SecretMatcher`s MUST NOT match an empty secret; `DefaultClient` never + matches an empty `Secret`. A public client has no secret: it + authenticates with `none` (RFC 7591 §2) and MUST use PKCE. Register as + public a client that cannot keep its credentials confidential (RFC 6749 + §2.1). `/introspect` answers `ClientConfidential` callers only: a public client (whatever the method, `none` included) or an untyped client gets `401 invalid_client`. - **Token introspection** — RFC 7662 §4 requires the authorization server @@ -134,20 +142,20 @@ configurable clock skew, and resolves keys by `kid` against a JWKS provider MUST NOT rely on a public client's authentication — even with a secret — to identify it (RFC 6749 §2.3). Accepting one would let anyone read the validity and claims of a token they held — through `none`, or through - Basic/Post with the client's registered secret (Basic: even an empty - one) — so `/introspect` refuses every caller that is not a confidential - client. By default, any authenticated confidential client may then - introspect any token. RFC 7662 §4 adds that protected resources SHOULD be - specifically authorized, and that when a token can be used only at certain - resource servers the authorization server MUST determine whether it can be - used at the caller: that decision is `ServerConfig.IntrospectionPolicy`, - which sees the authenticated caller and the token's metadata before - anything is disclosed. Refuse refresh tokens to resource servers — they are - meant for the client alone, and a resource server that ignores - `token_type` could accept one as a bearer token. A withheld token and a - failed policy answer the same bytes as an unknown token - (`{"active":false}`), so the response discloses nothing (its latency may - still differ); a policy failure is reported to `OnError`. + Basic/Post with the client's registered secret — so `/introspect` refuses + every caller that is not a confidential client. By default, any + authenticated confidential client may then introspect any token. + RFC 7662 §4 adds that protected resources SHOULD be specifically + authorized, and that when a token can be used only at certain resource + servers the authorization server MUST determine whether it can be used at + the caller: that decision is `ServerConfig.IntrospectionPolicy`, which + sees the authenticated caller and the token's metadata before anything is + disclosed. Refuse refresh tokens to resource servers — they are meant for + the client alone, and a resource server that ignores `token_type` could + accept one as a bearer token. A withheld token and a failed policy answer + the same bytes as an unknown token (`{"active":false}`), so the response + discloses nothing (its latency may still differ); a policy failure is + reported to `OnError`. ### Token-family revocation (storage guarantees) @@ -284,6 +292,10 @@ span catalog. - [ ] Keep the JWT allowlist asymmetric unless you truly need HMAC. - [ ] Use `Profile20BCP` or stricter; do not enable the `implicit` / `password` legacy grants without a documented reason. +- [ ] Give every confidential client that authenticates with a secret a + high-entropy one. Register a client that cannot keep a secret as + public (`none` + PKCE), and list its `GrantTypeValues`: an empty + list allows every grant, `client_credentials` included. - [ ] Register the resource servers calling `/introspect` as confidential clients, and set `ServerConfig.IntrospectionPolicy` when several resource servers or tenants share the authorization server. diff --git a/oauth2/client.go b/oauth2/client.go index ffed074..c189a04 100644 --- a/oauth2/client.go +++ b/oauth2/client.go @@ -53,9 +53,14 @@ type Client interface { // authentication methods (client_secret_basic, client_secret_post) to // verify the registered secret without exposing it. Implementations MUST // use constant-time comparison (or a hashed-secret scheme). +// +// An empty secret is never a credential: implementations MUST NOT match +// it — not even against a hash of the empty string — nor anything when no +// secret is registered. type SecretMatcher interface { // SecretMatches returns true when secret matches the registered one. - // Implementations MUST use constant-time comparison. + // Implementations MUST use constant-time comparison, and MUST return + // false when secret is empty or no secret is registered. SecretMatches(secret string) bool } @@ -74,6 +79,11 @@ type DefaultClient struct { // Secret is the cleartext client secret. DefaultClient stores it // verbatim for dev/test convenience; production deployments wrap a // hashed-secret store and implement SecretMatches themselves. + // + // Leave it empty for a client that does not authenticate with a secret + // (a public client using "none", or a client authenticated by another + // method): an empty Secret matches nothing, so client_secret_basic and + // client_secret_post never authenticate it. Secret string //nolint:gosec // dev/test convenience TypeValue ClientType RedirectURIValues []string @@ -101,10 +111,15 @@ func (c *DefaultClient) Scopes() []string { return c.ScopeValues } func (c *DefaultClient) AuthMethods() []string { return c.AuthMethodValues } // SecretMatches implements [SecretMatcher] using constant-time comparison. -// The DefaultClient stores secrets in cleartext for development convenience; -// production deployments SHOULD wrap a hashed-secret store and implement -// SecretMatches themselves. +// An empty Secret registers no secret and matches nothing, the empty string +// included. The DefaultClient stores secrets in cleartext for development +// convenience; production deployments SHOULD wrap a hashed-secret store and +// implement SecretMatches themselves. func (c *DefaultClient) SecretMatches(secret string) bool { + if c.Secret == "" { + return false + } + return subtle.ConstantTimeCompare([]byte(c.Secret), []byte(secret)) == 1 } diff --git a/oauth2/clientauth/basic.go b/oauth2/clientauth/basic.go index 3969fe4..475f87b 100644 --- a/oauth2/clientauth/basic.go +++ b/oauth2/clientauth/basic.go @@ -14,6 +14,12 @@ import ( // NewBasic returns a client_secret_basic authenticator. The client_id and // client_secret are read from the HTTP Basic Authorization header per // RFC 6749 §2.3.1. +// +// A header without a client_id or with an empty client_secret, such as +// Basic base64(":"), is refused with invalid_client before the +// client is loaded, as [NewPost] refuses an empty client_secret. A public +// client authenticates with [NewNone] instead (RFC 7591 §2: "none" is the +// method of a public client that has no secret). func NewBasic() ClientAuthenticator { return basicAuth{} } type basicAuth struct{} @@ -43,6 +49,14 @@ func (basicAuth) Authenticate(ctx context.Context, r *http.Request, store oauth2 return nil, oauth2.ErrInvalidClient.WithDescription("malformed Basic Authorization header") } + // A missing client_id or an empty password is no credential: refuse it + // before the store is consulted, like client_secret_post, so the answer + // does not depend on the client existing (RFC 6749 §2.3 lets the server + // choose the client authentication it accepts). + if id == "" || secret == "" { + return nil, oauth2.ErrInvalidClient.WithDescription("missing client_id or client_secret") + } + client, err := store.LoadClient(ctx, id) if err != nil { return nil, errInvalid(err) diff --git a/oauth2/clientauth/clientauth_test.go b/oauth2/clientauth/clientauth_test.go index c7e6721..e869611 100644 --- a/oauth2/clientauth/clientauth_test.go +++ b/oauth2/clientauth/clientauth_test.go @@ -12,6 +12,7 @@ import ( "net/http/httptest" "net/url" "strings" + "sync/atomic" "testing" "github.com/hyperscale-stack/security/oauth2" @@ -34,6 +35,18 @@ func (s fakeStore) LoadClient(_ context.Context, id string) (oauth2.Client, erro return s.clients[id], nil } +// countingStore is a fakeStore that counts its LoadClient calls. +type countingStore struct { + fakeStore + calls atomic.Int32 +} + +func (s *countingStore) LoadClient(ctx context.Context, id string) (oauth2.Client, error) { + s.calls.Add(1) + + return s.fakeStore.LoadClient(ctx, id) +} + // noMatcherClient implements oauth2.Client but NOT oauth2.SecretMatcher. type noMatcherClient struct { id string @@ -299,6 +312,79 @@ func TestPostAuthenticate(t *testing.T) { }) } +// --- secret-based methods ------------------------------------------------ + +// TestSecretMethodsRefuseMissingCredentials pins that client_secret_basic +// and client_secret_post refuse a missing client_id or an empty secret +// before the client store is consulted, with the same answer whatever the +// client: an empty secret is no credential, even for a client registered +// without one (#94). +func TestSecretMethodsRefuseMissingCredentials(t *testing.T) { + t.Parallel() + + clients := map[string]oauth2.Client{ + "c1": confidentialClient(), + "no-secret": &oauth2.DefaultClient{IDValue: "no-secret", TypeValue: oauth2.ClientConfidential}, + "public": &oauth2.DefaultClient{IDValue: "public", TypeValue: oauth2.ClientPublic}, + } + + methods := []struct { + name string + auth ClientAuthenticator + req func(id, secret string) *http.Request + }{ + { + name: "basic", + auth: NewBasic(), + req: func(id, secret string) *http.Request { + r := httptest.NewRequest(http.MethodPost, "/", nil) + r.Header.Set("Authorization", basicHeader(id, secret)) + + return r + }, + }, + { + name: "post", + auth: NewPost(), + req: func(id, secret string) *http.Request { + return postReq(url.Values{"client_id": {id}, "client_secret": {secret}}) + }, + }, + } + + cases := []struct { + name string + id, secret string + }{ + {"client with a secret", "c1", ""}, + {"confidential client without a secret", "no-secret", ""}, + {"public client without a secret", "public", ""}, + {"unknown client", "ghost", ""}, + {"empty client_id", "", "s3cr3t"}, + {"empty client_id and secret", "", ""}, + } + + for _, m := range methods { + for _, tc := range cases { + t.Run(m.name+"/"+tc.name, func(t *testing.T) { + t.Parallel() + + // A store per subtest, so that the count is this request's. + store := &countingStore{fakeStore: fakeStore{clients: clients}} + + c, err := m.auth.Authenticate(context.Background(), m.req(tc.id, tc.secret), store) + assert.Nil(t, c) + assertInvalidClient(t, err) + + var oe *oauth2.Error + require.ErrorAs(t, err, &oe) + assert.Equal(t, "missing client_id or client_secret", oe.Description) + assert.Zero(t, store.calls.Load(), "the client store must not be consulted") + }) + } + } +} + // --- none ---------------------------------------------------------------- func TestNoneMethodAndMatch(t *testing.T) { diff --git a/oauth2/empty_client_secret_test.go b/oauth2/empty_client_secret_test.go new file mode 100644 index 0000000..c476d52 --- /dev/null +++ b/oauth2/empty_client_secret_test.go @@ -0,0 +1,146 @@ +// Copyright 2026 Hyperscale. All rights reserved. +// Use of this source code is governed by a MIT +// license that can be found in the LICENSE file. + +package oauth2_test + +import ( + "net/http" + "net/url" + "testing" + "time" + + "github.com/hyperscale-stack/security/oauth2" + "github.com/hyperscale-stack/security/oauth2/clientauth" + "github.com/hyperscale-stack/security/oauth2/grant" + "github.com/hyperscale-stack/security/oauth2/storage/memory" + "github.com/hyperscale-stack/security/oauth2/token" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// missingCredentials is the error_description client_secret_basic and +// client_secret_post answer a missing client_id or an empty secret with. +const missingCredentials = "missing client_id or client_secret" + +// emptySecretNow is the instant newEmptySecretServer's clock is pinned to. +var emptySecretNow = time.Date(2026, 6, 1, 12, 0, 0, 0, time.UTC) + +// emptySecretClients lists the clients newEmptySecretServer registers: one +// with a secret, and one without a secret for each client type. +var emptySecretClients = []string{ + testClientID, "secretless-confidential", "secretless-public", "secretless-untyped", +} + +// newEmptySecretServer builds a Profile20BCP server pinned to +// emptySecretNow. It serves /token (client_credentials), /revoke and +// /introspect, authenticates clients with client_secret_basic, +// client_secret_post and none, and holds one active access token, at-, +// issued to each of emptySecretClients. +func newEmptySecretServer(t *testing.T) (*oauth2.Server, *memory.Store) { + t.Helper() + + store := memory.New() + clients := &staticClientStore{clients: map[string]oauth2.Client{ + testClientID: &oauth2.DefaultClient{ + IDValue: testClientID, Secret: testClientSecret, TypeValue: oauth2.ClientConfidential, + }, + "secretless-confidential": &oauth2.DefaultClient{ + IDValue: "secretless-confidential", TypeValue: oauth2.ClientConfidential, + }, + "secretless-public": &oauth2.DefaultClient{ + IDValue: "secretless-public", TypeValue: oauth2.ClientPublic, + }, + "secretless-untyped": &oauth2.DefaultClient{IDValue: "secretless-untyped"}, + }} + gcfg := grant.Config{Storage: store, AccessTokens: token.NewOpaque(32), AccessTTL: time.Hour} + + srv, err := oauth2.NewServer(oauth2.ServerConfig{ + Profile: oauth2.Profile20BCP, + Storage: store, + ClientStore: clients, + IssuerResolver: oauth2.StaticIssuer("https://auth.example", "api"), + Grants: []oauth2.Grant{grant.NewClientCredentials(gcfg)}, + ClientAuth: []oauth2.ClientAuthenticator{ + clientauth.NewBasic(), clientauth.NewPost(), clientauth.NewNone(), + }, + Now: func() time.Time { return emptySecretNow }, + }) + require.NoError(t, err) + + for _, id := range emptySecretClients { + require.NoError(t, store.SaveTokenPair(t.Context(), &oauth2.TokenPair{Access: oauth2.AccessToken{ + TokenHash: oauth2.HashToken(nil, "at-"+id), ClientID: id, Subject: id, Scope: "read", + IssuedAt: emptySecretNow, ExpiresAt: emptySecretNow.Add(time.Hour), + }})) + } + + return srv, store +} + +// TestEmptyBasicPasswordRefusedAtEveryEndpoint pins that an empty HTTP +// Basic password authenticates no client at /token, /revoke or /introspect +// (#94). Whether the client has a secret, has none — whatever its type — or +// does not exist, the answer is the same 401 invalid_client, so it tells +// nothing about the client, and the client's token is neither revoked nor +// disclosed. +func TestEmptyBasicPasswordRefusedAtEveryEndpoint(t *testing.T) { + t.Parallel() + + endpoints := []struct { + name string + handler func(*oauth2.Server) http.Handler + form func(id string) url.Values + }{ + { + name: "token", + handler: (*oauth2.Server).TokenHandler, + form: func(string) url.Values { return url.Values{"grant_type": {"client_credentials"}} }, + }, + { + name: "revoke", + handler: (*oauth2.Server).RevokeHandler, + form: func(id string) url.Values { return url.Values{"token": {"at-" + id}} }, + }, + { + name: "introspect", + handler: (*oauth2.Server).IntrospectHandler, + form: func(id string) url.Values { return url.Values{"token": {"at-" + id}} }, + }, + } + + for _, ep := range endpoints { + t.Run(ep.name, func(t *testing.T) { + t.Parallel() + + srv, store := newEmptySecretServer(t) + h := ep.handler(srv) + path := "/oauth2/" + ep.name + + // The reference answer, for a client that does not exist: + // RFC 6749 §5.2 invalid_client, with the challenge HTTP Basic + // calls for. + ref := serve(h, basicForm(path, ep.form("ghost"), "ghost", "")) + require.Equal(t, http.StatusUnauthorized, ref.Code) + assert.Equal(t, `Basic realm="oauth2"`, ref.Header().Get("WWW-Authenticate")) + assert.JSONEq(t, `{"error":"invalid_client","error_description":"`+missingCredentials+`"}`, ref.Body.String()) + + desc, _ := decodeBody(t, ref)["error_description"].(string) + assert.Regexp(t, descriptionCharset, desc) + + for _, id := range emptySecretClients { + rec := serve(h, basicForm(path, ep.form(id), id, "")) + + // The same status, headers and bytes as for an unknown + // client: the answer reveals nothing about the client... + assert.Equal(t, ref.Code, rec.Code, id) + assert.Equal(t, ref.Header(), rec.Header(), id) + assert.Equal(t, ref.Body.Bytes(), rec.Body.Bytes(), id) + + // ...and its token was not revoked. + _, err := store.LookupAccessToken(t.Context(), oauth2.HashToken(nil, "at-"+id)) + assert.NoError(t, err, id) + } + }) + } +} diff --git a/oauth2/introspect_endpoint_test.go b/oauth2/introspect_endpoint_test.go index 9c7a7d0..913df1e 100644 --- a/oauth2/introspect_endpoint_test.go +++ b/oauth2/introspect_endpoint_test.go @@ -305,9 +305,11 @@ func TestIntrospectCallerAuthentication(t *testing.T) { wantDesc: "none method reserved to public clients", }, { + // Refused by client_secret_basic itself (#94), before the + // confidential-only check. name: "basic public client with an empty password", req: introspectRequest(spaClientID, "", "at-app"), - wantDesc: introspectionRefused, + wantDesc: missingCredentials, }, { name: "basic public client with a secret", @@ -325,9 +327,11 @@ func TestIntrospectCallerAuthentication(t *testing.T) { wantDesc: introspectionRefused, }, { + // Refused by client_secret_basic itself (#94), before the + // confidential-only check. name: "basic untyped client with an empty password", req: introspectRequest(untypedNoSecretID, "", "at-app"), - wantDesc: introspectionRefused, + wantDesc: missingCredentials, }, { name: "wrong secret", diff --git a/oauth2/values_test.go b/oauth2/values_test.go index 00c2afa..2f2c95a 100644 --- a/oauth2/values_test.go +++ b/oauth2/values_test.go @@ -192,6 +192,12 @@ func TestDefaultClient(t *testing.T) { assert.True(t, c.SecretMatches("s3cr3t")) assert.False(t, c.SecretMatches("wrong")) assert.False(t, c.SecretMatches("")) + + // An empty Secret registers no secret: it matches nothing, the empty + // string included (#94). + secretless := &oauth2.DefaultClient{IDValue: "client-2"} + assert.False(t, secretless.SecretMatches("")) + assert.False(t, secretless.SecretMatches("s3cr3t")) } func TestStaticIssuer(t *testing.T) {