Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
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
2 changes: 2 additions & 0 deletions redisinsight/api/config/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ export default {
secretStoragePassword: process.env.RI_SECRET_STORAGE_PASSWORD,
agreementsPath: process.env.RI_AGREEMENTS_PATH,
encryptionKey: process.env.RI_ENCRYPTION_KEY,
acceptTermsAndConditions:
process.env.RI_ACCEPT_TERMS_AND_CONDITIONS === 'true',
tlsCert: process.env.RI_SERVER_TLS_CERT,
tlsKey: process.env.RI_SERVER_TLS_KEY,
staticContent: !!process.env.RI_SERVE_STATICS || true,
Expand Down
2 changes: 1 addition & 1 deletion redisinsight/api/src/constants/agreements-spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"since": "1.0.1",
"title": "Usage Data",
"label": "Usage Data",
"description": "Select the usage data option to help us improve Redis Insight. We use such usage data to understand how Redis Insight features are used, prioritize new features, and enhance the user experience."
"description": "Help improve Redis Insight by sharing anonymous usage data. This helps us understand feature usage and make the app better. By enabling this, you agree to our "
},
"notifications": {
"defaultValue": false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ export class EncryptionService {
return strategies;
}

/**
* Checks if any encryption strategy other than PLAIN is available
*/
async isEncryptionAvailable(): Promise<boolean> {
const strategies = await this.getAvailableEncryptionStrategies();
return strategies.length > 1 || (strategies.length === 1 && strategies[0] !== EncryptionStrategy.PLAIN);
}

/**
* Get encryption strategy based on app settings
* This strategy should be received from app settings but before it should be set by user.
Expand Down
9 changes: 9 additions & 0 deletions redisinsight/api/src/modules/settings/dto/settings.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,15 @@ export class GetAppSettingsResponse {
@Default(WORKBENCH_CONFIG.countBatch)
batchSize: number = WORKBENCH_CONFIG.countBatch;

@ApiProperty({
description: 'Flag indicating that terms and conditions are accepted via environment variable',
type: Boolean,
example: false,
})
@Expose()
@Default(false)
acceptTermsAndConditionsOverwritten: boolean = false;

@ApiProperty({
description: 'Agreements set by the user.',
type: GetUserAgreementsResponse,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
import { Agreements } from 'src/modules/settings/models/agreements';
import { SessionMetadata } from 'src/common/models';

export interface DefaultAgreementsOptions {
version?: string;
data?: Record<string, boolean>;
}

export abstract class AgreementsRepository {
abstract getOrCreate(sessionMetadata: SessionMetadata): Promise<Agreements>;
abstract getOrCreate(
sessionMetadata: SessionMetadata,
defaultOptions?: DefaultAgreementsOptions,
): Promise<Agreements>;
abstract update(
sessionMetadata: SessionMetadata,
agreements: Agreements,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ describe('LocalAgreementsRepository', () => {

await expect(service.getOrCreate()).rejects.toThrow(Error);
});
it('should create new agreements with default data when provided and no entity exists', async () => {
repository.findOneBy.mockResolvedValueOnce(null);
const defaultData = { eula: true, analytics: false };

await service.getOrCreate({ data: defaultData });

expect(repository.create).toHaveBeenCalledWith({
id: 1,
data: JSON.stringify(defaultData),
});
expect(repository.save).toHaveBeenCalled();
});
});

describe('update', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { InjectRepository } from '@nestjs/typeorm';
import { Repository } from 'typeorm';
import { classToClass } from 'src/utils';
import { AgreementsRepository } from 'src/modules/settings/repositories/agreements.repository';
import { AgreementsRepository, DefaultAgreementsOptions } from 'src/modules/settings/repositories/agreements.repository';
import { AgreementsEntity } from 'src/modules/settings/entities/agreements.entity';
import { Agreements } from 'src/modules/settings/models/agreements';
import { SessionMetadata } from 'src/common/models';
import { plainToInstance } from 'class-transformer';

export class LocalAgreementsRepository extends AgreementsRepository {
constructor(
Expand All @@ -14,15 +15,22 @@ export class LocalAgreementsRepository extends AgreementsRepository {
super();
}

async getOrCreate(): Promise<Agreements> {
async getOrCreate(
sessionMetadata: SessionMetadata,
defaultOptions: DefaultAgreementsOptions = {},
): Promise<Agreements> {
let entity = await this.repository.findOneBy({});

if (!entity) {
try {
entity = await this.repository.save(this.repository.create({ id: 1 }));
entity = await this.repository.save(
classToClass(AgreementsEntity, plainToInstance(Agreements, {
...defaultOptions,
id: 1,
})),
);
} catch (e) {
if (e.code === 'SQLITE_CONSTRAINT') {
return this.getOrCreate();
return this.getOrCreate(sessionMetadata);
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is a retry, shouldn't it pass the options as well?

Suggested change
return this.getOrCreate(sessionMetadata);
return this.getOrCreate(sessionMetadata, defaultOptions);

}

throw e;
Expand All @@ -33,13 +41,13 @@ export class LocalAgreementsRepository extends AgreementsRepository {
}

async update(
_: SessionMetadata,
sessionMetadata: SessionMetadata,
agreements: Agreements,
): Promise<Agreements> {
const entity = classToClass(AgreementsEntity, agreements);

await this.repository.save(entity);

return this.getOrCreate();
return this.getOrCreate(sessionMetadata);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ describe('SettingsAnalytics', () => {

describe('sendSettingsUpdatedEvent', () => {
const defaultSettings: GetAppSettingsResponse = {
acceptTermsAndConditionsOverwritten: false,
agreements: null,
scanThreshold: 10000,
batchSize: 5,
Expand Down
33 changes: 33 additions & 0 deletions redisinsight/api/src/modules/settings/settings.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ import { FeatureServerEvents } from 'src/modules/feature/constants';
import { KeyEncryptionStrategy } from 'src/modules/encryption/strategies/key-encryption.strategy';
import { DatabaseDiscoveryService } from 'src/modules/database-discovery/database-discovery.service';
import { ToggleAnalyticsReason } from 'src/modules/settings/constants/settings';
import { when } from 'jest-when';
import { classToClass } from 'src/utils';
import { GetAppSettingsResponse } from 'src/modules/settings/dto/settings.dto';

const REDIS_SCAN_CONFIG = config.get('redis_scan');
const WORKBENCH_CONFIG = config.get('workbench');
Expand All @@ -48,6 +51,7 @@ describe('SettingsService', () => {

beforeEach(async () => {
jest.clearAllMocks();

const module: TestingModule = await Test.createTestingModule({
providers: [
SettingsService,
Expand Down Expand Up @@ -107,6 +111,7 @@ describe('SettingsService', () => {
dateFormat: null,
timezone: null,
agreements: null,
acceptTermsAndConditionsOverwritten: false,
});

expect(eventEmitter.emit).not.toHaveBeenCalled();
Expand All @@ -120,13 +125,41 @@ describe('SettingsService', () => {

expect(result).toEqual({
...mockSettings.data,
acceptTermsAndConditionsOverwritten: false,
agreements: {
version: mockAgreements.version,
...mockAgreements.data,
},
});
});

it('should verify expected pre-accepted agreements format', async () => {
const preselectedAgreements = {
analytics: false,
encryption: true,
eula: true,
notifications: false,
acceptTermsAndConditionsOverwritten: true,
};
settingsRepository.getOrCreate.mockResolvedValue(mockSettings);

// Create a custom instance of the service with an override method
const customService = {
// Preserve the same data structure expected from the method
getAppSettings: async () => classToClass(GetAppSettingsResponse, {
...mockSettings.data,
agreements: preselectedAgreements,
}),
};

// Call the customized method
const result = await customService.getAppSettings();

// Verify the result matches the expected format when acceptTermsAndConditions is true
expect(result).toHaveProperty('agreements');
expect(result.agreements).toEqual(preselectedAgreements);
});

it('should throw InternalServerError', async () => {
agreementsRepository.getOrCreate.mockRejectedValue(
new Error('some error'),
Expand Down
25 changes: 23 additions & 2 deletions redisinsight/api/src/modules/settings/settings.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
GetAppSettingsResponse,
UpdateSettingsDto,
} from './dto/settings.dto';
import { EncryptionService } from '../encryption/encryption.service';

const SERVER_CONFIG = config.get('server') as Config['server'];

Expand All @@ -45,6 +46,7 @@ export class SettingsService {
private readonly analytics: SettingsAnalytics,
private readonly keytarEncryptionStrategy: KeytarEncryptionStrategy,
private readonly keyEncryptionStrategy: KeyEncryptionStrategy,
private readonly encryptionService: EncryptionService,
private eventEmitter: EventEmitter2,
) {}

Expand All @@ -56,16 +58,35 @@ export class SettingsService {
): Promise<GetAppSettingsResponse> {
this.logger.debug('Getting application settings.', sessionMetadata);
try {
const agreements =
await this.agreementRepository.getOrCreate(sessionMetadata);
const settings =
await this.settingsRepository.getOrCreate(sessionMetadata);

let defaultOptions: object;
if (SERVER_CONFIG.acceptTermsAndConditions) {
const isEncryptionAvailable = await this.encryptionService.isEncryptionAvailable();

defaultOptions = {
data: {
analytics: false,
encryption: isEncryptionAvailable,
eula: true,
notifications: false,
},
version: (await this.getAgreementsSpec()).version,
};
}

const agreements = await this.agreementRepository.getOrCreate(sessionMetadata, defaultOptions);

this.logger.debug(
'Succeed to get application settings.',
sessionMetadata,
);


return classToClass(GetAppSettingsResponse, {
...settings?.data,
acceptTermsAndConditionsOverwritten: SERVER_CONFIG.acceptTermsAndConditions,
agreements: agreements?.version
? {
...agreements?.data,
Expand Down
1 change: 1 addition & 0 deletions redisinsight/api/test/api/settings/GET-settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const responseSchema = Joi.object()
batchSize: Joi.number().required(),
dateFormat: Joi.string().allow(null),
timezone: Joi.string().allow(null),
acceptTermsAndConditionsOverwritten: Joi.bool().required(),
agreements: Joi.object()
.keys({
version: Joi.string().required(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const responseSchema = Joi.object()
batchSize: Joi.number().required(),
dateFormat: Joi.string().allow(null),
timezone: Joi.string().allow(null),
acceptTermsAndConditionsOverwritten: Joi.bool().required(),
agreements: Joi.object()
.keys({
version: Joi.string().required(),
Expand Down
1 change: 1 addition & 0 deletions redisinsight/api/test/helpers/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const APP_DEFAULT_SETTINGS = {
dateFormat: null,
timezone: null,
agreements: null,
acceptTermsAndConditionsOverwritten: false,
};
const TEST_LIBRARY_NAME = 'lib';
const TEST_ANALYTICS_PAGE = 'Settings';
Expand Down
31 changes: 31 additions & 0 deletions redisinsight/ui/src/components/config/Config.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -332,4 +332,35 @@ describe('Config', () => {
]),
)
})

it('should not show consent popup when acceptTermsAndConditionsOverwritten is true, regardless of consent differences', () => {
const userSettingsSelectorMock = jest.fn().mockReturnValue({
config: {
acceptTermsAndConditionsOverwritten: true,
agreements: {}, // Empty agreements - would normally cause a popup
},
spec: {
agreements: {
eula: {
defaultValue: false,
required: true,
editable: false,
since: '1.0.0',
title: 'EULA: Redis Insight License Terms',
label: 'Label',
},
},
},
})
userSettingsSelector.mockImplementation(userSettingsSelectorMock)

render(<Config />)

// Check that setSettingsPopupState is called with false
expect(store.getActions()).toEqual(
expect.arrayContaining([
setSettingsPopupState(false),
])
)
})
})
3 changes: 1 addition & 2 deletions redisinsight/ui/src/components/config/Config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,9 @@ const Config = () => {
const checkSettingsToShowPopup = () => {
const specConsents = spec?.agreements
const appliedConsents = config?.agreements

dispatch(
setSettingsPopupState(
isDifferentConsentsExists(specConsents, appliedConsents),
config?.acceptTermsAndConditionsOverwritten ? false : isDifferentConsentsExists(specConsents, appliedConsents),
),
)
}
Expand Down
Loading
Loading