Skip to content

Commit 3913cd1

Browse files
authored
fix: Don't treat generic exec plugin as OIDC (kyma-project#4651)
* fix: dont treat generic exec plugin treat as OIDC * adjust translation
1 parent 82587ae commit 3913cd1

File tree

9 files changed

+206
-53
lines changed

9 files changed

+206
-53
lines changed

public/i18n/en.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,16 @@ clusters:
141141
inMemory: In memory
142142
localStorage: Local storage
143143
sessionStorage: Session storage
144+
oidc-memory-warning: Memory storage is not compatible with browser-based OIDC login. After the authentication redirect, cluster data stored in memory is lost. Use session or local storage instead.
144145
storage-preference: Storage preference
145146
title: Storage Type
146147
token: Token
147148
wizard:
148149
auth:
149150
client-id: Client ID
150151
client-secret: Client Secret
152+
exec-command: Command
153+
exec-info: Run the above command to obtain a token, then paste it in the Token field.
151154
issuer-url: Issuer URL
152155
scopes: Scopes
153156
token: Token

src/components/Clusters/components/AddClusterWizard.jsx

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { useState } from 'react';
2-
import { Title, Wizard, WizardStep } from '@ui5/webcomponents-react';
2+
import {
3+
MessageStrip,
4+
Title,
5+
Wizard,
6+
WizardStep,
7+
} from '@ui5/webcomponents-react';
38
import { useTranslation } from 'react-i18next';
49
import { useAtomValue, useSetAtom } from 'jotai';
510

@@ -14,6 +19,7 @@ import { isFormOpenAtom } from 'state/formOpenAtom';
1419
import { checkAuthRequiredInputs } from '../helper';
1520

1621
import { addByContext, getUser, hasKubeconfigAuth } from '../shared';
22+
import { isOIDCExec } from './oidc-params';
1723
import { AuthForm } from './AuthForm';
1824
import { KubeconfigUpload } from './KubeconfigUpload/KubeconfigUpload';
1925
import { ContextChooser } from './ContextChooser/ContextChooser';
@@ -237,6 +243,16 @@ export function AddClusterWizard({ config = {} }) {
237243
setStorage={setStorage}
238244
hideLabel
239245
/>
246+
{isOIDCExec(getUser(kubeconfig)?.exec) &&
247+
storage === 'inMemory' && (
248+
<MessageStrip
249+
design="Critical"
250+
hideCloseButton
251+
className="sap-margin-top-small"
252+
>
253+
{t('clusters.storage.oidc-memory-warning')}
254+
</MessageStrip>
255+
)}
240256
</div>
241257
</WizardStep>
242258
<WizardStep

src/components/Clusters/components/AuthForm.jsx

Lines changed: 55 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import { useState, useEffect } from 'react';
22
import { useTranslation } from 'react-i18next';
33
import { MessageStrip, Switch, Title } from '@ui5/webcomponents-react';
44
import jp from 'jsonpath';
5-
import { createLoginCommand, tryParseOIDCparams } from './oidc-params';
5+
import {
6+
createLoginCommand,
7+
isOIDCExec,
8+
tryParseOIDCparams,
9+
} from './oidc-params';
610

711
import { ResourceForm } from 'shared/ResourceForm';
812
import * as Inputs from 'shared/ResourceForm/inputs';
@@ -70,23 +74,43 @@ const OIDCform = ({ resource, ...props }) => {
7074
);
7175
};
7276

73-
const TokenForm = ({ resource, ...props }) => {
77+
const TokenForm = ({ resource, setResource, onChange, ...props }) => {
7478
const { t } = useTranslation();
7579
const userIndex = getUserIndex(resource);
80+
const exec = resource?.users?.[userIndex]?.user?.exec;
81+
const isGenericExec = !!exec && !isOIDCExec(exec);
7682
const [token, setToken] = useState(resource?.users?.[userIndex]?.user?.token);
7783

7884
return (
79-
<ResourceForm.Wrapper resource={resource} {...props}>
85+
<ResourceForm.Wrapper
86+
resource={resource}
87+
setResource={setResource}
88+
{...props}
89+
>
90+
{isGenericExec && exec?.command && (
91+
<ResourceForm.FormField
92+
label={t('clusters.wizard.auth.exec-command')}
93+
input={Inputs.Text}
94+
value={exec.command}
95+
readOnly
96+
/>
97+
)}
8098
<ResourceForm.FormField
8199
required
82100
value={token}
83101
setValue={(val) => {
84102
setToken(val);
85103
jp.value(resource, `$.users[${userIndex}].user.token`, val);
104+
setResource?.({ ...resource });
105+
onChange?.();
86106
}}
87107
label={t('clusters.wizard.auth.token')}
88108
input={Inputs.Text}
89-
inputInfo={t('clusters.wizard.token-info')}
109+
inputInfo={
110+
isGenericExec
111+
? t('clusters.wizard.auth.exec-info')
112+
: t('clusters.wizard.token-info')
113+
}
90114
/>
91115
</ResourceForm.Wrapper>
92116
);
@@ -102,7 +126,10 @@ export function AuthForm({
102126
}) {
103127
const { t } = useTranslation();
104128

105-
const [useOidc, setUseOidc] = useState(!!getUser(resource)?.exec);
129+
const userExec = getUser(resource)?.exec;
130+
const isGenericExec = !!userExec && !isOIDCExec(userExec);
131+
132+
const [useOidc, setUseOidc] = useState(!isGenericExec && !!userExec);
106133

107134
useEffect(() => {
108135
revalidate();
@@ -120,6 +147,11 @@ export function AuthForm({
120147
setUseOidc(!useOidc);
121148
};
122149

150+
const incompleteContext =
151+
resource['current-context'] === '-all-'
152+
? resource.contexts[0]?.name
153+
: resource['current-context'];
154+
123155
return (
124156
<ResourceForm.Wrapper
125157
formElementRef={formElementRef}
@@ -134,30 +166,29 @@ export function AuthForm({
134166
hideCloseButton
135167
className="sap-margin-y-small"
136168
>
137-
{t('clusters.wizard.incomplete', {
138-
context:
139-
resource['current-context'] === '-all-'
140-
? resource.contexts[0]?.name
141-
: resource['current-context'],
142-
})}
169+
{t('clusters.wizard.incomplete', { context: incompleteContext })}
143170
</MessageStrip>
144171
{!useOidc && (
145172
<TokenForm onChange={checkRequiredInputs} resource={resource} />
146173
)}
147-
<ResourceForm.FormField
148-
label={t('clusters.wizard.auth.using-oidc')}
149-
input={(props) => (
150-
<Switch
151-
{...props}
152-
className="sap-margin-top-tiny"
153-
checked={useOidc}
154-
onChange={switchAuthVariant}
174+
{!isGenericExec && (
175+
<>
176+
<ResourceForm.FormField
177+
label={t('clusters.wizard.auth.using-oidc')}
178+
input={(props) => (
179+
<Switch
180+
{...props}
181+
className="sap-margin-top-tiny"
182+
checked={useOidc}
183+
onChange={switchAuthVariant}
184+
/>
185+
)}
186+
className="oidc-switch"
155187
/>
156-
)}
157-
className="oidc-switch"
158-
/>
159-
{useOidc && (
160-
<OIDCform onChange={checkRequiredInputs} resource={resource} />
188+
{useOidc && (
189+
<OIDCform onChange={checkRequiredInputs} resource={resource} />
190+
)}
191+
</>
161192
)}
162193
</div>
163194
</ResourceForm.Wrapper>

src/components/Clusters/components/ClusterPreview.tsx

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,33 @@ import {
66
findInitialValues,
77
} from '../views/EditCluster/EditCluster';
88
import { getUserIndex } from '../shared';
9+
import { isOIDCExec } from './oidc-params';
910
import {
1011
Kubeconfig,
1112
KubeconfigNonOIDCAuthToken,
1213
KubeconfigOIDCAuth,
1314
} from 'types';
1415
import { Tokens } from 'shared/components/Tokens';
1516

16-
const TokenData = ({ token }: { token: string }) => {
17+
const TokenData = ({
18+
token,
19+
execCommand,
20+
}: {
21+
token: string;
22+
execCommand?: string;
23+
}) => {
1724
const { t } = useTranslation();
1825

1926
return (
2027
<>
28+
{execCommand && (
29+
<>
30+
<p className="cluster-preview__data-header sap-margin-top-small sap-margin-bottom-tiny">
31+
{t('clusters.wizard.auth.exec-command')}:
32+
</p>
33+
<div>{execCommand}</div>
34+
</>
35+
)}
2136
<p className="cluster-preview__data-header sap-margin-top-small sap-margin-bottom-tiny">
2237
{`${t('clusters.token')}:`}
2338
</p>
@@ -92,11 +107,9 @@ export function ClusterPreview({
92107
}: ClusterPreviewProps) {
93108
const { t } = useTranslation();
94109
const userIndex = getUserIndex(kubeconfig);
95-
const authenticationType = (
96-
kubeconfig?.users?.[userIndex]?.user as KubeconfigOIDCAuth
97-
)?.exec
98-
? 'oidc'
99-
: 'token';
110+
const exec = (kubeconfig?.users?.[userIndex]?.user as KubeconfigOIDCAuth)
111+
?.exec;
112+
const authenticationType = exec && isOIDCExec(exec) ? 'oidc' : 'token';
100113

101114
const issuerUrl = findInitialValue(kubeconfig, 'oidc-issuer-url', userIndex);
102115
const clientId = findInitialValue(kubeconfig, 'oidc-client-id', userIndex);
@@ -149,7 +162,7 @@ export function ClusterPreview({
149162
<div className="cluster-preview__content sap-margin-top-small sap-margin-bottom-tiny">
150163
<div className="cluster-preview__auth">
151164
{authenticationType === 'token' ? (
152-
<TokenData token={token} />
165+
<TokenData token={token} execCommand={exec?.command} />
153166
) : (
154167
<OidcData
155168
issuerUrl={issuerUrl}

src/components/Clusters/components/oidc-params.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { KubeconfigOIDCAuth, LoginCommand } from 'types';
22

3+
export function isOIDCExec(exec: { args?: string[] } | undefined): boolean {
4+
return !!exec?.args?.some((arg) => arg?.startsWith('--oidc-issuer-url='));
5+
}
6+
37
const OIDC_PARAM_NAMES = new Map([
48
['--oidc-issuer-url', 'issuerUrl'],
59
['--oidc-client-id', 'clientId'],

src/components/Clusters/components/test/oidc-params.test.js

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,41 @@
1-
import { parseOIDCparams } from '../oidc-params';
1+
import { parseOIDCparams, isOIDCExec } from '../oidc-params';
2+
3+
describe('isOIDCExec', () => {
4+
it('returns true when exec args contain --oidc-issuer-url', () => {
5+
const exec = {
6+
command: 'kubectl',
7+
args: [
8+
'oidc-login',
9+
'get-token',
10+
'--oidc-issuer-url=https://example.com',
11+
'--oidc-client-id=myid',
12+
],
13+
};
14+
expect(isOIDCExec(exec)).toBe(true);
15+
});
16+
17+
it('returns false for a generic exec plugin without OIDC args', () => {
18+
const exec = {
19+
command: 'sh',
20+
args: ['mock-oidc.sh'],
21+
};
22+
expect(isOIDCExec(exec)).toBe(false);
23+
});
24+
25+
it('returns false when exec has empty args array', () => {
26+
const exec = { command: 'my-plugin', args: [] };
27+
expect(isOIDCExec(exec)).toBe(false);
28+
});
29+
30+
it('returns false when exec has no args property', () => {
31+
const exec = { command: 'my-plugin' };
32+
expect(isOIDCExec(exec)).toBe(false);
33+
});
34+
35+
it('returns false for undefined exec', () => {
36+
expect(isOIDCExec(undefined)).toBe(false);
37+
});
38+
});
239

340
describe('parseOIDCparams', () => {
441
it('Throw for empty data', () => {

src/components/Clusters/shared.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
ValidKubeconfig,
99
} from 'types';
1010
import { useClustersInfoType } from 'state/utils/getClustersInfo';
11-
import { tryParseOIDCparams } from './components/oidc-params';
11+
import { tryParseOIDCparams, isOIDCExec } from './components/oidc-params';
1212
import { hasNonOidcAuth, createUserManager } from 'state/authDataAtom';
1313
import { useNavigate } from 'react-router';
1414
import { useSetAtom } from 'jotai';
@@ -66,7 +66,8 @@ export function deleteCluster(
6666
setClusters((prev) => {
6767
// todo the same function when we switch cluster from oidc one
6868
const prevCredentials = prev?.[clusterName]?.currentContext.user.user;
69-
if (!hasNonOidcAuth(prevCredentials)) {
69+
const prevExec = (prevCredentials as KubeconfigOIDCAuth)?.exec;
70+
if (!hasNonOidcAuth(prevCredentials) && isOIDCExec(prevExec)) {
7071
const userManager = createUserManager(
7172
parseOIDCparams(prevCredentials as KubeconfigOIDCAuth),
7273
);
@@ -140,6 +141,7 @@ export function hasKubeconfigAuth(kubeconfig: Kubeconfig) {
140141
if ('client-certificate-data' in user) {
141142
return user['client-certificate-data'] && !!user['client-key-data'];
142143
}
144+
143145
const oidcData = tryParseOIDCparams(user as KubeconfigOIDCAuth);
144146

145147
if (oidcData?.issuerUrl && oidcData?.clientId && oidcData?.scopes) {

0 commit comments

Comments
 (0)