Repository navigation
feat(ui): refresh the component library and add five plugin packages - #173
Merged
Merged
Conversation
Tokens move to the reference palette: a blue primary, a warmer neutral ramp and a slightly larger radius. They are still tokens, so one line of CSS puts any of them back. Field heights do not change. The library was deliberately compacted below stock shadcn and it stays that way, so Button and Input keep h-[30px] at 13px. What they gain is everything other than size: a border you can see before you focus it, a focus ring that is the blue at low opacity rather than a hard outline offset from the field, and aria-invalid styling. AuthCard gets the type scale from the reference design and defaults to left-aligned, which is what every screen in it uses. Separator gains a LabelledSeparator, and OrDivider now uses it and takes a label. The old divider painted bg-card behind its text, which is a guess about what it is sitting on, and it guessed wrong the moment a form was rendered borderless on a page background.
One VerificationCodeInput replaces four hand-assembled InputOTP arrangements. It takes a length and a mode, numeric or alphanumeric, and alphanumeric upper-cases as you type because every code authsome issues is case-insensitive and a lowercase b rendered as B is a bug report waiting to happen. Boxes shrink to fit their column, so an eight character device code still sits inside a 400px card. It also takes focus back when it is re-enabled with an incomplete code. Disabling a field while its code is checked blurs it, and nothing gave the focus back, so the cost landed on exactly the person who had just got their code wrong and now had to click into the boxes again. That fixes MFA and email verification at the same time. PhoneInput is a country picker as an input addon plus a national number that formats itself, on libphonenumber-js. The value handed back is always E.164, which is what the API wants, while what you see is your own country's grouping, which is the only format you can check against the handset in your hand. InputGroup and InputAddon are the general version of that pattern. InputOTP and its slots stay exported for anyone who built their own arrangement. The caret animation they reference is now actually defined.
Two separate things that both answer "who is this". LastUsedAccount is the returning-visitor shortcut: who signed in last on this browser, and how. It is per-browser, in localStorage, written on every successful sign-in and kept for ninety days. Long enough that someone on their own laptop still gets the shortcut, short enough that a library computer stops offering a stranger's name and email to whoever sits down next. Signing out does not clear it, because being remembered is the point. It is a convenience and never an authorization. Nothing in that record is trusted and the person it names still authenticates. It reads through useSyncExternalStore so the server renders nothing and the chip appears after hydration, rather than setting state from an effect. useLastSignIn and AccountSummaryCard are the other one: server truth about when and where you last signed in. twoStepEnabled is a tri-state, and null hides the row rather than claiming two-step is off when nothing has answered the question. That is a lie you do not want in a security surface.
Three endpoints the UI needed and the server did not have. GET /v1/orgs/invitations/:inviteToken returns an invitation by its token, unauthenticated, because whoever is deciding whether to accept has not signed in yet and often has no account at all. It returns a composed view rather than the stored row: the row has no organization name and no inviter name, which are the two things the page needs, and it carries the token and the raw invited address, which the page does not. The address is masked. Expired, consumed and never-existed all answer 404, so a list of guessed tokens learns nothing. The path parameter is :inviteToken rather than :token because sdkgen names a path parameter and the auth argument from the same source, and :token generated getInvitation(token, token). GET /v1/mfa/enrollment answers whether two-step is on for the caller, with which factors and how many recovery codes are left. No secret, no code, nothing about anyone else. SCIM gains nine admin routes under /v1 for configurations and tokens. The service methods existed and none of them were reachable over HTTP. They authenticate as an organization owner or admin, not as a provisioning bearer token, which is why they are not under the SCIM base path: an identity provider holding a token has no business minting more. A caller who is not a member gets 404 rather than 403, and an app-level config, which has no organization to check a role against, is not reachable here at all.
The plugin mechanism landed in #168 with one package, subscription. These are the other five server plugins with a user-facing API: phone, consent, oauth2, organization and scim. Each exposes "." for the plugin definition, which is framework-free, and "./react" for its hooks and components, so a host that never touches React does not load them and a host that installs none of these ships none of their code. The components live in the plugin package next to the definition rather than in ui-components, because usePlugin looks a plugin up by its definition object and a component calling it would have to import one. The dependency runs one way. Two of these are easy to mix up. consent is GDPR purpose consent, keyed on a purpose string. oauth2 is the "App X wants access" screen, which belongs to oauth2provider. They are different plugins and different packages. AuthManager gains adoptSession, because phoneAuthVerify returns a perfectly good session and nothing could take it. Anything that mints a session out of band can use it now. Both test harnesses can register plugins. Before this the storybook mock made usePlugin a TypeError and test-support made it throw, which blocked every plugin component from being storied or tested at all.
components.mdx gains the session components, the verification code input, the phone input and the input group, and its CSS variable example now matches the tokens the library actually ships. plugins.mdx gains a section per package: install, register, hook, component, and the one caveat each has. The page already existed and covered the mechanism, so this is the half that was missing. playground.mdx was claiming 6 categories and 31 story files. It is 9 and 49 now.
The SCIM admin response types were called SCIMConfigListResponse and so on, which read as scim.SCIMConfigListResponse from outside and stutter. They are ConfigListResponse, TokenListResponse, TokenCreatedResponse and StatusResponse now, and the clients are regenerated for it. sdkgen keeps StatusResponse apart from the organization plugin's by itself. tokenForCaller returned a configuration nobody read. It returns the token ID and an error. The two org-admin handlers now name the parse error separately. Reusing err makes gocritic call the second assignment sloppy and shadowing it makes govet call it a shadow, so neither linter accepts the other's fix and distinct names are the way out. There is a comment saying so, since the obvious cleanup is to undo it. The MFA enrollment handler never read its request, so the parameter is _. It takes nothing: the enrollment it reports is always the caller's.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Codecov was right: three endpoints and nine SCIM admin routes landed with no Go tests at all, and admin_handlers.go was at zero. The SCIM tests spend most of their effort on the authorization boundary, because that is where this surface can actually hurt. An owner and an admin may manage a connection and a plain member may not. A non-member gets 404 rather than 403, and the test asserts that a real organization and an invented one answer identically, so probing IDs tells an outsider nothing. An admin of one organization cannot reach another's config, and cannot rotate or revoke a token by ID either, which is the check that stops a token ID alone being enough. An app-level config stays unreachable. The invitation preview tests pin what the response does not say as much as what it does: the address is masked, the token never appears in the body, and expired, accepted, declined and never-existed are the same 404. maskEmail and inviterName are tested directly through an export_test seam, including the one-character local part that would otherwise panic on a slice. The MFA tests pin that an unverified enrollment does not make two-step on, that the recovery count excludes used codes, that the TOTP secret never reaches the response, and that one user's enrollment says nothing about another's. admin_handlers.go goes from 0% to between 67% and 100% per function. What is left is the service-nil guards and the store-failure branches, which need fault injection to reach.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refreshes the React component library to match the reference auth screens, adds the two things that were missing from it, and fills in the client plugin packages that #168 left room for.
What you get
A refreshed look, at the same density. Tokens move to the reference palette: a blue primary, a warmer neutral ramp, a slightly larger radius. Field heights do not change. The library was deliberately compacted below stock shadcn and it stays that way, so
ButtonandInputkeeph-[30px]at 13px. What they gain is everything other than size: a border you can see before you focus it, a focus ring that is the blue at low opacity, not a hard outline floating off the edge of the field, andaria-invalidstyling. Every token is still a token, so one line of CSS puts any of this back.VerificationCodeInput, replacing four hand-assembledInputOTParrangements. It takes a length and a mode, numeric or alphanumeric, and alphanumeric upper-cases as you type. Boxes shrink to fit their column, so an eight-character device code still sits inside a 400px card.InputOTPand its slots stay exported for anyone who built their own arrangement.PhoneInput, a country picker as an input addon plus a national number that formats itself. The value handed back is always E.164, which is what the API wants and what a database should hold, while what you see is your own country's grouping.InputGroupandInputAddonare the general version of that pattern, for currency symbols, units, suffix buttons.LastUsedAccount, the returning-visitor chip: who signed in last on this browser, and how. It is per-browser, inlocalStorage, kept for ninety days. Long enough that someone on their own laptop still gets the shortcut, short enough that a library computer stops offering a stranger's name and email to whoever sits down next. Signing out does not clear it, because being remembered is the point.useLastSignInandAccountSummaryCard, which are the other half of that question and are server truth rather than browser memory.Five plugin packages: phone, consent, oauth2, organization and scim. Each exposes
.for the plugin definition, which is framework-free, and./reactfor its hooks and components, so a host that installs none of them ships none of their code.Three endpoints the server did not have
GET /v1/orgs/invitations/:inviteTokenreturns an invitation by its token, unauthenticated, because whoever is deciding whether to accept has not signed in yet and often has no account at all. It returns a composed view, because the stored row will not do: it has no organization name and no inviter name, which are the two things the page needs, and it carries the token and the raw invited address, which the page does not. The address is masked, and expired, consumed and never-existed all answer 404 so a list of guessed tokens learns nothing.GET /v1/mfa/enrollmentanswers whether two-step is on for the caller, with which factors and how many recovery codes are left. No secret, no code, nothing about anyone else.SCIM gains nine admin routes for configurations and tokens. The service methods existed and none of them were reachable over HTTP, so connecting an identity provider was not something a dashboard could do.
Worth a reviewer's attention
The SCIM admin authorization. These routes sit under
/v1rather than the SCIM base path on purpose: that group authenticates with a provisioning bearer token, and an identity provider holding one has no business minting more. They require an organization owner or admin instead, checked the same wayplugins/subscription/tenant.godoes it. A caller who is not a member gets 404, not 403, so an outsider probing organization IDs learns nothing. An app-level config, one with noOrgID, has no organization to check a role against and is not reachable here at all. If you want those manageable, that needs a decision about who may.The invitation route's path parameter is
:inviteToken, not:token.sdkgennames a path parameter and the auth argument from the same source, so:tokengeneratedgetInvitation(token: string, token: string), which does not compile. Worth knowing before the next route with a colliding parameter name.Plugin packages own their components. The first cut had them in
ui-componentstaking their data as props. That would have meant every host wiringusePhoneAuthinto<PhoneSignInForm>by hand, for a form whose whole purpose is to save them that.usePluginlooks a plugin up by its definition object, so anything calling it has to import one, andui-componentsimporting five plugin packages is the wrong direction. The dependency runs one way.AuthManager.adoptSessionis new public API on core.phoneAuthVerifyreturns a perfectly good session and nothing could take it, so phone sign-in could not actually sign anyone in. Anything that mints a session out of band can use it now.twoStepEnabledis a tri-state.nullhides the row. Claiming two-step is off when nothing has answered the question would be a different thing from saying nothing. That is a lie you do not want in a security surface, and it is why the MFA endpoint exists at all.On the rebase
This started from #170 and was rebased onto #168, which had already landed a
plugin-subscriptionpackage. Theirs is considerably better developed, with a snapshot model, plans, entitlements and its own components, so mine went and theirs stayed. The subscription section ofplugins.mdxdocuments the API that actually ships.Verification
go build ./...clean,go vetclean on the three changed plugins, and their tests pass. Inui/: lint 11/11, typecheck 20/20, tests 19/19, build 11/11. The docs site typechecks.I drove the phone flow end to end in Storybook, including the wrong-code path, and checked each plugin component renders.
Two things I did not do. The Go endpoints are verified by build, vet and the existing suites, not by a live request against a running server. And the five plugin packages have unit tests for their definitions but no component tests.