RBAC with domain and domain pattern returns invalid results #147 #148
Conversation
This file contains 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
added end to end test to validate that results are same as https://casbin.org/editor/
woprrr
reviewed
Nov 14, 2023
| g,u1,g1 | ||
| g,u2,ga1 | ||
| g,u3,r\d+ | ||
| g,u4,r1 |
src/Rbac/DefaultRoleManager/Role.php
Outdated
| @@ -172,4 +128,4 @@ public function getRoles(): array | |||
| return $role->name; | |||
| }, $this->roles); | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
Warning Blank Line at the end of file too
Comment on lines
89
to
124
| /** | ||
| * @param string $domain | ||
| * | ||
| * @return Roles | ||
| */ | ||
| protected function &generateTempRoles(string $domain): Roles | ||
| { | ||
| $this->loadOrStoreRoles($domain, new Roles()); | ||
|
|
||
| $patternDomain = [$domain]; | ||
|
|
||
| $domainMatchingFunc = $this->domainMatchingFunc; | ||
| if ($this->hasDomainPattern) { | ||
| foreach ($this->allDomains as $key => $roles) { | ||
| if ($domainMatchingFunc($domain, $key)) { | ||
| $patternDomain[] = $key; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| $allRoles = new Roles(); | ||
|
|
||
| foreach ($patternDomain as $domain) { | ||
| $roles = $this->loadOrStoreRoles($domain, new Roles()); | ||
| foreach ($roles->toArray() as $key => $role2) { | ||
| $role1 = &$allRoles->createRole($role2->name, $this->matchingFunc); | ||
| foreach ($role2->getRoles() as $name) { | ||
| $role3 = &$allRoles->createRole($name, $this->matchingFunc); | ||
| $role1->addRole($role3); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return $allRoles; | ||
| } | ||
|
|
There was a problem hiding this comment.
Some emprovements about code :
- Method Naming (descriptive, informative)
- SoC (Separate of Concern)
- Delete unecessary references & nested level
- Lisibility empowerment
- Clean Code
Suggested change
| /** | |
| * @param string $domain | |
| * | |
| * @return Roles | |
| */ | |
| protected function &generateTempRoles(string $domain): Roles | |
| { | |
| $this->loadOrStoreRoles($domain, new Roles()); | |
| $patternDomain = [$domain]; | |
| $domainMatchingFunc = $this->domainMatchingFunc; | |
| if ($this->hasDomainPattern) { | |
| foreach ($this->allDomains as $key => $roles) { | |
| if ($domainMatchingFunc($domain, $key)) { | |
| $patternDomain[] = $key; | |
| } | |
| } | |
| } | |
| $allRoles = new Roles(); | |
| foreach ($patternDomain as $domain) { | |
| $roles = $this->loadOrStoreRoles($domain, new Roles()); | |
| foreach ($roles->toArray() as $key => $role2) { | |
| $role1 = &$allRoles->createRole($role2->name, $this->matchingFunc); | |
| foreach ($role2->getRoles() as $name) { | |
| $role3 = &$allRoles->createRole($name, $this->matchingFunc); | |
| $role1->addRole($role3); | |
| } | |
| } | |
| } | |
| return $allRoles; | |
| } | |
| /** | |
| * @param string $domain | |
| * | |
| * @return Roles | |
| */ | |
| protected function generateTempRoles(string $domain): Roles | |
| { | |
| $this->initializeRolesForDomain($domain); | |
| $matchedDomains = $this->getMatchedDomains($domain); | |
| return $this->aggregateRolesFromDomains($matchedDomains); | |
| } | |
| private function initializeRolesForDomain(string $domain): void | |
| { | |
| $this->loadOrStoreRoles($domain, new Roles()); | |
| } | |
| private function getMatchedDomains(string $domain): array | |
| { | |
| $patternDomain = [$domain]; | |
| if ($this->hasDomainPattern) { | |
| $patternDomain = array_merge($patternDomain, $this->findMatchingDomains($domain)); | |
| } | |
| return $patternDomain; | |
| } | |
| private function findMatchingDomains(string $domain): array | |
| { | |
| return array_keys( | |
| array_filter( | |
| $this->allDomains, | |
| fn($key) => $this->domainMatchingFunc($domain, $key) | |
| ) | |
| ); | |
| } | |
| private function aggregateRolesFromDomains(array $domains): Roles | |
| { | |
| $aggregatedRoles = new Roles(); | |
| foreach ($domains as $domain) { | |
| $this->addRolesFromDomainToAggregate($domain, $aggregatedRoles); | |
| } | |
| return $aggregatedRoles; | |
| } | |
| private function addRolesFromDomainToAggregate(string $domain, Roles &$aggregatedRoles): void | |
| { | |
| $roles = $this->loadOrStoreRoles($domain, new Roles()); | |
| array_walk( | |
| $roles->toArray(), | |
| fn($role) => $this->addRoleWithDependencies($role, $aggregatedRoles) | |
| ); | |
| } | |
| private function addRoleWithDependencies(Role $role, Roles &$aggregatedRoles): void | |
| { | |
| $aggregatedRole = &$aggregatedRoles->createRole($role->name, $this->matchingFunc); | |
| foreach ($role->getRoles() as $associatedRoleName) { | |
| $associatedRole = &$aggregatedRoles->createRole($associatedRoleName, $this->matchingFunc); | |
| $aggregatedRole->addRole($associatedRole); | |
| } | |
| } |
|
|
||
| foreach ($patternDomain as $domain) { | ||
| $allRoles = &$this->loadOrStoreRoles($domain, new Roles()); | ||
| $allRoles = &$this->loadOrStoreRoles($domain[0], new Roles()); |
There was a problem hiding this comment.
IMHO we can simply all of this and obtain anythink like :
/**
* Adds the inheritance link between role: name1 and role: name2.
* aka role: name1 inherits role: name2.
* domain is a prefix to the roles.
*
* @param string $name1
* @param string $name2
* @param string ...$domain
*
* @throws CasbinException
*/
public function addLink(string $name1, string $name2, string ...$domain): void
{
$domainName = $this->validateAndResolveDomain($domain);
$allRoles = &$this->loadOrStoreRoles($domainName, new Roles());
$parentRole = &$allRoles->loadOrStore($name1, new Role($name1));
$childRole = &$allRoles->loadOrStore($name2, new Role($name2));
$parentRole->addRole($childRole);
}
/**
* Validates and resolves the domain name from the given arguments.
*
* @param array $domain
* @return string
* @throws CasbinException
*/
private function validateAndResolveDomain(array $domain): string
{
if (count($domain) > 1) {
throw new CasbinException('Error: Only one domain parameter is accepted.'); # Use proper const to do that & define one specific exception seem to be good.
}
return count($domain) === 0 ? self::DEFAULT_DOMAIN : $domain[0];
}- Here we can respect SoC again and uncouple methods of execution & exception (Adding custom specific exception for Casbin).
- Naming improvement to be more clear & maintanable.
|
|
||
| return $allRoles; | ||
| } | ||
| } |
| * | ||
| * @return Roles | ||
| */ | ||
| protected function &checkHasDomainPatternOrHasPattern(string $domain): Roles |
There was a problem hiding this comment.
Warning it's not a "check" but more a "get" because you recover all Roles at the end.
Comment on lines
317
to
332
| /** | ||
| * @param string $domain | ||
| * | ||
| * @return Roles | ||
| */ | ||
| protected function &checkHasDomainPatternOrHasPattern(string $domain): Roles | ||
| { | ||
| if ($this->hasDomainPattern || $this->hasPattern) { | ||
| $allRoles = &$this->generateTempRoles($domain); | ||
| } else { | ||
| $allRoles = &$this->loadOrStoreRoles($domain, new Roles()); | ||
| } | ||
|
|
||
| return $allRoles; | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggested change
| /** | |
| * @param string $domain | |
| * | |
| * @return Roles | |
| */ | |
| protected function &checkHasDomainPatternOrHasPattern(string $domain): Roles | |
| { | |
| if ($this->hasDomainPattern || $this->hasPattern) { | |
| $allRoles = &$this->generateTempRoles($domain); | |
| } else { | |
| $allRoles = &$this->loadOrStoreRoles($domain, new Roles()); | |
| } | |
| return $allRoles; | |
| } | |
| } | |
| /** | |
| * Retrieves roles based on domain patterns. | |
| * | |
| * @param string $domain | |
| * @return Roles | |
| */ | |
| protected function &getRolesBasedOnDomainPattern(string $domain): Roles | |
| { | |
| if ($this->shouldGenerateTempRoles()) { | |
| return $this->generateTempRoles($domain); | |
| } | |
| return $this->loadOrStoreRoles($domain, new Roles()); | |
| } | |
| /** | |
| * Determines if temporary roles should be generated based on domain patterns. | |
| * | |
| * @return bool | |
| */ | |
| private function shouldGenerateTempRoles(): bool | |
| { | |
| return $this->hasDomainPattern || $this->hasPattern; | |
| } |
Like previously seen the namming should be changed to be more revelant about his intentions.
Extract the conditional logic in proper method
IMHO references are unecessary, to be double checked.
Small clarification about DOCbloc (It's mandatory in constributed context).
| * | ||
| * @return Role | ||
| */ | ||
| public function &createRole(string $name): Role | ||
| public function &createRole(string $name, ?Closure $matchingFunc): Role |
There was a problem hiding this comment.
We need to became more clear and not breaking calishenics / clean code principles as much as possible.
The refactoring is too complex we can simplify & extract like that.
Suggested change
| public function &createRole(string $name, ?Closure $matchingFunc): Role | |
| public function &createRole(string $name, ?Closure $matchingFunc): Role | |
| { | |
| $role = &$this->loadOrStore($name, new Role($name)); | |
| if ($matchingFunc instanceof Closure) { | |
| $this->addMatchingRoles($role, $matchingFunc); | |
| } | |
| return $role; | |
| } | |
| private function addMatchingRoles(Role &$role, Closure $matchingFunc): void | |
| { | |
| foreach ($this->roles as $key => $value) { | |
| if ($this->isRoleMatching($name, $key, $matchingFunc)) { | |
| $relatedRole = &$this->loadOrStore($key, new Role($key)); | |
| $role->addRole($relatedRole); | |
| } | |
| } | |
| } | |
| private function isRoleMatching(string $roleName, string $key, Closure $matchingFunc): bool | |
| { | |
| return $matchingFunc($roleName, $key) && $roleName != $key; | |
| } |
tests/EnforcerTest.php
Outdated
Comment on lines
432
to
495
|
|
||
| public function testRbacWithRegexPatternMatching() | ||
| { | ||
| $e = new Enforcer( | ||
| $this->modelAndPolicyPath . '/rbac_with_regex_pattern_model.conf', | ||
| $this->modelAndPolicyPath . '/rbac_with_regex_pattern_policy.csv' | ||
| ); | ||
|
|
||
| $e->addNamedMatchingFunc('g', 'regexMatch', function (string $key1, string $key2) { | ||
| return BuiltinOperations::regexMatch($key1, $key2); | ||
| }); | ||
|
|
||
| $this->assertTrue($e->getRoleManager()->hasLink('u1', 'g1')); | ||
| $this->assertFalse($e->getRoleManager()->hasLink('u1', 'g2')); | ||
| $this->assertTrue($e->getRoleManager()->hasLink('u1', 'root')); | ||
|
|
||
| $this->assertTrue($e->enforce('u1', 'data1', 'read')); | ||
| $this->assertFalse($e->enforce('u2', 'data1', 'read')); | ||
| $this->assertFalse($e->enforce('u3', 'data2', 'read')); | ||
| $this->assertFalse($e->enforce('u4', 'data2', 'read')); | ||
| } | ||
|
|
||
| public function testRbacWithDomain() | ||
| { | ||
| $e = new Enforcer( | ||
| $this->modelAndPolicyPath . '/rbac_with_domain_pattern_model_and_keymatch_model.conf', | ||
| $this->modelAndPolicyPath . '/rbac_with_domain_pattern_model_and_keymatch_policy.csv' | ||
| ); | ||
|
|
||
| $e->addNamedDomainMatchingFunc('g', 'keyMatch', function (string $key1, string $key2) { | ||
| return BuiltinOperations::keyMatch($key1, $key2); | ||
| }); | ||
|
|
||
| $this->assertTrue($e->enforce('alice', 'domain1', 'data1', 'read')); | ||
| $this->assertTrue($e->enforce('alice', 'domain1', 'data1', 'write')); | ||
| $this->assertTrue($e->enforce('alice', 'domain1', 'data2', 'read')); | ||
| $this->assertTrue($e->enforce('alice', 'domain1', 'data2', 'write')); | ||
|
|
||
| $this->assertFalse($e->enforce('bob', 'domain1', 'data2', 'write')); | ||
|
|
||
| $this->assertTrue($e->enforce('alice', 'domain2', 'data1', 'read')); | ||
| $this->assertFalse($e->enforce('alice', 'domain2', 'data1', 'write')); | ||
| $this->assertTrue($e->enforce('alice', 'domain2', 'data2', 'read')); | ||
| $this->assertFalse($e->enforce('alice', 'domain2', 'data2', 'write')); | ||
|
|
||
| $this->assertFalse($e->enforce('alice', 'domain3', 'data1', 'read')); | ||
| $this->assertFalse($e->enforce('alice', 'domain3', 'data1', 'write')); | ||
| $this->assertFalse($e->enforce('alice', 'domain3', 'data2', 'read')); | ||
| $this->assertFalse($e->enforce('alice', 'domain3', 'data2', 'write')); | ||
|
|
||
| $this->assertFalse($e->enforce('badr', 'domain4', 'data1', 'write')); | ||
| $this->assertTrue($e->enforce('badr', 'domain4', 'data1', 'read')); | ||
| $this->assertTrue($e->enforce('badr', 'domain4', 'data2', 'read')); | ||
| $this->assertTrue($e->enforce('badr', 'domain4', 'data2', 'write')); | ||
|
|
||
| $this->assertTrue($e->enforce('stef', 'domain1', 'data1', 'read')); | ||
| $this->assertTrue($e->enforce('stef', 'domain1', 'data1', 'write')); | ||
| $this->assertTrue($e->enforce('stef', 'domain1', 'data2', 'read')); | ||
| $this->assertTrue($e->enforce('stef', 'domain1', 'data2', 'write')); | ||
|
|
||
| $this->assertFalse($e->enforce('stef', 'domain5', 'data3', 'read')); | ||
| $this->assertFalse($e->enforce('ben', 'domain5', 'data3', 'read')); | ||
| $this->assertTrue($e->enforce('leo', 'domain5', 'data3', 'read')); | ||
| } |
There was a problem hiding this comment.
I re write to be more maintainable & simple by usage of DataProvider.
Suggested change
| public function testRbacWithRegexPatternMatching() | |
| { | |
| $e = new Enforcer( | |
| $this->modelAndPolicyPath . '/rbac_with_regex_pattern_model.conf', | |
| $this->modelAndPolicyPath . '/rbac_with_regex_pattern_policy.csv' | |
| ); | |
| $e->addNamedMatchingFunc('g', 'regexMatch', function (string $key1, string $key2) { | |
| return BuiltinOperations::regexMatch($key1, $key2); | |
| }); | |
| $this->assertTrue($e->getRoleManager()->hasLink('u1', 'g1')); | |
| $this->assertFalse($e->getRoleManager()->hasLink('u1', 'g2')); | |
| $this->assertTrue($e->getRoleManager()->hasLink('u1', 'root')); | |
| $this->assertTrue($e->enforce('u1', 'data1', 'read')); | |
| $this->assertFalse($e->enforce('u2', 'data1', 'read')); | |
| $this->assertFalse($e->enforce('u3', 'data2', 'read')); | |
| $this->assertFalse($e->enforce('u4', 'data2', 'read')); | |
| } | |
| public function testRbacWithDomain() | |
| { | |
| $e = new Enforcer( | |
| $this->modelAndPolicyPath . '/rbac_with_domain_pattern_model_and_keymatch_model.conf', | |
| $this->modelAndPolicyPath . '/rbac_with_domain_pattern_model_and_keymatch_policy.csv' | |
| ); | |
| $e->addNamedDomainMatchingFunc('g', 'keyMatch', function (string $key1, string $key2) { | |
| return BuiltinOperations::keyMatch($key1, $key2); | |
| }); | |
| $this->assertTrue($e->enforce('alice', 'domain1', 'data1', 'read')); | |
| $this->assertTrue($e->enforce('alice', 'domain1', 'data1', 'write')); | |
| $this->assertTrue($e->enforce('alice', 'domain1', 'data2', 'read')); | |
| $this->assertTrue($e->enforce('alice', 'domain1', 'data2', 'write')); | |
| $this->assertFalse($e->enforce('bob', 'domain1', 'data2', 'write')); | |
| $this->assertTrue($e->enforce('alice', 'domain2', 'data1', 'read')); | |
| $this->assertFalse($e->enforce('alice', 'domain2', 'data1', 'write')); | |
| $this->assertTrue($e->enforce('alice', 'domain2', 'data2', 'read')); | |
| $this->assertFalse($e->enforce('alice', 'domain2', 'data2', 'write')); | |
| $this->assertFalse($e->enforce('alice', 'domain3', 'data1', 'read')); | |
| $this->assertFalse($e->enforce('alice', 'domain3', 'data1', 'write')); | |
| $this->assertFalse($e->enforce('alice', 'domain3', 'data2', 'read')); | |
| $this->assertFalse($e->enforce('alice', 'domain3', 'data2', 'write')); | |
| $this->assertFalse($e->enforce('badr', 'domain4', 'data1', 'write')); | |
| $this->assertTrue($e->enforce('badr', 'domain4', 'data1', 'read')); | |
| $this->assertTrue($e->enforce('badr', 'domain4', 'data2', 'read')); | |
| $this->assertTrue($e->enforce('badr', 'domain4', 'data2', 'write')); | |
| $this->assertTrue($e->enforce('stef', 'domain1', 'data1', 'read')); | |
| $this->assertTrue($e->enforce('stef', 'domain1', 'data1', 'write')); | |
| $this->assertTrue($e->enforce('stef', 'domain1', 'data2', 'read')); | |
| $this->assertTrue($e->enforce('stef', 'domain1', 'data2', 'write')); | |
| $this->assertFalse($e->enforce('stef', 'domain5', 'data3', 'read')); | |
| $this->assertFalse($e->enforce('ben', 'domain5', 'data3', 'read')); | |
| $this->assertTrue($e->enforce('leo', 'domain5', 'data3', 'read')); | |
| } | |
| /** | |
| * Data provider for testRbacWithRegexPatternMatching. | |
| */ | |
| public function regexPatternMatchingDataProvider() | |
| { | |
| yield 'user1 with group1' => ['user1', 'group1', true]; | |
| yield 'user1 with group2' => ['user1', 'group2', false]; | |
| yield 'user1 with root' => ['user1', 'root', true]; | |
| yield 'user2 with data1 read' => ['user2', 'data1', 'read', false]; | |
| yield 'user3 with data2 read' => ['user3', 'data2', 'read', false]; | |
| yield 'user4 with data2 read' => ['user4', 'data2', 'read', false]; | |
| } | |
| /** | |
| * @dataProvider regexPatternMatchingDataProvider | |
| */ | |
| public function testRbacWithRegexPatternMatching($user, $role, $expected) | |
| { | |
| $enforcer = new Enforcer( | |
| $this->modelAndPolicyPath . '/rbac_with_regex_pattern_model.conf', | |
| $this->modelAndPolicyPath . '/rbac_with_regex_pattern_policy.csv' | |
| ); | |
| $enforcer->addNamedMatchingFunc('g', 'regexMatch', function (string $keyOne, string $keyTwo) { | |
| return BuiltinOperations::regexMatch($keyOne, $keyTwo); | |
| }); | |
| $this->assertSame($expected, $enforcer->getRoleManager()->hasLink($user, $role)); | |
| } | |
| /** | |
| * Data provider for testRbacWithDomain. | |
| */ | |
| public function domainDataProvider() | |
| { | |
| yield 'alice in domain1 data1 read' => ['alice', 'domain1', 'data1', 'read', true]; | |
| yield 'alice in domain1 data1 write' => ['alice', 'domain1', 'data1', 'write', true]; | |
| yield 'alice in domain1 data2 read' => ['alice', 'domain1', 'data2', 'read', true]; | |
| yield 'alice in domain1 data2 write' => ['alice', 'domain1', 'data2', 'write', true]; | |
| yield 'bob in domain1 data2 write' => ['bob', 'domain1', 'data2', 'write', false]; | |
| yield 'alice in domain2 data1 read' => ['alice', 'domain2', 'data1', 'read', true]; | |
| yield 'alice in domain2 data1 write' => ['alice', 'domain2', 'data1', 'write', false]; | |
| yield 'alice in domain2 data2 read' => ['alice', 'domain2', 'data2', 'read', true]; | |
| yield 'alice in domain2 data2 write' => ['alice', 'domain2', 'data2', 'write', false]; | |
| yield 'alice in domain3 data1 read' => ['alice', 'domain3', 'data1', 'read', false]; | |
| yield 'alice in domain3 data1 write' => ['alice', 'domain3', 'data1', 'write', false]; | |
| yield 'alice in domain3 data2 read' => ['alice', 'domain3', 'data2', 'read', false]; | |
| yield 'alice in domain3 data2 write' => ['alice', 'domain3', 'data2', 'write', false]; | |
| yield 'badr in domain4 data1 write' => ['badr', 'domain4', 'data1', 'write', false]; | |
| yield 'badr in domain4 data1 read' => ['badr', 'domain4', 'data1', 'read', true]; | |
| yield 'badr in domain4 data2 read' => ['badr', 'domain4', 'data2', 'read', true]; | |
| yield 'badr in domain4 data2 write' => ['badr', 'domain4', 'data2', 'write', true]; | |
| yield 'stef in domain1 data1 read' => ['stef', 'domain1', 'data1', 'read', true]; | |
| yield 'stef in domain1 data1 write' => ['stef', 'domain1', 'data1', 'write', true]; | |
| yield 'stef in domain1 data2 read' => ['stef', 'domain1', 'data2', 'read', true]; | |
| yield 'stef in domain1 data2 write' => ['stef', 'domain1', 'data2', 'write', true]; | |
| yield 'stef in domain5 data3 read' => ['stef', 'domain5', 'data3', 'read', false]; | |
| yield 'ben in domain5 data3 read' => ['ben', 'domain5', 'data3', 'read', false]; | |
| yield 'leo in domain5 data3 read' => ['leo', 'domain5', 'data3', 'read', true]; | |
| } | |
| /** | |
| * @dataProvider domainDataProvider | |
| */ | |
| public function testRbacWithDomain($user, $domain, $resource, $action, $expected) | |
| { | |
| $enforcer = new Enforcer( | |
| $this->modelAndPolicyPath . '/rbac_with_domain_pattern_model_and_keymatch_model.conf', | |
| $this->modelAndPolicyPath . '/rbac_with_domain_pattern_model_and_keymatch_policy.csv' | |
| ); | |
| $enforcer->addNamedDomainMatchingFunc('g', 'keyMatch', function (string $keyOne, string $keyTwo) { | |
| return BuiltinOperations::keyMatch($keyOne, $keyTwo); | |
| }); | |
| $this->assertSame($expected, $enforcer->enforce($user, $domain, $resource, $action)); | |
| } |
To be consider using an Helper to store DataSet used in DataProvider in static class for more concise code.
Comment on lines
27
to
28
|
|
||
|
|
src/Rbac/DefaultRoleManager/Role.php
Outdated
| * | ||
| * @return bool | ||
| */ | ||
| public function hasRoleWithMatchingFunc(string $name, int $hierarchyLevel, Closure $matchingFunc): bool |
There was a problem hiding this comment.
You change the API. Are you sure this change is needed?
src/Rbac/DefaultRoleManager/Role.php
Outdated
| * | ||
| * @return bool | ||
| */ | ||
| public function hasDirectRoleWithMatchingFunc(string $name, Closure $matchingFunc): bool |
There was a problem hiding this comment.
You change the API. Are you sure this change is needed?
|
Is it created by AI? |
Pull Request Test Coverage Report for Build 6933024301
💛 - Coveralls |
No it’s not generated by ai 😅 juste by a craftsman & speedUp with tabnine (fine tuned & custom model). |
added 6 commits
November 20, 2023 15:52
removed spaces in policy file added missing space at end of file
added missing space at end of file
added missing space at end of file
added yield to unit test
added missing eol
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.