fix(auth)!: parse OAuth clients without a name - #1854
Conversation
Auth omits `client_name` for a client registered without one, so the cast in `OAuthClient.fromJson` threw and every `admin.oauth` call that returns a client failed on the whole response. `clientName` is now a `String?`, matching `OAuthAuthorizedClient.clientName`, which already models the same server value that way.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthrough
ChangesOAuth client nullability
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to OAuth clients without a name can now be parsed without failing admin OAuth operations, while callers are guided to use the client ID as a fallback. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Every
admin.oauthcall that returns a client throwstype 'Null' is not a subtype of type 'String' in type castwhen the client has no name, becauseOAuthClient.fromJsoncastsclient_nameto a non-nullableString.A client name is optional in Auth.
validateClientNameonly rejects names over 1024 characters, and the response field is taggedomitempty(internal/api/oauthserver/handlers.go), so a client registered without one comes back with noclient_namekey at all:{"client_id": "8f2a1c33-3a1e-4f56-9f0b-2d1d5b8a91c4", "client_type": "public", "token_endpoint_auth_method": "none", "registration_type": "dynamic", "redirect_uris": ["https://example.com/callback"], "grant_types": ["authorization_code"], "response_types": ["code"], "created_at": "2025-01-01T00:00:00Z", "updated_at": "2025-01-01T00:00:00Z"}clientNameis now aString?. The same shape reachescreateClient,getClient,updateClientandlistClients, andlistClientsis the worst case, since a single nameless client fails the parse of the whole page.The sibling type already models it this way:
OAuthAuthorizedClient.clientName, which the consent flow parses from the same column, is aString?.Compatibility
This is breaking against v2, where the field is non-nullable, so
MIGRATION.mdgets an entry next to the other OAuth ones.I left
CreateOAuthClientOptions.clientNamerequired. The server accepts a registration without a name, so it could be relaxed too, but that is a wider change and nothing forces it: clients you create through the SDK keep their name. Say the word if you want them to match.Test
test/src/types/oauth_client_test.dartfollows the other type tests: one case for a full response, one for a response withoutclient_name. Onmainthe second fails withand passes with the fix.
The admin OAuth suite could not cover this, because
CreateOAuthClientOptionscannot express a nameless client, so the parse is tested where it lives.I did not run the suites that need a live Auth server this time, since my local stack is down. The analyzer covers those files, so nothing in them stopped compiling, and the change is confined to parsing one field.
Summary by CodeRabbit
New Features
Documentation
Tests