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
445 changes: 275 additions & 170 deletions flutter/packages/authsome_core/lib/src/generated/api_client.dart

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ require (
github.com/x448/float16 v0.8.4 // indirect
github.com/xraph/confy v1.0.2 // indirect
github.com/xraph/dispatch v1.6.2
github.com/xraph/go-utils v1.1.7
github.com/xraph/go-utils v1.1.8
github.com/xraph/ledger v1.6.1
github.com/xraph/vault v1.6.1
github.com/yusufpapurcu/wmi v1.2.4 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -436,8 +436,8 @@ github.com/xraph/forge/extensions/auth v1.9.11 h1:toPgfeWOPFU74/+EPmqdA9copZIF7S
github.com/xraph/forge/extensions/auth v1.9.11/go.mod h1:iOQGV/iZA5+HY5vE47lDIAO+Qp2wIG8ZIDw41Pn5CDQ=
github.com/xraph/forgeui v1.4.1 h1:LHK1t/sZ+9zL+MNUZralO9/rc0f5UCa19dpbWTuRMNg=
github.com/xraph/forgeui v1.4.1/go.mod h1:rH/+wb1tt2pXSHotWAvoP+Lt846xlIjuwPDSpS5K5mw=
github.com/xraph/go-utils v1.1.7 h1:PvW6H5VZhiLt07qQrGTGQqPiVwsbVyeQg2oxuNmrBxA=
github.com/xraph/go-utils v1.1.7/go.mod h1:Mckdi+nR0bI4bUESKSYajJq4tNSPsvZiuLRYJ0+qDQw=
github.com/xraph/go-utils v1.1.8 h1:O8+Vie/u/ntn2cEbvh47jJLzQ6S7qwxhYwgRm2SL1sw=
github.com/xraph/go-utils v1.1.8/go.mod h1:Mckdi+nR0bI4bUESKSYajJq4tNSPsvZiuLRYJ0+qDQw=
github.com/xraph/grove v1.6.2 h1:O/3UyHTKQQ57CyZiLkDQi5T7xyzMSBz320VlK3C04Vo=
github.com/xraph/grove v1.6.2/go.mod h1:bgjHNhnmyfEyzbdpcppRt+Zf24nNcbGKlo450Mi4giI=
github.com/xraph/grove/drivers/mongodriver v1.6.2 h1:vyuSb2Fu6pRM7xb2DhtQattDCXyOm//TKUcMnwlkD9M=
Expand Down
14 changes: 6 additions & 8 deletions plugins/oauth2provider/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,11 +534,10 @@ type AuthorizeRequest struct {
State string `query:"state,omitempty"`
CodeChallenge string `query:"code_challenge,omitempty"`
CodeChallengeMethod string `query:"code_challenge_method,omitempty"`
// No resource field. go-utils' bindFormParam gained multi-value binding in
// v1.1.7, but bindQueryParam still reads one value through c.Query, so a
// []string query field silently keeps the first value and drops the rest.
// That is worse than the error it used to raise, so the authorization
// endpoint reads the raw query; see resourceParams.
// RFC 8707, repeatable. A field rather than a raw-request read, because
// only a declared field reaches the OpenAPI document, and only a described
// parameter reaches the generated clients.
Resource []string `query:"resource,omitempty"`
}

// TokenRequest is the OAuth2 token request.
Expand Down Expand Up @@ -754,9 +753,8 @@ func (p *Plugin) handleAuthorize(ctx forge.Context, req *AuthorizeRequest) (*api
return nil, err
}

// RFC 8707. Read off the raw query: the struct binder still collapses a
// repeated query parameter to its first value; see resourceParams.
resources, err := resolveResources(client, resourceParams(ctx.Request()))
// RFC 8707.
resources, err := resolveResources(client, req.Resource)
if err != nil {
return nil, err
}
Expand Down
24 changes: 0 additions & 24 deletions plugins/oauth2provider/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,6 @@ import (
"github.com/xraph/authsome/internal/resourceuri"
)

// resourceParams reads the repeatable RFC 8707 resource parameter off a query
// string.
//
// Form bodies no longer need this: go-utils v1.1.7 taught bindFormParam to
// fill a []string from every occurrence of a parameter, so the token and
// device endpoints bind theirs through the struct. bindQueryParam did not get
// the same treatment. It still reads a single value through c.Query, and
// setFieldValue's new slice case then splits that one value on commas, so a
// []string query field keeps the first resource and silently discards the
// rest. Reading the query directly is the only way the authorization endpoint
// sees every value it was sent.
//
// The cost is that the parameter stays out of the OpenAPI document, since
// forge describes query parameters by reflecting over the request struct and
// nothing else. No generated client can send a resource indicator to
// /authorize until bindQueryParam handles repeated values.
func resourceParams(r *http.Request) []string {
if r == nil {
return nil
}

return r.URL.Query()["resource"]
}

// resourceURISyntaxError checks a single RFC 8707 resource indicator against
// the syntax rule shared by request-time resolution and admin registration:
// the value must be an absolute URI and must not carry a fragment. It returns
Expand Down
445 changes: 275 additions & 170 deletions sdk/dart/lib/src/client.dart

Large diffs are not rendered by default.

56 changes: 56 additions & 0 deletions sdk/dart/test/form_encoding_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,60 @@ void main() {
);
});
});

group('query-string parameters', () {
// A query string carries repeated keys, the same as a form body does. The
// authorization endpoint reads every `resource` it is given, so a list has
// to be written one element at a time rather than stringified.
({AuthClient client, Uri Function() url}) clientRecordingUrl() {
var seen = Uri.parse('https://auth.example.com');

final client = AuthClient(AuthClientConfig(
baseUrl: 'https://auth.example.com',
httpClient: MockClient((request) async {
seen = request.url;
return http.Response('', 204);
}),
));

return (client: client, url: () => seen);
}

test('sends a one-element list as a single parameter', () async {
final recorder = clientRecordingUrl();

await recorder.client.oauth2Authorize(
responseType: 'code',
clientId: 'the-client',
resource: ['https://api.example.com'],
);

expect(recorder.url().queryParametersAll['resource'],
['https://api.example.com']);
});

test('repeats the parameter once per element for a two-element list',
() async {
final recorder = clientRecordingUrl();

await recorder.client.oauth2Authorize(
responseType: 'code',
clientId: 'the-client',
resource: ['https://a.example.com', 'https://b.example.com'],
);

expect(recorder.url().queryParametersAll['resource'],
['https://a.example.com', 'https://b.example.com']);
});

test('keeps an absent list off the query string', () async {
final recorder = clientRecordingUrl();

await recorder.client
.oauth2Authorize(responseType: 'code', clientId: 'the-client');

expect(recorder.url().queryParametersAll.containsKey('resource'),
isFalse);
});
});
}
21 changes: 14 additions & 7 deletions sdk/go/client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion sdk/typescript/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2162,7 +2162,7 @@ export class AuthClient {
* OAuth2 Authorization
* GET /v1/oauth/authorize
*/
async oauth2Authorize(response_type: string, client_id: string, redirect_uri?: string, scope?: string, state?: string, code_challenge?: string, code_challenge_method?: string): Promise<void> {
async oauth2Authorize(response_type: string, client_id: string, redirect_uri?: string, scope?: string, state?: string, code_challenge?: string, code_challenge_method?: string, resource?: string[]): Promise<void> {
const params = new URLSearchParams();
if (response_type !== undefined) params.set('response_type', String(response_type));
if (client_id !== undefined) params.set('client_id', String(client_id));
Expand All @@ -2171,6 +2171,10 @@ export class AuthClient {
if (state !== undefined) params.set('state', String(state));
if (code_challenge !== undefined) params.set('code_challenge', String(code_challenge));
if (code_challenge_method !== undefined) params.set('code_challenge_method', String(code_challenge_method));
// Repeated once per element: a query string carries repeated keys, not
// lists, which is how RFC 8707 sends `resource`. String() on an array
// would join it on commas and reach the server as one value.
if (resource !== undefined) for (const element of resource) params.append('resource', String(element));
const qs = params.toString();
const path = "/v1/oauth/authorize" + (qs ? `?${qs}` : '');
return this.request<void>(
Expand Down
54 changes: 54 additions & 0 deletions sdk/typescript/test/form-encoding.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,57 @@ describe('form-encoded request bodies', () => {
expect(new URLSearchParams(body()).has('resource')).toBe(false);
});
});

/**
* Builds a client whose transport records the URL it was called with.
*/
function clientRecordingURL(): { client: AuthClient; url: () => string } {
let sent = '';

const client = new AuthClient({
baseURL: 'https://auth.example.com',
fetch: async (url) => {
sent = String(url);
return new Response(null, { status: 204 });
},
});

return { client, url: () => sent };
}

describe('query-string parameters', () => {
// A query string carries repeated keys, the same as a form body does. The
// authorization endpoint reads every `resource` it is given, so an array has
// to be written one element at a time rather than stringified.
it('sends a one-element array as a single parameter', async () => {
const { client, url } = clientRecordingURL();

await client.oauth2Authorize('code', 'the-client', undefined, undefined, undefined, undefined, undefined, [
'https://api.example.com',
]);

expect(new URL(url()).searchParams.getAll('resource')).toEqual(['https://api.example.com']);
});

it('repeats the parameter once per element for a two-element array', async () => {
const { client, url } = clientRecordingURL();

await client.oauth2Authorize('code', 'the-client', undefined, undefined, undefined, undefined, undefined, [
'https://a.example.com',
'https://b.example.com',
]);

expect(new URL(url()).searchParams.getAll('resource')).toEqual([
'https://a.example.com',
'https://b.example.com',
]);
});

it('keeps an absent array off the query string', async () => {
const { client, url } = clientRecordingURL();

await client.oauth2Authorize('code', 'the-client');

expect(new URL(url()).searchParams.has('resource')).toBe(false);
});
});
16 changes: 3 additions & 13 deletions sdkgen/cmd/specgen/resource_parameter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,10 @@ func queryParameter(t *testing.T, spec map[string]any, path, method, name string
// if the spec describes it, and the spec only describes parameters that exist
// as fields on the handler's request struct.
//
// The field cannot exist yet. go-utils v1.1.7 taught bindFormParam to fill a
// []string from a repeated parameter, which is why the device endpoint below
// carries one, but bindQueryParam still reads a single value through c.Query.
// A []string query field would therefore bind the first resource and silently
// drop the rest, which is worse than the error the old binder raised, so
// handleAuthorize reads the raw query instead and the parameter stays
// undescribed.
//
// Unskip this once bindQueryParam handles repeated values: add
// `Resource []string` with a query tag to AuthorizeRequest, drop
// resourceParams, and regenerate.
// Reading the raw request instead left the endpoint working over curl and
// unreachable from every SDK, which is the state it was in until go-utils
// taught bindQueryParam to keep every occurrence of a repeated parameter.
func TestSpec_AuthorizeExposesRepeatableResource(t *testing.T) {
t.Skip("blocked on go-utils bindQueryParam, which collapses a repeated query parameter to its first value")

param := queryParameter(t, committedSpec(t), "/v1/oauth/authorize", "get", "resource")

require.NotNil(t, param, "the authorize endpoint should describe a resource query parameter")
Expand Down
6 changes: 6 additions & 0 deletions sdkgen/dart/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,12 @@ func (g *Generator) responseType(op *openapi.Operation) string {
func (g *Generator) renderTemplate(name string, data *TemplateData) (string, error) {
funcMap := template.FuncMap{
"lower": strings.ToLower,
// A list-typed parameter is written once per element rather than
// stringified, since List.toString() emits Dart's bracket notation and
// a query string carries repeated keys rather than lists.
"isArrayType": func(t string) bool {
return strings.HasPrefix(t, "List<")
},
"dartType": func(t string, optional bool) string {
if optional {
return t + "?"
Expand Down
25 changes: 20 additions & 5 deletions sdkgen/dart/templates/client.dart.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,31 @@ class AuthClient {
Future<{{ if isVoid $op.ResponseType }}void{{ else }}{{ $op.ResponseType }}{{ end }}> {{ $op.Name }}({{ buildDartParams $op }}) async {
final path = {{ buildDartPath $op.Path $op.PathParams }};
{{- if hasQueryParams $op.QueryParams }}
final queryParams = <String, String>{};
// A list of pairs rather than a map, because a query string may carry the
// same key more than once. RFC 8707 sends `resource` that way, and a map
// would keep only the last value.
final queryPairs = <MapEntry<String, String>>[];
{{- range $op.QueryParams }}
{{- if isArrayType .Type }}
{{- if .Required }}
queryParams['{{ .Name }}'] = {{ .DartName }}.toString();
for (final element in {{ .DartName }}) {
queryPairs.add(MapEntry('{{ .Name }}', element.toString()));
}
{{- else }}
for (final element in {{ .DartName }} ?? const []) {
queryPairs.add(MapEntry('{{ .Name }}', element.toString()));
}
{{- end }}
{{- else }}
if ({{ .DartName }} != null) queryParams['{{ .Name }}'] = {{ .DartName }}.toString();
{{- if .Required }}
queryPairs.add(MapEntry('{{ .Name }}', {{ .DartName }}.toString()));
{{- else }}
if ({{ .DartName }} != null) queryPairs.add(MapEntry('{{ .Name }}', {{ .DartName }}.toString()));
{{- end }}
{{- end }}
{{- end }}
final queryString = queryParams.isNotEmpty
? '?${queryParams.entries.map((e) => '${e.key}=${Uri.encodeComponent(e.value)}').join('&')}'
final queryString = queryPairs.isNotEmpty
? '?${queryPairs.map((e) => '${e.key}=${Uri.encodeComponent(e.value)}').join('&')}'
: '';
{{- end }}
{{ if isVoid $op.ResponseType }} await _request(
Expand Down
7 changes: 7 additions & 0 deletions sdkgen/golang/templates/client.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,13 @@ func (c *Client) {{ .Name }}(ctx context.Context
if params.{{ .GoName }} {
q.Set("{{ .Name }}", "true")
}
{{- else if eq .Type "[]string" }}
// Repeated once per element. A query string carries repeated keys, not
// lists, which is how RFC 8707 sends `resource`. A slice is also not
// comparable, so the zero-value check below would not compile for one.
for _, v := range params.{{ .GoName }} {
q.Add("{{ .Name }}", v)
}
{{- else }}
var zero{{ .GoName }} {{ .Type }}
if params.{{ .GoName }} != zero{{ .GoName }} {
Expand Down
10 changes: 10 additions & 0 deletions sdkgen/spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -27089,6 +27089,16 @@
"schema": {
"type": "string"
}
},
{
"in": "query",
"name": "resource",
"schema": {
"items": {
"type": "string"
},
"type": "array"
}
}
],
"responses": {
Expand Down
6 changes: 6 additions & 0 deletions sdkgen/typescript/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,12 @@ func (g *Generator) renderTemplate(name string, data *TemplateData) (string, err
"hasQueryParams": func(params []QueryParamDef) bool {
return len(params) > 0
},
// An array-typed parameter is written once per element rather than
// stringified, since String(["a","b"]) joins on commas and puts one
// value on the wire where the caller meant two.
"isArrayType": func(t string) bool {
return strings.HasSuffix(t, "[]")
},
"buildPath": func(path string, params []PathParamDef) string {
if len(params) == 0 {
return `"` + path + `"`
Expand Down
7 changes: 7 additions & 0 deletions sdkgen/typescript/templates/client.ts.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,14 @@ export class AuthClient {
{{- if hasQueryParams $op.QueryParams }}
const params = new URLSearchParams();
{{- range $op.QueryParams }}
{{- if isArrayType .Type }}
// Repeated once per element: a query string carries repeated keys, not
// lists, which is how RFC 8707 sends `resource`. String() on an array
// would join it on commas and reach the server as one value.
if ({{ .TSName }} !== undefined) for (const element of {{ .TSName }}) params.append('{{ .Name }}', String(element));
{{- else }}
if ({{ .TSName }} !== undefined) params.set('{{ .Name }}', String({{ .TSName }}));
{{- end }}
{{- end }}
const qs = params.toString();
const path = {{ buildPath $op.Path $op.PathParams }} + (qs ? `?${qs}` : '');
Expand Down
Loading