-
-
Notifications
You must be signed in to change notification settings - Fork 484
[4.x] Parameter validation and other DB manager improvements #1459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
lukinovec
wants to merge
62
commits into
master
Choose a base branch
from
validate-sql-parameters
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
62 commits
Select commit
Hold shift + click to select a range
808f527
Use select() instead of selectOne() in databaseExists() and userExists()
lukinovec ad7d229
Use parameter binding in SELECT queries
lukinovec bdf592c
Add parameter validation to DB managers
lukinovec 5adbc14
Test SQL parameter validation
lukinovec 182f3a2
Fix code style (php-cs-fixer)
github-actions[bot] d5087d1
Extract parameter validation into a trait
lukinovec db03997
Validate SQLite DB names in create/deleteDatabase()
lukinovec 0fdb8b2
Validate user passwords in DB managers
lukinovec 4a3e6ba
Test invalid passwords, improve test name and comments
lukinovec 740d53e
Rename ValidatesSqlParameters to ValidatesDatabaseParameters
lukinovec 8592949
Improve ValidatesDatabaseParameters docblocks
lukinovec f3f1ab9
Skip null parameters in validateParameter
lukinovec 75b74f2
Make validateParameter have void return type
lukinovec 322257f
Validate SQLite filename in databaseExists
lukinovec 46f73c4
Improve ValidatesDatabaseParameters comments, delete extra early return
lukinovec 4bdb877
Cover null parameter skipping
lukinovec 50ea524
Simplify test, improve comments
lukinovec bacbf93
Improve validation exception message
lukinovec 37a4c7d
Check if paremeter is string
lukinovec 2bd3a86
Quote database parameter in GRANT statement for consistency
lukinovec 76c324d
Add `validateFilename()`
lukinovec d3607f8
Use 'allowedCharacters' instead of 'allowlist', code quality
lukinovec e8168eb
Add string check to validateFilename, swap validation order
lukinovec 9611a05
Skip null parameters, throw for other non-string parameters
lukinovec f3836cc
Fix code style (php-cs-fixer)
github-actions[bot] 2bdda23
Disallow empty strings as filenames
lukinovec 1a01164
Make validateFilename accept string instead of ?string
lukinovec 665404e
Add `DatabaseTenancyBootstrapper::$harden`
lukinovec fbd1e02
Correct DatabaseTenancyBootstrapper test filename
lukinovec fc6a931
Fix code style (php-cs-fixer)
github-actions[bot] f5f5f1d
Fix DB bootstrapper test
lukinovec 52f6857
If harden throws an exception, revert connection back to central
lukinovec 0ce3d86
DATABASE_URL test: set config for both datasets
lukinovec 2ae1f79
Cover empty string parameters
lukinovec b1f0d0a
Get central DB from config in harden test
lukinovec 7363318
Make in-memory DB detection more strict
lukinovec 48b4837
Validate in-memory db names, move SQLite-specific methods to the SQLi…
lukinovec 7683bef
Fix code style (php-cs-fixer)
github-actions[bot] e48d822
Validate SQLite DB name unconditionally in getPath()
lukinovec 9a9adc0
Use getPath() in makeConnectionConfig()
lukinovec 7f93f44
Test that the SQLite DB manager recognizes in-memory DBs
lukinovec 7660ddd
Improve readability of harden() call
lukinovec 26c161a
Add regression test for makeConnectionConfig not working correctly wi…
lukinovec 429e098
Improve code quality and comments
lukinovec ea20eb1
Validate in-memory DBs outside of isInMemory
lukinovec 405aaaf
Handle MySQL charset and collation
lukinovec 2b3466f
Check the current DB name instead of configured one in harden()
lukinovec 338526d
Query for MySQL defaults instead of assuming them in charset test
lukinovec fec170a
Fix code style (php-cs-fixer)
github-actions[bot] 98a808b
Quote schema names in GRANT statements
lukinovec 6ed9975
Catch broader range of exceptions (harden() in DB bootstrapper)
lukinovec de91348
Specify exception message in assertions
lukinovec bdbfbd4
Remove extra variable
lukinovec e59195e
Improve coverage
lukinovec 66ae88a
Fix non-string parameter validation assertion
lukinovec 0331875
Specify charset and collation config in test
lukinovec bbd8f6f
Add parentheses to instanceof check
lukinovec 587f347
Restore default charset after assertion
lukinovec 099a666
Add valid password assertion
lukinovec 649c802
Use unique DB names and passwords in test
lukinovec 519c819
Delete user created in validation test
lukinovec d9ae274
Delete redundant cleanup
lukinovec File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Stancl\Tenancy\Database\Concerns; | ||
|
|
||
| use InvalidArgumentException; | ||
|
|
||
| /** | ||
| * Provides methods to validate database parameters (e.g. database names, usernames, passwords) | ||
| * before using them in SQL statements (or in file paths in the case of SQLiteDatabaseManager). | ||
| * | ||
| * Used where parameters can be provided by users, and where parameter binding cannot be used. | ||
| * | ||
| * @mixin \Stancl\Tenancy\Database\TenantDatabaseManagers\TenantDatabaseManager | ||
| * @mixin \Stancl\Tenancy\Database\TenantDatabaseManagers\SQLiteDatabaseManager | ||
| */ | ||
| trait ValidatesDatabaseParameters | ||
| { | ||
| /** | ||
| * Characters allowed in parameters. | ||
| * | ||
| * Used as the default allowlist in validateParameter(), which validates non-password | ||
| * parameters such as database names or usernames. | ||
| */ | ||
| protected function allowedParameterCharacters(): string | ||
| { | ||
| return 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-'; | ||
| } | ||
|
|
||
| /** | ||
| * Characters allowed in database user passwords. | ||
| * | ||
| * Passwords are always quoted in the SQL statements, so it's safe | ||
| * to allow a wider range of characters, as long as it doesn't include | ||
| * characters that can break out of the quoted SQL strings (so e.g. | ||
| * ', ", \, and ` aren't allowed). | ||
| */ | ||
| protected function allowedPasswordCharacters(): string | ||
| { | ||
| return ' !#$%&()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_abcdefghijklmnopqrstuvwxyz{|}~'; | ||
| } | ||
|
|
||
| /** | ||
| * Ensure that parameters (database names, usernames, etc.) | ||
| * only contain allowed characters before used in SQL statements | ||
| * (or paths in the case of SQLiteDatabaseManager). | ||
| * | ||
| * By default, only the characters in allowedParameterCharacters() are allowed. | ||
| * | ||
| * Null parameters are skipped. | ||
| * | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| protected function validateParameter(string|array|null $parameters, string|null $allowedCharacters = null): void | ||
| { | ||
| $allowedCharacters ??= $this->allowedParameterCharacters(); | ||
|
|
||
| foreach ((array) $parameters as $parameter) { | ||
| if (is_null($parameter)) { | ||
| // Skip if there's nothing to validate | ||
| // (e.g. when $tenant->database()->getUsername() of an | ||
| // improperly created tenant is null and it gets passed). | ||
| continue; | ||
| } | ||
|
|
||
| if (! is_string($parameter)) { | ||
| // E.g. if a parameter is retrieved from the config, it isn't necessarily a string | ||
| throw new InvalidArgumentException('Parameter has to be a string.'); | ||
| } | ||
|
|
||
| foreach (str_split($parameter) as $character) { | ||
| if (! str_contains($allowedCharacters, $character)) { | ||
| throw new InvalidArgumentException("Forbidden character '{$character}' in parameter."); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Ensure password only contains allowed characters (allowedPasswordCharacters()) | ||
| * before used in SQL statements. | ||
| * | ||
| * Used in permission controlled managers as a shorthand for calling validateParameter() | ||
| * with the less strict allowlist to validate database user passwords. | ||
| * | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| protected function validatePassword(string|null $password): void | ||
| { | ||
| $this->validateParameter($password, allowedCharacters: $this->allowedPasswordCharacters()); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.