gsc: handle stale Search Console grants during property listing (#241)
This commit is contained in:
parent
8ea55a3e77
commit
cc5d6bc632
@ -43,6 +43,16 @@ export function SearchConsoleConnectionCard({
|
|||||||
queryFn: () => listGscSites({ data: { projectId } }),
|
queryFn: () => listGscSites({ data: { projectId } }),
|
||||||
enabled: Boolean(showPicker && !selfHostedNeedsSetup),
|
enabled: Boolean(showPicker && !selfHostedNeedsSetup),
|
||||||
});
|
});
|
||||||
|
const requiresReconnect = Boolean(sitesQuery.data?.requiresReconnect);
|
||||||
|
|
||||||
|
React.useEffect(() => {
|
||||||
|
if (!requiresReconnect) return;
|
||||||
|
|
||||||
|
void queryClient.invalidateQueries({
|
||||||
|
queryKey: ["gscConnection", projectId],
|
||||||
|
});
|
||||||
|
void queryClient.invalidateQueries({ queryKey: GRANT_STATUS_KEY });
|
||||||
|
}, [requiresReconnect, queryClient, projectId]);
|
||||||
|
|
||||||
const setSiteMutation = useMutation({
|
const setSiteMutation = useMutation({
|
||||||
mutationFn: (siteUrl: string) =>
|
mutationFn: (siteUrl: string) =>
|
||||||
@ -105,7 +115,7 @@ export function SearchConsoleConnectionCard({
|
|||||||
) : showPicker ? (
|
) : showPicker ? (
|
||||||
<SitePicker
|
<SitePicker
|
||||||
loading={sitesQuery.isLoading}
|
loading={sitesQuery.isLoading}
|
||||||
error={sitesQuery.isError}
|
error={sitesQuery.isError || requiresReconnect}
|
||||||
sites={sitesQuery.data?.sites ?? []}
|
sites={sitesQuery.data?.sites ?? []}
|
||||||
selectedSiteUrl={selectedSiteUrl}
|
selectedSiteUrl={selectedSiteUrl}
|
||||||
onSelect={setSelectedSiteUrl}
|
onSelect={setSelectedSiteUrl}
|
||||||
|
|||||||
@ -68,6 +68,15 @@ function GscConnect({ projectId }: { projectId: string }) {
|
|||||||
queryFn: () => listGscSites({ data: { projectId } }),
|
queryFn: () => listGscSites({ data: { projectId } }),
|
||||||
enabled: hasGrant && !connected && !needsSetup,
|
enabled: hasGrant && !connected && !needsSetup,
|
||||||
});
|
});
|
||||||
|
const requiresReconnect = Boolean(sitesQuery.data?.requiresReconnect);
|
||||||
|
|
||||||
|
React.useEffect(() => {
|
||||||
|
if (!requiresReconnect) return;
|
||||||
|
|
||||||
|
void queryClient.invalidateQueries({
|
||||||
|
queryKey: ["gscConnection", projectId],
|
||||||
|
});
|
||||||
|
}, [requiresReconnect, queryClient, projectId]);
|
||||||
|
|
||||||
const setSiteMutation = useMutation({
|
const setSiteMutation = useMutation({
|
||||||
mutationFn: (siteUrl: string) =>
|
mutationFn: (siteUrl: string) =>
|
||||||
@ -107,7 +116,7 @@ function GscConnect({ projectId }: { projectId: string }) {
|
|||||||
return (
|
return (
|
||||||
<SitePicker
|
<SitePicker
|
||||||
loading={sitesQuery.isLoading}
|
loading={sitesQuery.isLoading}
|
||||||
error={sitesQuery.isError}
|
error={sitesQuery.isError || requiresReconnect}
|
||||||
sites={sitesQuery.data?.sites ?? []}
|
sites={sitesQuery.data?.sites ?? []}
|
||||||
selectedSiteUrl={selectedSiteUrl}
|
selectedSiteUrl={selectedSiteUrl}
|
||||||
onSelect={setSelectedSiteUrl}
|
onSelect={setSelectedSiteUrl}
|
||||||
|
|||||||
@ -1,20 +1,41 @@
|
|||||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
|
|
||||||
const mocks = vi.hoisted(() => ({
|
const mocks = vi.hoisted(() => {
|
||||||
|
class GscApiError extends Error {
|
||||||
|
constructor(
|
||||||
|
public readonly status: number,
|
||||||
|
message: string,
|
||||||
|
) {
|
||||||
|
super(message);
|
||||||
|
this.name = "GscApiError";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
class GscTokenError extends Error {
|
||||||
|
constructor(message = "token unavailable") {
|
||||||
|
super(message);
|
||||||
|
this.name = "GscTokenError";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
listSites: vi.fn(),
|
listSites: vi.fn(),
|
||||||
upsert: vi.fn(),
|
upsert: vi.fn(),
|
||||||
getByProjectId: vi.fn(),
|
getByProjectId: vi.fn(),
|
||||||
deleteByProjectId: vi.fn(),
|
deleteByProjectId: vi.fn(),
|
||||||
existsForConnector: vi.fn(),
|
existsForConnector: vi.fn(),
|
||||||
dbDelete: vi.fn(() => ({ where: vi.fn().mockResolvedValue(undefined) })),
|
dbDelete: vi.fn(() => ({ where: vi.fn().mockResolvedValue(undefined) })),
|
||||||
}));
|
GscApiError,
|
||||||
|
GscTokenError,
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
vi.mock("cloudflare:workers", () => ({ env: {} }));
|
vi.mock("cloudflare:workers", () => ({ env: {} }));
|
||||||
vi.mock("@/db", () => ({ db: { delete: mocks.dbDelete } }));
|
vi.mock("@/db", () => ({ db: { delete: mocks.dbDelete } }));
|
||||||
vi.mock("@/server/lib/gscClient", () => ({
|
vi.mock("@/server/lib/gscClient", () => ({
|
||||||
createGscClient: () => ({ listSites: mocks.listSites }),
|
createGscClient: () => ({ listSites: mocks.listSites }),
|
||||||
GscApiError: class extends Error {},
|
GscApiError: mocks.GscApiError,
|
||||||
GscTokenError: class extends Error {},
|
GscTokenError: mocks.GscTokenError,
|
||||||
}));
|
}));
|
||||||
vi.mock("@/server/features/gsc/repositories/GscConnectionRepository", () => ({
|
vi.mock("@/server/features/gsc/repositories/GscConnectionRepository", () => ({
|
||||||
GscConnectionRepository: {
|
GscConnectionRepository: {
|
||||||
@ -82,6 +103,73 @@ describe("GscService.setSite", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("GscService.listSitesForUserWithGrantStatus", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
mocks.listSites.mockReset();
|
||||||
|
mocks.dbDelete.mockClear();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns available sites when the grant is healthy", async () => {
|
||||||
|
mocks.listSites.mockResolvedValue([
|
||||||
|
{ siteUrl: "https://x/", permissionLevel: "siteOwner" },
|
||||||
|
]);
|
||||||
|
const { GscService } = await import("./GscService");
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
GscService.listSitesForUserWithGrantStatus("u1"),
|
||||||
|
).resolves.toEqual({
|
||||||
|
sites: [{ siteUrl: "https://x/", permissionLevel: "siteOwner" }],
|
||||||
|
requiresReconnect: false,
|
||||||
|
});
|
||||||
|
expect(mocks.dbDelete).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("unlinks the dead grant and asks for reconnect when no token can be minted", async () => {
|
||||||
|
mocks.listSites.mockRejectedValue(new mocks.GscTokenError());
|
||||||
|
const { GscService } = await import("./GscService");
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
GscService.listSitesForUserWithGrantStatus("u1"),
|
||||||
|
).resolves.toEqual({ sites: [], requiresReconnect: true });
|
||||||
|
expect(mocks.dbDelete).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("asks for reconnect without unlinking on a 403 (revoked vs. quota is ambiguous)", async () => {
|
||||||
|
mocks.listSites.mockRejectedValue(
|
||||||
|
new mocks.GscApiError(403, "Search Console denied access"),
|
||||||
|
);
|
||||||
|
const { GscService } = await import("./GscService");
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
GscService.listSitesForUserWithGrantStatus("u1"),
|
||||||
|
).resolves.toEqual({ sites: [], requiresReconnect: true });
|
||||||
|
expect(mocks.dbDelete).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("asks for reconnect without unlinking on a 401", async () => {
|
||||||
|
mocks.listSites.mockRejectedValue(
|
||||||
|
new mocks.GscApiError(401, "unauthenticated"),
|
||||||
|
);
|
||||||
|
const { GscService } = await import("./GscService");
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
GscService.listSitesForUserWithGrantStatus("u1"),
|
||||||
|
).resolves.toEqual({ sites: [], requiresReconnect: true });
|
||||||
|
expect(mocks.dbDelete).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps non-auth GSC API errors reportable", async () => {
|
||||||
|
const rateLimit = new mocks.GscApiError(429, "slow down");
|
||||||
|
mocks.listSites.mockRejectedValue(rateLimit);
|
||||||
|
const { GscService } = await import("./GscService");
|
||||||
|
|
||||||
|
await expect(GscService.listSitesForUserWithGrantStatus("u1")).rejects.toBe(
|
||||||
|
rateLimit,
|
||||||
|
);
|
||||||
|
expect(mocks.dbDelete).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("GscService.disconnect", () => {
|
describe("GscService.disconnect", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
mocks.getByProjectId.mockReset();
|
mocks.getByProjectId.mockReset();
|
||||||
|
|||||||
@ -5,6 +5,7 @@ import { GSC_OAUTH_PROVIDER_ID } from "@/shared/gsc";
|
|||||||
import { AppError } from "@/server/lib/errors";
|
import { AppError } from "@/server/lib/errors";
|
||||||
import {
|
import {
|
||||||
createGscClient,
|
createGscClient,
|
||||||
|
GscApiError,
|
||||||
GscTokenError,
|
GscTokenError,
|
||||||
type GscSite,
|
type GscSite,
|
||||||
type UrlInspectionResult,
|
type UrlInspectionResult,
|
||||||
@ -31,6 +32,11 @@ type GscPerformanceResult = {
|
|||||||
rows: GscSearchAnalyticsRow[];
|
rows: GscSearchAnalyticsRow[];
|
||||||
};
|
};
|
||||||
|
|
||||||
|
type GscSiteListResult = {
|
||||||
|
sites: GscSite[];
|
||||||
|
requiresReconnect: boolean;
|
||||||
|
};
|
||||||
|
|
||||||
/** Thrown when a project has no connected GSC property. */
|
/** Thrown when a project has no connected GSC property. */
|
||||||
export class GscNotConnectedError extends Error {
|
export class GscNotConnectedError extends Error {
|
||||||
constructor(public readonly projectId: string) {
|
constructor(public readonly projectId: string) {
|
||||||
@ -64,6 +70,44 @@ async function listSitesForUser(userId: string): Promise<GscSite[]> {
|
|||||||
return createGscClient({ userId }).listSites();
|
return createGscClient({ userId }).listSites();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Expected ways a stored grant fails to reach Search Console: no token could be
|
||||||
|
* minted (refresh token revoked or expired), or Google rejected the call
|
||||||
|
* (401/403). These surface a reconnect prompt instead of being routed through
|
||||||
|
* error tracking. Other statuses (429, 5xx) are genuine faults and propagate. */
|
||||||
|
function isExpectedGrantFailure(error: unknown): boolean {
|
||||||
|
if (error instanceof GscTokenError) return true;
|
||||||
|
return (
|
||||||
|
error instanceof GscApiError &&
|
||||||
|
(error.status === 401 || error.status === 403)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** List properties for the picker UI. When the stored grant can't currently
|
||||||
|
* reach GSC, return a reconnect signal instead of throwing, so an expected
|
||||||
|
* external-auth failure doesn't land in error tracking.
|
||||||
|
*
|
||||||
|
* Only a GscTokenError unlinks the stored grant — the one unambiguous "this
|
||||||
|
* grant is dead" signal (Better Auth couldn't mint/refresh a token, i.e. the
|
||||||
|
* user revoked access or the refresh token expired). A bare 401/403 from
|
||||||
|
* sites.list is left in place: Search Console also returns 403 for quota/rate
|
||||||
|
* limits, so destroying the grant there would force needless reconnects across
|
||||||
|
* every project on it. Reconnecting re-upserts the grant either way. */
|
||||||
|
async function listSitesForUserWithGrantStatus(
|
||||||
|
userId: string,
|
||||||
|
): Promise<GscSiteListResult> {
|
||||||
|
try {
|
||||||
|
return { sites: await listSitesForUser(userId), requiresReconnect: false };
|
||||||
|
} catch (error) {
|
||||||
|
if (!isExpectedGrantFailure(error)) {
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
|
if (error instanceof GscTokenError) {
|
||||||
|
await unlinkUserGrant(userId);
|
||||||
|
}
|
||||||
|
return { sites: [], requiresReconnect: true };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/** Map a verified property to a project. Rejects unverified properties and
|
/** Map a verified property to a project. Rejects unverified properties and
|
||||||
* properties not present on the connector's grant. */
|
* properties not present on the connector's grant. */
|
||||||
async function setSite(input: {
|
async function setSite(input: {
|
||||||
@ -210,7 +254,7 @@ async function inspectUrls(input: {
|
|||||||
export const GscService = {
|
export const GscService = {
|
||||||
getConnection,
|
getConnection,
|
||||||
userHasGrant,
|
userHasGrant,
|
||||||
listSitesForUser,
|
listSitesForUserWithGrantStatus,
|
||||||
setSite,
|
setSite,
|
||||||
disconnect,
|
disconnect,
|
||||||
getPerformance,
|
getPerformance,
|
||||||
|
|||||||
@ -55,12 +55,13 @@ export const listGscSites = createServerFn({ method: "POST" })
|
|||||||
.middleware(requireProjectContext)
|
.middleware(requireProjectContext)
|
||||||
.inputValidator((data: unknown) => projectScopedSchema.parse(data))
|
.inputValidator((data: unknown) => projectScopedSchema.parse(data))
|
||||||
.handler(async ({ context }) => {
|
.handler(async ({ context }) => {
|
||||||
const [sites, connection] = await Promise.all([
|
const [siteList, connection] = await Promise.all([
|
||||||
GscService.listSitesForUser(context.userId),
|
GscService.listSitesForUserWithGrantStatus(context.userId),
|
||||||
GscService.getConnection(context.projectId),
|
GscService.getConnection(context.projectId),
|
||||||
]);
|
]);
|
||||||
return {
|
return {
|
||||||
sites: sites.map((s) => ({
|
requiresReconnect: siteList.requiresReconnect,
|
||||||
|
sites: siteList.sites.map((s) => ({
|
||||||
siteUrl: s.siteUrl,
|
siteUrl: s.siteUrl,
|
||||||
permissionLevel: s.permissionLevel,
|
permissionLevel: s.permissionLevel,
|
||||||
selectable: s.permissionLevel !== "siteUnverifiedUser",
|
selectable: s.permissionLevel !== "siteUnverifiedUser",
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user