refactor

[Artuomka]

No description provided.

[Copilot]

Pull request overview

This PR performs a significant refactoring of the connection validation and error handling logic in the backend. The changes consolidate duplicate code, improve type safety, fix spelling errors, and optimize database queries.

Changes:

• Fixed spelling error in CONNECTION_TEST_FAILED message constant and updated its usage
• Extracted and modularized host validation logic into a dedicated is-host-allowed.ts file with improved error handling
• Consolidated duplicate connection validation code from connection.controller.ts into the validate-create-connection-data utility
• Added master password verification before updating connections, improved type safety for DAO interfaces, and optimized permissions queries
• Implemented caching for SSL certificate reading and improved type definitions across DTOs

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
backend/src/exceptions/text/messages.ts	Fixed typo: CONNECTION_TEST_FILED → CONNECTION_TEST_FAILED
backend/src/entities/connection/utils/validate-create-connection-data.ts	Refactored validation logic, removed duplicate checkIsHostAllowed function, added Redis URL handling and FQDN validation
backend/src/entities/connection/utils/is-host-allowed.ts	New dedicated file for host validation with improved error handling using HttpException instead of console.error
backend/src/entities/connection/use-cases/update-connection.use.case.ts	Added master password verification before updating encrypted connections, removed commented-out code
backend/src/entities/connection/use-cases/test-connection.use.case.ts	Fixed typo usage, improved type safety by replacing 'any' types with specific interfaces
backend/src/entities/connection/use-cases/get-permissions-for-group-in-connection.use.case.ts	Optimized permissions fetching by replacing N individual queries with a single bulk query
backend/src/entities/connection/ssl-certificate/read-certificate.ts	Added caching mechanism to avoid repeated file reads
backend/src/entities/connection/repository/custom-connection-repository-extension.ts	Refactored to reuse findAllUserConnections method, removing code duplication
backend/src/entities/connection/connection.module.ts	Removed commented-out route configuration
backend/src/entities/connection/connection.entity.ts	Removed cert field assignment in test connection initialization (line 192)
backend/src/entities/connection/connection.controller.ts	Removed duplicate validation logic, @Injectable decorator, and console.log; renamed file import; refactored validation to use shared utility
backend/src/entities/connection/application/dto/updated-connection-response.dto.ts	New DTO file created (renamed from updated-connection-responce.dto.ts to fix spelling)
backend/src/entities/connection/application/dto/create-connection.dto.ts	Enhanced SSH field validation using ValidateIf and IsNotEmpty decorators for conditional required fields
backend/src/entities/connection/application/data-structures/found-connections.ds.ts	Improved type safety by replacing 'any' with ConnectionPropertiesEntity

Comments suppressed due to low confidence (1)

backend/src/entities/connection/utils/validate-create-connection-data.ts:88

• The isHostAllowed function now throws an HttpException with status 403 FORBIDDEN on DNS lookup errors, but here it's also being checked for a false return value which would throw a 403 FORBIDDEN. This creates redundant error handling. If isHostAllowed throws on DNS errors, then the if (!checkingResult) check on line 81 will never be reached for DNS errors, only for forbidden IP ranges. Consider either removing the throw from isHostAllowed.catch and returning false instead (keeping this error handling), or removing lines 81-87 and letting isHostAllowed handle all error cases.

		const checkingResult = await isHostAllowed(createConnectionData.connection_parameters);
		if (!checkingResult) {
			throw new HttpException(
				{
					message: Messages.CANNOT_CREATE_CONNECTION_TO_THIS_HOST,
				},
				HttpStatus.FORBIDDEN,
			);
		}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The cert assignment is removed here but still exists on line 189 (and throughout the rest of the file). This creates an inconsistency where the cert field is being set for test connections in one place but not here. Either the removal should be reverted, or line 189 should also be removed along with other references to ensure consistent behavior.

[Copilot]

The master password verification occurs AFTER Object.assign has already merged the connection_parameters into the foundConnectionToUpdate (line 76). If the password is incorrect, the connection entity has already been modified in memory. While this won't be saved to the database due to the exception, it's better practice to verify the master password before modifying the entity. Consider moving this validation block before line 76.

[Copilot]

When catching DNS lookup errors, the error information is now lost. The catch block throws a new exception without preserving the original error details. Consider logging the original error before throwing the new exception to aid in debugging DNS lookup issues. For example: console.error('DNS lookup error:', e.message);