Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Use Environment variables for secrets capabilities #1702 #1877

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app/common/public/locales/en/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -295,5 +295,8 @@
"gestureNameCannotBeEmptyError": "Gesture name cannot be empty",
"gestureInvalidJsonError": "Invalid JSON file. Unable to parse the file content",
"gestureImportedFrom": "Gesture imported from '{{fileName}}'",
"invalidSessionFile": "Invalid session file"
"invalidSessionFile": "Invalid session file",
"Environment Variables": "Environment Variables",
"Variable Name": "Variable Name",
"Delete Variable": "Delete Variable"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This specific key is not used anymore

}
17 changes: 16 additions & 1 deletion app/common/renderer/actions/Inspector.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import _ from 'lodash';

import {SAVED_FRAMEWORK, SET_SAVED_GESTURES} from '../../shared/setting-defs';
import {
ENVIRONMENT_VARIABLES,
SAVED_FRAMEWORK,
SET_SAVED_GESTURES,
} from '../../shared/setting-defs';
import {POINTER_TYPES} from '../constants/gestures';
import {APP_MODE, NATIVE_APP} from '../constants/session-inspector';
import i18n from '../i18next';
Expand Down Expand Up @@ -116,6 +120,7 @@ export const TOGGLE_SHOW_ATTRIBUTES = 'TOGGLE_SHOW_ATTRIBUTES';
export const TOGGLE_REFRESHING_STATE = 'TOGGLE_REFRESHING_STATE';

export const SET_GESTURE_UPLOAD_ERROR = 'SET_GESTURE_UPLOAD_ERROR';
export const SET_ENVIRONMENT_VARIABLES = 'SET_ENVIRONMENT_VARIABLES';

const KEEP_ALIVE_PING_INTERVAL = 20 * 1000;
const NO_NEW_COMMAND_LIMIT = 24 * 60 * 60 * 1000; // Set timeout to 24 hours
Expand Down Expand Up @@ -357,6 +362,9 @@ export function quitSession(reason, killedByUser = true) {
const applyAction = applyClientMethod({methodName: 'quit'});
await applyAction(dispatch, getState);
dispatch({type: QUIT_SESSION_DONE});
// Reload environment variables from persistent settings after session ends
const loadEnvAction = loadEnvironmentVariables();
await loadEnvAction(dispatch);
Comment on lines +365 to +367
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this code is necessary

Copy link
Collaborator

@eglitise eglitise Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned above, I don't think this code is necessary because we only care about loading the env var values in the context of an active session. Quitting a session won't delete them from the persistent settings in any case.

if (!killedByUser) {
showError(new Error(reason || i18n.t('Session has been terminated')), {secs: 0});
}
Expand Down Expand Up @@ -919,6 +927,13 @@ export function setGestureUploadErrors(errors) {
};
}

export function loadEnvironmentVariables() {
return async (dispatch) => {
const envVars = (await getSetting(ENVIRONMENT_VARIABLES)) || [];
dispatch({type: SET_ENVIRONMENT_VARIABLES, envVars});
};
}

export function uploadGesturesFromFile(fileList) {
return async (dispatch) => {
const gestures = await readTextFromUploadedFiles(fileList);
Expand Down
57 changes: 57 additions & 0 deletions app/common/renderer/actions/Session.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@ import {APP_MODE} from '../constants/session-inspector';
import i18n from '../i18next';
import {getSetting, ipcRenderer, setSetting} from '../polyfills';
import {fetchSessionInformation, formatSeleniumGridSessions} from '../utils/attaching-to-session';
import {interpolateEnvironmentVariables} from '../utils/env-utils';
import {downloadFile, parseSessionFileContents} from '../utils/file-handling';
import {log} from '../utils/logger';
import {addVendorPrefixes} from '../utils/other';
import {quitSession, setSessionDetails} from './Inspector';

export const ENVIRONMENT_VARIABLES = 'ENVIRONMENT_VARIABLES';

export const NEW_SESSION_REQUESTED = 'NEW_SESSION_REQUESTED';
export const NEW_SESSION_LOADING = 'NEW_SESSION_LOADING';
export const NEW_SESSION_DONE = 'NEW_SESSION_DONE';
Expand Down Expand Up @@ -68,6 +71,10 @@ export const SET_CAPABILITY_NAME_ERROR = 'SET_CAPABILITY_NAME_ERROR';
export const SET_STATE_FROM_URL = 'SET_STATE_FROM_URL';
export const SET_STATE_FROM_FILE = 'SET_STATE_FROM_FILE';

export const SET_ENVIRONMENT_VARIABLES = 'SET_ENVIRONMENT_VARIABLES';
export const ADD_ENVIRONMENT_VARIABLE = 'ADD_ENVIRONMENT_VARIABLE';
export const DELETE_ENVIRONMENT_VARIABLE = 'DELETE_ENVIRONMENT_VARIABLE';

const APPIUM_SESSION_FILE_VERSION = '1.0';

const CAPS_NEW_COMMAND = 'appium:newCommandTimeout';
Expand Down Expand Up @@ -229,7 +236,25 @@ export function newSession(caps, attachSessId = null) {

dispatch({type: NEW_SESSION_REQUESTED, caps});

// Get environment variables from state
const environmentVariables = session.environmentVariables || [];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handling for default value is not needed since it is guaranteed to be set


// Get capabilities and interpolate environment variables
let desiredCapabilities = caps ? getCapsObject(caps) : {};

// Modify this section to handle W3C capabilities format
if (desiredCapabilities.alwaysMatch) {
desiredCapabilities.alwaysMatch = interpolateEnvironmentVariables(
desiredCapabilities.alwaysMatch,
environmentVariables,
);
} else {
desiredCapabilities = interpolateEnvironmentVariables(
desiredCapabilities,
environmentVariables,
);
}

let host, port, username, accessKey, https, path, token;
desiredCapabilities = addCustomCaps(desiredCapabilities);

Expand Down Expand Up @@ -1190,3 +1215,35 @@ export function initFromQueryString(loadNewSession) {
}
};
}

export function setEnvironmentVariables(envVars) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method isn't used anywhere

return async (dispatch) => {
await setSetting(ENVIRONMENT_VARIABLES, envVars);
dispatch({type: SET_ENVIRONMENT_VARIABLES, envVars});
};
}

export function loadEnvironmentVariables() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be renamed to setEnvironmentVariables for consistency with setVisibleProviders. I was initially confused by its call in components/Session/Session.jsx.

return async (dispatch) => {
const envVars = (await getSetting(ENVIRONMENT_VARIABLES)) || [];
dispatch({type: SET_ENVIRONMENT_VARIABLES, envVars});
};
}

export function addEnvironmentVariable(key, value) {
return async (dispatch, getState) => {
const currentEnvVars = getState().inspector.environmentVariables || [];
Copy link
Collaborator

@eglitise eglitise Jan 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why these are retrieved from the inspector reducer (they aren't relevant anymore at that point). The variable also has a default value, so the additional empty array is not needed.

Suggested change
const currentEnvVars = getState().inspector.environmentVariables || [];
const currentEnvVars = getState().session.environmentVariables;

const newEnvVars = [...currentEnvVars, {key, value}];
await setSetting(ENVIRONMENT_VARIABLES, newEnvVars);
dispatch({type: ADD_ENVIRONMENT_VARIABLE, key, value});
};
}

export function deleteEnvironmentVariable(key) {
return async (dispatch, getState) => {
const currentEnvVars = getState().inspector.environmentVariables || [];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Suggested change
const currentEnvVars = getState().inspector.environmentVariables || [];
const currentEnvVars = getState().session.environmentVariables;

const newEnvVars = currentEnvVars.filter((v) => v.key !== key);
await setSetting(ENVIRONMENT_VARIABLES, newEnvVars);
dispatch({type: DELETE_ENVIRONMENT_VARIABLE, key});
};
}
26 changes: 13 additions & 13 deletions app/common/renderer/components/Inspector/Inspector.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {downloadFile} from '../../utils/file-handling';
import Commands from './Commands.jsx';
import GestureEditor from './GestureEditor.jsx';
import HeaderButtons from './HeaderButtons.jsx';
import InspectorStyles from './Inspector.module.css';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the changes in this file are renaming and can be omitted in this PR

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still not resolved

import styles from './Inspector.module.css';
import Recorder from './Recorder.jsx';
import SavedGestures from './SavedGestures.jsx';
import Screenshot from './Screenshot.jsx';
Expand Down Expand Up @@ -221,7 +221,7 @@ const Inspector = (props) => {
}, [showKeepAlivePrompt]);

const screenShotControls = (
<div className={InspectorStyles['screenshot-controls']}>
<div className={styles['screenshot-controls']}>
<Space size="middle">
<Tooltip
title={t(showCentroids ? 'Hide Element Handles' : 'Show Element Handles')}
Expand Down Expand Up @@ -263,22 +263,22 @@ const Inspector = (props) => {
);

const main = (
<div className={InspectorStyles['inspector-main']}>
<div className={styles['inspector-main']}>
<div
id="screenshotContainer"
className={InspectorStyles['screenshot-container']}
className={styles['screenshot-container']}
ref={screenshotContainerEl}
>
{screenShotControls}
{showScreenshot && <Screenshot {...props} scaleRatio={scaleRatio} />}
{screenshotError && t('couldNotObtainScreenshot', {screenshotError})}
{!showScreenshot && (
<Spin size="large" spinning={true}>
<div className={InspectorStyles.screenshotBox} />
<div className={styles.screenshotBox} />
</Spin>
)}
</div>
<div id="sourceTreeContainer" className={InspectorStyles['interaction-tab-container']}>
<div id="sourceTreeContainer" className={styles['interaction-tab-container']}>
<Tabs
activeKey={selectedInspectorTab}
size="small"
Expand Down Expand Up @@ -331,15 +331,15 @@ const Inspector = (props) => {
</div>
<div
id="selectedElementContainer"
className={`${InspectorStyles['interaction-tab-container']} ${InspectorStyles['element-detail-container']} action-col`}
className={`${styles['interaction-tab-container']} ${styles['element-detail-container']} action-col`}
>
<Card
title={
<span>
<TagOutlined /> {t('selectedElement')}
</span>
}
className={InspectorStyles['selected-element-card']}
className={styles['selected-element-card']}
>
{selectedElement.path && <SelectedElement {...props} />}
{!selectedElement.path && <i>{t('selectElementInSource')}</i>}
Expand All @@ -359,7 +359,7 @@ const Inspector = (props) => {
<ThunderboltOutlined /> {t('Execute Commands')}
</span>
}
className={InspectorStyles['interaction-tab-card']}
className={styles['interaction-tab-card']}
>
<Commands {...props} />
</Card>
Expand All @@ -376,7 +376,7 @@ const Inspector = (props) => {
<HighlightOutlined /> {t('Gesture Builder')}
</span>
}
className={InspectorStyles['interaction-tab-card']}
className={styles['interaction-tab-card']}
>
<GestureEditor {...props} />
</Card>
Expand All @@ -387,7 +387,7 @@ const Inspector = (props) => {
<HighlightOutlined /> {t('Saved Gestures')}
</span>
}
className={InspectorStyles['interaction-tab-card']}
className={styles['interaction-tab-card']}
>
<SavedGestures {...props} />
</Card>
Expand All @@ -410,7 +410,7 @@ const Inspector = (props) => {
<InfoCircleOutlined /> {t('Session Information')}
</span>
}
className={InspectorStyles['interaction-tab-card']}
className={styles['interaction-tab-card']}
>
<SessionInfo {...props} />
</Card>
Expand All @@ -423,7 +423,7 @@ const Inspector = (props) => {
);

return (
<div className={InspectorStyles['inspector-container']}>
<div className={styles['inspector-container']}>
<HeaderButtons quitCurrentSession={quitCurrentSession} {...props} />
{main}
<Modal
Expand Down
101 changes: 101 additions & 0 deletions app/common/renderer/components/Session/EnvironmentVariables.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import {DeleteOutlined, PlusOutlined, SettingOutlined} from '@ant-design/icons';
import {Button, Card, Input, Popconfirm, Space, Table, Tooltip} from 'antd';
import {useState} from 'react';

import SessionStyles from './Session.module.css';

const EnvironmentVariables = ({t, envVars, addVariable, deleteVariable}) => {
const [newVar, setNewVar] = useState({key: '', value: ''});

const tableData = Array.from(envVars, ([key, value]) => ({key, value}));

const columns = [
{
title: t('Variable Name'),
dataIndex: 'key',
key: 'key',
width: '40%',
},
{
title: t('Value'),
dataIndex: 'value',
key: 'value',
width: '40%',
render: (text) => <Input.Password value={text} readOnly />,
},
{
title: t('Actions'),
key: 'action',
width: '20%',
render: (_, record) => (
<Tooltip zIndex={3} title={t('Delete')}>
<Popconfirm
zIndex={4}
title={t('confirmDeletion')}
placement="topRight"
okText={t('OK')}
cancelText={t('Cancel')}
onConfirm={() => deleteVariable(record.key)}
>
<Button icon={<DeleteOutlined />} />
</Popconfirm>
</Tooltip>
),
},
];

const handleAddVariable = () => {
if (!newVar.key || !newVar.value) {
return;
}

// Check for duplicate keys is no longer needed since Map handles this automatically
addVariable(newVar.key, newVar.value);
setNewVar({key: '', value: ''});
};

return (
<Card
title={
<span>
<SettingOutlined /> {t('Environment Variables')}
</span>
}
className={SessionStyles['interaction-tab-card']}
>
<div className={SessionStyles.container}>
<div className={SessionStyles.addForm}>
<Space.Compact style={{width: '100%'}}>
<Input
placeholder={t('Variable Name')}
value={newVar.key}
onChange={(e) => setNewVar({...newVar, key: e.target.value})}
/>
<Input.Password
placeholder={t('Value')}
value={newVar.value}
onChange={(e) => setNewVar({...newVar, value: e.target.value})}
/>
<Button
type="primary"
icon={<PlusOutlined />}
onClick={handleAddVariable}
disabled={!newVar.key || !newVar.value}
>
{t('Add')}
</Button>
</Space.Compact>
</div>
<Table
columns={columns}
dataSource={tableData}
pagination={false}
rowKey="key"
size="small"
/>
</div>
</Card>
);
};

export default EnvironmentVariables;
Comment on lines +1 to +101
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really a fan of the Card view, how about this?
I've also adjusted the table for better handling of more rows (added starting point for scroll + sticky header)

Suggested change
import {DeleteOutlined, PlusOutlined, SettingOutlined} from '@ant-design/icons';
import {Button, Card, Input, Popconfirm, Space, Table, Tooltip} from 'antd';
import {useState} from 'react';
import SessionStyles from './Session.module.css';
const EnvironmentVariables = ({t, envVars, addVariable, deleteVariable}) => {
const [newVar, setNewVar] = useState({key: '', value: ''});
const tableData = Array.from(envVars, ([key, value]) => ({key, value}));
const columns = [
{
title: t('Variable Name'),
dataIndex: 'key',
key: 'key',
width: '40%',
},
{
title: t('Value'),
dataIndex: 'value',
key: 'value',
width: '40%',
render: (text) => <Input.Password value={text} readOnly />,
},
{
title: t('Actions'),
key: 'action',
width: '20%',
render: (_, record) => (
<Tooltip zIndex={3} title={t('Delete')}>
<Popconfirm
zIndex={4}
title={t('confirmDeletion')}
placement="topRight"
okText={t('OK')}
cancelText={t('Cancel')}
onConfirm={() => deleteVariable(record.key)}
>
<Button icon={<DeleteOutlined />} />
</Popconfirm>
</Tooltip>
),
},
];
const handleAddVariable = () => {
if (!newVar.key || !newVar.value) {
return;
}
// Check for duplicate keys is no longer needed since Map handles this automatically
addVariable(newVar.key, newVar.value);
setNewVar({key: '', value: ''});
};
return (
<Card
title={
<span>
<SettingOutlined /> {t('Environment Variables')}
</span>
}
className={SessionStyles['interaction-tab-card']}
>
<div className={SessionStyles.container}>
<div className={SessionStyles.addForm}>
<Space.Compact style={{width: '100%'}}>
<Input
placeholder={t('Variable Name')}
value={newVar.key}
onChange={(e) => setNewVar({...newVar, key: e.target.value})}
/>
<Input.Password
placeholder={t('Value')}
value={newVar.value}
onChange={(e) => setNewVar({...newVar, value: e.target.value})}
/>
<Button
type="primary"
icon={<PlusOutlined />}
onClick={handleAddVariable}
disabled={!newVar.key || !newVar.value}
>
{t('Add')}
</Button>
</Space.Compact>
</div>
<Table
columns={columns}
dataSource={tableData}
pagination={false}
rowKey="key"
size="small"
/>
</div>
</Card>
);
};
export default EnvironmentVariables;
import {DeleteOutlined, PlusOutlined} from '@ant-design/icons';
import {Button, Col, Form, Input, Popconfirm, Row, Table, Tooltip} from 'antd';
import {useState} from 'react';
const EnvironmentVariables = ({t, envVars, addVariable, deleteVariable}) => {
const [newVar, setNewVar] = useState({key: '', value: ''});
const tableData = Array.from(envVars, ([key, value]) => ({key, value}));
const columns = [
{
title: t('Variable Name'),
dataIndex: 'key',
key: 'key',
width: '45%',
},
{
title: t('Value'),
dataIndex: 'value',
key: 'value',
width: '45%',
render: (text) => <Input.Password value={text} readOnly />,
},
{
title: t('Actions'),
key: 'action',
width: '10%',
render: (_, record) => (
<Tooltip zIndex={3} title={t('Delete')}>
<Popconfirm
zIndex={4}
title={t('confirmDeletion')}
placement="topRight"
okText={t('OK')}
cancelText={t('Cancel')}
onConfirm={() => deleteVariable(record.key)}
>
<Button icon={<DeleteOutlined />} />
</Popconfirm>
</Tooltip>
),
},
];
const handleAddVariable = () => {
if (!newVar.key || !newVar.value) {
return;
}
// Check for duplicate keys is no longer needed since Map handles this automatically
addVariable(newVar.key, newVar.value);
setNewVar({key: '', value: ''});
};
return (
<>
<Row gutter={8}>
<Col span={11}>
<Form.Item>
<Input
placeholder={t('Variable Name')}
value={newVar.key}
onChange={(e) => setNewVar({...newVar, key: e.target.value})}
/>
</Form.Item>
</Col>
<Col span={11}>
<Form.Item>
<Input.Password
placeholder={t('Value')}
value={newVar.value}
onChange={(e) => setNewVar({...newVar, value: e.target.value})}
/>
</Form.Item>
</Col>
<Col span={2}>
<Form.Item>
<Button
block
type="primary"
icon={<PlusOutlined />}
onClick={handleAddVariable}
disabled={!newVar.key || !newVar.value}
>
{t('Add')}
</Button>
</Form.Item>
</Col>
</Row>
<Table
columns={columns}
dataSource={tableData}
pagination={false}
sticky={true}
rowKey="key"
scroll={{y: 'calc(100vh - 34em)'}}
size="small"
/>
</>
);
};
export default EnvironmentVariables;

Loading
Loading