Skip to content

feat(oauth2provider): let clients send resource indicators to /authorize - #76

Merged
juicycleff merged 2 commits into
fix/oauth2-resource-form-bindingfrom
feat/oauth2-authorize-resource-param
Aug 25, 2026
Merged

feat(oauth2provider): let clients send resource indicators to /authorize#76
juicycleff merged 2 commits into
fix/oauth2-resource-form-bindingfrom
feat/oauth2-authorize-resource-param

Conversation

@juicycleff

Copy link
Copy Markdown
Contributor

Draft, and red on purpose. It needs xraph/go-utils#4 merged and released, then
a go.mod bump here. Three tests in plugins/oauth2provider fail against
go-utils v1.1.7, all downstream of the repeated query parameter that PR fixes.
Everything below passes with the fix applied through a local replace.

Stacked on #75, which is stacked on #72. Read those first.

What this finishes

The authorization endpoint has honoured a repeatable resource all along, but
read it off the raw request. Forge describes query parameters by reflecting
over the handler's request struct, so a parameter with no field is a parameter
with no spec entry, and a parameter with no spec entry reaches no generated
client. RFC 8707 worked there over curl and nowhere else.

It is an ordinary []string with a query tag now, resourceParams is gone, and
the parameter is in the document.

Exposing it turned up the same bug one layer over

The query-string builders had never met an array parameter, because until now
the API did not have one. All three were wrong, in three different ways:

// Go: does not compile. A slice is not comparable.
var zeroResource []string
if params.Resource != zeroResource {
    q.Set("resource", fmt.Sprint(params.Resource))
}
// TypeScript: joins on commas, sends one value where two were meant.
params.set('resource', String(resource));

Dart was the interesting one. It built a Map<String, String> and joined the
entries, and a map cannot hold a repeated key at all, so only the last value
would ever have survived. That one needed a shape change rather than a patch:
it builds a list of pairs now.

All three write one entry per element, the same treatment the form encoders got
in #72.

The Go breakage bit twice

A generated SDK that does not compile also blocks the generator, because
dump-spec builds the module the SDK lives in. So the first regeneration after
adding the parameter produced a broken sdk/go/client.go, and every subsequent
regeneration failed on it. Recovering meant restoring sdk/go from git before
regenerating with the fixed template.

Worth knowing about if you hit it: the error points at the generated file, not
at the template that wrote it.

Tests

Wire-form tests on the query path in both TypeScript and Dart, covering one
element and two, matching the form-body tests already in those files. Same
approach: inject a transport and read what actually went out, rather than test
the builder against itself.

The spec test for /authorize was added skipped in #75 with the unskip
procedure written into it. This is that procedure, so the skip is gone.

Before merging

  1. Merge and release fix(http): bind every occurrence of a repeated query parameter go-utils#4
  2. go get github.com/xraph/go-utils@<new version> here
  3. CI goes green on its own, no code changes needed

Verified

With the local replace in place: go test ./sdkgen/... ./plugins/oauth2provider/
green, go build ./... and sdk/go both clean, regeneration idempotent,
goimports clean, npm test 8/8, dart test 8/8, dart analyze clean,
flutter analyze clean apart from one pre-existing info, pnpm run typecheck
8/8.

BLOCKED until xraph/go-utils#4 ships and go.mod picks it up. Three tests
in plugins/oauth2provider fail against go-utils v1.1.7, all of them
downstream of the repeated query parameter that PR fixes.

The authorization endpoint has honoured a repeatable `resource` all
along, but read it off the raw request, so forge never saw a field to
describe and the parameter reached no generated client. RFC 8707 worked
there over curl and nowhere else. It is a `[]string` with a query tag
now, resourceParams is gone, and the parameter is in the spec.

Exposing it turned up the same defect one layer over. The query-string
builders had never met an array parameter, because until now there was
not one:

  Go          q.Set("resource", fmt.Sprint(params.Resource))
              preceded by a zero-value comparison that does not compile
              for a slice, so the generated SDK failed to build
  TypeScript  params.set('resource', String(resource))
              which joins on commas and sends one value
  Dart        a Map<String, String>, which cannot hold a repeated key
              at all, so only the last value would survive

All three now write one entry per element, the same treatment the form
encoders got. Dart builds a list of pairs rather than a map, since the
map was the thing that made repetition impossible.

The Go breakage is worth noting: a generated SDK that does not compile
also blocks the generator, because dump-spec builds the module the SDK
lives in. Recovering meant restoring sdk/go before regenerating.

Wire-form tests cover one element and two on the query path in both
TypeScript and Dart, matching the form-body tests already there.
xraph/go-utils#4 shipped as v1.1.8. bindQueryParam now fills a []string from
every occurrence of a repeated query parameter, the way bindFormParam already
did, and a lone value still expands on commas so scope=openid,profile is
unaffected.

That is the release the previous commit said it was waiting for. The three
tests it listed as failing against v1.1.7 pass now, and they fail for the right
reason without this bump: two_resources_both_land_on_the_code kept only the
first resource, and the two TestTokenResource cases were downstream of the same
collapse.

Nothing else moves. The spec and all three SDKs regenerate byte-identical, the
full suite passes and the linter is quiet, because the code this unblocks was
already written and only the dependency was missing.
@juicycleff
juicycleff marked this pull request as ready for review August 25, 2026 20:59
@juicycleff
juicycleff merged commit c98d092 into fix/oauth2-resource-form-binding Aug 25, 2026
@juicycleff
juicycleff deleted the feat/oauth2-authorize-resource-param branch August 25, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant