Skip to content
Merged
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ legacy packages (`authentication/`, `authorization/`, the in-tree
- **Sessions** (`session`): stateless AES-256-GCM encrypted cookies with key
rotation, a `Manager` (Login/Get/Touch/Rotate/Logout), and a
synchronizer-token CSRF helper.
- **OAuth2 error hook**: `ServerConfig.OnError` and `grant.Config.OnError`
(both `oauth2.ErrorHook`) observe every error the authorization server
turns into an RFC 6749 §5.2 response — carrying the cause a `server_error`
never puts on the wire — plus the errors it swallows on purpose, such as a
best-effort revocation (RFC 7009 §2.2) or a family revocation that failed
during reuse detection. Purely observational: the responses are unchanged.
- **Observability**: OpenTelemetry spans emitted directly by the core,
`httpsec`, `grpcsec`, `connectrpcsec`, `jwtsec`, and `session`. See
[docs/observability.md](docs/observability.md).
Expand Down
47 changes: 46 additions & 1 deletion docs/observability.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,52 @@ dependency. Basic/Bearer authentication is still observable: the core
`security.Manager.Authenticate` span records which authenticator ran via
the `security.authenticator.name` attribute and an `authenticator.try`
event per candidate. OAuth2 HTTP endpoints are observable through the host
server's HTTP instrumentation (e.g. `otelhttp`).
server's HTTP instrumentation (e.g. `otelhttp`) and, for their errors,
through the OAuth2 error hook described below.

## OAuth2 error hook

An RFC 6749 §5.2 response carries a code, a description and a URI — never
the cause. A `server_error` therefore reaches the client as an opaque 500,
and RFC 7009 §2.2 goes further: a revocation answers `200 OK` even when the
revocation itself failed. Both are protocol requirements, and both leave an
operator with nothing to diagnose.

`oauth2.ServerConfig.OnError` closes that gap. It is an `oauth2.ErrorHook`
(`func(ctx context.Context, err error)`) called with the `*oauth2.Error`
envelope — cause intact — at the point the server decides on its answer:

```go
srv, err := oauth2.NewServer(oauth2.ServerConfig{
// ...
OnError: func(ctx context.Context, err error) {
if oauth2.IsCode(err) != oauth2.CodeServerError {
return // expected 4xx traffic
}

slog.ErrorContext(ctx, "oauth2 server error", "err", err)
},
})
```

The hook fires for:

- every error serialized as an RFC 6749 §5.2 body (`/token`, `/revoke`,
`/introspect`, metadata) and every error redirected back to the client by
`/authorize`, including the pre-redirect refusals answered with a bare
400 (unknown client, unregistered `redirect_uri`);
- the best-effort revocations `/revoke` swallows to honour RFC 7009 §2.2.

`grant.Config.OnError` is the same hook for the errors a grant swallows
before returning — today, a family revocation that failed during BCP §8.10.3
reuse detection. Errors a grant *returns* travel to the server and reach
`ServerConfig.OnError`, so wire both fields to the same sink.

The hook is purely observational: it MUST NOT influence the response, and it
runs synchronously on the request goroutine, so keep it fast. It receives no
secret — the envelope carries the code, the description and the wrapped Go
error, never a token or a client secret — but the cause comes from your own
storage layer, so apply the same redaction rules you apply to your logs.

## Span catalog

Expand Down
14 changes: 14 additions & 0 deletions examples/oauth2/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,18 @@ type principal struct{ sub string }

func (p principal) Subject() string { return p.sub }

// logOAuthError is the [oauth2.ErrorHook] shared by the server and the
// grants. The RFC 6749 §5.2 body carries no cause, so this is the only
// place a server_error becomes diagnosable; the expected 4xx traffic is
// filtered out to keep the log readable.
func logOAuthError(_ context.Context, err error) {
if oauth2.IsCode(err) != oauth2.CodeServerError {
return
}

log.Printf("oauth2: server error: %v", err)
}

// buildServer wires the authorization server and the Bearer-protected
// resource server onto a single mux. It is separate from main so the
// end-to-end test can exercise the exact same wiring.
Expand Down Expand Up @@ -121,6 +133,7 @@ func buildServer() (http.Handler, error) {
AccessTTL: time.Hour,
RefreshTTL: 24 * time.Hour,
RotateRefreshTokens: true,
OnError: logOAuthError,
}

srv, err := oauth2.NewServer(oauth2.ServerConfig{
Expand All @@ -134,6 +147,7 @@ func buildServer() (http.Handler, error) {
grant.NewRefreshToken(gcfg),
},
ClientAuth: []oauth2.ClientAuthenticator{clientauth.NewBasic(), clientauth.NewPost()},
OnError: logOAuthError,
})
if err != nil {
return nil, fmt.Errorf("oauth2.NewServer: %w", err)
Expand Down
37 changes: 23 additions & 14 deletions oauth2/authorize_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,28 +174,33 @@ func (s *Server) AuthorizeHandler(cfg AuthorizeConfig, consent ConsentFunc) http

func (s *Server) serveAuthorize(cfg AuthorizeConfig, consent ConsentFunc, w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet && r.Method != http.MethodPost {
s.notifyError(r.Context(), ErrInvalidRequest.WithDescription("/authorize requires GET or POST"))
http.Error(w, "oauth2: /authorize requires GET or POST", http.StatusMethodNotAllowed)

return
}

if err := r.ParseForm(); err != nil {
s.notifyError(r.Context(), ErrInvalidRequest.WithCause(err))
http.Error(w, "oauth2: malformed authorization request", http.StatusBadRequest)

return
}

// Client and redirect URI come first: a failure here MUST NOT redirect
// (the redirect target is not yet trusted).
// (the redirect target is not yet trusted). The refusal is deliberately
// opaque on the wire, so the hook is the only place the cause shows up.
client, err := s.cfg.ClientStore.LoadClient(r.Context(), r.FormValue("client_id"))
if err != nil || client == nil {
s.notifyError(r.Context(), ErrInvalidClient.WithDescription("unknown or invalid client").WithCause(err))
http.Error(w, "oauth2: unknown or invalid client", http.StatusBadRequest)

return
}

redirectURI, ok := resolveRedirectURI(client, r.FormValue("redirect_uri"))
if !ok {
s.notifyError(r.Context(), ErrInvalidRequest.WithDescription("missing or unregistered redirect_uri"))
http.Error(w, "oauth2: missing or unregistered redirect_uri", http.StatusBadRequest)

return
Expand All @@ -208,7 +213,7 @@ func (s *Server) serveAuthorize(cfg AuthorizeConfig, consent ConsentFunc, w http
flow, oerr := resolveFlow(cfg, r.FormValue("response_type"))
if oerr != nil {
// The response type is unknown — default to a query-string error.
redirectAuthorizeError(w, r, redirectURI, state, oerr, false)
s.redirectAuthorizeError(w, r, redirectURI, state, oerr, false)

return
}
Expand All @@ -217,14 +222,14 @@ func (s *Server) serveAuthorize(cfg AuthorizeConfig, consent ConsentFunc, w http

ar, oerr := s.parseAuthorizeRequest(r, client, redirectURI, flow)
if oerr != nil {
redirectAuthorizeError(w, r, redirectURI, state, oerr, useFragment)
s.redirectAuthorizeError(w, r, redirectURI, state, oerr, useFragment)

return
}

decision, err := consent(w, r, ar)
if err != nil {
redirectAuthorizeError(w, r, redirectURI, ar.State,
s.redirectAuthorizeError(w, r, redirectURI, ar.State,
ErrServerError.WithDescription("consent handler failed"), useFragment)

return
Expand All @@ -236,7 +241,7 @@ func (s *Server) serveAuthorize(cfg AuthorizeConfig, consent ConsentFunc, w http
}

if !decision.Approved {
redirectAuthorizeError(w, r, redirectURI, ar.State,
s.redirectAuthorizeError(w, r, redirectURI, ar.State,
ErrAccessDenied.WithDescription("the resource owner denied the request"), useFragment)

return
Expand Down Expand Up @@ -334,15 +339,15 @@ func (s *Server) issueAuthorizationCode(
) {
granted, err := grantedScope(ar, decision)
if err != nil {
redirectAuthorizeError(w, r, ar.RedirectURI, ar.State,
s.redirectAuthorizeError(w, r, ar.RedirectURI, ar.State,
ErrInvalidScope.WithDescription("granted scope exceeds the request"), false)

return
}

raw, err := randomCode()
if err != nil {
redirectAuthorizeError(w, r, ar.RedirectURI, ar.State, ErrServerError.WithCause(err), false)
s.redirectAuthorizeError(w, r, ar.RedirectURI, ar.State, ErrServerError.WithCause(err), false)

return
}
Expand All @@ -365,7 +370,7 @@ func (s *Server) issueAuthorizationCode(
}

if err := s.cfg.Storage.SaveAuthorizationCode(r.Context(), code); err != nil {
redirectAuthorizeError(w, r, ar.RedirectURI, ar.State, ErrServerError.WithCause(err), false)
s.redirectAuthorizeError(w, r, ar.RedirectURI, ar.State, ErrServerError.WithCause(err), false)

return
}
Expand All @@ -390,22 +395,22 @@ func (s *Server) issueImplicitToken(
) {
granted, err := grantedScope(ar, decision)
if err != nil {
redirectAuthorizeError(w, r, ar.RedirectURI, ar.State,
s.redirectAuthorizeError(w, r, ar.RedirectURI, ar.State,
ErrInvalidScope.WithDescription("granted scope exceeds the request"), true)

return
}

_, audience, ierr := s.resolveIssuer(r.Context(), r)
if ierr != nil {
redirectAuthorizeError(w, r, ar.RedirectURI, ar.State, ErrServerError.WithCause(ierr), true)
s.redirectAuthorizeError(w, r, ar.RedirectURI, ar.State, ErrServerError.WithCause(ierr), true)

return
}

raw, hash, err := cfg.ImplicitTokens.Generate(r.Context())
if err != nil {
redirectAuthorizeError(w, r, ar.RedirectURI, ar.State, ErrServerError.WithCause(err), true)
s.redirectAuthorizeError(w, r, ar.RedirectURI, ar.State, ErrServerError.WithCause(err), true)

return
}
Expand All @@ -423,7 +428,7 @@ func (s *Server) issueImplicitToken(
}

if err := s.cfg.Storage.SaveAccessToken(r.Context(), at); err != nil {
redirectAuthorizeError(w, r, ar.RedirectURI, ar.State, ErrServerError.WithCause(err), true)
s.redirectAuthorizeError(w, r, ar.RedirectURI, ar.State, ErrServerError.WithCause(err), true)

return
}
Expand Down Expand Up @@ -497,14 +502,18 @@ func authorizeScope(requested string, allowed []string) (string, error) {
}

// redirectAuthorizeError sends an RFC 6749 §4.1.2.1 / §4.2.2.1 error
// response by redirecting back to the client's redirect URI.
func redirectAuthorizeError(
// response by redirecting back to the client's redirect URI. The envelope —
// cause included — is handed to the configured [ErrorHook] first: the
// redirect carries only the code and the description.
func (s *Server) redirectAuthorizeError(
w http.ResponseWriter,
r *http.Request,
redirectURI, state string,
oerr *Error,
useFragment bool,
) {
s.notifyError(r.Context(), oerr)

params := url.Values{"error": {oerr.Code}}
if oerr.Description != "" {
params.Set("error_description", oerr.Description)
Expand Down
4 changes: 4 additions & 0 deletions oauth2/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
// the jwt sub-module (no hard dependency from oauth2 to jwt).
// - Stores expose atomic ConsumeAuthorizationCode and RotateRefreshToken
// to guarantee single-use semantics and reuse-detection.
// - ServerConfig.OnError (and grant.Config.OnError) observe the errors the
// protocol hides: the cause of a server_error, which never reaches the
// wire, and the revocations RFC 7009 §2.2 requires to answer 200 OK
// regardless. See ErrorHook.
//
// Allowed dependencies:
// - github.com/hyperscale-stack/security (core)
Expand Down
16 changes: 16 additions & 0 deletions oauth2/grant/grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package grant

import (
"context"
"time"

"github.com/hyperscale-stack/security/oauth2"
Expand Down Expand Up @@ -46,6 +47,21 @@ type Config struct {
// /token?grant_type=refresh_token call and marks the old one
// consumed; reuse triggers family revocation. Default true in BCP/21.
RotateRefreshTokens bool
// OnError, when set, observes the errors a grant swallows to keep the
// protocol response intact — today, a family revocation that failed
// during reuse detection. Errors returned to the server travel to
// [oauth2.ServerConfig.OnError] instead, so wire both to the same sink.
// Optional; see [oauth2.ErrorHook].
OnError oauth2.ErrorHook
}

// notifyError hands err to the configured [oauth2.ErrorHook], if any.
func (c Config) notifyError(ctx context.Context, err error) {
if c.OnError == nil || err == nil {
return
}

c.OnError(ctx, err)
}

// Request and Response are type aliases anchoring the contract in the
Expand Down
Loading
Loading