fix: close module access lifecycle gaps
This commit is contained in:
@@ -8,7 +8,7 @@
|
||||
- Worktree: D:\Datas\OthersProjects\makelore-module-access-6f2a91c4
|
||||
- Base commit: f7171a471ab1a39380ef666b1e1be9a1f689e43f
|
||||
- Owner: codex
|
||||
- Status: Ready For Integration
|
||||
- Status: In Progress
|
||||
|
||||
## Scope
|
||||
|
||||
@@ -32,17 +32,20 @@
|
||||
- Implemented a shared four-field policy normalizer, Main-owned `/api/auth/me` projection, startup/login/refresh policy hydration, chooser disable state, and direct-route guard.
|
||||
- Bumped the persisted authentication state to schema version 2 so existing installations normalize the new policy field during upgrade; a new login falls back to all-enabled rather than inheriting another account's cached policy.
|
||||
- Disabled Programming routes no longer initialize providers, and all four disabled module families redirect before `MainLayout` or module workspaces mount.
|
||||
- Corrected the first-review lifecycle gaps: Provider initialization now waits for authenticated policy hydration, a terminal current-user 401 clears both Main and Renderer session state, and global `/settings` remains available when Code is disabled.
|
||||
|
||||
## Verification
|
||||
|
||||
- Red: `module-navigation.test.tsx` failed at `toBeDisabled()` while the static module definition remained enabled.
|
||||
- Green: focused Vitest (`module-navigation`, `auth-store`, `auth-routes`, `app-module-provider-gate`) — 58 passed.
|
||||
- Full Vitest — 175 files, 2036 tests passed. The first sandboxed run had one environmental `EPERM` because the test could not create worktree `.tmp`; the unrestricted rerun of the same suite passed completely.
|
||||
- Green after first review corrections: focused Vitest (`module-navigation`, `auth-store`, `auth-routes`, `app-module-provider-gate`) — 64 passed.
|
||||
- Full Vitest — 175 files, 2042 tests passed. An earlier sandboxed run before the review corrections had one environmental `EPERM` because the test could not create worktree `.tmp`; every unrestricted full-suite rerun passed completely.
|
||||
- TypeScript `tsc --noEmit` — passed.
|
||||
- Scoped ESLint for all changed TypeScript/TSX files — passed.
|
||||
- Vite production build — passed (Renderer, Electron Main, and preload); existing chunk-size/dynamic-import warnings remain unchanged.
|
||||
- `git diff --check` — passed.
|
||||
- Electron E2E was not extended because the shared fixture deliberately bypasses authentication and cannot express a Main-owned Works `/api/auth/me` policy; the user-visible chooser and direct-route behavior are covered at rendered App/Router seams.
|
||||
- First independent Sol review — FAIL: found a cold-start Programming provider race, terminal `/api/auth/me` 401 fallback, and global `/settings` misclassification. The task returned to In Progress for corrections and re-review.
|
||||
- Regression-first correction: the cold-start test failed before the Provider gate fix (`initProviders` called once while auth was unresolved), then passed after the fix. Added startup/login/refresh 401 lifecycle, Main-session clear, and Code-disabled global-settings coverage.
|
||||
|
||||
## Follow-ups
|
||||
|
||||
|
||||
@@ -466,7 +466,7 @@ async function handleSessionActivity(
|
||||
});
|
||||
}
|
||||
|
||||
async function handleCurrentUser(res: ServerResponse): Promise<void> {
|
||||
async function handleCurrentUser(res: ServerResponse, ctx: HostApiContext): Promise<void> {
|
||||
const accessToken = await getValidWorksSquareAccessToken({ forceRefresh: false });
|
||||
if (!accessToken) {
|
||||
sendJson(res, 401, { success: false, error: '登录已过期,请重新授权。' });
|
||||
@@ -481,6 +481,13 @@ async function handleCurrentUser(res: ServerResponse): Promise<void> {
|
||||
});
|
||||
const payload = await readResponsePayload(response);
|
||||
if (!response.ok) {
|
||||
if (response.status === 401) {
|
||||
clearWorksSquareSession();
|
||||
await Promise.allSettled([
|
||||
flushWorksSquareSessionPersistence(),
|
||||
clearManagedWorksSquareRuntimeBestEffort(ctx, 'terminal current-user lookup'),
|
||||
]);
|
||||
}
|
||||
sendJson(res, response.status === 401 ? 401 : 502, {
|
||||
success: false,
|
||||
error: response.status === 401
|
||||
@@ -649,7 +656,7 @@ export async function handleAuthRoutes(
|
||||
}
|
||||
|
||||
if (url.pathname === '/api/auth/me' && req.method === 'GET') {
|
||||
await handleCurrentUser(res);
|
||||
await handleCurrentUser(res, ctx);
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
25
src/App.tsx
25
src/App.tsx
@@ -35,6 +35,7 @@ import {
|
||||
AI_MODULE_SELECTION_PATH,
|
||||
getGuardedAiModuleForPath,
|
||||
isAiModuleAllowed,
|
||||
isProgrammingProviderRoute,
|
||||
} from './lib/ai-modules';
|
||||
import { useUserSyncStore } from './stores/user-sync';
|
||||
import { flushPendingAgentSessionSync } from '@/lib/agent-session-sync';
|
||||
@@ -122,10 +123,6 @@ function getReturnPath(location: ReturnType<typeof useLocation>): string {
|
||||
return `${location.pathname}${location.search}`;
|
||||
}
|
||||
|
||||
function isProgrammingRoute(pathname: string): boolean {
|
||||
return getGuardedAiModuleForPath(pathname) === 'programming';
|
||||
}
|
||||
|
||||
function ProtectedLayout({
|
||||
authReady,
|
||||
authRequired,
|
||||
@@ -321,10 +318,24 @@ function App() {
|
||||
useEffect(() => {
|
||||
if (rendererOnlyPreview) return;
|
||||
if (!setupReady) return;
|
||||
if (!programmingModuleAllowed) return;
|
||||
if (!isProgrammingRoute(location.pathname)) return;
|
||||
if (authRequired && !authInitialized) return;
|
||||
if (authRequired && !authenticated) return;
|
||||
if (!isProgrammingProviderRoute(location.pathname)) return;
|
||||
if (
|
||||
getGuardedAiModuleForPath(location.pathname) === 'programming'
|
||||
&& !programmingModuleAllowed
|
||||
) return;
|
||||
initProviders();
|
||||
}, [initProviders, location.pathname, programmingModuleAllowed, rendererOnlyPreview, setupReady]);
|
||||
}, [
|
||||
authInitialized,
|
||||
authRequired,
|
||||
authenticated,
|
||||
initProviders,
|
||||
location.pathname,
|
||||
programmingModuleAllowed,
|
||||
rendererOnlyPreview,
|
||||
setupReady,
|
||||
]);
|
||||
|
||||
useEffect(() => {
|
||||
if (rendererOnlyPreview) return;
|
||||
|
||||
@@ -76,6 +76,10 @@ const PROGRAMMING_ROUTE_PREFIXES = [
|
||||
'/projects',
|
||||
'/sessions',
|
||||
'/models',
|
||||
] as const;
|
||||
|
||||
const PROGRAMMING_PROVIDER_ROUTE_PREFIXES = [
|
||||
...PROGRAMMING_ROUTE_PREFIXES,
|
||||
'/settings',
|
||||
] as const;
|
||||
|
||||
@@ -87,6 +91,10 @@ export function isAiModuleAllowed(moduleId: AiModuleId, access: ModuleAccess): b
|
||||
return access[moduleAccessKeyById[moduleId]];
|
||||
}
|
||||
|
||||
export function isProgrammingProviderRoute(pathname: string): boolean {
|
||||
return PROGRAMMING_PROVIDER_ROUTE_PREFIXES.some((route) => matchesRoute(pathname, route));
|
||||
}
|
||||
|
||||
export function getGuardedAiModuleForPath(pathname: string): AiModuleId | null {
|
||||
if (pathname === '/image-canvas'
|
||||
|| pathname.startsWith('/image-canvas/')
|
||||
|
||||
@@ -236,7 +236,8 @@ async function readCurrentModuleAccess(fallback: ModuleAccess): Promise<ModuleAc
|
||||
const response = await hostApiFetch<ModuleAccessResponse>('/api/auth/me');
|
||||
if (!response.success) return fallback;
|
||||
return normalizeModuleAccess(response.moduleAccess);
|
||||
} catch {
|
||||
} catch (error) {
|
||||
if (isTerminalAuthError(error)) throw error;
|
||||
return fallback;
|
||||
}
|
||||
}
|
||||
@@ -347,9 +348,25 @@ export const useAuthStore = create<AuthState>()(
|
||||
return;
|
||||
}
|
||||
|
||||
const moduleAccess = await readCurrentModuleAccess(
|
||||
normalizeModuleAccess(state.moduleAccess),
|
||||
);
|
||||
let moduleAccess: ModuleAccess;
|
||||
try {
|
||||
moduleAccess = await readCurrentModuleAccess(
|
||||
normalizeModuleAccess(state.moduleAccess),
|
||||
);
|
||||
} catch (error) {
|
||||
if (!isCurrentAuthSessionEpoch(operationEpoch)) return;
|
||||
if (isTerminalAuthError(error)) {
|
||||
advanceAuthSessionEpoch();
|
||||
set({
|
||||
initialized: true,
|
||||
loading: false,
|
||||
error: '登录已过期,请重新授权。',
|
||||
...getClearedSession(),
|
||||
});
|
||||
return;
|
||||
}
|
||||
moduleAccess = normalizeModuleAccess(state.moduleAccess);
|
||||
}
|
||||
if (!isCurrentAuthSessionEpoch(operationEpoch)) return;
|
||||
set({ initialized: true, loading: false, error: null, moduleAccess });
|
||||
},
|
||||
@@ -383,7 +400,10 @@ export const useAuthStore = create<AuthState>()(
|
||||
});
|
||||
} catch (error) {
|
||||
if (!isCurrentAuthSessionEpoch(operationEpoch)) return;
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
const terminal = isTerminalAuthError(error);
|
||||
const message = terminal
|
||||
? '登录已过期,请重新授权。'
|
||||
: (error instanceof Error ? error.message : String(error));
|
||||
advanceAuthSessionEpoch();
|
||||
set({ loading: false, error: message, ...getClearedSession() });
|
||||
throw new Error(message, { cause: error });
|
||||
@@ -431,8 +451,10 @@ export const useAuthStore = create<AuthState>()(
|
||||
return session.accessToken;
|
||||
} catch (error) {
|
||||
if (!isCurrentAuthSessionEpoch(operationEpoch)) return null;
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
const terminal = isTerminalAuthError(error);
|
||||
const message = terminal
|
||||
? '登录已过期,请重新授权。'
|
||||
: (error instanceof Error ? error.message : String(error));
|
||||
if (terminal) advanceAuthSessionEpoch();
|
||||
set({
|
||||
initialized: true,
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { render, screen, waitFor } from '@testing-library/react';
|
||||
import { act, render, screen, waitFor } from '@testing-library/react';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { MemoryRouter, Outlet } from 'react-router-dom';
|
||||
import App from '@/App';
|
||||
@@ -27,6 +27,10 @@ vi.mock('@/pages/ModuleSelection', () => ({
|
||||
ModuleSelection: () => <div>Module chooser</div>,
|
||||
}));
|
||||
|
||||
vi.mock('@/pages/Settings', () => ({
|
||||
Settings: () => <div>Global settings</div>,
|
||||
}));
|
||||
|
||||
describe('App programming provider initialization gate', () => {
|
||||
const initProviders = vi.fn();
|
||||
|
||||
@@ -84,6 +88,37 @@ describe('App programming provider initialization gate', () => {
|
||||
await waitFor(() => expect(initProviders).toHaveBeenCalledTimes(1));
|
||||
});
|
||||
|
||||
it('waits for the startup module policy before initializing programming providers', async () => {
|
||||
useAuthStore.setState({
|
||||
initialized: false,
|
||||
moduleAccess: {
|
||||
programming: true,
|
||||
design: true,
|
||||
learning: true,
|
||||
robot: true,
|
||||
},
|
||||
});
|
||||
|
||||
await renderAt('/opencode-chat');
|
||||
|
||||
expect(initProviders).not.toHaveBeenCalled();
|
||||
|
||||
act(() => {
|
||||
useAuthStore.setState({
|
||||
initialized: true,
|
||||
moduleAccess: {
|
||||
programming: false,
|
||||
design: true,
|
||||
learning: true,
|
||||
robot: true,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
expect(await screen.findByText('Module chooser')).toBeInTheDocument();
|
||||
expect(initProviders).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
['/opencode-chat', 'programming'],
|
||||
['/image-canvas', 'design'],
|
||||
@@ -107,4 +142,20 @@ describe('App programming provider initialization gate', () => {
|
||||
expect(initProviders).not.toHaveBeenCalled();
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps global settings available when Programming is disabled', async () => {
|
||||
useAuthStore.setState({
|
||||
moduleAccess: {
|
||||
programming: false,
|
||||
design: true,
|
||||
learning: true,
|
||||
robot: true,
|
||||
},
|
||||
});
|
||||
|
||||
await renderAt('/settings');
|
||||
|
||||
expect(await screen.findByText('Global settings')).toBeInTheDocument();
|
||||
expect(screen.queryByText('Module chooser')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -145,6 +145,34 @@ describe('auth host api routes', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('clears the Main session when the current-user lookup is unauthorized', async () => {
|
||||
storeWorksSquareSession({
|
||||
accessToken: 'expired-access-token',
|
||||
refreshToken: 'expired-refresh-token',
|
||||
expiresAt: Date.now() + 60_000,
|
||||
lastActiveAt: Date.now(),
|
||||
});
|
||||
vi.stubGlobal('fetch', vi.fn().mockResolvedValueOnce(
|
||||
new Response(JSON.stringify({ detail: 'upstream secret' }), { status: 401 }),
|
||||
));
|
||||
const response = createResponse();
|
||||
|
||||
await handleAuthRoutes(
|
||||
createRequest('GET'),
|
||||
response.res,
|
||||
new URL('http://127.0.0.1:13210/api/auth/me'),
|
||||
{} as never,
|
||||
);
|
||||
|
||||
expect(response.statusCode).toBe(401);
|
||||
expect(response.json()).toEqual({
|
||||
success: false,
|
||||
error: '登录已过期,请重新授权。',
|
||||
});
|
||||
expect(JSON.stringify(response.json())).not.toContain('upstream secret');
|
||||
expect(getWorksSquareSessionSnapshot()).toBeNull();
|
||||
});
|
||||
|
||||
it('exchanges username and AES-encrypted password through the app SSO token endpoint', async () => {
|
||||
const fetchMock = vi.fn().mockResolvedValueOnce(
|
||||
new Response(JSON.stringify({
|
||||
|
||||
@@ -150,6 +150,100 @@ describe('auth store', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('clears restored auth when the current-user policy lookup is unauthorized', async () => {
|
||||
hostApiFetchMock
|
||||
.mockResolvedValueOnce({
|
||||
success: true,
|
||||
session: {
|
||||
accessToken: 'persisted-access-token',
|
||||
tokenType: 'Bearer',
|
||||
expiresAt: Date.now() + 60_000,
|
||||
lastActiveAt: Date.now(),
|
||||
canRefresh: true,
|
||||
},
|
||||
})
|
||||
.mockRejectedValueOnce(Object.assign(new Error('Unauthorized'), {
|
||||
details: { status: 401 },
|
||||
}));
|
||||
useAuthStore.setState({
|
||||
authBase: 'https://biz.nianxx.cn/auth/',
|
||||
accessToken: 'persisted-access-token',
|
||||
tokenType: 'Bearer',
|
||||
expiresAt: Date.now() + 60_000,
|
||||
lastActiveAt: Date.now(),
|
||||
canRefresh: true,
|
||||
moduleAccess: {
|
||||
programming: true,
|
||||
design: false,
|
||||
learning: true,
|
||||
robot: true,
|
||||
},
|
||||
user: {
|
||||
username: 'zhangsan',
|
||||
userId: '1',
|
||||
tenantId: null,
|
||||
deptId: null,
|
||||
authorities: [],
|
||||
},
|
||||
});
|
||||
|
||||
await useAuthStore.getState().init();
|
||||
|
||||
expect(useAuthStore.getState()).toMatchObject({
|
||||
initialized: true,
|
||||
loading: false,
|
||||
error: '登录已过期,请重新授权。',
|
||||
accessToken: null,
|
||||
user: null,
|
||||
moduleAccess: {
|
||||
programming: true,
|
||||
design: true,
|
||||
learning: true,
|
||||
robot: true,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects a new login when its current-user policy lookup is unauthorized', async () => {
|
||||
hostApiFetchMock
|
||||
.mockResolvedValueOnce({
|
||||
success: true,
|
||||
token: {
|
||||
access_token: 'access-token',
|
||||
token_type: 'Bearer',
|
||||
username: 'zhangsan',
|
||||
user_id: '1',
|
||||
},
|
||||
session: {
|
||||
accessToken: 'access-token',
|
||||
tokenType: 'Bearer',
|
||||
expiresAt: Date.now() + 60_000,
|
||||
lastActiveAt: Date.now(),
|
||||
canRefresh: true,
|
||||
},
|
||||
})
|
||||
.mockRejectedValueOnce(Object.assign(new Error('Unauthorized'), {
|
||||
details: { status: 401 },
|
||||
}));
|
||||
|
||||
await expect(useAuthStore.getState().loginWithBrowser()).rejects.toThrow(
|
||||
'登录已过期,请重新授权。',
|
||||
);
|
||||
|
||||
expect(useAuthStore.getState()).toMatchObject({
|
||||
initialized: false,
|
||||
loading: false,
|
||||
accessToken: null,
|
||||
user: null,
|
||||
moduleAccess: {
|
||||
programming: true,
|
||||
design: true,
|
||||
learning: true,
|
||||
robot: true,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('surfaces browser authorization failures and does not keep a partial session', async () => {
|
||||
hostApiFetchMock.mockResolvedValueOnce({
|
||||
success: false,
|
||||
@@ -541,6 +635,52 @@ describe('auth store', () => {
|
||||
expect(useAuthStore.getState().user?.username).toBe('zhangsan');
|
||||
});
|
||||
|
||||
it('clears auth when a refreshed session cannot read the current user', async () => {
|
||||
hostApiFetchMock
|
||||
.mockResolvedValueOnce({
|
||||
success: true,
|
||||
session: {
|
||||
accessToken: 'new-access-token',
|
||||
tokenType: 'Bearer',
|
||||
expiresAt: Date.now() + 120_000,
|
||||
lastActiveAt: Date.now(),
|
||||
canRefresh: true,
|
||||
},
|
||||
})
|
||||
.mockRejectedValueOnce(Object.assign(new Error('Unauthorized'), {
|
||||
details: { status: 401 },
|
||||
}));
|
||||
useAuthStore.setState({
|
||||
initialized: true,
|
||||
accessToken: 'old-access-token',
|
||||
tokenType: 'Bearer',
|
||||
expiresAt: Date.now() + 1_000,
|
||||
lastActiveAt: Date.now(),
|
||||
canRefresh: true,
|
||||
user: {
|
||||
username: 'zhangsan',
|
||||
userId: '1',
|
||||
tenantId: null,
|
||||
deptId: null,
|
||||
authorities: [],
|
||||
},
|
||||
});
|
||||
|
||||
await expect(useAuthStore.getState().refreshSession()).resolves.toBeNull();
|
||||
|
||||
expect(useAuthStore.getState()).toMatchObject({
|
||||
error: '登录已过期,请重新授权。',
|
||||
accessToken: null,
|
||||
user: null,
|
||||
moduleAccess: {
|
||||
programming: true,
|
||||
design: true,
|
||||
learning: true,
|
||||
robot: true,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('restores an expired access token when Main reports it can refresh', async () => {
|
||||
const lastActiveAt = Date.now() - 24 * 60 * 60 * 1000;
|
||||
hostApiFetchMock
|
||||
|
||||
Reference in New Issue
Block a user