From 8ee9161e6bde236993226bf29ad2354983283bcd Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Sat, 7 Jan 2012 15:37:07 -0600 Subject: [PATCH 1/3] [Security] Adding more extensive PHPDoc to UserInterface, AdvancedUserInterface and UserProviderInterface --- .../Core/User/AdvancedUserInterface.php | 34 ++++++++++++++++- .../Security/Core/User/UserInterface.php | 37 ++++++++++++++++++- .../Core/User/UserProviderInterface.php | 26 +++++++++---- 3 files changed, 87 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Component/Security/Core/User/AdvancedUserInterface.php b/src/Symfony/Component/Security/Core/User/AdvancedUserInterface.php index ba528a1050..f2d59fcbf3 100644 --- a/src/Symfony/Component/Security/Core/User/AdvancedUserInterface.php +++ b/src/Symfony/Component/Security/Core/User/AdvancedUserInterface.php @@ -12,8 +12,20 @@ namespace Symfony\Component\Security\Core\User; /** - * AdvancedUserInterface adds status flags to a regular account. + * Adds extra features to a user class related to account status flags. * + * This interface can be implemented in place of UserInterface if you'd like + * the authentication system to consider different account status flags + * during authentication. If any of the methods in this interface return + * false, authentication will fail. + * + * If you need to perform custom logic for any of these situations, then + * you will need to register an exception listener and watch for the specific + * exception instances thrown in each case. All exceptions are a subclass + * of AccountStatusException + * + * @see UserInterface + * @see Symfony\Component\Security\Core\Exception\AccountStatusException * @author Fabien Potencier */ interface AdvancedUserInterface extends UserInterface @@ -21,6 +33,11 @@ interface AdvancedUserInterface extends UserInterface /** * Checks whether the user's account has expired. * + * Internally, if this method returns false, the authentication system + * will throw an AccountExpiredException and prevent login. + * + * @see Symfony\Component\Security\Core\Exception\AccountExpiredException + * * @return Boolean true if the user's account is non expired, false otherwise */ function isAccountNonExpired(); @@ -28,6 +45,11 @@ interface AdvancedUserInterface extends UserInterface /** * Checks whether the user is locked. * + * Internally, if this method returns false, the authentication system + * will throw a LockedException and prevent login. + * + * @see Symfony\Component\Security\Core\Exception\LockedException + * * @return Boolean true if the user is not locked, false otherwise */ function isAccountNonLocked(); @@ -35,6 +57,11 @@ interface AdvancedUserInterface extends UserInterface /** * Checks whether the user's credentials (password) has expired. * + * Internally, if this method returns false, the authentication system + * will throw a CredentialsExpiredException and prevent login. + * + * @see Symfony\Component\Security\Core\Exception\CredentialsExpiredException + * * @return Boolean true if the user's credentials are non expired, false otherwise */ function isCredentialsNonExpired(); @@ -42,6 +69,11 @@ interface AdvancedUserInterface extends UserInterface /** * Checks whether the user is enabled. * + * Internally, if this method returns false, the authentication system + * will throw a DisabledException and prevent login. + * + * @see Symfony\Component\Security\Core\Exception\DisabledException + * * @return Boolean true if the user is enabled, false otherwise */ function isEnabled(); diff --git a/src/Symfony/Component/Security/Core/User/UserInterface.php b/src/Symfony/Component/Security/Core/User/UserInterface.php index 3b6695670b..7382cff35c 100644 --- a/src/Symfony/Component/Security/Core/User/UserInterface.php +++ b/src/Symfony/Component/Security/Core/User/UserInterface.php @@ -12,8 +12,20 @@ namespace Symfony\Component\Security\Core\User; /** - * UserInterface is the interface that user classes must implement. + * Represents the interface that all user classes must implement. * + * This interface is useful because the authentication layer can deal with + * the object through its lifecycle, using the object to get the encoded + * password (for checking against a submitted password), assigning roles + * and so on. + * + * Regardless of how your user are loaded or where they come from (a database, + * configuration, web service, etc), you will have a class that implements + * this interface. Objects that implement this interface are created and + * loaded by different objects that implement UserProviderInterface + * + * @see UserProviderInterface + * @see AdvancedUserInterface * @author Fabien Potencier */ interface UserInterface @@ -21,6 +33,17 @@ interface UserInterface /** * Returns the roles granted to the user. * + * + * public function getRoles() + * { + * return array('ROLE_USER'); + * } + * + * + * Alternatively, the roles might be stored on a ``roles`` property, + * and populated in any number of different ways when the user object + * is created. + * * @return Role[] The user roles */ function getRoles(); @@ -28,12 +51,17 @@ interface UserInterface /** * Returns the password used to authenticate the user. * + * This should be the encoded password. On authentication, a plain-text + * password will be salted, encoded, and then compared to this value. + * * @return string The password */ function getPassword(); /** - * Returns the salt. + * Returns the salt that was originally used to encode the password. + * + * This can return null if the password was not encoded using a salt. * * @return string The salt */ @@ -49,11 +77,16 @@ interface UserInterface /** * Removes sensitive data from the user. * + * This is important if, at any given point, sensitive information like + * the plain-text password is stored on this object. + * * @return void */ function eraseCredentials(); /** + * Returns whether or not the given user is equivalent to *this* user. + * * The equality comparison should neither be done by referential equality * nor by comparing identities (i.e. getId() === getId()). * diff --git a/src/Symfony/Component/Security/Core/User/UserProviderInterface.php b/src/Symfony/Component/Security/Core/User/UserProviderInterface.php index 11fd62cdac..d79b7a196f 100644 --- a/src/Symfony/Component/Security/Core/User/UserProviderInterface.php +++ b/src/Symfony/Component/Security/Core/User/UserProviderInterface.php @@ -12,9 +12,19 @@ namespace Symfony\Component\Security\Core\User; /** - * UserProviderInterface is the implementation that all user provider must - * implement. + * Represents a class that loads UserInterface objects from some source for the authentication system. * + * In a typical authentication configuration, a username (i.e. some unique + * user identifier) credential enters the system (via form login, or any + * method). The user provider that is configured with that authentication + * method is asked to load the UserInterface object for the given username + * (via loadUserByUsername) so that the rest of the process can continue. + * + * Internally, a user provider can load users from any source (databases, + * configuration, web service). This is totally independent of how the authentication + * information is submitted or what the UserInterface object looks like. + * + * @see Symfony\Component\Security\Core\User\UserInterface * @author Fabien Potencier */ interface UserProviderInterface @@ -25,7 +35,8 @@ interface UserProviderInterface * This method must throw UsernameNotFoundException if the user is not * found. * - * @throws UsernameNotFoundException if the user is not found + * @see UsernameNotFoundException + * @throws Symfony\Component\Security\Core\Exception\UsernameNotFoundException if the user is not found * @param string $username The username * * @return UserInterface @@ -35,11 +46,12 @@ interface UserProviderInterface /** * Refreshes the user for the account interface. * - * It is up to the implementation if it decides to reload the user data - * from the database, or if it simply merges the passed User into the - * identity map of an entity manager. + * It is up to the implementation to decide if the user data should be + * totally reloaded (e.g. from the database), or if the UserInterface + * object can just be merged into some internal array of users / identity + * map. * - * @throws UnsupportedUserException if the account is not supported + * @throws Symfony\Component\Security\Core\Exception\UnsupportedUserException if the account is not supported * @param UserInterface $user * * @return UserInterface From 2f0afb2b5a8f66f21853dde2f6f7d6b2cf10395d Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 9 Jan 2012 12:13:49 +0100 Subject: [PATCH 2/3] fixed CS --- .../Core/User/AdvancedUserInterface.php | 25 ++++++++++++------- .../Security/Core/User/UserInterface.php | 1 + .../Core/User/UserProviderInterface.php | 17 +++++++++---- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/Symfony/Component/Security/Core/User/AdvancedUserInterface.php b/src/Symfony/Component/Security/Core/User/AdvancedUserInterface.php index f2d59fcbf3..e951c65eb0 100644 --- a/src/Symfony/Component/Security/Core/User/AdvancedUserInterface.php +++ b/src/Symfony/Component/Security/Core/User/AdvancedUserInterface.php @@ -11,6 +11,12 @@ namespace Symfony\Component\Security\Core\User; +use Symfony\Component\Security\Core\Exception\AccountStatusException; +use Symfony\Component\Security\Core\Exception\AccountExpiredException; +use Symfony\Component\Security\Core\Exception\LockedException; +use Symfony\Component\Security\Core\Exception\CredentialsExpiredException; +use Symfony\Component\Security\Core\Exception\DisabledException; + /** * Adds extra features to a user class related to account status flags. * @@ -25,7 +31,8 @@ namespace Symfony\Component\Security\Core\User; * of AccountStatusException * * @see UserInterface - * @see Symfony\Component\Security\Core\Exception\AccountStatusException + * @see AccountStatusException + * * @author Fabien Potencier */ interface AdvancedUserInterface extends UserInterface @@ -36,9 +43,9 @@ interface AdvancedUserInterface extends UserInterface * Internally, if this method returns false, the authentication system * will throw an AccountExpiredException and prevent login. * - * @see Symfony\Component\Security\Core\Exception\AccountExpiredException - * * @return Boolean true if the user's account is non expired, false otherwise + * + * @see AccountExpiredException */ function isAccountNonExpired(); @@ -48,9 +55,9 @@ interface AdvancedUserInterface extends UserInterface * Internally, if this method returns false, the authentication system * will throw a LockedException and prevent login. * - * @see Symfony\Component\Security\Core\Exception\LockedException - * * @return Boolean true if the user is not locked, false otherwise + * + * @see LockedException */ function isAccountNonLocked(); @@ -60,9 +67,9 @@ interface AdvancedUserInterface extends UserInterface * Internally, if this method returns false, the authentication system * will throw a CredentialsExpiredException and prevent login. * - * @see Symfony\Component\Security\Core\Exception\CredentialsExpiredException - * * @return Boolean true if the user's credentials are non expired, false otherwise + * + * @see CredentialsExpiredException */ function isCredentialsNonExpired(); @@ -72,9 +79,9 @@ interface AdvancedUserInterface extends UserInterface * Internally, if this method returns false, the authentication system * will throw a DisabledException and prevent login. * - * @see Symfony\Component\Security\Core\Exception\DisabledException - * * @return Boolean true if the user is enabled, false otherwise + * + * @see DisabledException */ function isEnabled(); } diff --git a/src/Symfony/Component/Security/Core/User/UserInterface.php b/src/Symfony/Component/Security/Core/User/UserInterface.php index 7382cff35c..85356b77a0 100644 --- a/src/Symfony/Component/Security/Core/User/UserInterface.php +++ b/src/Symfony/Component/Security/Core/User/UserInterface.php @@ -26,6 +26,7 @@ namespace Symfony\Component\Security\Core\User; * * @see UserProviderInterface * @see AdvancedUserInterface + * * @author Fabien Potencier */ interface UserInterface diff --git a/src/Symfony/Component/Security/Core/User/UserProviderInterface.php b/src/Symfony/Component/Security/Core/User/UserProviderInterface.php index d79b7a196f..dbd7924a96 100644 --- a/src/Symfony/Component/Security/Core/User/UserProviderInterface.php +++ b/src/Symfony/Component/Security/Core/User/UserProviderInterface.php @@ -11,6 +11,9 @@ namespace Symfony\Component\Security\Core\User; +use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; +use Symfony\Component\Security\Core\Exception\UnsupportedUserException; + /** * Represents a class that loads UserInterface objects from some source for the authentication system. * @@ -24,7 +27,8 @@ namespace Symfony\Component\Security\Core\User; * configuration, web service). This is totally independent of how the authentication * information is submitted or what the UserInterface object looks like. * - * @see Symfony\Component\Security\Core\User\UserInterface + * @see UserInterface + * * @author Fabien Potencier */ interface UserProviderInterface @@ -35,11 +39,14 @@ interface UserProviderInterface * This method must throw UsernameNotFoundException if the user is not * found. * - * @see UsernameNotFoundException - * @throws Symfony\Component\Security\Core\Exception\UsernameNotFoundException if the user is not found * @param string $username The username * * @return UserInterface + * + * @see UsernameNotFoundException + * + * @throws UsernameNotFoundException if the user is not found + * */ function loadUserByUsername($username); @@ -50,11 +57,11 @@ interface UserProviderInterface * totally reloaded (e.g. from the database), or if the UserInterface * object can just be merged into some internal array of users / identity * map. - * - * @throws Symfony\Component\Security\Core\Exception\UnsupportedUserException if the account is not supported * @param UserInterface $user * * @return UserInterface + * + * @throws UnsupportedUserException if the account is not supported */ function refreshUser(UserInterface $user); From 0507840b67d6375f791ead7e00e7424ec12f901e Mon Sep 17 00:00:00 2001 From: Matt Robinson Date: Mon, 9 Jan 2012 11:40:00 +0000 Subject: [PATCH 3/3] Prevent parameters from overwriting the template filename. Fixes a potential arbitrary file execution exploit. --- src/Symfony/Component/Templating/PhpEngine.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Templating/PhpEngine.php b/src/Symfony/Component/Templating/PhpEngine.php index 32c8c4564d..1953866bad 100644 --- a/src/Symfony/Component/Templating/PhpEngine.php +++ b/src/Symfony/Component/Templating/PhpEngine.php @@ -150,15 +150,20 @@ class PhpEngine implements EngineInterface, \ArrayAccess protected function evaluate(Storage $template, array $parameters = array()) { $__template__ = $template; + + if (isset($parameters['__template__'])) { + throw new \InvalidArgumentException('Invalid parameter (__template__)'); + } + if ($__template__ instanceof FileStorage) { - extract($parameters); + extract($parameters, EXTR_SKIP); $view = $this; ob_start(); require $__template__; return ob_get_clean(); } elseif ($__template__ instanceof StringStorage) { - extract($parameters); + extract($parameters, EXTR_SKIP); $view = $this; ob_start(); eval('; ?>'.$__template__.'