Skip to content

Commit 13ef69a

Browse files
authored
refactor(greenhouse): add TypeScript types and Zod validation for auth system (#1532)
* chore(greenhouse): adds authentication types to improve readability and simplify code * chore(greenhouse): fix tests * chore(greenhouse): remove @ts-expect-error directives * chore(greenhouse): addressed copilot issues * chore(greenhouse): removed warnings * chore(greenhouse): adds changeset * chore(greenhouse): added appProp basePath to the readme * chore(greenhouse): fix type * chore(greenhouse): removes as OidcSessionInstance
1 parent 68ba389 commit 13ef69a

14 files changed

Lines changed: 226 additions & 72 deletions

File tree

.changeset/happy-pigs-think.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@cloudoperators/juno-app-greenhouse": patch
3+
---
4+
5+
Correctly extract organisation name from URL with basePath and add TypeScript types and Zod validation for auth system.

apps/greenhouse/README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ These are the customizable application properties (appProps) that you can define
5252
- **demoOrg** (optional): `"demo"`. If the organization name matches this value, the app will use the demo user token for authentication.
5353
- **demoUserToken** (optional): `"token for demo user"`. Used for authentication if `demoOrg` and `demoUserToken` are set, and the organization name matches `demoOrg`.
5454
**Note:** This is ignored if `mockAuth` is set.
55+
- **basePath** (optional, default: `/`):
56+
Specifies the root path under which the application is served. Useful for deploying the app to a subdirectory. If not provided, defaults to the root path `/`.
5557

5658
## Contributing
5759

apps/greenhouse/package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
"clean:cache": "rm -rf .turbo"
4848
},
4949
"dependencies": {
50+
"@cloudoperators/greenhouse-auth-provider": "workspace:*",
5051
"@cloudoperators/juno-app-doop": "workspace:*",
5152
"@cloudoperators/juno-app-heureka": "workspace:*",
5253
"@cloudoperators/juno-app-supernova": "workspace:*",
@@ -55,7 +56,6 @@
5556
"@cloudoperators/juno-oauth": "workspace:*",
5657
"@cloudoperators/juno-ui-components": "workspace:*",
5758
"@cloudoperators/juno-url-state-provider": "workspace:*",
58-
"@cloudoperators/greenhouse-auth-provider": "workspace:*",
5959
"@codemirror/lang-yaml": "^6.1.2",
6060
"@codemirror/language": "^6.12.2",
6161
"@codemirror/state": "^6.5.4",
@@ -64,6 +64,7 @@
6464
"@tanstack/react-query": "5.90.21",
6565
"@tanstack/react-router": "1.161.3",
6666
"js-yaml": "4.1.1",
67-
"lodash": "4.17.23"
67+
"lodash": "4.17.23",
68+
"zod": "4.3.6"
6869
}
6970
}

apps/greenhouse/src/Shell.tsx

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import StoreProvider, { useGlobalsApiEndpoint } from "./components/StoreProvider
1616
import { AuthProvider, useAuth } from "./components/AuthProvider"
1717
import { routeTree } from "./routeTree.gen"
1818
import { getRouterBasePath } from "./utils/organizationResolver"
19+
import type { AuthContextValue, MockAuthValue } from "./types/auth"
1920

2021
// Create a new query client instance
2122
const queryClient = new QueryClient()
@@ -44,24 +45,21 @@ export type AppProps = {
4445
authClientId?: string
4546
authIssuerUrl?: string
4647
demoOrg?: string
47-
mockAuth?: boolean
48+
mockAuth?: MockAuthValue
4849
demoUserToken?: string
4950
currentHost?: string
5051
enableHashedRouting?: boolean
5152
basePath?: string
5253
}
5354

54-
const getUser = (auth: unknown) => ({
55-
// @ts-expect-error - auth?.data type needs to be properly defined
56-
organization: auth?.data?.raw?.groups?.find((g: any) => g.startsWith("organization:"))?.split(":")[1] ?? "",
57-
// @ts-expect-error - auth?.data type needs to be properly defined
55+
const getUser = (auth: AuthContextValue) => ({
56+
organization: auth?.data?.raw?.groups?.find((g) => g.startsWith("organization:"))?.split(":")[1] ?? "",
5857
supportGroups: auth?.data?.parsed?.supportGroups ?? [],
5958
})
6059

6160
function App(props: AppProps) {
6261
const auth = useAuth()
6362
const apiEndpoint = useGlobalsApiEndpoint()
64-
// @ts-expect-error - useAuth return type is not properly typed
6563
const token = auth?.data?.JWT
6664
// Create k8s client if apiEndpoint and token are available
6765
// @ts-expect-error - apiEndpoint type needs to be properly typed as string
@@ -75,7 +73,6 @@ function App(props: AppProps) {
7573
* want the app to use browser history.
7674
*/
7775
router.update({
78-
// @ts-expect-error - auth?.data type needs to be properly defined
7976
basepath: getRouterBasePath(auth?.data?.raw?.groups, props.basePath),
8077
context: { appProps: props, apiClient, user },
8178
stringifySearch: encodeV2,

apps/greenhouse/src/components/Auth.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import { useAuth } from "./AuthProvider"
1919
*
2020
*/
2121
const Auth = ({ children }: any) => {
22-
// @ts-expect-error TS(2339): Property 'isProcessing' does not exist on type 'un... Remove this comment to see the full error message
2322
const { isProcessing: authIsProcessing, loggedIn: authLoggedIn, error: authError, login } = useAuth()
2423

2524
if (authLoggedIn) {

apps/greenhouse/src/components/AuthProvider.test.tsx

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,8 @@ describe("AuthProvider", () => {
2323
it("initializes default the oidc session but return error because no issuerUrl or clientId provided", () => {
2424
const wrapper = ({ children }: any) => <AuthProvider options={{}}>{children}</AuthProvider>
2525
const { result } = renderHook(() => useAuth(), { wrapper })
26-
// @ts-expect-error TS(2531): Object is possibly 'null'.
2726
expect(result.current.loggedIn).toBe(false)
28-
// @ts-expect-error TS(2531): Object is possibly 'null'.
2927
expect(result.current.isDemoMode).toBe(false)
30-
// @ts-expect-error TS(2531): Object is possibly 'null'.
3128
expect(result.current.error).not.toBe(null)
3229
})
3330
})
@@ -107,6 +104,52 @@ describe("AuthProvider", () => {
107104
// @ts-expect-error TS(2531): Object is possibly 'null'.
108105
expect(result.current.data.parsed.groups).toEqual(mockAuthData.groups)
109106
})
107+
108+
it("preserves case in JSON string mockAuth data", () => {
109+
const replaceStateSpy = vi.spyOn(window.history, "replaceState").mockImplementation(() => {})
110+
111+
// Test with mixed-case data that should NOT be lowercased
112+
const mockAuthData = {
113+
email: "Jane.Doe@Example.com",
114+
name: "Jane Doe",
115+
groups: ["organization:TestOrg", "team:DevTeam"],
116+
}
117+
const mockAuthDataString = JSON.stringify(mockAuthData)
118+
119+
const wrapper = ({ children }: any) => (
120+
<AuthProvider options={{ mockAuth: mockAuthDataString }}>{children}</AuthProvider>
121+
)
122+
123+
const { result } = renderHook(() => useAuth(), { wrapper })
124+
125+
expect(replaceStateSpy).toHaveBeenCalledTimes(1)
126+
// @ts-expect-error TS(2531): Object is possibly 'null'.
127+
expect(result.current.data.raw.email).toEqual("Jane.Doe@Example.com")
128+
// @ts-expect-error TS(2531): Object is possibly 'null'.
129+
expect(result.current.data.raw.name).toEqual("Jane Doe")
130+
// @ts-expect-error TS(2531): Object is possibly 'null'.
131+
expect(result.current.data.raw.groups).toContain("organization:TestOrg")
132+
})
133+
134+
it("rejects invalid mockAuth values (number, array, etc)", () => {
135+
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
136+
137+
// Test with number (invalid)
138+
const wrapper1 = ({ children }: any) => <AuthProvider options={{ mockAuth: 123 }}>{children}</AuthProvider>
139+
const { result: result1 } = renderHook(() => useAuth(), { wrapper: wrapper1 })
140+
expect(result1.current.loggedIn).toBe(false)
141+
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("Invalid mockAuth value"), 123)
142+
143+
consoleWarnSpy.mockClear()
144+
145+
// Test with array (invalid)
146+
const wrapper2 = ({ children }: any) => <AuthProvider options={{ mockAuth: [1, 2, 3] }}>{children}</AuthProvider>
147+
const { result: result2 } = renderHook(() => useAuth(), { wrapper: wrapper2 })
148+
expect(result2.current.loggedIn).toBe(false)
149+
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("Invalid mockAuth value"), [1, 2, 3])
150+
151+
consoleWarnSpy.mockRestore()
152+
})
110153
})
111154

112155
describe("initializes demo auth session", () => {

apps/greenhouse/src/components/AuthProvider.tsx

Lines changed: 65 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@ import React, { createContext, useContext, useState, useMemo, useRef, useEffect
77
import { oidcSession, mockedSession, tokenSession } from "@cloudoperators/juno-oauth"
88
import { createAuthStore, AuthStore } from "@cloudoperators/greenhouse-auth-provider"
99
import { extractOrganizationName } from "../utils/organizationResolver"
10+
import { TokenDataSchema } from "../types/auth"
11+
import type { OidcSessionState, AuthContextValue, OidcSessionInstance, TokenData, MockAuthValue } from "../types/auth"
1012

11-
const setOrganizationToUrl = (groups: any, enableHashedRouting: boolean) => {
12-
const orgName = groups?.find((g: any) => g.startsWith("organization:"))?.split(":")[1]
13+
const setOrganizationToUrl = (groups: string[] | undefined, enableHashedRouting: boolean) => {
14+
const orgName = groups?.find((g: string) => g.startsWith("organization:"))?.split(":")[1]
1315

1416
if (!orgName) return
1517

@@ -34,43 +36,58 @@ const setOrganizationToUrl = (groups: any, enableHashedRouting: boolean) => {
3436
} else {
3537
url.pathname = pathWithOrg
3638
}
37-
// @ts-expect-error TS(2345): Argument of type 'null' is not assignable to param... Remove this comment to see the full error message
38-
window.history.replaceState(null, null, url.href)
39+
window.history.replaceState({}, "", url.href)
3940
}
4041

41-
function resolveMockAuth(value: any) {
42-
const result = { isMock: false, parsedAuth: null }
43-
42+
/**
43+
* Resolves mock authentication configuration with runtime validation
44+
*
45+
* @param value - Can be:
46+
* - boolean: true enables mock with default token, false disables
47+
* - TokenData: Plain object with token attributes (iss, sub, aud, exp, iat, nonce, email, groups, etc.)
48+
* - string: "true" or JSON string that will be parsed
49+
*
50+
* @returns Object with isMock flag and parsedAuth token data
51+
*/
52+
function resolveMockAuth(value: MockAuthValue): { isMock: boolean; parsedAuth: TokenData | null } {
4453
if (typeof value === "boolean") {
45-
// If value is a boolean, set `isMock` accordingly
46-
// and return an empty object for `true`, otherwise `null`
47-
result.isMock = value
48-
// @ts-expect-error TS(2322): Type '{} | null' is not assignable to type 'null'.
49-
result.parsedAuth = value ? {} : null
50-
} else if (typeof value === "string") {
51-
const trimmed = value.trim().toLowerCase()
52-
if (trimmed === "true") {
53-
// If the string is "true", treat it as a mock with an empty object
54-
result.isMock = true
55-
// @ts-expect-error TS(2322): Type '{}' is not assignable to type 'null'.
56-
result.parsedAuth = {}
57-
} else {
58-
try {
59-
// Try parsing the string as JSON
60-
result.isMock = true
61-
result.parsedAuth = JSON.parse(value)
62-
} catch {
63-
result.isMock = false
64-
result.parsedAuth = null
54+
return {
55+
isMock: value,
56+
parsedAuth: value ? {} : null,
57+
}
58+
}
59+
60+
if (typeof value === "string") {
61+
const trimmed = value.trim()
62+
// Only lowercase for boolean check, not for JSON parsing
63+
if (trimmed.toLowerCase() === "true") {
64+
return { isMock: true, parsedAuth: {} }
65+
}
66+
67+
// Try parsing JSON string (use original case-sensitive trimmed string)
68+
try {
69+
const parsed = JSON.parse(trimmed)
70+
// Validate the parsed JSON
71+
const validation = TokenDataSchema.safeParse(parsed)
72+
if (validation.success) {
73+
return { isMock: true, parsedAuth: validation.data }
6574
}
75+
} catch {
76+
// Invalid JSON, return disabled mock
6677
}
67-
} else if (typeof value === "object" && value !== null) {
68-
// If value is a non-null object, treat it as a mock
69-
result.isMock = true
70-
result.parsedAuth = value
78+
79+
return { isMock: false, parsedAuth: null }
80+
}
81+
82+
// It's an object - validate with Zod to ensure runtime safety
83+
const validation = TokenDataSchema.safeParse(value)
84+
if (validation.success) {
85+
return { isMock: true, parsedAuth: validation.data }
7186
}
7287

73-
return result
88+
// Invalid object (e.g., array, number, invalid structure)
89+
console.warn("Invalid mockAuth value: expected boolean, TokenData object, or JSON string", value)
90+
return { isMock: false, parsedAuth: null }
7491
}
7592

7693
const initializeDemoAuth = (
@@ -163,14 +180,13 @@ const initializeRealOidc = (
163180
})
164181
}
165182

166-
// @ts-expect-error TS(2554): Expected 1 arguments, but got 0.
167-
const AuthContext = createContext()
183+
const AuthContext = createContext<AuthContextValue | undefined>(undefined)
168184

169185
export const AuthProvider = ({ options, children }: any) => {
170-
const [authData, setAuthData] = useState(null)
186+
const [authData, setAuthData] = useState<OidcSessionState | null>(null)
171187
const [isDemoMode, setIsDemoMode] = useState(false)
172-
const [oidcError, setOidcError] = useState(null)
173-
const oidcInstance = useRef(null)
188+
const [oidcError, setOidcError] = useState<string | null>(null)
189+
const oidcInstance = useRef<OidcSessionInstance | null>(null)
174190
const pluginAuthRef = useRef<AuthStore | null>(null)
175191

176192
if (!pluginAuthRef.current) {
@@ -201,7 +217,6 @@ export const AuthProvider = ({ options, children }: any) => {
201217
if (demoOrg === orgName && !isMock) {
202218
console.debug("Initializing new demo auth session")
203219
setIsDemoMode(true)
204-
// @ts-ignore
205220
oidcInstance.current = initializeDemoAuth(
206221
orgName,
207222
demoUserToken,
@@ -245,42 +260,34 @@ export const AuthProvider = ({ options, children }: any) => {
245260

246261
const error = "Invalid OIDC configuration, issuerURL and clientID are required"
247262
console.error(error)
248-
// @ts-expect-error TS(2345): Argument of type '"Invalid OIDC configuration, iss... Remove this comment to see the full error message
249263
setOidcError(error)
250-
return
264+
return null
251265
}
252266

253267
// Memoized login function
254268
const login = () => {
255-
// @ts-expect-error TS(2339): Property 'login' does not exist on type 'never'.
256-
oidcInstance.current?.login?.()
269+
oidcInstance.current?.login()
257270
}
258271

259272
// Memoized logout function
260-
const logout = () => {
261-
// @ts-expect-error TS(2339): Property 'logout' does not exist on type 'never'.
262-
oidcInstance.current?.logout?.({
263-
resetOIDCSession: true,
264-
silent: true,
273+
const logout = (options?: { resetOIDCSession?: boolean; silent?: boolean }) => {
274+
oidcInstance.current?.logout({
275+
resetOIDCSession: options?.resetOIDCSession ?? true,
276+
silent: options?.silent ?? true,
265277
})
266278
setAuthData(null)
267279
setOidcError(null)
268280
}
269281

270282
useEffect(() => {
271-
// @ts-expect-error TS(2322): Type 'null | undefined' is not assignable to type ... Remove this comment to see the full error message
272283
oidcInstance.current = initializeOidc()
273284
}, [options])
274285

275286
const contextValue = useMemo(
276287
() => ({
277-
// @ts-expect-error TS(2339): Property 'isProcessing' does not exist on type 'ne... Remove this comment to see the full error message
278288
isProcessing: authData ? authData?.isProcessing : false,
279-
// @ts-expect-error TS(2339): Property 'loggedIn' does not exist on type 'never'... Remove this comment to see the full error message
280289
loggedIn: authData ? authData?.loggedIn : false,
281-
// @ts-expect-error TS(2339): Property 'error' does not exist on type 'never'.
282290
error: authData ? authData?.error : oidcError,
283-
// @ts-expect-error TS(2339): Property 'auth' does not exist on type 'never'.
284291
data: authData ? authData?.auth : null,
285292
pluginAuth,
286293
isDemoMode,
@@ -293,6 +300,10 @@ export const AuthProvider = ({ options, children }: any) => {
293300
return <AuthContext.Provider value={contextValue}>{children}</AuthContext.Provider>
294301
}
295302

296-
export const useAuth = () => {
297-
return useContext(AuthContext)
303+
export const useAuth = (): AuthContextValue => {
304+
const context = useContext(AuthContext)
305+
if (context === undefined) {
306+
throw new Error("useAuth must be used within an AuthProvider")
307+
}
308+
return context
298309
}

apps/greenhouse/src/components/NotificationsContainer.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { Messages } from "@cloudoperators/juno-messages-provider"
99
import { useAuth } from "./AuthProvider"
1010

1111
const NotificationsContainer = () => {
12-
// @ts-expect-error TS(2339): Property 'isDemoMode' does not exist on type 'unkn... Remove this comment to see the full error message
1312
const { isDemoMode } = useAuth()
1413

1514
return (

apps/greenhouse/src/components/nav/PluginNav.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ const PluginNav = () => {
7070
const isAdminRoute = matches.some((match) => match.routeId.startsWith("/admin"))
7171
// If we're on /admin route, set activeApp to "admin", otherwise use extension ID
7272
const activeApp = isAdminRoute ? "admin" : extensionIdMatch
73-
// @ts-expect-error TS(2339): Property 'data' does not exist on type 'unknown'.
7473
const { data: authData, loggedIn, login, logout } = useAuth()
7574

7675
const navigateToApp = (appId: string) => {

apps/greenhouse/src/hooks/useApi.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { useAuth } from "../components/AuthProvider"
1414
// get plugin configs from k8s api
1515
// @ts-ignore
1616
const useApi = () => {
17-
// @ts-expect-error TS(2339): Property 'data' does not exist on type 'unknown'.
1817
const { data: authData } = useAuth()
1918
const apiEndpoint = useGlobalsApiEndpoint()
2019
const assetsHost = useGlobalsAssetsHost()

0 commit comments

Comments
 (0)