From f125274dea2d34aff3af98f7f33980f154b5665d Mon Sep 17 00:00:00 2001 From: Henry Stern Date: Sun, 21 Jun 2026 09:22:09 -0300 Subject: [PATCH 1/3] add producer-side constructors for RISC events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every RISC event embeds the unexported subjectEvent struct, so a consumer in another package could not set the required subject in a composite literal — risc.AccountDisabled{Subject: sub} does not compile because Subject is promoted, not a direct field. The only path was a two-step var declaration plus assignment, which makes "forgot the subject" a runtime AddTo failure rather than a compile error. A downstream consumer hit exactly this and fell back to hand-rolling the SET as raw JSON, leaving the typed producer path untested from the consumer side. Each non-deprecated event type now has a New… constructor that takes the required subject positionally, so the subject can never be omitted at the call site. credential-compromise carries a second required field, so NewCredentialCompromise takes credential_type positionally as well. The deprecated sessions-revoked event deliberately has no constructor: the RISC profile steers emitters to the CAEP session-revoked event, and the producer API should not encourage emitting the deprecated form. Optional members (Reason, NewValue, the credential-compromise reasons and timestamp) stay exported fields set on the returned value rather than constructor arguments or functional options — the only thing that needed help was the unexported-subject footgun, and a functional-options layer for already-exported fields would be ceremony for nothing. Exporting the embedded struct as Base was the considered alternative; it was rejected because it keeps the "remember to set Subject" footgun the constructors exist to remove. This changes no wire shape and no decode path. Construction stays lenient in keeping with the library's Postel's-law contract: the constructors do not validate subject format, so identifier-changed/-recycled events still have their email-or-phone requirement enforced only at the AddTo/Validate marshal boundary. A consumer-side round-trip test covers every constructor (construct → AddTo → encode SET → secevent.Parse → Events.Typed yields a wire-equal event), with a coverage guard that fails if a new event type ships without a constructor or if a constructor is ever added for the deprecated sessions-revoked event. ExampleAddTo and the README now show the constructor instead of the var-plus-assign dance. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 16 ++++-- construct.go | 98 +++++++++++++++++++++++++++++++++++++ construct_test.go | 122 ++++++++++++++++++++++++++++++++++++++++++++++ example_test.go | 7 +-- 4 files changed, 236 insertions(+), 7 deletions(-) create mode 100644 construct.go create mode 100644 construct_test.go diff --git a/README.md b/README.md index 8fb2b3f..c635c50 100644 --- a/README.md +++ b/README.md @@ -76,14 +76,16 @@ An event-type URI this build does not import stays raw in ## Build a RISC event into a SET -The `subject` is carried on every RISC event; set it by assignment, -then `AddTo` validates and stores the event under its URI. +Each event type has a `New…` constructor that takes the required +`subject` (and any other required field) positionally; optional members +are set on the returned value. `AddTo` then validates and stores the +event under its URI. ```go events := secevent.Events{} -e := risc.AccountDisabled{Reason: "hijacking"} -e.Subject = subjectid.IssSubID{Iss: "https://idp.example.com/", Sub: "user-7f3e2a"} +e := risc.NewAccountDisabled(subjectid.IssSubID{Iss: "https://idp.example.com/", Sub: "user-7f3e2a"}) +e.Reason = "hijacking" if err := risc.AddTo(events, e); err != nil { return err // e.g. a credential-compromise missing its credential_type @@ -91,6 +93,12 @@ if err := risc.AddTo(events, e); err != nil { // events now carries the account-disabled member, ready for a *secevent.SET. ``` +`credential-compromise` carries a second required field, so its +constructor takes it positionally: +`risc.NewCredentialCompromise(subject, "password")`. The deprecated +`sessions-revoked` event has no constructor by design; prefer the CAEP +`session-revoked` event. + `AddTo` applies the library's strict-marshal contract: it calls `Validate` first and refuses to emit a half-built event. diff --git a/construct.go b/construct.go new file mode 100644 index 0000000..c22872f --- /dev/null +++ b/construct.go @@ -0,0 +1,98 @@ +// Copyright 2026 The go-risc Authors +// SPDX-License-Identifier: Apache-2.0 + +package risc + +import "github.com/hstern/go-subjectid" + +// This file provides the producer-side constructors for the RISC event types. +// Each event embeds the unexported subjectEvent, so a consumer cannot set the +// required subject in a composite literal; the New… constructors take the +// subject (and any other required field) positionally, making "forgot the +// subject" a compile error rather than a runtime AddTo failure. +// +// Optional members (AccountDisabled.Reason, IdentifierChanged.NewValue, +// CredentialCompromise.ReasonAdmin, …) are exported fields set on the returned +// value. Construction is lenient: the constructors do not validate the subject +// format (identifier-changed/-recycled require email or phone_number); that +// check stays at the marshal boundary in AddTo/Validate (Postel's law). +// +// The deprecated sessions-revoked event has no constructor by design, so the +// API does not encourage emitting it; prefer the CAEP session-revoked event. + +// NewAccountCredentialChangeRequired builds an account-credential-change-required +// event for subject. +func NewAccountCredentialChangeRequired(subject subjectid.SubjectIdentifier) AccountCredentialChangeRequired { + return AccountCredentialChangeRequired{subjectEvent{Subject: subject}} +} + +// NewAccountPurged builds an account-purged event for subject. +func NewAccountPurged(subject subjectid.SubjectIdentifier) AccountPurged { + return AccountPurged{subjectEvent{Subject: subject}} +} + +// NewAccountDisabled builds an account-disabled event for subject. Set the +// optional Reason on the returned value. +func NewAccountDisabled(subject subjectid.SubjectIdentifier) AccountDisabled { + return AccountDisabled{subjectEvent: subjectEvent{Subject: subject}} +} + +// NewAccountEnabled builds an account-enabled event for subject. +func NewAccountEnabled(subject subjectid.SubjectIdentifier) AccountEnabled { + return AccountEnabled{subjectEvent{Subject: subject}} +} + +// NewIdentifierChanged builds an identifier-changed event for subject, which +// names the old identifier and must be in email or phone_number format. Set +// the optional NewValue on the returned value. +func NewIdentifierChanged(subject subjectid.SubjectIdentifier) IdentifierChanged { + return IdentifierChanged{subjectEvent: subjectEvent{Subject: subject}} +} + +// NewIdentifierRecycled builds an identifier-recycled event for subject, which +// must be in email or phone_number format. +func NewIdentifierRecycled(subject subjectid.SubjectIdentifier) IdentifierRecycled { + return IdentifierRecycled{subjectEvent{Subject: subject}} +} + +// NewCredentialCompromise builds a credential-compromise event for subject. +// credentialType is required (the CAEP credential-type vocabulary, e.g. +// "password", "pin", "x509"). Set the optional EventTimestamp, ReasonAdmin, +// and ReasonUser on the returned value. +func NewCredentialCompromise(subject subjectid.SubjectIdentifier, credentialType string) CredentialCompromise { + return CredentialCompromise{ + subjectEvent: subjectEvent{Subject: subject}, + CredentialType: credentialType, + } +} + +// NewOptIn builds an opt-in event for subject. +func NewOptIn(subject subjectid.SubjectIdentifier) OptIn { + return OptIn{subjectEvent{Subject: subject}} +} + +// NewOptOutInitiated builds an opt-out-initiated event for subject. +func NewOptOutInitiated(subject subjectid.SubjectIdentifier) OptOutInitiated { + return OptOutInitiated{subjectEvent{Subject: subject}} +} + +// NewOptOutCancelled builds an opt-out-cancelled event for subject. +func NewOptOutCancelled(subject subjectid.SubjectIdentifier) OptOutCancelled { + return OptOutCancelled{subjectEvent{Subject: subject}} +} + +// NewOptOutEffective builds an opt-out-effective event for subject. +func NewOptOutEffective(subject subjectid.SubjectIdentifier) OptOutEffective { + return OptOutEffective{subjectEvent{Subject: subject}} +} + +// NewRecoveryActivated builds a recovery-activated event for subject. +func NewRecoveryActivated(subject subjectid.SubjectIdentifier) RecoveryActivated { + return RecoveryActivated{subjectEvent{Subject: subject}} +} + +// NewRecoveryInformationChanged builds a recovery-information-changed event for +// subject. +func NewRecoveryInformationChanged(subject subjectid.SubjectIdentifier) RecoveryInformationChanged { + return RecoveryInformationChanged{subjectEvent{Subject: subject}} +} diff --git a/construct_test.go b/construct_test.go new file mode 100644 index 0000000..a329e88 --- /dev/null +++ b/construct_test.go @@ -0,0 +1,122 @@ +// Copyright 2026 The go-risc Authors +// SPDX-License-Identifier: Apache-2.0 + +package risc_test + +import ( + "encoding/json" + "testing" + + risc "github.com/hstern/go-risc" + secevent "github.com/hstern/go-secevent" + subjectid "github.com/hstern/go-subjectid" +) + +// Concrete subject fixtures for the producer-side constructors. The identifier +// events require an email or phone_number subject (see validate.go). +var ( + ctorIssSub = subjectid.IssSubID{Iss: "https://idp.example.com/", Sub: "user-7f3e2a"} + ctorEmail = subjectid.EmailID{Email: "user@example.com"} + ctorPhone = subjectid.PhoneNumberID{PhoneNumber: "+12065550100"} +) + +// constructorCase pairs an event built through its New… constructor with the +// URI it must carry. The deprecated sessions-revoked event has no constructor +// and so does not appear here. +type constructorCase struct { + name string + ev secevent.Event +} + +func constructorCases() []constructorCase { + return []constructorCase{ + {"account-credential-change-required", risc.NewAccountCredentialChangeRequired(ctorIssSub)}, + {"account-purged", risc.NewAccountPurged(ctorIssSub)}, + {"account-disabled", risc.NewAccountDisabled(ctorIssSub)}, + {"account-enabled", risc.NewAccountEnabled(ctorIssSub)}, + {"identifier-changed", risc.NewIdentifierChanged(ctorEmail)}, + {"identifier-recycled", risc.NewIdentifierRecycled(ctorPhone)}, + {"credential-compromise", risc.NewCredentialCompromise(ctorIssSub, "password")}, + {"opt-in", risc.NewOptIn(ctorIssSub)}, + {"opt-out-initiated", risc.NewOptOutInitiated(ctorIssSub)}, + {"opt-out-cancelled", risc.NewOptOutCancelled(ctorIssSub)}, + {"opt-out-effective", risc.NewOptOutEffective(ctorIssSub)}, + {"recovery-activated", risc.NewRecoveryActivated(ctorIssSub)}, + {"recovery-information-changed", risc.NewRecoveryInformationChanged(ctorIssSub)}, + } +} + +// TestConstructorCoverage guards that every non-deprecated event type has a +// constructor case, and that the deprecated sessions-revoked event does not — +// so adding an event type without a constructor (or adding a constructor for +// sessions-revoked) fails here. +func TestConstructorCoverage(t *testing.T) { + covered := map[string]bool{} + for _, c := range constructorCases() { + covered[c.ev.EventTypeURI()] = true + } + for _, uri := range allEventURIs { + if uri == risc.SessionsRevokedURI { + if covered[uri] { + t.Errorf("deprecated %s should have no constructor", uri) + } + continue + } + if !covered[uri] { + t.Errorf("event type %s has no constructor", uri) + } + } +} + +// TestConstructorRoundTrip is the consumer-side acceptance check: build each +// event through its constructor, place it in a SET with AddTo, parse the SET +// back, and assert the decoded event is equal to the one constructed. +// +// Equality is checked on the marshaled wire form (jsonEqual), matching the +// repo's TestRoundTrip: the decode path canonicalizes a subject to a pointer +// (*subjectid.IssSubID) where a constructor may hold a value, so the Go values +// differ representationally while the events are identical on the wire — which +// is the round-trip contract this library makes. +func TestConstructorRoundTrip(t *testing.T) { + for _, c := range constructorCases() { + t.Run(c.name, func(t *testing.T) { + events := secevent.Events{} + if err := risc.AddTo(events, c.ev); err != nil { + t.Fatalf("AddTo: %v", err) + } + raw, ok := events[c.ev.EventTypeURI()] + if !ok { + t.Fatalf("AddTo did not store event under %s", c.ev.EventTypeURI()) + } + back := typedEvent(t, c.ev.EventTypeURI(), string(raw)) + out, err := json.Marshal(back) + if err != nil { + t.Fatalf("Marshal decoded: %v", err) + } + if !jsonEqual(t, out, raw) { + t.Errorf("round-trip mismatch\n got: %s\nwant: %s", out, raw) + } + }) + } +} + +// TestConstructorSetsSubject confirms the required subject is carried on the +// returned value — the footgun the constructors exist to remove. +func TestConstructorSetsSubject(t *testing.T) { + e := risc.NewAccountDisabled(ctorIssSub) + if e.Subject != ctorIssSub { + t.Errorf("Subject = %#v, want %#v", e.Subject, ctorIssSub) + } +} + +// TestNewCredentialCompromiseSetsRequiredFields confirms credential-compromise +// takes its second required field (credential_type) positionally. +func TestNewCredentialCompromiseSetsRequiredFields(t *testing.T) { + e := risc.NewCredentialCompromise(ctorIssSub, "password") + if e.Subject != ctorIssSub { + t.Errorf("Subject = %#v, want %#v", e.Subject, ctorIssSub) + } + if e.CredentialType != "password" { + t.Errorf("CredentialType = %q, want password", e.CredentialType) + } +} diff --git a/example_test.go b/example_test.go index 8a1acd0..a24d27a 100644 --- a/example_test.go +++ b/example_test.go @@ -43,12 +43,13 @@ func Example() { } // ExampleAddTo builds a RISC event and places it in a SET's events claim. The -// subject is set by assignment because it is carried on every RISC event. +// constructor takes the required subject positionally; optional members such +// as Reason are set on the returned value. func ExampleAddTo() { events := secevent.Events{} - e := risc.AccountDisabled{Reason: "bulk-account"} - e.Subject = subjectid.IssSubID{Iss: "https://idp.example.com/", Sub: "user-7f3e2a"} + e := risc.NewAccountDisabled(subjectid.IssSubID{Iss: "https://idp.example.com/", Sub: "user-7f3e2a"}) + e.Reason = "bulk-account" if err := risc.AddTo(events, e); err != nil { panic(err) From 1ae53609d7c57cc4a1417382f7f16ce149d2a4e4 Mon Sep 17 00:00:00 2001 From: Henry Stern Date: Sun, 21 Jun 2026 11:53:47 -0300 Subject: [PATCH 2/3] correct the Example godoc on how decoders are registered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Example function's comment said "the blank import of go-risc ... wires every RISC decoder", but the example uses a named import (it references risc.AccountDisabledURI and the typed value). A reader following the comment and switching to a blank import would lose the risc. namespace the example needs. Reword to say that importing the package — by name here, for the URI constants — registers the decoders via the package init, and note the blank import as the side-effect-only alternative. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) --- example_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/example_test.go b/example_test.go index a24d27a..7cb385f 100644 --- a/example_test.go +++ b/example_test.go @@ -13,8 +13,9 @@ import ( ) // Example decodes a SET carrying a RISC account-disabled event into its typed -// Go value. The blank import of go-risc (see the package documentation) wires -// every RISC decoder into go-secevent's registry. +// Go value. Importing go-risc (here by name, for its URI constants; a blank +// import works when only the side effect is wanted) registers every RISC +// decoder into go-secevent's registry via the package init. func Example() { payload := []byte(`{ "iss": "https://idp.example.com/", From e445ac09dbdfee3eb7e04a9e94edc35a4fbcb380 Mon Sep 17 00:00:00 2001 From: Henry Stern Date: Sun, 21 Jun 2026 11:57:43 -0300 Subject: [PATCH 3/3] document the constructor API as the 0.2.0 release Move the producer-side constructors from Unreleased into a [0.2.0] section and add its compare link. 0.2.0 is a minor bump: the constructors are a backward-compatible addition to the public API over 0.1.0, with no breaking change and no wire-shape or decode-path change. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26ad88e..0938720 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.2.0] + +### Added + +- Producer-side `New…` constructors for every non-deprecated event type + (`NewAccountDisabled`, `NewCredentialCompromise`, …). The required + subject is a positional argument, so an event can be built in a single + expression and "forgot the subject" is a compile error rather than a + runtime `AddTo` failure. `NewCredentialCompromise` also takes its + required `credential_type` positionally. The deprecated + `sessions-revoked` event has no constructor by design. + +### Changed + +- `ExampleAddTo` and the README now build events with the constructors + instead of declaring a value and assigning `Subject` separately. No + wire-shape or decode-path change. + ## [0.1.0] Initial release: the OpenID RISC Profile 1.0 event vocabulary. @@ -29,5 +47,6 @@ Initial release: the OpenID RISC Profile 1.0 event vocabulary. - Byte-stable preservation of unrecognized event-payload members via each event's `Extra` map. -[Unreleased]: https://github.com/hstern/go-risc/compare/v0.1.0...HEAD +[Unreleased]: https://github.com/hstern/go-risc/compare/v0.2.0...HEAD +[0.2.0]: https://github.com/hstern/go-risc/compare/v0.1.0...v0.2.0 [0.1.0]: https://github.com/hstern/go-risc/releases/tag/v0.1.0