8000 Bug-Fix /_api/user/<user>/config (#15860) · meta-foundation/arangodb@bf37153 · GitHub 8000
[go: up one dir, main page]
Skip to content

Commit bf37153

Browse files
hkernbachjsteemannKVS85
authored
Bug-Fix /_api/user/<user>/config (arangodb#15860)
* fix RestUserHandler PUT config route * clang format * added suite to test DELETE, PUT of /_api/user/<user>/config * changelog * Update tests/js/common/http/grants-spec.js Co-authored-by: Jan <jsteemann@users.noreply.github.com> * Update tests/js/common/http/grants-spec.js Co-authored-by: Jan <jsteemann@users.noreply.github.com> * Update tests/js/common/http/grants-spec.js Co-authored-by: Jan <jsteemann@users.noreply.github.com> * Update tests/js/common/http/grants-spec.js * also reset root users config to default after each test * Update tests/js/common/http/grants-spec.js Co-authored-by: Jan <jsteemann@users.noreply.github.com> Co-authored-by: Jan <jsteemann@users.noreply.github.com> Co-authored-by: Vadim <vadim@arangodb.com>
1 parent 3278282 commit bf37153

File tree

3 files changed

+245
-20
lines changed

3 files changed

+245
-20
lines changed

‎CHANGELOG

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
devel
22
-----
33

4+
* Fixed ES-1078: The REST API endpoint for handling `/_api/user/${user}/config`
5+
did not work properly. The supplied data by sending a PUT request has not been
6+
stored to the correct location. The Web UI uses this endpoint to store its
7+
graph properties for storing the visualization properties. As this endpoint
8+
did not work as expected, the graph visualization properties did not get
9+
persisted as well. This is now resolved.
10+
411
* Updated arangosync to 2.9.0.
512

613
* Speed up initial sync (in case there is already data present) by prefetching

‎arangod/RestHandler/RestUsersHandler.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -445,10 +445,10 @@ RestStatus RestUsersHandler::putRequest(auth::UserManager* um) {
445445
// to a remove of the config option.
446446
res = um->updateUser(name, [&](auth::User& u) {
447447
VPackSlice newVal = body;
448-
VPackSlice oldConf = u.userData();
448+
VPackSlice oldConf = u.configData();
449449
if (!newVal.isObject() || !newVal.hasKey("value")) {
450450
if (oldConf.isObject() && oldConf.hasKey(key)) {
451-
u.setUserData(VPackCollection::remove(
451+
u.setConfigData(VPackCollection::remove(
452452
oldConf, std::unordered_set<std::string>{key}));
453453
} // Nothing to do. We do not have a config yet.
454454
} else { // We need to merge the new key into the config
@@ -457,9 +457,10 @@ RestStatus RestUsersHandler::putRequest(auth::UserManager* um) {
457457

458458
if (oldConf.isObject() &&
459459
!oldConf.isEmptyObject()) { // merge value in
460-
u.setUserData(VPackCollection::merge(oldConf, b.slice(), false));
460+
u.setConfigData(
461+
VPackCollection::merge(oldConf, b.slice(), false));
461462
} else {
462-
u.setUserData(std::move(b));
463+
u.setConfigData(std::move(b));
463464
}
464465
}
465466

‎tests/js/common/http/grants-spec.js

Lines changed: 233 additions & 16 deletions
323
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,19 @@ const request = require('@arangodb/request');
2929
const users = require('@arangodb/users');
3030
const db = require('@arangodb').db;
3131

32-
describe('Grants', function() {
33-
before(function() {
32+
describe('Grants', function () {
33+
before(function () {
3434
db._drop('grants');
3535
db._create('grants');
3636
});
37-
beforeEach(function() {
37+
beforeEach(function () {
3838
try {
3939
// remove any such user if it exists
4040
users.remove('hans');
41-
} catch (err) {}
41+
} catch (err) {
42+
}
4243
});
43-
afterEach(function() {
44+
afterEach(function () {
4445
let resp = request.put({
4546
url: '/_admin/server/mode',
4647
body: {'mode': 'default'},
@@ -51,10 +52,10 @@ describe('Grants', function() {
5152
require('internal').wait(5.0);
5253
users.remove('hans');
5354
});
54-
after(function() {
55+
after(function () {
5556
db._drop('grants');
5657
});
57-
it('should show the effective rights for a user', function() {
58+
it('should show the effective rights for a user', function () {
5859
users.save('hans');
5960
users.grantDatabase('hans', '_system', 'rw');
6061
let resp = request.get({
@@ -63,11 +64,11 @@ describe('Grants', function() {
6364
});
6465
expect(JSON.parse(resp.body)).to.have.property('result', 'rw');
6566
});
66-
67-
it('should show the effective rights when readonly mode is on', function() {
67+
68+
it('should show the effective rights when readonly mode is on', function () {
6869
users.save('hans');
6970
users.grantDatabase('hans', '_system', 'rw');
70-
71+
7172
let resp;
7273
resp = request.put({
7374
url: '/_admin/server/mode',
@@ -81,10 +82,10 @@ describe('Grants', function() {
8182
expect(JSON.parse(resp.body)).to.have.property('result', 'ro');
8283
});
8384

84-
it('should show the configured rights when readonly mode is on and configured is requested', function() {
85+
it('should show the configured rights when readonly mode is on and configured is requested', function () {
8586
users.save('hans');
8687
users.grantDatabase('hans', '_system', 'rw');
87-
88+
8889
let resp;
8990
resp = request.put({
9091
url: '/_admin/server/mode',
@@ -98,11 +99,11 @@ describe('Grants', function() {
9899
expect(JSON.parse(resp.body)).to.have.property('result', 'rw');
99100
});
100101

101-
it('should show the effective rights when readonly mode is on', function() {
102+
it('should show the effective rights when readonly mode is on', function () {
102103
users.save('hans');
103104
users.grantDatabase('hans', '_system', 'rw');
104105
users.grantCollection('hans', '_system', 'grants');
105-
106+
106107
let resp;
107108
resp = request.put({
108109
url: '/_admin/server/mode',
@@ -116,11 +117,11 @@ describe('Grants', function() {
116117
expect(JSON.parse(resp.body)).to.have.property('result', 'ro');
117118
});
118119

119-
it('should show the configured rights when readonly mode is on and configured is requested', function() {
120+
it('should show the configured rights when readonly mode is on and configured is requested', function () {
120121
users.save('hans');
121122
users.grantDatabase('hans', '_system', 'rw');
122123
users.grantCollection('hans', '_system', 'grants');
123-
124+
124125
let resp;
125126
resp = request.put({
126127
url: '/_admin/server/mode',
@@ -134,3 +135,219 @@ describe('Grants', function() {
134135
expect(JSON.parse(resp.body)).to.have.property('result', 'rw');
135136
});
136137
});
138+
139+
describe('UserProperties', function () {
140+
const rootUser = 'root';
141+
const rootDB = '_system';
142+
143+
const nonRootDB = 'nonRootDB';
144+
const nonRootUser = 'notRoot';
145+
146+
const configPropertyAttribute = 'graphs';
147+
const additionalPropertyAttribute = 'someAdditionalAttribute';
148+
149+
const initConfigObject = {
150+
a: true,
151+
b: false,
152+
c: 1,
153+
d: 1.1,
154+
e: "aString",
155+
f: [1, 2, 3],
156+
g: {"anObjectKey": "anObjectValue"}
157+
};
158+
159+
const overwriteConfigObject = {
160+
c: 1337,
161+
someNewParam: "MJ7"
162+
};
163+
164+
const mergeConfigObject = {
165+
"thisMustStay": true
166+
};
167+
168+
const verifyEmptyConfig = (result) => {
169+
expect(result.error).to.be.false;
170+
expect(result).to.have.property('code', 200);
171+
expect(result).to.have.property('result', null);
172+
};
173+
174+
const verifyInitObject = (result) => {
175+
expect(result.error).to.be.false;
176+
expect(result).to.have.property('code', 200);
177+
178+
result = result.result[configPropertyAttribute];
179+
expect(result.a).to.be.true;
180+
expect(result.b).to.be.false;
181+
expect(result).to.have.property('c', 1);
182+
expect(result).to.have.property('d', 1.1);
183+
expect(result).to.have.property('e', "aString");
184+
expect(result.f).to.be.an('array');
185+
expect(result.g).to.be.an('object');
186+
};
187+
188+
const verifyOverwriteConfigObject = (result) => {
189+
// We do have one "global" property we can modify. If we write to that same top-level attribute again, it will be
190+
// fully replaced by the supplied object given. In case we write to another top-level attribute, it will be merged with
191+
// the other already available ones (This is tested in verifyMergedConfigObject).
192+
expect(result.error).to.be.false;
193+
expect(result).to.have.property('code', 200);
194+
195+
result = result.result[configPropertyAttribute];
196+
expect(result).to.have.property('c', 1337);
197+
expect(result).to.have.property('someNewParam', "MJ7");
198+
};
199+
200+
const verifyMergedConfigObject = (result) => {
201+
// needs to be called after we've written `overwriteConfigObject`
202+
verifyOverwriteConfigObject(result);
203+
const expectedMergeObject = result.result[additionalPropertyAttribute];
204+
expect(expectedMergeObject.thisMustStay).to.be.true;
205+
};
206+
207+
const generateReadConfigUrl = (user, database) => {
208+
return `/_db/${database}/_api/user/${user}/config`;
209+
};
210+
211+
const generateWriteConfigUrl = (user, database, attribute) => {
212+
if (!attribute) {
213+
attribute = configPropertyAttribute;
214+
}
215+
return `/_db/${database}/_api/user/${user}/config/${attribute}`;
216+
};
217+
218+
const generateConfigBody = (object) => {
219+
return {
220+
value: object
221+
};
222+
};
223+
224+
before(function () {
225+
db._createDatabase(nonRootDB);
226+
});
227+
228+
beforeEach(function () {
229+
try {
230+
// remove any such user if it exists
231+
users.remove(nonRootUser);
232+
} catch (ignore) {
233+
}
234+
});
235+
236+
afterEach(function () {
237+
try {
238+
users.remove(nonRootUser);
239+
} catch (ignore) {
240+
}
241+
242+
// cleanup root users config we might have been modified in any of our tests
243+
const cleanupRootUrl = generateWriteConfigUrl(rootUser, rootDB);
244+
request.delete({
245+
url: cleanupRootUrl,
246+
json: true
247+
});
248+
});
249+
250+
after(function () {
251+
db._dropDatabase(nonRootDB);
252+
});
253+
254+
const testSuiteHelper = (user, database) => {
255+
const readUrl = generateReadConfigUrl(user, database);
256+
const writeUrl = generateWriteConfigUrl(user, database);
257+
let response;
258+
259+
// should be empty by default
260+
response = request.get({
261+
url: readUrl,
262+
json: true
263+
});
264+
verifyEmptyConfig(JSON.parse(response.body));
265+
266+
// stores the init object
267+
response = request.put({
268+
url: writeUrl,
269+
body: generateConfigBody(initConfigObject),
270+
json: true
271+
});
272+
expect(response.statusCode).to.equal(200);
273+
274+
// should now be the same as init object
275+
response = request.get({
276+
url: readUrl,
277+
json: true
278+
});
279+
verifyInitObject(JSON.parse(response.body));
280+
281+
// now try to overwrite as well
282+
response = request.put({
283+
url: writeUrl,
284+
body: generateConfigBody(overwriteConfigObject),
285+
json: true
286+
});
287+
expect(response.statusCode).to.equal(200);
288+
289+
response = request.get({
290+
url: readUrl,
291+
json: true
292+
});
293+
verifyOverwriteConfigObject(JSON.parse(response.body));
294+
295+
// now try a merge with another storage attribute
296+
const additionalWriteUrl = generateWriteConfigUrl(user, database, additionalPropertyAttribute);
297+
response = request.put({
298+
url: additionalWriteUrl,
299+
body: generateConfigBody(mergeConfigObject),
300+
json: true
301+
});
302+
expect(response.statusCode).to.equal(200);
303+
response = request.get({
304+
url: readUrl,
305+
json: true
306+
});
307+
verifyMergedConfigObject(JSON.parse(response.body));
308+
309+
// test delete (remove additional key)
310+
response = request.delete({
311+
url: additionalWriteUrl,
312+
json: true
313+
});
314+
response = request.get({
315+
url: readUrl,
316+
json: true
317+
});
318+
verifyOverwriteConfigObject(JSON.parse(response.body));
319+
320+
// finally, test delete of graph attribute
321+
response = request.delete({
322+
url: writeUrl,
+
json: true
324+
});
325+
// should be fully empty now again
326+
response = request.get({
327+
url: readUrl,
328+
json: true
329+
});
330+
verifyEmptyConfig(JSON.parse(response.body));
331+
};
332+
333+
it('should store/patch/delete root config data in _system database', function () {
334+
testSuiteHelper(rootUser, rootDB);
335+
});
336+
337+
it('should store/patch/delete root config data in non-system database', function () {
338+
testSuiteHelper(rootUser, nonRootDB);
339+
});
340+
341+
it('should store/patch/delete non-root user config data in _system database', function () {
342+
users.save(nonRootUser);
343+
users.grantDatabase(nonRootUser, rootDB, 'rw');
344+
testSuiteHelper(nonRootUser, rootDB);
345+
});
346+
347+
it('should store/patch/delete non-root user config data in non-system database', function () {
348+
users.save(nonRootUser);
349+
users.grantDatabase(nonRootUser, nonRootDB, 'rw');
350+
testSuiteHelper(nonRootUser, nonRootDB);
351+
});
352+
353+
});

0 commit comments

Comments
 (0)
0