Feature v 12 0 2 #8
Conversation
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 19515427 | Triggered | Generic High Entropy Secret | eef1a6d | tests/Fixtures/Saloon/get-vaults.json | View secret |
| 19515428 | Triggered | Generic High Entropy Secret | f3e6198 | tests/Fixtures/Saloon/get-vaults.json | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
There was a problem hiding this comment.
Pull Request Overview
This PR implements a major refactor to consolidate the authentication system and improve cache management for the M-Files Laravel package. The changes move from a fragmented authentication approach to a unified system with a dedicated cache key manager.
Key changes:
- Unified authentication token system with improved caching
- New CacheKeyManager for centralized cache operations
- Updated API requests to use consistent authentication patterns
Reviewed Changes
Copilot reviewed 71 out of 72 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Helpers/CacheKeyManagerTest.php | New test file for the cache key manager functionality |
| tests/Unit/DTO/PropertyValueTest.php | Comprehensive test coverage for PropertyValue DTO |
| src/Helpers/CacheKeyManager.php | New centralized cache management helper |
| src/DTO/AuthenticationToken.php | Simplified authentication token DTO |
| src/DTO/ConfigWithCredentials.php | Updated configuration DTO structure |
| src/Requests/LogInToVaultRequest.php | Refactored authentication request |
| src/Requests/LogOutFromVaultRequest.php | New logout request implementation |
| src/Connectors/MFilesConnector.php | Updated connector with new auth system |
Comments suppressed due to low confidence (2)
tests/Feature/Requests/GetVaultsRequestTest.php:12
- The test name describes logout functionality but the test is actually for getting vaults. The test name should be 'can get vaults' or similar.
test('can logout from vault and clear authentication token from cache', function () {
tests/Fixtures/Saloon/get-vauls-login-to-vault.json:1
- Filename contains a typo: 'vauls' should be 'vaults'.
{
| $this->assertEquals($expected, $result); | ||
| } | ||
|
|
||
| public function test_to_array_with_multiselectlookup_data_type_with_collection(): void |
There was a problem hiding this comment.
The test expects an incorrect data structure when using a collection. The expected result shows ['Item' => [201, 202], 'Version' => -1] and ['Item' => false, 'Version' => -1], which indicates the implementation doesn't properly handle Laravel collections. The collection should be converted to an array before processing.
| $resonse = $connector->send(new UploadFileRequest( | ||
| fileContent: $fileContent, | ||
| fileName: $fileName | ||
| ))->dto(); | ||
|
|
||
| expect($result)->toBeArray(); | ||
| expect($result['ID'])->toBe(456); | ||
| expect($result['Title'])->toBe('test-1'); | ||
| expect($result['Extension'])->toBe('pdf'); | ||
| expect($result)->not->toHaveKey('FileInformationType'); | ||
| }); | ||
|
|
||
| test('throws exception when file content is empty', function () { | ||
| expect(fn () => new UploadFileRequest( | ||
| fileContent: '', | ||
| fileName: 'test.pdf' | ||
| ))->toThrow(\InvalidArgumentException::class, 'File content is required'); | ||
| }); | ||
|
|
||
| test('throws exception when file name is empty', function () { | ||
| expect(fn () => new UploadFileRequest( | ||
| fileContent: 'test content', | ||
| fileName: '' | ||
| ))->toThrow(\InvalidArgumentException::class, 'File name is required'); | ||
| }); | ||
| )); | ||
|
|
||
| expect(Arr::get($resonse->dto(), 'UploadID'))->toBe(1); | ||
| expect(Arr::get($resonse->dto(), 'Size'))->toBe(8785); | ||
| expect(Arr::get($resonse->dto(), 'Title'))->toBe('test-1'); | ||
| expect(Arr::get($resonse->dto(), 'Extension'))->toBe('pdf'); |
There was a problem hiding this comment.
Variable name 'resonse' is misspelled, should be 'response'.
| $fileExtension = $fileName ? pathinfo($fileName, PATHINFO_EXTENSION) : null; | ||
|
|
||
| return new DownloadedFile( | ||
| content: $response->body(), | ||
| name: $name, | ||
| extension: $extension, | ||
| size: $contentLength ? (int) $contentLength : null, | ||
| lastModified: $lastModified ? \Carbon\CarbonImmutable::parse($lastModified) : null, | ||
| contentType: $contentType, | ||
| name: $fileName, |
There was a problem hiding this comment.
The filename extraction logic may not work correctly. The variable $fileName is used for both the full filename and the name without extension, but pathinfo($fileName, PATHINFO_FILENAME) is not called to extract just the name part.
src/Helpers/CacheKeyManager.php
Outdated
| } | ||
|
|
||
| /** | ||
| * Get authentication token from cache |
There was a problem hiding this comment.
The method lacks a docstring explaining what type of token is returned or when it might return null.
| * Get authentication token from cache | |
| * Retrieves the authentication token from the cache. | |
| * | |
| * @return mixed Returns the authentication token (type depends on what was stored), or null if no token is found in the cache. |
No description provided.