Browse Source

Applied my review

Christophe Coevoet 12 years ago
parent
commit
cb29de7185

+ 1 - 1
app/Resources/FOSUserBundle/views/Security/login.html.twig

@@ -5,7 +5,7 @@
     <div>{{ error }}</div>
 {% endif %}
 
-<form action="/login_check" method="post">
+<form action="{{ path('login_check') }}" method="post">
     <div>
         <label for="username">{{ 'security.login.username'|trans({}, 'FOSUserBundle') }}</label>
         <input type="text" id="username" name="_username" value="{{ last_username }}" />

+ 5 - 7
src/Packagist/WebBundle/Form/Handler/OAuthRegistrationFormHandler.php

@@ -16,6 +16,7 @@ use FOS\UserBundle\Model\UserManagerInterface;
 use FOS\UserBundle\Util\TokenGeneratorInterface;
 use HWI\Bundle\OAuthBundle\Form\RegistrationFormHandlerInterface;
 use HWI\Bundle\OAuthBundle\OAuth\Response\UserResponseInterface;
+use HWI\Bundle\OAuthBundle\OAuth\Response\AdvancedUserResponseInterface;
 use Symfony\Component\Form\Form;
 use Symfony\Component\HttpFoundation\Request;
 
@@ -33,6 +34,7 @@ class OAuthRegistrationFormHandler implements RegistrationFormHandlerInterface
      * Constructor.
      *
      * @param UserManagerInterface $userManager
+     * @param TokenGeneratorInterface $tokenGenerator
      */
     public function __construct(UserManagerInterface $userManager, TokenGeneratorInterface $tokenGenerator)
     {
@@ -43,32 +45,28 @@ class OAuthRegistrationFormHandler implements RegistrationFormHandlerInterface
     /**
      * {@inheritDoc}
      */
-    function process(Request $request, Form $form, UserResponseInterface $userInformation)
+    public function process(Request $request, Form $form, UserResponseInterface $userInformation)
     {
         $user = $this->userManager->createUser();
 
         $form->setData($user);
 
         if ('POST' === $request->getMethod()) {
-            $form->bindRequest($request);
+            $form->bind($request);
 
             if ($form->isValid()) {
                 $randomPassword = $this->tokenGenerator->generateToken();
-                $form->getData()->setPassword($randomPassword);
+                $user->setPlainPassword($randomPassword);
 
                 return true;
             }
         // if the form is not posted we'll try to set some properties
         } else {
-            $user = $form->getData();
-
             $user->setUsername($this->getUniqueUsername($userInformation->getUsername()));
 
             if ($userInformation instanceof AdvancedUserResponseInterface) {
                 $user->setEmail($userInformation->getEmail());
             }
-
-            $form->setData($user);
         }
 
         return false;

+ 1 - 2
src/Packagist/WebBundle/Resources/config/services.yml

@@ -43,9 +43,8 @@ services:
             - { name: form.type, alias: packagist_oauth_user_registration }
 
     packagist.oauth.registration_form:
-        factory_method: createNamed
+        factory_method: create
         factory_service: form.factory
         class: Symfony\Component\Form\Form
         arguments:
             - 'packagist_oauth_user_registration'
-            - 'packagist_oauth_user_registration'

+ 2 - 2
src/Packagist/WebBundle/Resources/views/layout.html.twig

@@ -33,7 +33,7 @@
         <div class="container">
             <div class="user">
                 {% if app.user %}
-                    <a href="{{ path('fos_user_profile_show') }}">{{ app.user.username }}</a> | <a href="/logout">Logout</a>
+                    <a href="{{ path('fos_user_profile_show') }}">{{ app.user.username }}</a> | <a href="{{ path('logout') }}">Logout</a>
                 {% else %}
                     <a href="{{ path('fos_user_registration_register') }}">Create a new account</a>
                     |
@@ -81,7 +81,7 @@
             <ul>
                 {% if app.user %}
                     <li><a href="{{ path('fos_user_profile_show') }}">{{ 'menu.profile'|trans }}</a></li>
-                    <li><a href="/logout">{{ 'menu.logout'|trans }}</a></li>
+                    <li><a href="{{ path('logout') }}">{{ 'menu.logout'|trans }}</a></li>
                 {% else %}
                     <li><a href="{{ path('fos_user_registration_register') }}">{{ 'menu.register'|trans }}</a></li>
                     <li><a href="{{ path('hwi_oauth_connect') }}">{{ 'menu.login'|trans }}</a></li>

+ 8 - 1
src/Packagist/WebBundle/Security/Provider/UserProvider.php

@@ -43,8 +43,15 @@ class UserProvider implements OAuthAwareUserProviderInterface, UserProviderInter
     {
         $username = $response->getUsername();
 
+        $previousUser = $this->userManager->findUserBy(array('githubId' => $username));
+
+        // The account is already connected. Do nothing
+        if ($previousUser === $user) {
+            return;
+        }
+
         // 'disconnect' a previous account
-        if (null !== $previousUser = $this->userManager->findUserBy(array('githubId' => $username))) {
+        if (null !== $previousUser) {
             $previousUser->setGithubId(null);
             $this->userManager->updateUser($previousUser);
         }