Skip to content

Commit df7ea59

Browse files
authored
Fixes #38593 - Prevent unintentional password updates (#11451)
1 parent 6e242c7 commit df7ea59

File tree

5 files changed

+150
-19
lines changed

5 files changed

+150
-19
lines changed

app/views/katello/api/v2/flatpak_remotes/base.json.rabl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,7 @@ extends 'katello/api/v2/common/org_reference'
33

44
attributes :name
55
attributes :url, :description, :username, :seeded, :registry_url
6+
7+
node :upstream_password_exists do |fr|
8+
fr.token.present?
9+
end

webpack/scenes/FlatpakRemotes/CreateEdit/FlatpakRemoteform.js

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,27 +30,27 @@ const FlatpakRemotesForm = ({ setModalOpen, remoteData }) => {
3030
name: editingName,
3131
url: editingUrl,
3232
username: editingUsername,
33-
password: editingPassword,
33+
upstream_password_exists: passwordExists,
3434
} = remoteData || {};
3535

3636
const isEditing = !!editingName;
3737
const dispatch = useDispatch();
3838
const [name, setName] = useState(editingName || '');
39-
const [url, seturl] = useState(editingUrl || '');
39+
const [url, setUrl] = useState(editingUrl || '');
4040
const [username, setUsername] = useState(editingUsername || '');
41-
const [password, setpassword] = useState(editingPassword || '');
41+
const [password, setPassword] = useState(passwordExists ? '*****' : '');
4242
const [redirect, setRedirect] = useState(false);
4343
const [saving, setSaving] = useState(false);
4444

45-
const [urlValidated, seturlValidated] = useState('default');
45+
const [urlValidated, setUrlValidated] = useState('default');
4646
const handleUrlChange = (newurl, _event) => {
47-
seturl(newurl);
47+
setUrl(newurl);
4848
if (newurl === '') {
49-
seturlValidated('default');
49+
setUrlValidated('default');
5050
} else if (/^(http(s):\/\/.)[-a-zA-Z0-9@:%._~#=]{2,256}\.[a-z]{2,6}\b([-a-zA-Z0-9@:%_.~#?&//=]*)$/g.test(newurl)) {
51-
seturlValidated('success');
51+
setUrlValidated('success');
5252
} else {
53-
seturlValidated('error');
53+
setUrlValidated('error');
5454
}
5555
};
5656

@@ -70,14 +70,24 @@ const FlatpakRemotesForm = ({ setModalOpen, remoteData }) => {
7070

7171
const onSave = () => {
7272
setSaving(true);
73+
let editingParams = {};
74+
if (isEditing && password === '*****') {
75+
editingParams = {
76+
name,
77+
url,
78+
username,
79+
};
80+
} else {
81+
editingParams = {
82+
name,
83+
url,
84+
username,
85+
token: password,
86+
};
87+
}
7388
dispatch(isEditing ?
7489
updateFlatpakRemote(
75-
editingId, {
76-
name,
77-
url,
78-
username,
79-
token: password,
80-
}, () => window.location.assign(`/flatpak_remotes/${editingId}`),
90+
editingId, editingParams, () => window.location.assign(`/flatpak_remotes/${editingId}`),
8191
() => setModalOpen(false),
8292
) :
8393
createFlatpakRemote({
@@ -168,9 +178,9 @@ const FlatpakRemotesForm = ({ setModalOpen, remoteData }) => {
168178
id="password"
169179
ouiaId="input_password"
170180
name="password"
171-
aria-label="password"
181+
aria-label="input_password"
172182
value={password}
173-
onChange={(_event, value) => setpassword(value)}
183+
onChange={(_event, value) => setPassword(value)}
174184
/>
175185
</FormGroup>
176186

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
import React from 'react';
2+
import { renderWithRedux, patientlyWaitFor, fireEvent } from 'react-testing-lib-wrapper';
3+
import { nockInstance, assertNockRequest } from '../../../../test-utils/nockWrapper';
4+
import FlatpakRemotesForm from '../FlatpakRemoteform';
5+
import api from '../../../../services/api';
6+
7+
const mockFn = jest.fn();
8+
delete window.location;
9+
window.location = { assign: mockFn };
10+
11+
afterEach(() => {
12+
mockFn.mockClear();
13+
});
14+
15+
const createFlatpakPath = api.getApiUrl('/flatpak_remotes');
16+
const updateFlatpakPath = id => api.getApiUrl(`/flatpak_remotes/${id}`);
17+
18+
const mockRemoteData = {
19+
id: 1,
20+
name: 'Test Remote',
21+
url: 'https://example.com',
22+
username: 'testuser',
23+
upstream_password_exists: true,
24+
};
25+
26+
test('Renders FlatpakRemotesForm fields correctly', () => {
27+
const { getByText } = renderWithRedux(<FlatpakRemotesForm setModalOpen={mockFn} />);
28+
expect(getByText('Name')).toBeInTheDocument();
29+
expect(getByText('URL')).toBeInTheDocument();
30+
expect(getByText('Username')).toBeInTheDocument();
31+
expect(getByText('Password')).toBeInTheDocument();
32+
});
33+
34+
test('Can save Flatpak remote from form', async (done) => {
35+
const createScope = nockInstance
36+
.post(createFlatpakPath, {
37+
organization_id: 1,
38+
name: 'New Remote',
39+
url: 'https://example.com',
40+
username: 'testuser',
41+
token: 'password123',
42+
})
43+
.reply(201, { id: 2 });
44+
45+
const { getByLabelText, getByText } = renderWithRedux(<FlatpakRemotesForm
46+
setModalOpen={mockFn}
47+
/>);
48+
fireEvent.change(getByLabelText('input_name'), { target: { value: 'New Remote' } });
49+
fireEvent.change(getByLabelText('input_url'), { target: { value: 'https://example.com' } });
50+
fireEvent.change(getByLabelText('input_username'), { target: { value: 'testuser' } });
51+
fireEvent.change(getByLabelText('input_password'), { target: { value: 'password123' } });
52+
53+
getByText('Create').click();
54+
55+
await patientlyWaitFor(() => {
56+
expect(window.location.assign).toHaveBeenCalledWith('/flatpak_remotes/2');
57+
});
58+
59+
assertNockRequest(createScope);
60+
done();
61+
});
62+
63+
test('Can update Flatpak remote from form', async (done) => {
64+
const updateScope = nockInstance
65+
.put(updateFlatpakPath(mockRemoteData.id), {
66+
name: 'Updated Remote',
67+
url: 'https://updated.com',
68+
username: 'updateduser',
69+
})
70+
.reply(200);
71+
72+
const { getByLabelText, getByText } = renderWithRedux(<FlatpakRemotesForm
73+
setModalOpen={mockFn}
74+
remoteData={mockRemoteData}
75+
/>);
76+
77+
fireEvent.change(getByLabelText('input_name'), { target: { value: 'Updated Remote' } });
78+
fireEvent.change(getByLabelText('input_url'), { target: { value: 'https://updated.com' } });
79+
fireEvent.change(getByLabelText('input_username'), { target: { value: 'updateduser' } });
80+
81+
getByText('Update').click();
82+
83+
await patientlyWaitFor(() => {
84+
expect(window.location.assign).toHaveBeenCalledWith('/flatpak_remotes/1');
85+
});
86+
87+
assertNockRequest(updateScope);
88+
done();
89+
});
90+
91+
test('Can update Flatpak remote password from placeholder', async (done) => {
92+
const updateScope = nockInstance
93+
.put(updateFlatpakPath(mockRemoteData.id), {
94+
name: mockRemoteData.name,
95+
url: mockRemoteData.url,
96+
username: mockRemoteData.username,
97+
token: 'newpassword123',
98+
})
99+
.reply(200);
100+
101+
const { getByLabelText, getByText } = renderWithRedux(<FlatpakRemotesForm
102+
setModalOpen={mockFn}
103+
remoteData={mockRemoteData}
104+
/>);
105+
106+
// Simulate changing the password from the placeholder to a new value
107+
fireEvent.change(getByLabelText('input_password'), { target: { value: 'newpassword123' } });
108+
109+
getByText('Update').click();
110+
111+
await patientlyWaitFor(() => {
112+
expect(window.location.assign).toHaveBeenCalledWith('/flatpak_remotes/1');
113+
});
114+
115+
assertNockRequest(updateScope);
116+
done();
117+
});

webpack/scenes/FlatpakRemotes/Details/FlatpakRemoteDetailActions.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export const updateFlatpakRemote = (frId, params, handleSuccess, handleError) =>
3131
url: api.getApiUrl(`/flatpak_remotes/${frId}`),
3232
handleSuccess,
3333
handleError,
34-
params: { include_permissions: true, ...params },
34+
params,
3535
successToast: () => __('Flatpak remote updated'),
3636
errorToast: error => getResponseErrorMsgs(error.response),
3737
updateData: (_prevState, respState) => respState,

webpack/scenes/FlatpakRemotes/FlatpakRemotesPage.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import Pagination from 'foremanReact/components/Pagination';
1313
import { urlBuilder } from 'foremanReact/common/urlHelpers';
1414
import { STATUS } from 'foremanReact/constants';
1515
import { selectFlatpakRemotes, selectFlatpakRemotesError, selectFlatpakRemotesStatus } from './FlatpakRemotesSelectors';
16-
import { truncate } from '../../utils/helpers';
16+
import { getResponseErrorMsgs, truncate } from '../../utils/helpers';
1717
import CreateFlatpakModal from './CreateEdit/CreateFlatpakRemoteModal';
1818
import EditFlatpakModal from './CreateEdit/EditFlatpakRemotesModal';
1919
import { deleteFlatpakRemote, scanFlatpakRemote } from './Details/FlatpakRemoteDetailActions';
@@ -126,7 +126,7 @@ const FlatpakRemotesPage = () => {
126126
<EmptyPage message={{ type: 'empty' }} />
127127
)}
128128
{error && (
129-
<EmptyPage message={{ type: 'error', text: error }} />
129+
<EmptyPage message={{ type: 'error', text: getResponseErrorMsgs(error?.response) }} />
130130
)}
131131
{results.length > 0 && (
132132
<Table variant="compact" ouiaId="flatpak-remotes-table" isStriped>

0 commit comments

Comments
 (0)