Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client Authentication #105

Open
lukaszreszke opened this issue Mar 2, 2022 · 8 comments
Open

Client Authentication #105

lukaszreszke opened this issue Mar 2, 2022 · 8 comments
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@lukaszreszke
Copy link
Collaborator

Currently, a client can "log in" by navigating to /client URL, selecting desired client name, and clicking the login button.
The outcome of this issue should be a possibility to log in using the client login and password.

@krzykamil krzykamil added enhancement New feature or request good first issue Good for newcomers labels May 25, 2022
@andrzejkrzywda
Copy link
Contributor

I've had a long deliberation blocker here - whether to go with Devise or start from scratch. I now think going from scratch to have an event sourced Authentication module is the way to go. However with this ticket I imagine to go with something super simplest. Maybe just have commands for setting the accounts (login/password) for clients (without the registration UI yet), use them in the seeds. Then some UI (login/password) only for the customer panel.

@andrzejkrzywda andrzejkrzywda changed the title Authentication Client Authentication Jun 9, 2022
@pstrzalk
Copy link
Contributor

Hi, @andrzejkrzywda

As I have mentioned, I would love to help out with this one.

I've seen the work around Client idea in 0af1cae and f962d0b

Do you see Client entity/commands/events as a part of Ordering context or should it rather get its own one?
Or perhaps the Client referred in this issue should in fact be a Customer and that's Customer who should be given a login/password?

I would love to align this before I ship my idea of the solution

@andrzejkrzywda
Copy link
Contributor

I think we can start with a new BC called Authentication (similarly as in the red book - the Identity and Access BC).

The Account seems like a good building block here.
For the start we probably need:

  • RegisterAccount(id)
  • SetLogin(account_id, login)
  • SetPasswordHash(account_id, password_hash)
  • ConnectAccountToClient(account_id, client_id)

The consequence would be that we allow multiple accounts per client, which is fine by me.

Those commands would then be used in the seeds to set up the existing clients. We will create UI for creating accounts as a separate ticket.

For the login process we probably need something like LoginSession.

  • StartSession(uid, credentials)

Let's make the Logout feature as a separate ticket.

BTW, currently we use client/customer as synonyms.

How does it sound?

@pstrzalk
Copy link
Contributor

Sounds great!
Thank you very much for the detailed description. I will jump into it next week and hopefully deliver shortly after.

@andrzejkrzywda
Copy link
Contributor

There was a bit of progress about it recently. Instead of using URL, we now store cookies.

some context: https://dev.to/andrzejkrzywda/implementing-authentication-in-tiny-steps-jhn

@pstrzalk how is it going? need any help?

@pstrzalk
Copy link
Contributor

Thank you very much for the head's up @andrzejkrzywda

Actually, I need 48h days too keep up with all the plans and promisses... I'm terribly sorry to keep you waiting for so long.

Back when I started, I've added the new BC and the first events. But I had to put it on a shelf for much longer than I expected.
I will be back with my mac in 5-6 days. I will make sure that what I had prepared makes sense and push forward

@pstrzalk
Copy link
Contributor

I have re-visited my changes and adjusted a bit. I've created a WIP PR at #175

So far I've

  • added Authentication BC
  • added commands & events
    • RegisterAccount
    • SetLogin
    • SetPasswordHash
    • ConnectAccountToClient
  • added tests and made sure mutation tests work fine

When you have a chance, please take a look and let me know what you think. Again - sorry for the delay.

The next step, if I understand correctly, is to:

  • add LoginSession AggregateRoot with start_session method
  • add StartSession command & SessionStarted event
  • raise error if credentials don't match
  • use this all in the login action of the Client::ClientsController

The thing I'm not sure about is - what's the intended approach for checking the credentials?
@andrzejkrzywda

@tomaszpatrzek
Copy link
Collaborator

Added the possibility to login with password.

Extracted tasks:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants