diff --git a/.scrutinizer.yml b/.scrutinizer.yml deleted file mode 100644 index 5333c199..00000000 --- a/.scrutinizer.yml +++ /dev/null @@ -1,12 +0,0 @@ -filter: - excluded_paths: - - tests/* -build: - image: default-bionic - environment: - php: 8.1.2 - nodes: - analysis: - tests: - override: - - command: php-scrutinizer-run diff --git a/readme.md b/readme.md index efa8e810..9a0a53f9 100644 --- a/readme.md +++ b/readme.md @@ -8,9 +8,6 @@ - - - diff --git a/src/DetailedError.php b/src/DetailedError.php index dc3424ad..c7000ba6 100644 --- a/src/DetailedError.php +++ b/src/DetailedError.php @@ -8,8 +8,8 @@ class DetailedError * Constructor. */ public function __construct( - protected ?int $errorCode, - protected ?string $errorMessage, + protected int $errorCode, + protected string $errorMessage, protected ?string $diagnosticMessage ) { } @@ -17,7 +17,7 @@ public function __construct( /** * Returns the LDAP error code. */ - public function getErrorCode(): ?int + public function getErrorCode(): int { return $this->errorCode; } @@ -25,7 +25,7 @@ public function getErrorCode(): ?int /** * Returns the LDAP error message. */ - public function getErrorMessage(): ?string + public function getErrorMessage(): string { return $this->errorMessage; } diff --git a/src/LdapResultResponse.php b/src/LdapResultResponse.php index 0d7e53e0..4f18b3d1 100644 --- a/src/LdapResultResponse.php +++ b/src/LdapResultResponse.php @@ -21,7 +21,7 @@ public function __construct( */ public function successful(): bool { - return $this->errorCode === 0 && empty($this->errorMessage); + return $this->errorCode === 0; } /** diff --git a/src/Models/ActiveDirectory/Computer.php b/src/Models/ActiveDirectory/Computer.php index aaa5bd16..3900c61a 100644 --- a/src/Models/ActiveDirectory/Computer.php +++ b/src/Models/ActiveDirectory/Computer.php @@ -2,6 +2,7 @@ namespace LdapRecord\Models\ActiveDirectory; +use LdapRecord\Models\ActiveDirectory\Concerns\HasAccountControl; use LdapRecord\Models\ActiveDirectory\Concerns\HasPrimaryGroup; use LdapRecord\Models\ActiveDirectory\Relations\HasOnePrimaryGroup; use LdapRecord\Models\Relations\HasMany; @@ -9,6 +10,7 @@ class Computer extends Entry { + use HasAccountControl; use HasPrimaryGroup; /** diff --git a/src/Models/ActiveDirectory/Concerns/HasAccountControl.php b/src/Models/ActiveDirectory/Concerns/HasAccountControl.php new file mode 100644 index 00000000..425cc548 --- /dev/null +++ b/src/Models/ActiveDirectory/Concerns/HasAccountControl.php @@ -0,0 +1,46 @@ +isDisabled(); + } + + /** + * Determine if the user's account is disabled. + */ + public function isDisabled(): bool + { + return $this->accountControl()->hasFlag(AccountControl::ACCOUNTDISABLE); + } + + /** + * Get the user's account control. + */ + public function accountControl(): AccountControl + { + return new AccountControl( + $this->getFirstAttribute('userAccountControl') + ); + } + + /** + * Set the user's account control attribute. + */ + public function setUserAccountControlAttribute(mixed $value): void + { + if ($value instanceof AccountControl) { + $value = $value->getValue(); + } + + $this->attributes['useraccountcontrol'] = [(int) $value]; + } +} diff --git a/src/Models/ActiveDirectory/User.php b/src/Models/ActiveDirectory/User.php index 7ddfd080..b0582772 100644 --- a/src/Models/ActiveDirectory/User.php +++ b/src/Models/ActiveDirectory/User.php @@ -4,9 +4,9 @@ use Carbon\Carbon; use Illuminate\Contracts\Auth\Authenticatable; +use LdapRecord\Models\ActiveDirectory\Concerns\HasAccountControl; use LdapRecord\Models\ActiveDirectory\Concerns\HasPrimaryGroup; use LdapRecord\Models\ActiveDirectory\Scopes\RejectComputerObjectClass; -use LdapRecord\Models\Attributes\AccountControl; use LdapRecord\Models\Concerns\CanAuthenticate; use LdapRecord\Models\Concerns\HasPassword; use LdapRecord\Models\Relations\HasMany; @@ -18,6 +18,7 @@ class User extends Entry implements Authenticatable use HasPassword; use HasPrimaryGroup; use CanAuthenticate; + use HasAccountControl; /** * The password's attribute name. @@ -63,33 +64,7 @@ protected static function boot(): void // class. This is needed due to computer objects containing all // of the ActiveDirectory 'user' object classes. Without // this scope, they would be included in results. - static::addGlobalScope(new RejectComputerObjectClass()); - } - - /** - * Determine if the user's account is enabled. - */ - public function isEnabled(): bool - { - return ! $this->isDisabled(); - } - - /** - * Determine if the user's account is disabled. - */ - public function isDisabled(): bool - { - return $this->accountControl()->hasFlag(AccountControl::ACCOUNTDISABLE); - } - - /** - * Get the user's account control. - */ - public function accountControl(): AccountControl - { - return new AccountControl( - $this->getFirstAttribute('userAccountControl') - ); + static::addGlobalScope(new RejectComputerObjectClass); } /** diff --git a/src/Models/Concerns/HasAttributes.php b/src/Models/Concerns/HasAttributes.php index 46658d8f..6eb536b0 100644 --- a/src/Models/Concerns/HasAttributes.php +++ b/src/Models/Concerns/HasAttributes.php @@ -20,6 +20,11 @@ trait HasAttributes */ protected array $original = []; + /** + * The models changed attributes. + */ + protected array $changes = []; + /** * The models attributes. */ @@ -207,7 +212,7 @@ protected function encodeValue(string $value): string return $value; } - return utf8_encode($value); + return mb_convert_encoding($value, 'UTF-8', 'ISO-8859-1'); } /** @@ -257,6 +262,16 @@ public function syncOriginal(): static return $this; } + /** + * Sync the changed attributes. + */ + public function syncChanges(): static + { + $this->changes = $this->getDirty(); + + return $this; + } + /** * Fills the entry with the supplied attributes. */ @@ -823,7 +838,7 @@ public function filterRawAttributes(array $attributes = [], array $keys = ['coun */ public function hasAttribute(int|string $key): bool { - return [] !== ($this->attributes[$this->normalizeAttributeKey($key)] ?? []); + return ($this->attributes[$this->normalizeAttributeKey($key)] ?? []) !== []; } /** @@ -869,6 +884,14 @@ public function getDirty(): array return $dirty; } + /** + * Get the attributes that have been changed since the model was last saved. + */ + public function getChanges(): array + { + return $this->changes; + } + /** * Determine if the given attribute is dirty. */ @@ -877,6 +900,42 @@ public function isDirty(string $key): bool return ! $this->originalIsEquivalent($key); } + /** + * Determine if given attribute has remained the same. + */ + public function isClean(string $key): bool + { + return ! $this->isDirty($key); + } + + /** + * Discard attribute changes and reset the attributes to their original state. + */ + public function discardChanges(): static + { + [$this->attributes, $this->changes] = [$this->original, []]; + + return $this; + } + + /** + * Determine if the model or any of the given attribute(s) were changed when the model was last saved. + */ + public function wasChanged(array|string $attributes = null): bool + { + if (func_num_args() === 0) { + return count($this->changes) > 0; + } + + foreach ((array) $attributes as $attribute) { + if (array_key_exists($attribute, $this->changes)) { + return true; + } + } + + return false; + } + /** * Get the accessors being appended to the models array form. */ diff --git a/src/Models/Model.php b/src/Models/Model.php index b0b47ae1..1aafac77 100644 --- a/src/Models/Model.php +++ b/src/Models/Model.php @@ -509,7 +509,7 @@ public function jsonSerialize(): array protected function convertAttributesForJson(array $attributes = []): array { // If the model has a GUID set, we need to convert it to its - // string format, due to it being in binary. Otherwise + // string format, due to it being in binary. Otherwise, // we will receive a JSON serialization exception. if (isset($attributes[$this->guidKey])) { $attributes[$this->guidKey] = [$this->getConvertedGuid( @@ -914,6 +914,8 @@ protected function performUpdate(): void $this->dispatch('updated'); + $this->syncChanges(); + $this->syncOriginal(); } @@ -1054,7 +1056,7 @@ protected function deleteLeafNodes(): void ->in($this->dn) ->list() ->each(function (Model $model) { - $model->delete($recursive = true); + $model->delete(recursive: true); }); } diff --git a/src/Query/Builder.php b/src/Query/Builder.php index 79ad506c..8359a98e 100644 --- a/src/Query/Builder.php +++ b/src/Query/Builder.php @@ -156,7 +156,7 @@ public function setCache(Cache $cache = null): static /** * Returns a new Query Builder instance. */ - public function newInstance(string $baseDn = null): static + public function newInstance(string $baseDn = null): Builder { return (new static($this->connection))->setDn( is_null($baseDn) ? $this->getDn() : $baseDn @@ -166,7 +166,7 @@ public function newInstance(string $baseDn = null): static /** * Returns a new nested Query Builder instance. */ - public function newNestedInstance(Closure $closure = null): static + public function newNestedInstance(Closure $closure = null): Builder { $query = $this->newInstance()->nested(); @@ -1022,7 +1022,7 @@ public function whereNotContains(string $field, string $value): static */ public function whereIn(string $field, array $values): static { - return $this->orFilter(function (self $query) use ($field, $values) { + return $this->orFilter(function (Builder $query) use ($field, $values) { foreach ($values as $value) { $query->whereEquals($field, $value); } diff --git a/src/Query/Filter/ConditionNode.php b/src/Query/Filter/ConditionNode.php index 4fe9eafe..4dfed722 100644 --- a/src/Query/Filter/ConditionNode.php +++ b/src/Query/Filter/ConditionNode.php @@ -68,14 +68,10 @@ protected function extractComponents(string $filter): array $components = Str::whenContains( $filter, $this->operators, - fn ($operator, $filter) => explode($this->operator = $operator, $filter), + fn ($operator, $filter) => explode($this->operator = $operator, $filter, 2), fn ($filter) => throw new ParserException("Invalid query condition. No operator found in [$filter]"), ); - if (count($components) !== 2) { - throw new ParserException("Invalid query filter [$filter]"); - } - return $components; } } diff --git a/src/Query/Model/Builder.php b/src/Query/Model/Builder.php index f97c915a..7c694730 100644 --- a/src/Query/Model/Builder.php +++ b/src/Query/Model/Builder.php @@ -91,7 +91,7 @@ public function getModel(): Model /** * Get a new model query builder instance. */ - public function newInstance(string $baseDn = null): static + public function newInstance(string $baseDn = null): BaseBuilder { return parent::newInstance($baseDn)->model($this->model); } @@ -236,7 +236,7 @@ public function findManyByAnr(array $values = [], array|string $columns = ['*']) */ protected function prepareAnrEquivalentQuery(string $value): static { - return $this->orFilter(function (self $query) use ($value) { + return $this->orFilter(function (BaseBuilder $query) use ($value) { foreach ($this->model->getAnrAttributes() as $attribute) { $query->whereEquals($attribute, $value); } diff --git a/src/Testing/LdapExpectation.php b/src/Testing/LdapExpectation.php index 3c9b1f2a..ff78e6e5 100644 --- a/src/Testing/LdapExpectation.php +++ b/src/Testing/LdapExpectation.php @@ -54,19 +54,19 @@ class LdapExpectation protected bool $indefinitely = true; /** - * Whether the expectation should return errors. + * Whether the expectation should return an error. */ protected bool $errors = false; /** - * The error number to return. + * The error code to return. */ - protected ?string $errorCode = null; + protected int $errorCode = 1; /** - * The last error string to return. + * The error message to return. */ - protected ?string $errorMessage = null; + protected string $errorMessage = 'Unknown error'; /** * The diagnostic message string to return. @@ -130,7 +130,7 @@ public function andReturnFalse(): static /** * The error message to return from the expectation. */ - public function andReturnError(int $errorCode = 1, string $errorMessage = '', string $diagnosticMessage = ''): static + public function andReturnError(int $errorCode = 1, string $errorMessage = 'Unknown error', string $diagnosticMessage = null): static { $this->errors = true; @@ -287,7 +287,7 @@ public function isReturningError(): bool /** * Get the expected error code. */ - public function getExpectedErrorCode(): ?int + public function getExpectedErrorCode(): int { return $this->errorCode; } diff --git a/src/Testing/LdapFake.php b/src/Testing/LdapFake.php index a64583f2..7ae9f205 100644 --- a/src/Testing/LdapFake.php +++ b/src/Testing/LdapFake.php @@ -32,12 +32,12 @@ class LdapFake implements LdapInterface /** * The default fake last error string. */ - protected string $lastError = ''; + protected string $lastError = 'Unknown error'; /** * The default fake diagnostic message string. */ - protected string $diagnosticMessage = ''; + protected ?string $diagnosticMessage = null; /** * Create a new expected operation. @@ -152,7 +152,7 @@ public function shouldReturnErrorNumber(int $number = 1): static /** * Set the last error of a failed bind attempt. */ - public function shouldReturnError(string $message = ''): static + public function shouldReturnError(string $message): static { $this->lastError = $message; @@ -162,7 +162,7 @@ public function shouldReturnError(string $message = ''): static /** * Set the diagnostic message of a failed bind attempt. */ - public function shouldReturnDiagnosticMessage(string $message = ''): static + public function shouldReturnDiagnosticMessage(?string $message): static { $this->diagnosticMessage = $message; @@ -188,7 +188,7 @@ public function getLastError(): string /** * Return a fake diagnostic message. */ - public function getDiagnosticMessage(): string + public function getDiagnosticMessage(): ?string { return $this->diagnosticMessage; } diff --git a/tests/Integration/CacheTest.php b/tests/Integration/CacheTest.php index b80c0efe..8880c159 100644 --- a/tests/Integration/CacheTest.php +++ b/tests/Integration/CacheTest.php @@ -53,12 +53,12 @@ public function test_that_results_are_fetched_from_cache() { $this->resetConnection(cache: new ArrayCacheStore); - $this->assertEquals([], $this->getUserCnsFromCache()); + $this->assertEmpty($this->getUserCnsFromCache()); $user = $this->makeUser($this->ou); $user->save(); - $this->assertEquals([], $this->getUserCnsFromCache()); + $this->assertEmpty($this->getUserCnsFromCache()); } public function test_that_results_are_fetched_from_cache2() diff --git a/tests/Unit/Exceptions/ConstraintViolationExceptionTest.php b/tests/Unit/Exceptions/ConstraintViolationExceptionTest.php index 4b4944f5..28f11982 100644 --- a/tests/Unit/Exceptions/ConstraintViolationExceptionTest.php +++ b/tests/Unit/Exceptions/ConstraintViolationExceptionTest.php @@ -17,7 +17,7 @@ public function test_caused_by_password_policy() $error = new DetailedError( 0, 'Constraint violation', - $diagMessage = '0000052D: AtrErr: DSID-03190FD6' + '0000052D: AtrErr: DSID-03190FD6' ); $e->setDetailedError($error); @@ -34,7 +34,7 @@ public function test_caused_by_incorrect_password() $error = new DetailedError( 0, 'Constraint violation', - $diagMessage = '00000056: AtrErr: DSID-03190FD6' + '00000056: AtrErr: DSID-03190FD6' ); $e->setDetailedError($error); diff --git a/tests/Unit/Models/ActiveDirectory/ModelTest.php b/tests/Unit/Models/ActiveDirectory/ModelTest.php index bea7f406..56d1c17a 100644 --- a/tests/Unit/Models/ActiveDirectory/ModelTest.php +++ b/tests/Unit/Models/ActiveDirectory/ModelTest.php @@ -2,7 +2,11 @@ namespace LdapRecord\Tests\Unit\Models\ActiveDirectory; +use LdapRecord\Connection; +use LdapRecord\Container; use LdapRecord\Models\ActiveDirectory\Entry; +use LdapRecord\Models\Relations\HasMany; +use LdapRecord\Query\Model\ActiveDirectoryBuilder; use LdapRecord\Query\Model\Builder; use LdapRecord\Tests\TestCase; use Mockery as m; @@ -88,6 +92,26 @@ public function test_restore() $this->assertEquals('CN=John Doe,DC=local,DC=com', $m->getDn()); } + + public function test_relation_query_can_be_created() + { + Container::addConnection(new Connection); + + $entry = new class extends Entry + { + public function relation(): HasMany + { + return $this->hasMany(Entry::class, 'dn'); + } + }; + + /** @var HasMany $relation */ + $relation = $entry->relation()->whereIn('foo', ['foo']); + + $this->assertInstanceOf(HasMany::class, $relation); + $this->assertInstanceOf(ActiveDirectoryBuilder::class, $query = $relation->getQuery()); + $this->assertEquals('(|(foo=\66\6f\6f))', $query->getQuery()); + } } class TestModelRestoreStub extends Entry diff --git a/tests/Unit/Models/ActiveDirectory/UserTest.php b/tests/Unit/Models/ActiveDirectory/UserTest.php index d7722863..2488ec0b 100644 --- a/tests/Unit/Models/ActiveDirectory/UserTest.php +++ b/tests/Unit/Models/ActiveDirectory/UserTest.php @@ -189,6 +189,17 @@ public function test_user_with_account_control_returns_hydrated_account_control_ $this->assertFalse($uac->hasFlag(AccountControl::DONT_EXPIRE_PASSWORD)); } + public function test_user_can_have_account_control_object_set_on_attribute() + { + $user = new User(); + + $uac = $user->accountControl(); + + $user->userAccountControl = $uac->setAccountIsNormal(); + + $this->assertEquals(AccountControl::NORMAL_ACCOUNT, $user->accountControl()->getValue()); + } + public function test_user_is_disabled() { $user = new User(); diff --git a/tests/Unit/Models/ModelTest.php b/tests/Unit/Models/ModelTest.php index d8f5fd32..28a2e084 100644 --- a/tests/Unit/Models/ModelTest.php +++ b/tests/Unit/Models/ModelTest.php @@ -330,6 +330,40 @@ public function test_dirty_attributes() ], $model->getDirty()); } + public function test_changed_attributes() + { + $model = new Entry(['foo' => 1, 'bar' => 2]); + $model->syncOriginal(); + $model->foo = 1; + $model->bar = 20; + $model->baz = 30; + + $model->syncChanges(); + + $this->assertFalse($model->wasChanged('')); + $this->assertFalse($model->wasChanged([])); + $this->assertFalse($model->wasChanged('foo')); + $this->assertFalse($model->wasChanged(['foo'])); + + $this->assertTrue($model->wasChanged()); + $this->assertTrue($model->wasChanged('bar')); + $this->assertTrue($model->wasChanged('baz')); + $this->assertTrue($model->wasChanged(['bar', 'baz'])); + + $this->assertTrue($model->isClean('foo')); + $this->assertFalse($model->isClean('bar')); + + $this->assertEquals([ + 'bar' => [20], + 'baz' => [30], + ], $model->getChanges()); + + $model->discardChanges(); + + $this->assertEmpty($model->getChanges()); + $this->assertEquals(['foo' => [1], 'bar' => [2]], $model->getAttributes()); + } + public function test_reset_integer_is_kept_in_tact_when_batch_modifications_are_generated() { $model = new Entry(); diff --git a/tests/Unit/Query/Filter/ParserTest.php b/tests/Unit/Query/Filter/ParserTest.php index 29116000..03555545 100644 --- a/tests/Unit/Query/Filter/ParserTest.php +++ b/tests/Unit/Query/Filter/ParserTest.php @@ -85,6 +85,27 @@ public function test_parsing_nested_filter_groups() $this->assertEquals('(&(objectCategory=person)(objectClass=contact)(|(sn=Smith)(sn=Johnson)))', Parser::assemble($group)); } + public function test_parser_can_parse_value_with_equal_sign() + { + $nodes = Parser::parse('(&(objectClass=inetOrgPerson)(memberof=cn=foo,ou=Groups,dc=example,dc=org))'); + + $this->assertCount(1, $nodes); + $this->assertInstanceOf(GroupNode::class, $nodes[0]); + + $this->assertCount(2, $nodes[0]->getNodes()); + + $groupNodes = $nodes[0]->getNodes(); + $this->assertInstanceOf(ConditionNode::class, $groupNodes[0]); + $this->assertEquals('objectClass', $groupNodes[0]->getAttribute()); + $this->assertEquals('=', $groupNodes[0]->getOperator()); + $this->assertEquals('inetOrgPerson', $groupNodes[0]->getValue()); + + $this->assertInstanceOf(ConditionNode::class, $groupNodes[1]); + $this->assertEquals('memberof', $groupNodes[1]->getAttribute()); + $this->assertEquals('=', $groupNodes[1]->getOperator()); + $this->assertEquals('cn=foo,ou=Groups,dc=example,dc=org', $groupNodes[1]->getValue()); + } + public function test_parser_throws_exception_when_missing_open_parenthesis_is_detected() { $this->expectException(ParserException::class); @@ -167,13 +188,6 @@ public function test_parser_can_process_single_node() $this->assertEquals('(foo=bar)', Parser::assemble($node)); } - public function test_parser_throws_exception_with_invalid_filter_with_additional_equals() - { - $this->expectExceptionMessage('Invalid query filter [foo=bar=baz]'); - - Parser::parse('(foo=bar=baz)'); - } - public function test_parser_throws_exception_during_assemble_when_invalid_nodes_given() { $this->expectException(TypeError::class);