fix(auth): stabilize SSO option loading
This commit is contained in:
parent
9c3bd44a1b
commit
1e91cfba9d
5 changed files with 109 additions and 12 deletions
|
|
@ -50,6 +50,8 @@ export const AuthLinkPrompt = ({ prompt, to, label }: { prompt: string; to: stri
|
||||||
</p>
|
</p>
|
||||||
);
|
);
|
||||||
|
|
||||||
|
export const AuthOptionsLoading = () => <div className="h-9 w-full animate-pulse rounded-md bg-muted/60" aria-hidden="true" />;
|
||||||
|
|
||||||
const AuthPageLayout = ({ chip, title, subtitle, hideExplore, children }: Props) => {
|
const AuthPageLayout = ({ chip, title, subtitle, hideExplore, children }: Props) => {
|
||||||
const t = useTranslate();
|
const t = useTranslate();
|
||||||
const { generalSetting, profile } = useInstance();
|
const { generalSetting, profile } = useInstance();
|
||||||
|
|
|
||||||
|
|
@ -12,12 +12,15 @@ const EMPTY_LIST: IdentityProvider[] = [];
|
||||||
|
|
||||||
// Hook to fetch the configured identity providers. Pass `enabled: false` on
|
// Hook to fetch the configured identity providers. Pass `enabled: false` on
|
||||||
// pages/branches that never render provider buttons to skip the request.
|
// pages/branches that never render provider buttons to skip the request.
|
||||||
export function useIdentityProviderList(enabled = true): IdentityProvider[] {
|
export function useIdentityProviderList(enabled = true) {
|
||||||
const { data } = useQuery({
|
const { data, isLoading } = useQuery({
|
||||||
queryKey: identityProviderKeys.list(),
|
queryKey: identityProviderKeys.list(),
|
||||||
queryFn: async () => (await identityProviderServiceClient.listIdentityProviders({})).identityProviders,
|
queryFn: async () => (await identityProviderServiceClient.listIdentityProviders({})).identityProviders,
|
||||||
staleTime: 60_000,
|
staleTime: 60_000,
|
||||||
enabled,
|
enabled,
|
||||||
});
|
});
|
||||||
return data ?? EMPTY_LIST;
|
return {
|
||||||
|
identityProviderList: data ?? EMPTY_LIST,
|
||||||
|
isLoading,
|
||||||
|
};
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,6 @@
|
||||||
import { ArrowRightIcon, LockIcon } from "lucide-react";
|
import { ArrowRightIcon, LockIcon } from "lucide-react";
|
||||||
import { Link, useSearchParams } from "react-router-dom";
|
import { Link, useSearchParams } from "react-router-dom";
|
||||||
import AuthPageLayout, { AuthEmptyState, AuthLinkPrompt } from "@/components/AuthPageLayout";
|
import AuthPageLayout, { AuthEmptyState, AuthLinkPrompt, AuthOptionsLoading } from "@/components/AuthPageLayout";
|
||||||
import IdentityProviderButtons from "@/components/IdentityProviderButtons";
|
import IdentityProviderButtons from "@/components/IdentityProviderButtons";
|
||||||
import PasswordSignInForm from "@/components/PasswordSignInForm";
|
import PasswordSignInForm from "@/components/PasswordSignInForm";
|
||||||
import { Separator } from "@/components/ui/separator";
|
import { Separator } from "@/components/ui/separator";
|
||||||
|
|
@ -14,18 +14,21 @@ const SignIn = () => {
|
||||||
const t = useTranslate();
|
const t = useTranslate();
|
||||||
const { generalSetting: instanceGeneralSetting } = useInstance();
|
const { generalSetting: instanceGeneralSetting } = useInstance();
|
||||||
const [searchParams] = useSearchParams();
|
const [searchParams] = useSearchParams();
|
||||||
const identityProviderList = useIdentityProviderList();
|
const { identityProviderList, isLoading: identityProvidersLoading } = useIdentityProviderList();
|
||||||
const redirectTarget = getSafeRedirectPath(searchParams.get(AUTH_REDIRECT_PARAM));
|
const redirectTarget = getSafeRedirectPath(searchParams.get(AUTH_REDIRECT_PARAM));
|
||||||
const signUpPath = appendSearchParams(ROUTES.AUTH_SIGNUP, searchParams);
|
const signUpPath = appendSearchParams(ROUTES.AUTH_SIGNUP, searchParams);
|
||||||
|
|
||||||
const passwordAuthAllowed = !instanceGeneralSetting.disallowPasswordAuth;
|
const passwordAuthAllowed = !instanceGeneralSetting.disallowPasswordAuth;
|
||||||
const hasIdentityProviders = identityProviderList.length > 0;
|
const hasIdentityProviders = identityProviderList.length > 0;
|
||||||
|
|
||||||
const subtitle = passwordAuthAllowed || hasIdentityProviders ? t("auth.welcome-back") : undefined;
|
// Shared by the subtitle and the body branch so they can't disagree.
|
||||||
|
const showAuthOptions = identityProvidersLoading || passwordAuthAllowed || hasIdentityProviders;
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<AuthPageLayout title={t("common.sign-in")} subtitle={subtitle}>
|
<AuthPageLayout title={t("common.sign-in")} subtitle={showAuthOptions ? t("auth.welcome-back") : undefined}>
|
||||||
{passwordAuthAllowed || hasIdentityProviders ? (
|
{identityProvidersLoading ? (
|
||||||
|
<AuthOptionsLoading />
|
||||||
|
) : showAuthOptions ? (
|
||||||
<>
|
<>
|
||||||
{hasIdentityProviders && <IdentityProviderButtons identityProviderList={identityProviderList} redirectTarget={redirectTarget} />}
|
{hasIdentityProviders && <IdentityProviderButtons identityProviderList={identityProviderList} redirectTarget={redirectTarget} />}
|
||||||
{hasIdentityProviders && passwordAuthAllowed && (
|
{hasIdentityProviders && passwordAuthAllowed && (
|
||||||
|
|
|
||||||
|
|
@ -5,7 +5,7 @@ import { useState } from "react";
|
||||||
import { toast } from "react-hot-toast";
|
import { toast } from "react-hot-toast";
|
||||||
import { useSearchParams } from "react-router-dom";
|
import { useSearchParams } from "react-router-dom";
|
||||||
import { setAccessToken } from "@/auth-state";
|
import { setAccessToken } from "@/auth-state";
|
||||||
import AuthPageLayout, { AuthChip, AuthEmptyState, AuthLinkPrompt } from "@/components/AuthPageLayout";
|
import AuthPageLayout, { AuthChip, AuthEmptyState, AuthLinkPrompt, AuthOptionsLoading } from "@/components/AuthPageLayout";
|
||||||
import CredentialFields from "@/components/CredentialFields";
|
import CredentialFields from "@/components/CredentialFields";
|
||||||
import IdentityProviderButtons from "@/components/IdentityProviderButtons";
|
import IdentityProviderButtons from "@/components/IdentityProviderButtons";
|
||||||
import { Button } from "@/components/ui/button";
|
import { Button } from "@/components/ui/button";
|
||||||
|
|
@ -37,7 +37,9 @@ const SignUp = () => {
|
||||||
const registrationOpen = !instanceGeneralSetting.disallowUserRegistration;
|
const registrationOpen = !instanceGeneralSetting.disallowUserRegistration;
|
||||||
const needsSetup = profile.needsSetup;
|
const needsSetup = profile.needsSetup;
|
||||||
// Provider buttons only render on the SSO-provisioned branch below; skip the request elsewhere.
|
// Provider buttons only render on the SSO-provisioned branch below; skip the request elsewhere.
|
||||||
const identityProviderList = useIdentityProviderList(!needsSetup && registrationOpen && !passwordAuthAllowed);
|
const { identityProviderList, isLoading: identityProvidersLoading } = useIdentityProviderList(
|
||||||
|
!needsSetup && registrationOpen && !passwordAuthAllowed,
|
||||||
|
);
|
||||||
const hasIdentityProviders = identityProviderList.length > 0;
|
const hasIdentityProviders = identityProviderList.length > 0;
|
||||||
|
|
||||||
const handleFormSubmit = async (e: React.FormEvent<HTMLFormElement>) => {
|
const handleFormSubmit = async (e: React.FormEvent<HTMLFormElement>) => {
|
||||||
|
|
@ -141,9 +143,13 @@ const SignUp = () => {
|
||||||
|
|
||||||
// Password sign-up disallowed: accounts come from the identity provider.
|
// Password sign-up disallowed: accounts come from the identity provider.
|
||||||
if (!passwordAuthAllowed) {
|
if (!passwordAuthAllowed) {
|
||||||
|
// Shared by the subtitle and the body branch so they can't disagree.
|
||||||
|
const showSsoOptions = identityProvidersLoading || hasIdentityProviders;
|
||||||
return (
|
return (
|
||||||
<AuthPageLayout title={t("auth.create-your-account")} subtitle={hasIdentityProviders ? t("auth.sso-signup-tip") : undefined}>
|
<AuthPageLayout title={t("auth.create-your-account")} subtitle={showSsoOptions ? t("auth.sso-signup-tip") : undefined}>
|
||||||
{hasIdentityProviders ? (
|
{identityProvidersLoading ? (
|
||||||
|
<AuthOptionsLoading />
|
||||||
|
) : showSsoOptions ? (
|
||||||
<IdentityProviderButtons identityProviderList={identityProviderList} redirectTarget={redirectTarget} />
|
<IdentityProviderButtons identityProviderList={identityProviderList} redirectTarget={redirectTarget} />
|
||||||
) : (
|
) : (
|
||||||
<AuthEmptyState
|
<AuthEmptyState
|
||||||
|
|
|
||||||
83
web/tests/sign-in-page.test.tsx
Normal file
83
web/tests/sign-in-page.test.tsx
Normal file
|
|
@ -0,0 +1,83 @@
|
||||||
|
import { render, screen } from "@testing-library/react";
|
||||||
|
import { MemoryRouter } from "react-router-dom";
|
||||||
|
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
|
import SignIn from "@/pages/SignIn";
|
||||||
|
|
||||||
|
const state = vi.hoisted(() => ({
|
||||||
|
generalSetting: {
|
||||||
|
disallowPasswordAuth: true,
|
||||||
|
},
|
||||||
|
identityProviders: {
|
||||||
|
identityProviderList: [] as { name: string; title: string }[],
|
||||||
|
isLoading: true,
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/contexts/InstanceContext", () => ({
|
||||||
|
useInstance: () => ({
|
||||||
|
generalSetting: state.generalSetting,
|
||||||
|
profile: { instanceUrl: "" },
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/hooks/useIdentityProviderQueries", () => ({
|
||||||
|
useIdentityProviderList: () => state.identityProviders,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/components/AuthFooter", () => ({ default: () => null }));
|
||||||
|
|
||||||
|
vi.mock("@/components/IdentityProviderButtons", () => ({
|
||||||
|
default: ({ identityProviderList }: { identityProviderList: { title: string }[] }) => (
|
||||||
|
<div data-testid="identity-providers">{identityProviderList.map((provider) => provider.title).join(", ")}</div>
|
||||||
|
),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/components/PasswordSignInForm", () => ({
|
||||||
|
default: () => <div data-testid="password-sign-in" />,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/utils/i18n", () => ({
|
||||||
|
useTranslate: () => (key: string) => key,
|
||||||
|
}));
|
||||||
|
|
||||||
|
const renderPage = () =>
|
||||||
|
render(
|
||||||
|
<MemoryRouter>
|
||||||
|
<SignIn />
|
||||||
|
</MemoryRouter>,
|
||||||
|
);
|
||||||
|
|
||||||
|
describe("<SignIn>", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
state.generalSetting.disallowPasswordAuth = true;
|
||||||
|
state.identityProviders.identityProviderList = [];
|
||||||
|
state.identityProviders.isLoading = true;
|
||||||
|
});
|
||||||
|
|
||||||
|
it("waits for identity providers before choosing the sign-in method", () => {
|
||||||
|
const { container, rerender } = renderPage();
|
||||||
|
|
||||||
|
expect(container.querySelector(".animate-pulse")).toBeInTheDocument();
|
||||||
|
expect(screen.queryByText("auth.signin-unavailable-title")).not.toBeInTheDocument();
|
||||||
|
expect(screen.queryByTestId("password-sign-in")).not.toBeInTheDocument();
|
||||||
|
expect(screen.queryByTestId("identity-providers")).not.toBeInTheDocument();
|
||||||
|
|
||||||
|
state.identityProviders.identityProviderList = [{ name: "identityProviders/acme", title: "Acme SSO" }];
|
||||||
|
state.identityProviders.isLoading = false;
|
||||||
|
rerender(
|
||||||
|
<MemoryRouter>
|
||||||
|
<SignIn />
|
||||||
|
</MemoryRouter>,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(screen.getByTestId("identity-providers")).toHaveTextContent("Acme SSO");
|
||||||
|
expect(screen.queryByText("auth.signin-unavailable-title")).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows the unavailable state only after an empty provider response", () => {
|
||||||
|
state.identityProviders.isLoading = false;
|
||||||
|
renderPage();
|
||||||
|
|
||||||
|
expect(screen.getByText("auth.signin-unavailable-title")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue