hwi / HWIOAuthBundle

OAuth client integration for Symfony. Supports both OAuth1.0a and OAuth2.
MIT License
2.27k stars 800 forks source link

[DX] Revamp the documentation #637

Closed javiereguiluz closed 8 years ago

javiereguiluz commented 10 years ago

I'd love to revamp the documentation of this bundle, but before starting this huge task, I'd like to know if the bundle creators and/or managers are willing to include major changes in the documentation.

This is a short overview of the changes that I'm considering:

Aryess commented 10 years ago

That's a good idea, the documentation is a bit sparse. One thing that could be nice too is on how to add a confirmation page where users can provide missing info, for services like Twitter that does not provide email address.

aranw commented 10 years ago

It would also be useful to show how to link accounts after the fact a service has already been used for log in.

For example users register's with just an ordinary username and password style process, but later on they decide they want to link Facebook or Twitter to the account.

Also adding more details about settings/urls and configuration of services would be useful.

aranw commented 10 years ago

@javiereguiluz @Aryess I was also wondering if there is any WIP documentation anywhere? As am currently trying to add Twitter login but getting stuck. I am also trying to do as mention in my previous comment about adding additional accounts after a user has already authenticated, for example I have Facebook login working but now I want/need to link up Twitter as well.

javiereguiluz commented 10 years ago

@aranw I'm afraid I don't have any WIP documentation yet. And I'm afraid that this is going to take some time because the work to do is daunting.

aranw commented 10 years ago

@javiereguiluz understanding, don't suppose you have any resources you can link in the mean time for "Add information about how to show a confirmation page where users can provide missing info, for services like Twitter that does not provide email address" as am currently stuck on this bit and can't figure out how to handle this problem.

Ideally I need to do this before HWIOAuth directs my request into the WebServiceUserProvider but am stuck on how to do this.

jeffxpx commented 9 years ago

@javiereguiluz Is there anything I can do to help you improve the docs? I'm using this bundle for a project I'm doing and I've had to become somewhat intimately familiar with it. Sorry if this response hijacks this issue.

@aranw It is possible to do some or most of what you're suggesting with some work. In my project, I've got G+, Facebook, and FOS UserBundle email/password combo logins. I'm not completely done, but I'm getting close to the system you're describing.

The direction I'd point you in is this. I'd start with this: https://gist.github.com/danvbe/4476697, but I'd only use the connect() function from the UserProvider he mentions. The "connect" functionality is specifically for taking an OAuth response and showing a registration or account linking form. The loadUserByOAuthUserResponse code in that gist short circuits that and essentially creates an account behind the scenes.

Additionally, you'll probably want your own registration form handler so you can import as many fields as you want. If you're using the code from the gist, in your services.yml file, you'll probably want to add something like this (this is from my code, but you'll want to adapt it to your profiles):

# in MyUserBundle/Resources/config/services.yml
my_user.registration.form.handler:
        class: My\UserBundle\Form\RegistrationFormHandler
        arguments:
            - @fos_user.user_manager
            - @fos_user.mailer
            - @fos_user.util.token_generator

# in app/config/config.yml
hwi_oauth:
    connect:
        confirmation: true
        account_connector: my_user_provider
        registration_form_handler: my_user.registration.form.handler

Then in your registration handler, you'll probably want something like this:

// this is part of the class My\UserBundle\Form\RegistrationFormHandler which extends HWI\Bundle\OAuthBundle\Form\FOSUBRegistrationFormHandler

    // this is to override the plainPassword field
    public function process(Request $request, Form $form, UserResponseInterface $userInformation)
    {
        // not sure if this code even works in later versions FOSUserBundle...
        if (null !== $this->registrationFormHandler) {
            $formHandler = $this->reconstructFormHandler($request, $form);

            // make FOSUB process the form already
            $processed = $formHandler->process();

            // if the form is not posted we'll try to set some properties
            if (!$request->isMethod('POST')) {
                $form->setData($this->setUserInformation($form->getData(), $userInformation));
            }

            return $processed;
        }

        $user = $this->userManager->createUser();
        $user->setEnabled(true);

        $user = $this->setUserInformation($user, $userInformation);

        // this is a bit of a hack...can't disable validation at this point, so create a random password that isn't used anywhere
        if ($form->has('plainPassword')) {
            $form->remove('plainPassword');
            $form->add('plainPassword', 'hidden', [
                'data' => sha1($user->getUsername() . uniqid() . time() . rand(1000000, 9999999))
            ]);
        }

        $form->setData($user);

        if ($request->isMethod('POST')) {
            $form->handleRequest($request);

            return $form->isValid();
        }

        return false;
    }

    // this is what takes the OAuth Response and populates a user with it
    protected function setUserInformation(UserInterface $user, UserResponseInterface $response)
    {
        // this will be tried next time!
        $username = $response->getUsername();

        $service = $response->getResourceOwner()->getName();
        $setter = 'set'.ucfirst($service);
        $setter_id = $setter.'Id';
        $setter_token = $setter.'AccessToken';

        $email = $response->getEmail();

        /* @var $actualUser My\UserBundle\Entity\User */
        $actualUser = $user;
        $actualUser->$setter_id($username);
        $actualUser->$setter_token($response->getAccessToken());

        $actualUser->ensureUuid();

        if ($email != null) {
            $actualUser->setEmail($email);
        }

        if ($actualUser->getUsername() == null) {
            $actualUser->setUsername($service . '_' . $username);
        }

        if ($actualUser->getPassword() == null) {
            $actualUser->setPassword($username);
        }

        $responseData = $response->getResponse();

        switch($service) {
            case "google":
                if (isset($responseData['given_name'])) {
                    $actualUser->setFirstName($responseData['given_name']);
                }
                if (isset($responseData['family_name'])) {
                    $actualUser->setLastName($responseData['family_name']);
                }

                if (isset($responseData['gender'])) {
                    switch($responseData['gender']) {
                        case "male":
                            $actualUser->setGender('m');
                            break;
                        case "female":
                            $actualUser->setGender('f');
                            break;
                    }
                }

                if (isset($responseData['picture'])) {
                    $actualUser->setProfilePicUrl($responseData['picture']);
                }
                break;

            case "facebook":
                if(isset($responseData['first_name'])) {
                    $actualUser->setFirstName($responseData['first_name']);
                }

                if (isset($responseData['last_name'])) {
                    $actualUser->setLastName($responseData['last_name']);
                }

                if (isset($responseData['gender'])) {
                    switch(strtolower($responseData['gender'])) {
                        case "male":
                            $actualUser->setGender('m');
                            break;
                        case "female":
                            $actualUser->setGender('f');
                            break;
                    }
                }

                if (isset($responseData['birthday'])) {
                    $birthDate = new \DateTime($responseData['birthday'], new \DateTimeZone("UTC"));
                    $actualUser->setBirthDate($birthDate);
                }

                break;

            default:
                throw new \Exception("This code doesn't handle: " . $service);
        }

        return $actualUser;
    }

This is working for me but it is still kinda kludgy. I have to have essentially two registration templates (the normal extended UserBundle one and the HWIOAuth registration one).

I almost need to make a simple proof-of-concept symfony installation with all the various bits in place to show how I got it working.

In my current version I'm still having issues where if someone logs in with Facebook at first and creates an account using the code above and then they come back and forgot they logged in with Facebook and when they're not logged in, they decide to login with Google+ second and both accounts use the same e-mail address.

If they're not logged in to an account and they log into a social account that has an e-mail address that matches an account that exists, it currently shows the HWI registration screen instead of either showing the HWI "link accounts" screen or at the very least ask them to login with their original account that's on file and then somehow connect them that way.

If they go to a login with url while they're logged into another matching account, it works like you'd expect and shows the "link accounts" screen.

javiereguiluz commented 9 years ago

@jeffxpx I'm glad that you can take over the initial effort to revamp the documentation of this bundle :)

psrpinto commented 9 years ago

I definitely agree the documentation needs to be significantly improved. I have a reasonably good understanding of the architecture (I've contributed PRs in the past) but everytime I need to get back to this bundle, it's still very hard to have a broad picture of what's going on. It's usually debugger in one hand, pen and paper in the other one, trying to reverse engineer it.

Don't get me wrong, the code is high-quality and is relatively simple considering the huge complexity of the problem at hand. However, reading the code only gets you so far: there are implicit notions that can only be explained by means of documentation. For example: what does Connect mean? It's a crucial feature of this bundle and it's effectively inexistent for many users because it's not documented.

I realize the maintainers are busy with real work and that it takes many hours to maintain a project this size but it's a matter of recruiting new blood to help you out. Here you already have three potential volunteers who you can put in charge of the docs (@javiereguiluz, @jeffxpx and myself).

That said, @javiereguiluz lists many of the problems with the docs right now, but I think the documentation needs to be written from scratch. This could be done over several PRs, with the first one defining the structure of the documentation to come (and "refactoring" the existing docs into this new structure). As I said, I'm willing to put time on this but before I do, I need to know that this effort fits the vision the maintainers have for the project. @stloyd @asm89

javiereguiluz commented 8 years ago

Closing this ancient issue as "won't fix". Thank you all for your comments.