Просмотр исходного кода

MDM check improvements (#4770)

* Better check for production clerk

* Send to account tab if required

* Fix tests
Matt Rubens 6 месяцев назад
Родитель
Сommit
e95c1e88d7

+ 7 - 2
packages/cloud/src/Config.ts

@@ -1,2 +1,7 @@
-export const getClerkBaseUrl = () => process.env.CLERK_BASE_URL || "https://clerk.roocode.com"
-export const getRooCodeApiUrl = () => process.env.ROO_CODE_API_URL || "https://app.roocode.com"
+// Production constants
+export const PRODUCTION_CLERK_BASE_URL = "https://clerk.roocode.com"
+export const PRODUCTION_ROO_CODE_API_URL = "https://app.roocode.com"
+
+// Functions with environment variable fallbacks
+export const getClerkBaseUrl = () => process.env.CLERK_BASE_URL || PRODUCTION_CLERK_BASE_URL
+export const getRooCodeApiUrl = () => process.env.ROO_CODE_API_URL || PRODUCTION_ROO_CODE_API_URL

+ 6 - 10
src/core/webview/ClineProvider.ts

@@ -529,11 +529,6 @@ export class ClineProvider
 			>
 		> = {},
 	) {
-		// Check MDM compliance before proceeding
-		if (!this.checkMdmCompliance()) {
-			return // Block task creation if not compliant
-		}
-
 		const {
 			apiConfiguration,
 			organizationAllowList,
@@ -1252,6 +1247,11 @@ export class ClineProvider
 
 		// Update VSCode context for experiments
 		await this.updateVSCodeContext()
+
+		// Check MDM compliance and send user to account tab if not compliant
+		if (!this.checkMdmCompliance()) {
+			await this.postMessageToWebview({ type: "action", action: "accountButtonClicked" })
+		}
 	}
 
 	/**
@@ -1466,6 +1466,7 @@ export class ClineProvider
 				codebaseIndexEmbedderBaseUrl: "",
 				codebaseIndexEmbedderModelId: "",
 			},
+			mdmCompliant: this.checkMdmCompliance(),
 		}
 	}
 
@@ -1721,11 +1722,6 @@ export class ClineProvider
 		const compliance = this.mdmService.isCompliant()
 
 		if (!compliance.compliant) {
-			vscode.window.showErrorMessage(compliance.reason, "Sign In").then((selection) => {
-				if (selection === "Sign In") {
-					this.postMessageToWebview({ type: "action", action: "accountButtonClicked" })
-				}
-			})
 			return false
 		}
 

+ 2 - 2
src/services/mdm/MdmService.ts

@@ -4,7 +4,7 @@ import * as os from "os"
 import * as vscode from "vscode"
 import { z } from "zod"
 
-import { CloudService } from "@roo-code/cloud"
+import { CloudService, getClerkBaseUrl, PRODUCTION_CLERK_BASE_URL } from "@roo-code/cloud"
 import { Package } from "../../shared/package"
 
 // MDM Configuration Schema
@@ -145,7 +145,7 @@ export class MdmService {
 	 */
 	private getMdmConfigPath(): string {
 		const platform = os.platform()
-		const isProduction = process.env.NODE_ENV === "production"
+		const isProduction = getClerkBaseUrl() === PRODUCTION_CLERK_BASE_URL
 		const configFileName = isProduction ? "mdm.json" : "mdm.dev.json"
 
 		switch (platform) {

+ 18 - 8
src/services/mdm/__tests__/MdmService.spec.ts

@@ -19,6 +19,8 @@ vi.mock("@roo-code/cloud", () => ({
 			getOrganizationId: vi.fn(),
 		},
 	},
+	getClerkBaseUrl: vi.fn(),
+	PRODUCTION_CLERK_BASE_URL: "https://clerk.roocode.com",
 }))
 
 vi.mock("vscode", () => ({
@@ -44,12 +46,13 @@ import * as fs from "fs"
 import * as os from "os"
 import * as vscode from "vscode"
 import { MdmService } from "../MdmService"
-import { CloudService } from "@roo-code/cloud"
+import { CloudService, getClerkBaseUrl, PRODUCTION_CLERK_BASE_URL } from "@roo-code/cloud"
 
 const mockFs = fs as any
 const mockOs = os as any
 const mockCloudService = CloudService as any
 const mockVscode = vscode as any
+const mockGetClerkBaseUrl = getClerkBaseUrl as any
 
 describe("MdmService", () => {
 	let originalPlatform: string
@@ -64,6 +67,9 @@ describe("MdmService", () => {
 		// Set default platform for tests
 		mockOs.platform.mockReturnValue("darwin")
 
+		// Setup default mock for getClerkBaseUrl to return development URL
+		mockGetClerkBaseUrl.mockReturnValue("https://dev.clerk.roocode.com")
+
 		// Setup VSCode mocks
 		const mockConfig = {
 			get: vi.fn().mockReturnValue(false),
@@ -73,6 +79,8 @@ describe("MdmService", () => {
 
 		// Reset mocks
 		vi.clearAllMocks()
+		// Re-setup the default after clearing
+		mockGetClerkBaseUrl.mockReturnValue("https://dev.clerk.roocode.com")
 	})
 
 	afterEach(() => {
@@ -142,7 +150,7 @@ describe("MdmService", () => {
 		it("should use correct path for Windows in production", async () => {
 			mockOs.platform.mockReturnValue("win32")
 			process.env.PROGRAMDATA = "C:\\ProgramData"
-			process.env.NODE_ENV = "production"
+			mockGetClerkBaseUrl.mockReturnValue(PRODUCTION_CLERK_BASE_URL)
 
 			mockFs.existsSync.mockReturnValue(false)
 
@@ -154,7 +162,7 @@ describe("MdmService", () => {
 		it("should use correct path for Windows in development", async () => {
 			mockOs.platform.mockReturnValue("win32")
 			process.env.PROGRAMDATA = "C:\\ProgramData"
-			process.env.NODE_ENV = "development"
+			mockGetClerkBaseUrl.mockReturnValue("https://dev.clerk.roocode.com")
 
 			mockFs.existsSync.mockReturnValue(false)
 
@@ -165,7 +173,7 @@ describe("MdmService", () => {
 
 		it("should use correct path for macOS in production", async () => {
 			mockOs.platform.mockReturnValue("darwin")
-			process.env.NODE_ENV = "production"
+			mockGetClerkBaseUrl.mockReturnValue(PRODUCTION_CLERK_BASE_URL)
 
 			mockFs.existsSync.mockReturnValue(false)
 
@@ -176,7 +184,7 @@ describe("MdmService", () => {
 
 		it("should use correct path for macOS in development", async () => {
 			mockOs.platform.mockReturnValue("darwin")
-			process.env.NODE_ENV = "development"
+			mockGetClerkBaseUrl.mockReturnValue("https://dev.clerk.roocode.com")
 
 			mockFs.existsSync.mockReturnValue(false)
 
@@ -187,7 +195,7 @@ describe("MdmService", () => {
 
 		it("should use correct path for Linux in production", async () => {
 			mockOs.platform.mockReturnValue("linux")
-			process.env.NODE_ENV = "production"
+			mockGetClerkBaseUrl.mockReturnValue(PRODUCTION_CLERK_BASE_URL)
 
 			mockFs.existsSync.mockReturnValue(false)
 
@@ -198,7 +206,7 @@ describe("MdmService", () => {
 
 		it("should use correct path for Linux in development", async () => {
 			mockOs.platform.mockReturnValue("linux")
-			process.env.NODE_ENV = "development"
+			mockGetClerkBaseUrl.mockReturnValue("https://dev.clerk.roocode.com")
 
 			mockFs.existsSync.mockReturnValue(false)
 
@@ -209,7 +217,7 @@ describe("MdmService", () => {
 
 		it("should default to dev config when NODE_ENV is not set", async () => {
 			mockOs.platform.mockReturnValue("darwin")
-			delete process.env.NODE_ENV
+			mockGetClerkBaseUrl.mockReturnValue("https://dev.clerk.roocode.com")
 
 			mockFs.existsSync.mockReturnValue(false)
 
@@ -248,6 +256,7 @@ describe("MdmService", () => {
 			mockFs.existsSync.mockReturnValue(true)
 			mockFs.readFileSync.mockReturnValue(JSON.stringify(mockConfig))
 
+			// Mock CloudService to indicate no instance or no active session
 			mockCloudService.hasInstance.mockReturnValue(false)
 
 			const service = await MdmService.createInstance()
@@ -267,6 +276,7 @@ describe("MdmService", () => {
 			mockFs.existsSync.mockReturnValue(true)
 			mockFs.readFileSync.mockReturnValue(JSON.stringify(mockConfig))
 
+			// Mock CloudService to have instance and active session but wrong org
 			mockCloudService.hasInstance.mockReturnValue(true)
 			mockCloudService.instance.hasActiveSession.mockReturnValue(true)
 			mockCloudService.instance.getOrganizationId.mockReturnValue("different-org-456")

+ 17 - 8
webview-ui/src/App.tsx

@@ -42,6 +42,7 @@ const App = () => {
 		experiments,
 		cloudUserInfo,
 		cloudIsAuthenticated,
+		mdmCompliant,
 	} = useExtensionState()
 
 	// Create a persistent state manager
@@ -63,15 +64,23 @@ const App = () => {
 	const settingsRef = useRef<SettingsViewRef>(null)
 	const chatViewRef = useRef<ChatViewRef>(null)
 
-	const switchTab = useCallback((newTab: Tab) => {
-		setCurrentSection(undefined)
+	const switchTab = useCallback(
+		(newTab: Tab) => {
+			// Check MDM compliance before allowing tab switching
+			if (mdmCompliant === false && newTab !== "account") {
+				return
+			}
 
-		if (settingsRef.current?.checkUnsaveChanges) {
-			settingsRef.current.checkUnsaveChanges(() => setTab(newTab))
-		} else {
-			setTab(newTab)
-		}
-	}, [])
+			setCurrentSection(undefined)
+
+			if (settingsRef.current?.checkUnsaveChanges) {
+				settingsRef.current.checkUnsaveChanges(() => setTab(newTab))
+			} else {
+				setTab(newTab)
+			}
+		},
+		[mdmCompliant],
+	)
 
 	const [currentSection, setCurrentSection] = useState<string | undefined>(undefined)
 

+ 1 - 0
webview-ui/src/context/ExtensionStateContext.tsx

@@ -37,6 +37,7 @@ export interface ExtensionStateContextType extends ExtensionState {
 	cloudIsAuthenticated: boolean
 	sharingEnabled: boolean
 	maxConcurrentFileReads?: number
+	mdmCompliant?: boolean
 	condensingApiConfigId?: string
 	setCondensingApiConfigId: (value: string) => void
 	customCondensingPrompt?: string