fix: extract the param row constant and cover the manager bridge (#2018)
Copilot round 3 (both findings suppressed; both valid). - The a11y fix in the previous commit added `align="flex-start"` to the row's `Group`, giving it two static layout props and so tripping the .withProps() rule in AGENTS.md — which applies to a single-use element too. Extracted as `AuthorizationParamRow`. (The sibling `KeyValueRows` Group carries one static prop and stays inline, correctly.) - Added OAuthManager coverage for the config→provider bridge. The point is sharp: the provider test proves the merge and the runner test proves the options object, but nothing crossed the seam in `createOAuthProvider`, so deleting the `authorizationParams:` line left the feature dead with every test green. Verified by doing exactly that — both new cases now fail with `expected null to be 'corp-idp'` — then restoring it. The second case also pins the reserved-key drop through the manager path. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
55734f3776
commit
f36278538e
2 changed files with 56 additions and 2 deletions
|
|
@ -202,6 +202,13 @@ const FieldDescription = Text.withProps({
|
||||||
c: "dimmed",
|
c: "dimmed",
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// `align="flex-start"` keeps the key/value/remove controls top-aligned when the
|
||||||
|
// key input grows an error message, so the row doesn't jump as it appears.
|
||||||
|
const AuthorizationParamRow = Group.withProps({
|
||||||
|
grow: true,
|
||||||
|
align: "flex-start",
|
||||||
|
});
|
||||||
|
|
||||||
const ClearStoredOAuthButton = Button.withProps({
|
const ClearStoredOAuthButton = Button.withProps({
|
||||||
variant: "light",
|
variant: "light",
|
||||||
color: "red",
|
color: "red",
|
||||||
|
|
@ -288,7 +295,7 @@ function AuthorizationParamRows({
|
||||||
{params.map((param, index) => {
|
{params.map((param, index) => {
|
||||||
const rowLabel = param.key.trim() || `row ${index + 1}`;
|
const rowLabel = param.key.trim() || `row ${index + 1}`;
|
||||||
return (
|
return (
|
||||||
<Group key={index} grow align="flex-start">
|
<AuthorizationParamRow key={index}>
|
||||||
<ClearableTextInput
|
<ClearableTextInput
|
||||||
aria-label={`Authorization parameter name, ${rowLabel}`}
|
aria-label={`Authorization parameter name, ${rowLabel}`}
|
||||||
placeholder="Parameter (e.g. kc_idp_hint)"
|
placeholder="Parameter (e.g. kc_idp_hint)"
|
||||||
|
|
@ -324,7 +331,7 @@ function AuthorizationParamRows({
|
||||||
>
|
>
|
||||||
X
|
X
|
||||||
</RemoveIcon>
|
</RemoveIcon>
|
||||||
</Group>
|
</AuthorizationParamRow>
|
||||||
);
|
);
|
||||||
})}
|
})}
|
||||||
</>
|
</>
|
||||||
|
|
|
||||||
|
|
@ -1574,6 +1574,53 @@ describe("OAuthManager", () => {
|
||||||
const provider = await manager.createOAuthProviderForTransport();
|
const provider = await manager.createOAuthProviderForTransport();
|
||||||
expect(provider.constructor.name).toBe("EmaTransportOAuthProvider");
|
expect(provider.constructor.name).toBe("EmaTransportOAuthProvider");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// #2018 — the manager→provider bridge for the custom authorization
|
||||||
|
// parameters. The provider test proves the merge and the runner test proves
|
||||||
|
// the options object, but neither crosses this seam: without this case,
|
||||||
|
// deleting `authorizationParams:` from `createOAuthProvider` would leave the
|
||||||
|
// feature dead with every other test still green.
|
||||||
|
it("forwards configured authorizationParams to the provider it builds", async () => {
|
||||||
|
const params = createMockParams();
|
||||||
|
const manager = new OAuthManager(params);
|
||||||
|
manager.setOAuthConfig({
|
||||||
|
authorizationParams: { kc_idp_hint: "corp-idp", prompt: "login" },
|
||||||
|
});
|
||||||
|
|
||||||
|
const provider = await manager.createOAuthProviderForTransport();
|
||||||
|
provider.redirectToAuthorization(
|
||||||
|
new URL("https://as.example.com/authorize?client_id=abc&state=xyz"),
|
||||||
|
);
|
||||||
|
|
||||||
|
const navigate = params.initialConfig.navigation
|
||||||
|
?.navigateToAuthorization as ReturnType<typeof vi.fn>;
|
||||||
|
const navigated = navigate.mock.calls[0]?.[0] as URL;
|
||||||
|
expect(navigated.searchParams.get("kc_idp_hint")).toBe("corp-idp");
|
||||||
|
expect(navigated.searchParams.get("prompt")).toBe("login");
|
||||||
|
// The flow's own parameters are untouched.
|
||||||
|
expect(navigated.searchParams.get("state")).toBe("xyz");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("drops a reserved key configured on the manager", async () => {
|
||||||
|
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||||
|
const params = createMockParams();
|
||||||
|
const manager = new OAuthManager(params);
|
||||||
|
manager.setOAuthConfig({
|
||||||
|
authorizationParams: { state: "spoofed", kc_idp_hint: "corp-idp" },
|
||||||
|
});
|
||||||
|
|
||||||
|
const provider = await manager.createOAuthProviderForTransport();
|
||||||
|
provider.redirectToAuthorization(
|
||||||
|
new URL("https://as.example.com/authorize?client_id=abc&state=xyz"),
|
||||||
|
);
|
||||||
|
|
||||||
|
const navigate = params.initialConfig.navigation
|
||||||
|
?.navigateToAuthorization as ReturnType<typeof vi.fn>;
|
||||||
|
const navigated = navigate.mock.calls[0]?.[0] as URL;
|
||||||
|
expect(navigated.searchParams.get("state")).toBe("xyz");
|
||||||
|
expect(navigated.searchParams.get("kc_idp_hint")).toBe("corp-idp");
|
||||||
|
expect(warn).toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("getOAuthState (enterprise managed / scope)", () => {
|
describe("getOAuthState (enterprise managed / scope)", () => {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue