Skip to content

Commit

Permalink
feature #31189 [Security] Add IS_IMPERSONATOR, IS_ANONYMOUS and IS_RE…
Browse files Browse the repository at this point in the history
…MEMBERED (HeahDude)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[Security] Add IS_IMPERSONATOR, IS_ANONYMOUS and IS_REMEMBERED

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #29848
| License       | MIT
| Doc PR        | symfony/symfony-docs#11487

This continues work of @HeahDude and finally finishes one of the code PRs I've been working on during the ⭐️ EUFOSSA Hackathon.

Changes
---

The PRs modifies some of the attributes used by the `AuthenticatedVoter`:

* New `IS_IMPERSONATOR`, `IS_ANONYMOUS` and `IS_REMEMBERED` attributes are introduced to indicate the user either impersonated, anonymous or rembered.
* <s>`IS_AUTHENTICATED_ANONYMOUSLY` actually meant "is authenticated, either anonymous or fully". As this is confusing, it is replaced by `IS_AUTHENTICATED`.</s>
* <s>All `is_*()` functions in expressions are deprecated in favor of `is_granted('IS_*')`. It's not worth duplicating the `AuthenticatedVoter` logic in two places now we have shorter `IS_*` attributes</s>

**Before**

```php
if ($authorizationChecker->isGranted('ROLE_PREVIOUS_ADMIN')) {
    // ...
}
```
<s>

```yaml
security:
  # ...

  access_control:
    - { path: ^/protected, roles: 'IS_AUTHENTICATED_ANONYMOUSLY' }
```
</s>

**After**

```php
if ($authorizationChecker->isGranted('IS_IMPERSONATOR')) {
    // ...
}
```
<s>

```yaml
security:
  # ...

  access_control:
    - { path: ^/protected, roles: 'IS_AUTHENTICATED' }
```
</s>

<s>Discussion
---

The only thing I'm wondering is how we combine this with the `is_authenticated()` expression function:

https://github.com/symfony/symfony/blob/98929dc2927c59ba3e36b5547f2eae6316aa4740/src/Symfony/Component/Security/Core/Authorization/ExpressionLanguageProvider.php#L33-L37

As you can see, the `IS_AUTHENTICATED` attribute and `is_authenticated()` expression function do not have the same meaning. Should we somehow deprecate the current behavior of `is_authenticated()` or should we find another name for `IS_AUTHENTICATED` (that would be a shame imo).</s>

Commits
-------

6c522a7 Added IS_ANONYMOUS, IS_REMEMBERED, IS_IMPERSONATOR
  • Loading branch information
fabpot committed Feb 24, 2020
2 parents c231214 + 6c522a7 commit 6178c63
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/Symfony/Component/Security/CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@ CHANGELOG
-----

* Added access decision strategy to override access decisions by voter service priority
* Added `IS_ANONYMOUS`, `IS_REMEMBERED`, `IS_IMPERSONATOR`

5.0.0
-----
Expand Down
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Security\Core\Authorization\Voter;

use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface;
use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;

/**
Expand All @@ -28,6 +29,9 @@ class AuthenticatedVoter implements VoterInterface
const IS_AUTHENTICATED_FULLY = 'IS_AUTHENTICATED_FULLY';
const IS_AUTHENTICATED_REMEMBERED = 'IS_AUTHENTICATED_REMEMBERED';
const IS_AUTHENTICATED_ANONYMOUSLY = 'IS_AUTHENTICATED_ANONYMOUSLY';
const IS_ANONYMOUS = 'IS_ANONYMOUS';
const IS_IMPERSONATOR = 'IS_IMPERSONATOR';
const IS_REMEMBERED = 'IS_REMEMBERED';

private $authenticationTrustResolver;

Expand All @@ -45,7 +49,10 @@ public function vote(TokenInterface $token, $subject, array $attributes)
foreach ($attributes as $attribute) {
if (null === $attribute || (self::IS_AUTHENTICATED_FULLY !== $attribute
&& self::IS_AUTHENTICATED_REMEMBERED !== $attribute
&& self::IS_AUTHENTICATED_ANONYMOUSLY !== $attribute)) {
&& self::IS_AUTHENTICATED_ANONYMOUSLY !== $attribute
&& self::IS_ANONYMOUS !== $attribute
&& self::IS_IMPERSONATOR !== $attribute
&& self::IS_REMEMBERED !== $attribute)) {
continue;
}

Expand All @@ -68,6 +75,18 @@ public function vote(TokenInterface $token, $subject, array $attributes)
|| $this->authenticationTrustResolver->isFullFledged($token))) {
return VoterInterface::ACCESS_GRANTED;
}

if (self::IS_REMEMBERED === $attribute && $this->authenticationTrustResolver->isRememberMe($token)) {
return VoterInterface::ACCESS_GRANTED;
}

if (self::IS_ANONYMOUS === $attribute && $this->authenticationTrustResolver->isAnonymous($token)) {
return VoterInterface::ACCESS_GRANTED;
}

if (self::IS_IMPERSONATOR === $attribute && $token instanceof SwitchUserToken) {
return VoterInterface::ACCESS_GRANTED;
}
}

return $result;
Expand Down
Expand Up @@ -49,6 +49,15 @@ public function getVoteTests()
['fully', ['IS_AUTHENTICATED_FULLY'], VoterInterface::ACCESS_GRANTED],
['remembered', ['IS_AUTHENTICATED_FULLY'], VoterInterface::ACCESS_DENIED],
['anonymously', ['IS_AUTHENTICATED_FULLY'], VoterInterface::ACCESS_DENIED],

['fully', ['IS_ANONYMOUS'], VoterInterface::ACCESS_DENIED],
['remembered', ['IS_ANONYMOUS'], VoterInterface::ACCESS_DENIED],
['anonymously', ['IS_ANONYMOUS'], VoterInterface::ACCESS_GRANTED],

['fully', ['IS_IMPERSONATOR'], VoterInterface::ACCESS_DENIED],
['remembered', ['IS_IMPERSONATOR'], VoterInterface::ACCESS_DENIED],
['anonymously', ['IS_IMPERSONATOR'], VoterInterface::ACCESS_DENIED],
['impersonated', ['IS_IMPERSONATOR'], VoterInterface::ACCESS_GRANTED],
];
}

Expand All @@ -58,6 +67,8 @@ protected function getToken($authenticated)
return $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
} elseif ('remembered' === $authenticated) {
return $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\RememberMeToken')->setMethods(['setPersistent'])->disableOriginalConstructor()->getMock();
} elseif ('impersonated' === $authenticated) {
return $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken')->disableOriginalConstructor()->getMock();
} else {
return $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\AnonymousToken')->setConstructorArgs(['', ''])->getMock();
}
Expand Down

0 comments on commit 6178c63

Please sign in to comment.