Add github login

[mablae]

Copy from gitter chat, @codeliner 's idea, from my memory:

It would be nice to have github/oauth login

[codeliner]

Details can be discussed as soon as someone wants to take over the exercise.

[shulard]

Hello guys,

I'm interested to take a look at this issue 😄. Doesn't seems easy to handle but I really want to dive in one of these exercises !

[codeliner]

@shulard That's cool and if you get this feature in then you're the new prooph-do hero 🥇

First we need to think about the requirements. We should start as simple as possible and split tasks in smaller exercises.

At the moment it is only possible to register a single user with his or her username and email.
A login is not implemented. Once you have registered, you cannot change that or log out.

At the PHP Developer Day I talked with @oqq about user management. He told me about an interesting way how to organize user and identity:

A User can have one or more Identities. An Identity is a separate aggregate. You can log in with an identity and the application looks up the connected user of that identity.

So we can now decide which way we want to go:

1.) Add a Github API Service that can register the github user in proophessor-do using existing commands.
2.) Add log in functionality and split User and Identity. Add Github Login in a second step, so that we have two types of Identity then.

Details Option 1

If no user exists in the app we redirect the user to new page where they can choose between manual registration (enter username + email) OR github registration. First option uses existing registration mechanism and the new github option uses a new Github API Service. We can use a PHP Github lib. We just need to check what libs are available and what is the best one to use. The user authenticates against Github and we use the Github username and email to register the user in proophessor-do using the existing RegisterUser command. We should add a new property that tells us if app or github registration was used.

This option does not include a big change in domain model and focuses more on the Github integration. The intesting thing here is to integrate a third-party service and use a process manager to auto register a user.

Details Option 2

Option 2 requires more work with the domain model. A new Identity aggregate needs to be added and the current user registration should be refactored.
We also need a way to log in. Log in and log out events are recorded by the Identity aggregate so you get a full log in history.

We can also discuss other options. I'm open for suggestions ;)

[oqq]

Here we go!
I am about to create an example implementation for an event sourced user login.
https://github.com/oqq/es_user_login

@codeliner i am not sure about Log in and log out events are recorded by the Identity aggregate. How about an UserLoggedInWithIdentity event, triggered by the user aggregate?

[shulard]

Hello @codeliner,

Thank you for all the details !

In the project a user can connect with 2 identity as the same user, so option 2 is more interesting in this exercise because it help handling the concept of Identity. Also it opens the ability to integrate different identity logic more easily (other than Github).

Also if in the future you want to integrate 2FA the concept of Identity is mandatory I think...

[codeliner]

great. Option 2 is my favorite, too. @shulard Good point regarding 2FA. That is an interesting feature and a good exercise once we have the Identity aggregate.

Side notes: I thought about the roadmap of proophessor-do. We have some big maintenance tasks in the pipeline:

• we want to merge the different framework variants into one repo: Merge this repo into proophessor-do proophessor-do-symfony#6
• And that requires a rewrite of the frontend part to use a SPA and API-only backend

When these tasks are done I'd like to host proophessor-do somewhere (maybe we find a sponsor). A real login is mandatory for that, so working on that feature now is great.

Having said this, we should only focus at the backend side for the login functionality and keep in mind, that framework variants will be merged. Everything used by the domain needs to be defined by an interface, so that the web frameworks can provide alternative implementations (for example mailer, OAuth client, ...)

@oqq Sounds good. Can you provide a rough message flow?

I think first step is to expand user registration:

1. RegisterUser command should take a new property password.
2. User::register method takes a new PasswordHashService as argument to hash the password before recording the UserWasRegistered domain event
3. a process manager listens on UserWasRegistered events and dispatches new AddIdentity commands for each registered user, so that a new Identity aggregate is added with user email as aggregate id and the hashed password.
4. Identity should be in state "email not confirmed" and another event listener should listen on IdentityWasAdded to send a confirmation email with an activation link (uuid for the link should be generated by the Identity Aggregate - using a service - and stored in internal state of the Identity aggregate). The activation uuid should be part of event payload of IdentityWasAdded so that the event listener can take that information and send out the email.

I think that is enough for one exercise. Activating the Identity can be the next excersice as well as login in and log out commands.

Github login is also moved to a later exercise.

What do you think @shulard and @oqq /cc @prolic

[shulard]

Seems a nice first step, I'll start working on it ASAP.

[prolic]

looks good to me

[oqq]

@codeliner what if the email address is already in use?
In my opinion we need to know that before the UserWasRegistered event was triggered.
In that case, an different event on the already exists user aggregate has to be triggered which could send an email with "Your Email is already in use .. Lost your password? Click here" as content.

[codeliner]

@oqq Good catch, so we need to switch the order?

1. AddIdentityForNewUser command (later we will also have a AddIdentityForExistingUser command). Then the Identity takes over the task to hash the password, which is better because the Identity also verifies the password on log in.
2. process manager reacts on IdentityForNewUserWasAdded event and dispatches a RegisterUser command.
   ...

"The lost your password" scenario is again a nice excercise.

[oqq]

Sounds good!
I like the way to use a process manager for that.

Is there a 'best practice' to run all process managers listening for an event in a transaction?
Maybe we need a new plugin in prooph event bus for that.

[codeliner]

each command should run in its own transaction. The aggregate defines the transaction boundary, not the process managers. If you need to coordinate work across different aggregates you should use a saga for that.

[kochen]

@oqq nice work on your implementation.

Regardless of the GitHub account/login and just to elaborate a bit more, this issue should/could also be taken much further into Real Life use cases:

• Real Life (API) application would use some sort of Authentication like OAuth2, and in this case probably a Password Grant Type, so there needs to be some considerations on how the token is returned to the newly registered user, while the command itself doesn't (or at least shouldn't) return any response.
• Taking it further, a user can only change his own password, and not a password of any given user_id/identity_id.
• One more step further, user_id/identity_id should not be passed to any command explicitly but deducted from the token that is being sent with the call - taking this into Metadata Enrichment?

[burzum]

@oqq are you still working on this?

[oqq]

@burzum i am not sure what you are referring to. Is it the ticket, or the event sourced user login example?

[burzum]

@oqq the Github user login. I'm asking because I want to do something as well with proophessor-do and looking for a task. :)

[oqq]

@burzum sorry, but I have never worked on this issue. Feel free to catch the task. 👍

[ahmed-alaa]

Hi, I would like to catch up and get more into Prooph so I think catching up this task or maybe another task would be a great idea.

Any objection or anyone is working on this task already? @codeliner

[prolic]

@ahmed-alaa go for it!

[ahmed-alaa]

@oqq Good catch, so we need to switch the order?

1. `AddIdentityForNewUser` command (later we will also have a `AddIdentityForExistingUser` command). Then the Identity takes over the task to hash the password, which is better because the Identity also verifies the password on log in.

2. process manager reacts on `IdentityForNewUserWasAdded` event and dispatches a `RegisterUser` command.
   ...

"The lost your password" scenario is again a nice excercise.

@codeliner Hi, I started to work on this task, I was reading the comments on the ticket.

1. I would rather go with your first approach (Add github login #129 (comment)) to have a projection that listens to UserRegistered that creates Identity depending on the type of the Registration process.

2. Identity will be used to Login and Logout, which means that the hashing will happen during event persistence of the identity

3. Checking that Email exists can be done using Read models of read_user which actually fire another command rather than RegisterUser

What do you think?

[codeliner]

@ahmed-alaa sounds good. Let's try it this way and see how it works out.