Skip to content

Commit 14dfd61

Browse files
vlad-dargelvlad-dargel
authored
Merge pull request #1503 from RedisInsight/feature/RI-3574_set_password_recommendation
#RI-3574 - add set password recommendation
2 parents bcae1c1 + ee095e9 commit 14dfd61

File tree

9 files changed

+236
-40
lines changed

9 files changed

+236
-40
lines changed

‎redisinsight/api/src/__mocks__/errors.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@ export const mockRedisNoAuthError: ReplyError = {
66
message: 'NOAUTH authentication is required',
77
};
88

9+
export const mockRedisNoPasswordError: ReplyError = {
10+
name: 'ReplyError',
11+
command: 'AUTH',
12+
message: 'ERR Client sent AUTH, but no password is set',
13+
};
14+
915
export const mockRedisNoPermError: ReplyError = {
1016
name: 'ReplyError',
1117
command: 'GET',

‎redisinsight/api/src/constants/recommendations.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,5 @@ export const RECOMMENDATION_NAMES = Object.freeze({
1212
COMPRESS_HASH_FIELD_NAMES: 'compressHashFieldNames',
1313
COMPRESSION_FOR_LIST: 'compressionForList',
1414
ZSET_HASHTABLE_TO_ZIPLIST: 'zSetHashtableToZiplist',
15+
SET_PASSWORD: 'setPassword',
1516
});

‎redisinsight/api/src/modules/recommendation/providers/recommendation.provider.spec.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import IORedis from 'ioredis';
22
import { when } from 'jest-when';
33
import { RECOMMENDATION_NAMES } from 'src/constants';
4+
import { mockRedisNoAuthError, mockRedisNoPasswordError } from 'src/__mocks__';
45
import { RecommendationProvider } from 'src/modules/recommendation/providers/recommendation.provider';
56

67
const nodeClient = Object.create(IORedis.prototype);
@@ -21,6 +22,16 @@ const mockRedisConfigResponse = ['name', '512'];
2122
const mockRedisClientsResponse_1: string = '# Clients\r\nconnected_clients:100\r\n';
2223
const mockRedisClientsResponse_2: string = '# Clients\r\nconnected_clients:101\r\n';
2324

25+
const mockRedisAclListResponse_1: string[] = [
26+
'user <pass off resetchannels -@all',
27+
'user default on #d74ff0ee8da3b9806b18c877dbf29bbde50b5bd8e4dad7a3a725000feb82e8f1 ~* &* +@all',
28+
];
29+
30+
const mockRedisAclListResponse_2: string[] = [
31+
...mockRedisAclListResponse_1,
32+
'user test_2 on nopass ~* &* +@all',
33+
];
34+
2435
const mockKeys = [
2536
{
2637
name: Buffer.from('name'), type: 'string', length: 10, memory: 10, ttl: -1,
@@ -389,4 +400,59 @@ describe('RecommendationProvider', () => {
389400
expect(connectionClientsRecommendation).toEqual(null);
390401
});
391402
});
403+
404+
describe('determineSetPasswordRecommendation', () => {
405+
it('should not return setPassword recommendation', async () => {
406+
when(nodeClient.sendCommand)
407+
.calledWith(jasmine.objectContaining({ name: 'acl' }))
408+
.mockResolvedValue(mockRedisAclListResponse_1);
409+
410+
const setPasswordRecommendation = await service
411+
.determineSetPasswordRecommendation(nodeClient);
412+
expect(setPasswordRecommendation).toEqual(null);
413+
});
414+
415+
it('should return setPassword recommendation', async () => {
416+
when(nodeClient.sendCommand)
417+
.calledWith(jasmine.objectContaining({ name: 'acl' }))
418+
.mockResolvedValue(mockRedisAclListResponse_2);
419+
420+
const setPasswordRecommendation = await service
421+
.determineSetPasswordRecommendation(nodeClient);
422+
expect(setPasswordRecommendation).toEqual({ name: RECOMMENDATION_NAMES.SET_PASSWORD });
423+
});
424+
425+
it('should not return setPassword recommendation when acl command executed with error',
426+
async () => {
427+
when(nodeClient.sendCommand)
428+
.calledWith(jasmine.objectContaining({ name: 'acl' }))
429+
.mockRejectedValue('some error');
430+
431+
const setPasswordRecommendation = await service
432+
.determineSetPasswordRecommendation(nodeClient);
433+
expect(setPasswordRecommendation).toEqual(null);
434+
});
435+
436+
it('should not return setPassword recommendation when acl command executed with error',
437+
async () => {
438+
when(nodeClient.sendCommand)
439+
.calledWith(jasmine.objectContaining({ name: 'auth' }))
440+
.mockRejectedValue(mockRedisNoAuthError);
441+
442+
const setPasswordRecommendation = await service
443+
.determineSetPasswordRecommendation(nodeClient);
444+
expect(setPasswordRecommendation).toEqual(null);
445+
});
446+
447+
it('should return setPassword recommendation when acl command executed with no password error',
448+
async () => {
449+
when(nodeClient.sendCommand)
450+
.calledWith(jasmine.objectContaining({ name: 'auth' }))
451+
.mockRejectedValue(mockRedisNoPasswordError);
452+
453+
const setPasswordRecommendation = await service
454+
.determineSetPasswordRecommendation(nodeClient);
455+
expect(setPasswordRecommendation).toEqual({ name: RECOMMENDATION_NAMES.SET_PASSWORD });
456+
});
457+
});
392458
});

‎redisinsight/api/src/modules/recommendation/providers/recommendation.provider.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,4 +280,43 @@ export class RecommendationProvider {
280280
return null;
281281
}
282282
}
283+
284+
/**
285+
* Check set password recommendation
286+
* @param redisClient
287+
*/
288+
289+
async determineSetPasswordRecommendation(
290+
redisClient: Redis | Cluster,
291+
): Promise<Recommendation> {
292+
if (await this.checkAuth(redisClient)) {
293+
return { name: RECOMMENDATION_NAMES.SET_PASSWORD };
294+
}
295+
296+
try {
297+
const users = await redisClient.sendCommand(
298+
new Command('acl', ['list'], { replyEncoding: 'utf8' }),
299+
) as string[];
300+
301+
const nopassUser = users.some((user) => user.split(' ')[3] === 'nopass');
302+
303+
return nopassUser ? { name: RECOMMENDATION_NAMES.SET_PASSWORD } : null;
304+
} catch (err) {
305+
this.logger.error('Can not determine set password recommendation', err);
306+
return null;
307+
}
308+
}
309+
310+
private async checkAuth(redisClient: Redis | Cluster): Promise<boolean> {
311+
try {
312+
await redisClient.sendCommand(
313+
new Command('auth', ['pass']),
314+
);
315+
} catch (err) {
316+
if (err.message.includes('Client sent AUTH, but no password is set')) {
317+
return true;
318+
}
319+
}
320+
return false;
321+
}
283322
}

‎redisinsight/api/src/modules/recommendation/recommendation.service.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export class RecommendationService {
4848
await this.recommendationProvider.determineZSetHashtableToZiplistRecommendation(client, keys),
4949
await this.recommendationProvider.determineBigSetsRecommendation(keys),
5050
await this.recommendationProvider.determineConnectionClientsRecommendation(client),
51+
await this.recommendationProvider.determineSetPasswordRecommendation(client),
5152
]));
5253
}
5354
}

‎redisinsight/api/test/api/database-analysis/POST-databases-id-analysis.test.ts‎

Lines changed: 71 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,59 @@ describe('POST /databases/:instanceId/analysis', () => {
156156
await rte.data.truncate();
157157
});
158158

159+
describe('useSmallerKeys recommendation', () => {
160+
// generate 1M keys take a lot of time
161+
requirements('!rte.type=CLUSTER');
162+
163+
[
164+
{
165+
name: 'Should create new database analysis with useSmallerKeys recommendation',
166+
data: {
167+
delimiter: '-',
168+
},
169+
statusCode: 201,
170+
responseSchema,
171+
before: async () => {
172+
await rte.data.truncate();
173+
const KEYS_NUMBER = 1_000_006;
174+
await rte.data.generateNKeys(KEYS_NUMBER, false);
175+
},
176+
checkFn: async ({ body }) => {
177+
expect(body.recommendations).to.include.deep.members([
178+
constants.TEST_SMALLER_KEYS_DATABASE_ANALYSIS_RECOMMENDATION,
179+
constants.TEST_COMBINE_SMALL_STRING_TO_HASHES_RECOMMENDATION,
180+
]);
181+
},
182+
after: async () => {
183+
expect(await repository.count()).to.eq(5);
184+
}
185+
},
186+
].map(mainCheckFn);
187+
});
188+
189+
190+
describe('setPassword recommendation', () => {
191+
requirements('!rte.pass');
192+
[
193+
{
194+
name: 'Should create new database analysis with useSmallerKeys recommendation',
195+
data: {
196+
delimiter: '-',
197+
},
198+
statusCode: 201,
199+
responseSchema,
200+
checkFn: async ({ body }) => {
201+
expect(body.recommendations).to.include.deep.members([
202+
constants.TEST_SET_PASSWORD_RECOMMENDATION,
203+
]);
204+
},
205+
after: async () => {
206+
expect(await repository.count()).to.eq(5);
207+
}
208+
},
209+
].map(mainCheckFn);
210+
});
211+
159212
[
160213
{
161214
name: 'Should create new database analysis with bigHashes recommendation',
@@ -169,7 +222,7 @@ describe('POST /databases/:instanceId/analysis', () => {
169222
await rte.data.generateHugeNumberOfFieldsForHashKey(NUMBERS_OF_HASH_FIELDS, true);
170223
},
171224
checkFn: async ({ body }) => {
172-
expect(body.recommendations).to.deep.eq([
225+
expect(body.recommendations).to.include.deep.members([
173226
constants.TEST_BIG_HASHES_DATABASE_ANALYSIS_RECOMMENDATION,
174227
constants.TEST_HASH_HASHTABLE_TO_ZIPLIST_RECOMMENDATION,
175228
constants.TEST_COMPRESS_HASH_FIELD_NAMES_RECOMMENDATION,
@@ -191,7 +244,9 @@ describe('POST /databases/:instanceId/analysis', () => {
191244
await rte.data.generateHugeNumberOfMembersForSetKey(NUMBERS_OF_SET_MEMBERS, true);
192245
},
193246
checkFn: async ({ body }) => {
194-
expect(body.recommendations).to.deep.eq([constants.TEST_INCREASE_SET_MAX_INTSET_ENTRIES_RECOMMENDATION]);
247+
expect(body.recommendations).to.include.deep.members([
248+
constants.TEST_INCREASE_SET_MAX_INTSET_ENTRIES_RECOMMENDATION,
249+
]);
195250
},
196251
after: async () => {
197252
expect(await repository.count()).to.eq(5);
@@ -208,7 +263,9 @@ describe('POST /databases/:instanceId/analysis', () => {
208263
await rte.data.generateStrings(true);
209264
},
210265
checkFn: async ({ body }) => {
211-
expect(body.recommendations).to.deep.eq([constants.TEST_COMBINE_SMALL_STRING_TO_HASHES_RECOMMENDATION]);
266+
expect(body.recommendations).to.include.deep.members([
267+
constants.TEST_COMBINE_SMALL_STRING_TO_HASHES_RECOMMENDATION,
268+
]);
212269
},
213270
after: async () => {
214271
expect(await repository.count()).to.eq(5);
@@ -226,7 +283,7 @@ describe('POST /databases/:instanceId/analysis', () => {
226283
await rte.data.generateHugeNumberOfFieldsForHashKey(NUMBERS_OF_HASH_FIELDS, true);
227284
},
228285
checkFn: async ({ body }) => {
229-
expect(body.recommendations).to.deep.eq([
286+
expect(body.recommendations).to.include.deep.members([
230287
constants.TEST_HASH_HASHTABLE_TO_ZIPLIST_RECOMMENDATION,
231288
]);
232289
},
@@ -246,7 +303,7 @@ describe('POST /databases/:instanceId/analysis', () => {
246303
await rte.data.generateHugeNumberOfFieldsForHashKey(NUMBERS_OF_HASH_FIELDS, true);
247304
},
248305
checkFn: async ({ body }) => {
249-
expect(body.recommendations).to.deep.eq([
306+
expect(body.recommendations).to.include.deep.members([
250307
constants.TEST_HASH_HASHTABLE_TO_ZIPLIST_RECOMMENDATION,
251308
constants.TEST_COMPRESS_HASH_FIELD_NAMES_RECOMMENDATION,
252309
]);
@@ -267,7 +324,7 @@ describe('POST /databases/:instanceId/analysis', () => {
267324
await rte.data.generateHugeElementsForListKey(NUMBERS_OF_LIST_ELEMENTS, true);
268325
},
269326
checkFn: async ({ body }) => {
270-
expect(body.recommendations).to.deep.eq([
327+
expect(body.recommendations).to.include.deep.members([
271328
constants.TEST_COMPRESSION_FOR_LIST_RECOMMENDATION,
272329
]);
273330
},
@@ -289,7 +346,9 @@ describe('POST /databases/:instanceId/analysis', () => {
289346
await rte.data.sendCommand('set', [constants.TEST_STRING_KEY_1, bigStringValue]);
290347
},
291348
checkFn: async ({ body }) => {
292-
expect(body.recommendations).to.deep.eq([constants.TEST_BIG_STRINGS_RECOMMENDATION]);
349+
expect(body.recommendations).to.include.deep.members([
350+
constants.TEST_BIG_STRINGS_RECOMMENDATION,
351+
]);
293352
},
294353
after: async () => {
295354
expect(await repository.count()).to.eq(5);
@@ -307,7 +366,7 @@ describe('POST /databases/:instanceId/analysis', () => {
307366
await rte.data.generateHugeMembersForSortedListKey(NUMBERS_OF_ZSET_MEMBERS, true);
308367
},
309368
checkFn: async ({ body }) => {
310-
expect(body.recommendations).to.deep.eq([
369+
expect(body.recommendations).to.include.deep.members([
311370
constants.TEST_ZSET_HASHTABLE_TO_ZIPLIST_RECOMMENDATION,
312371
]);
313372
},
@@ -327,7 +386,7 @@ describe('POST /databases/:instanceId/analysis', () => {
327386
await rte.data.generateHugeNumberOfMembersForSetKey(NUMBERS_OF_SET_MEMBERS, true);
328387
},
329388
checkFn: async ({ body }) => {
330-
expect(body.recommendations).to.deep.eq([
389+
expect(body.recommendations).to.include.deep.members([
331390
// by default max_intset_entries = 512
332391
constants.TEST_INCREASE_SET_MAX_INTSET_ENTRIES_RECOMMENDATION,
333392
constants.TEST_BIG_SETS_RECOMMENDATION,
@@ -348,43 +407,15 @@ describe('POST /databases/:instanceId/analysis', () => {
348407
await rte.data.generateNCachedScripts(11, true);
349408
},
350409
checkFn: async ({ body }) => {
351-
expect(body.recommendations).to.deep.eq([constants.TEST_LUA_DATABASE_ANALYSIS_RECOMMENDATION]);
410+
expect(body.recommendations).to.include.deep.members([
411+
constants.TEST_LUA_DATABASE_ANALYSIS_RECOMMENDATION,
412+
]);
352413
},
353414
after: async () => {
354415
await rte.data.sendCommand('script', ['flush']);
355416
expect(await repository.count()).to.eq(5);
356417
}
357418
},
358419
].map(mainCheckFn);
359-
360-
describe('useSmallerKeys recommendation', () => {
361-
// generate 1M keys take a lot of time
362-
requirements('!rte.type=CLUSTER');
363-
364-
[
365-
{
366-
name: 'Should create new database analysis with useSmallerKeys recommendation',
367-
data: {
368-
delimiter: '-',
369-
},
370-
statusCode: 201,
371-
responseSchema,
372-
before: async () => {
373-
await rte.data.truncate();
374-
const KEYS_NUMBER = 1_000_006;
375-
await rte.data.generateNKeys(KEYS_NUMBER, false);
376-
},
377-
checkFn: async ({ body }) => {
378-
expect(body.recommendations).to.deep.eq([
379-
constants.TEST_SMALLER_KEYS_DATABASE_ANALYSIS_RECOMMENDATION,
380-
constants.TEST_COMBINE_SMALL_STRING_TO_HASHES_RECOMMENDATION,
381-
]);
382-
},
383-
after: async () => {
384-
expect(await repository.count()).to.eq(5);
385-
}
386-
},
387-
].map(mainCheckFn);
388-
});
389420
});
390421
});

‎redisinsight/api/test/helpers/constants.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,5 +488,9 @@ export const constants = {
488488
name: RECOMMENDATION_NAMES.BIG_SETS,
489489
},
490490

491+
TEST_SET_PASSWORD_RECOMMENDATION: {
492+
name: RECOMMENDATION_NAMES.SET_PASSWORD,
493+
},
494+
491495
// etc...
492496
}

‎redisinsight/ui/src/constants/dbAnalysisRecommendations.json‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,5 +434,38 @@
434434
}
435435
],
436436
"badges": ["code_changes"]
437+
},
438+
"setPassword": {
439+
"id": "setPassword",
440+
"title":"Set the password",
441+
"content": [
442+
{
443+
"id": "1",
444+
"type": "span",
445+
"value": "Protect your database by setting a password and using the "
446+
},
447+
{
448+
"id": "2",
449+
"type": "link",
450+
"value": {
451+
"href": "https://redis.io/commands/auth/",
452+
"name": "AUTH"
453+
}
454+
},
455+
{
456+
"id": "3",
457+
"type": "span",
458+
"value": " command to authenticate the connection. "
459+
},
460+
{
461+
"id": "4",
462+
"type": "link",
463+
"value": {
464+
"href": "https://redis.io/docs/management/security/",
465+
"name": "Read more"
466+
}
467+
}
468+
],
469+
"badges": ["configuration_changes"]
437470
}
438471
}

0 commit comments

Comments
 (0)