tm-pe2 / ad-website

Frontend web app
0 stars 0 forks source link

Login registration #34

Closed sdd2001 closed 2 years ago

sdd2001 commented 2 years ago

I've made a login and registration component. I've commented what all code does in the .ts files in case there's a problem.

Right now you can log in using a mock user (in the login.service.ts file)

I've added some code in the app-module.html file for checking if the login redirects after you press the login button.

I've also installed ts-md5 which normally updated the package.json files

vdbe commented 2 years ago

Is there a reason you hash the password client side? If there is I would advice against using md5 it is not safe by today's standards.

Could you also maybe put the password for the mock user somewhere was not hard to find but would be handy.

DrunkenToast commented 2 years ago

As Ewout said, md5 is not safe if your intent is password hashing. If you intend to replace it because you need hashing that's fine. Your unit tests failed as well, you can run npm run test or check the details to see what failed, you can adjust the unit test files themselves or your code wherever necessary.

sdd2001 commented 2 years ago

I used Md5 because it's the one I know, I didn't know it was unsafe so I will look for another way. 😅

I also forgot to do the hashing in the services it seems so I'll fix that as well + I'll do the npm run test to see what is failing

DriesOlbrechts commented 2 years ago

@sdd2001 either way at some point hashing should happen in the backend (API)

sdd2001 commented 2 years ago

okay, so do I just keep them unhashed for now?

vdbe commented 2 years ago

I would just sent the password without hash to the api

DrunkenToast commented 2 years ago

Through https (encrypted) (although http for now) you send the password to the API, API will then hash the password properly and store it in the database.

sdd2001 commented 2 years ago

I've removed the hashing so the passwords so they can be send normally to the API.

Dries also fixed the errors I was getting when I did npm run test since I couldn't figure it out.

DriesOlbrechts commented 2 years ago

@sdd2001 No shame in not figuring that one out. Really weird issue.

sdd2001 commented 2 years ago

do I save the service in a service folder or do I keep it in on of the component folders?

DrunkenToast commented 2 years ago

I'd suggest putting services in app/services. I am not certain if a service is even best practise for something like this. But in general we need something to access user data globally across the app easily. I am mostly guessing that service injection would be one way to do it.

sdd2001 commented 2 years ago

Can someone else take a look at these errors please, I can't seem to fix them.

sdd2001 commented 2 years ago

omg thanks! and now I see what I was missing xD