Require typed issuer refusals in live registration verification
All checks were successful
Build and Publish Container Image / build-and-push (push) Successful in 40s
All checks were successful
Build and Publish Container Image / build-and-push (push) Successful in 40s
Assistant: codex Assistant-Model: gpt-5.6-luna Assistant-Session: 01a07ff8-19d0-7820-b4d0-1353833cb7fc
This commit is contained in:
parent
471465df22
commit
dcebd46fa6
6 changed files with 168 additions and 13 deletions
|
|
@ -43,6 +43,19 @@ func New(issuer string) (*Client, error) {
|
|||
return &Client{Issuer: issuer, HTTP: &http.Client{Timeout: 30 * time.Second, CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }}}, nil
|
||||
}
|
||||
|
||||
// providerRejection preserves only the typed refusal needed by verification.
|
||||
// Error() never includes response text, credentials, endpoint or claim values.
|
||||
type providerRejection struct {
|
||||
status int
|
||||
endpoint string
|
||||
code string
|
||||
feature string
|
||||
}
|
||||
|
||||
func (e *providerRejection) Error() string {
|
||||
return fmt.Sprintf("provider rejected request (HTTP %d)", e.status)
|
||||
}
|
||||
|
||||
func (c *Client) request(ctx context.Context, method, endpoint string, form url.Values, id, secret string, out any) error {
|
||||
req, err := http.NewRequestWithContext(ctx, method, endpoint, strings.NewReader(form.Encode()))
|
||||
if err != nil {
|
||||
|
|
@ -60,7 +73,18 @@ func (c *Client) request(ctx context.Context, method, endpoint string, form url.
|
|||
}
|
||||
defer response.Body.Close()
|
||||
if response.StatusCode != http.StatusOK {
|
||||
return fmt.Errorf("provider rejected request (HTTP %d)", response.StatusCode)
|
||||
e := &providerRejection{status: response.StatusCode, endpoint: endpoint}
|
||||
body, readErr := io.ReadAll(io.LimitReader(response.Body, 4097))
|
||||
if readErr == nil && len(body) <= 4096 {
|
||||
var refusal struct {
|
||||
Code string `json:"error"`
|
||||
Feature string `json:"feature"`
|
||||
}
|
||||
if json.Unmarshal(body, &refusal) == nil {
|
||||
e.code, e.feature = refusal.Code, refusal.Feature
|
||||
}
|
||||
}
|
||||
return e
|
||||
}
|
||||
body, err := io.ReadAll(io.LimitReader(response.Body, 1024*1024+1))
|
||||
if err != nil || len(body) > 1024*1024 {
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import (
|
|||
"flag"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"os"
|
||||
"sort"
|
||||
|
|
@ -124,7 +125,7 @@ func runVerify(ctx context.Context, c *Client, o verifyOptions, stderr io.Writer
|
|||
for _, s := range strings.Fields(o.DenyScope) {
|
||||
excess := url.Values{"grant_type": {"client_credentials"}, "scope": {s}}
|
||||
_, err := c.Exchange(ctx, d, excess, o.ClientID, o.Secret, o.Audience, "")
|
||||
report.record("excess scope refused: "+s, mustFail(err))
|
||||
report.record("excess scope refused: "+s, mustReject(err, d.Token, http.StatusBadRequest, "scope"))
|
||||
}
|
||||
|
||||
// Predecessor rejection, for a rotation. Only meaningful after step 3 of the
|
||||
|
|
@ -136,7 +137,7 @@ func runVerify(ctx context.Context, c *Client, o verifyOptions, stderr io.Writer
|
|||
report.record("predecessor secret refused", errors.New("predecessor and current secret are identical; no rotation occurred"))
|
||||
} else {
|
||||
_, err := c.Exchange(ctx, d, form, o.ClientID, o.Previous, o.Audience, "")
|
||||
report.record("predecessor secret refused", mustFail(err))
|
||||
report.record("predecessor secret refused", mustReject(err, d.Token, http.StatusUnauthorized, "Authorization"))
|
||||
}
|
||||
}
|
||||
return report.result()
|
||||
|
|
@ -223,9 +224,14 @@ func noExcessScope(claims map[string]any, requested string) error {
|
|||
return nil
|
||||
}
|
||||
|
||||
func mustFail(err error) error {
|
||||
func mustReject(err error, endpoint string, status int, feature string) error {
|
||||
if err == nil {
|
||||
return errors.New("the issuer accepted a request it must have refused")
|
||||
}
|
||||
return nil
|
||||
var rejection *providerRejection
|
||||
if errors.As(err, &rejection) && rejection.endpoint == endpoint &&
|
||||
rejection.status == status && rejection.code == "invalid_profile_usage" && rejection.feature == feature {
|
||||
return nil
|
||||
}
|
||||
return errors.New("the expected token-endpoint refusal was not proved; transport, server, or token-validation failures are not denial evidence")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,6 +3,10 @@ package authclient
|
|||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
|
|
@ -160,3 +164,62 @@ func TestHasAllRolesNamesWhatIsMissing(t *testing.T) {
|
|||
}
|
||||
|
||||
var _ = domain.Client{}
|
||||
|
||||
type verifyTransport func(*http.Request) (*http.Response, error)
|
||||
|
||||
func (f verifyTransport) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }
|
||||
|
||||
func TestVerifyClientDoesNotConfuseFailuresWithRefusals(t *testing.T) {
|
||||
for _, target := range []string{"scope", "predecessor"} {
|
||||
for _, failure := range []string{"transport", "server", "invalid_token", "unrelated_refusal", "malformed_refusal"} {
|
||||
t.Run(target+"/"+failure, func(t *testing.T) {
|
||||
c, _, _ := provider(t)
|
||||
original := c.HTTP.Transport
|
||||
o := verifyOpts()
|
||||
if target == "predecessor" {
|
||||
o.PreviousNamed, o.Previous = true, "old-secret"
|
||||
}
|
||||
c.HTTP.Transport = verifyTransport(func(r *http.Request) (*http.Response, error) {
|
||||
if r.Method == http.MethodPost && r.URL.Path == "/token" {
|
||||
body, _ := io.ReadAll(r.Body)
|
||||
r.Body = io.NopCloser(bytes.NewReader(body))
|
||||
form, _ := url.ParseQuery(string(body))
|
||||
_, password, _ := r.BasicAuth()
|
||||
password, _ = url.QueryUnescape(password)
|
||||
negative := target == "scope" && form.Get("scope") != o.Scope || target == "predecessor" && password == o.Previous
|
||||
if negative {
|
||||
status, payload := 503, `{"error":"unavailable"}`
|
||||
switch failure {
|
||||
case "transport":
|
||||
return nil, errors.New("transport failed")
|
||||
case "invalid_token":
|
||||
status, payload = 200, `{"token_type":"Bearer","access_token":"invalid","expires_in":900}`
|
||||
case "unrelated_refusal":
|
||||
status, payload = 400, `{"error":"invalid_profile_usage","feature":"client_id","description":"must not disclose this"}`
|
||||
case "malformed_refusal":
|
||||
status, payload = 400, `not-json-sensitive-body`
|
||||
}
|
||||
return &http.Response{StatusCode: status, Header: http.Header{}, Body: io.NopCloser(strings.NewReader(payload)), Request: r}, nil
|
||||
}
|
||||
}
|
||||
return original.RoundTrip(r)
|
||||
})
|
||||
var out bytes.Buffer
|
||||
if err := runVerify(context.Background(), c, o, &out); err == nil {
|
||||
t.Fatal("a failed negative check was accepted as issuer refusal")
|
||||
}
|
||||
if !strings.Contains(out.String(), "PASS exchange and JWKS signature for granted scopes") {
|
||||
t.Fatalf("test did not reach the negative checks: %s", out.String())
|
||||
}
|
||||
if !strings.Contains(out.String(), "expected token-endpoint refusal was not proved") {
|
||||
t.Fatalf("wrong failure: %s", out.String())
|
||||
}
|
||||
for _, value := range []string{o.Secret, o.Previous, "must not disclose this", "not-json-sensitive-body", "eyJ"} {
|
||||
if value != "" && strings.Contains(out.String(), value) {
|
||||
t.Fatal("verification disclosed input or provider payload")
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue