Skip to content
This repository has been archived by the owner on May 7, 2023. It is now read-only.

Improvement of notifying sign in error #454

Closed
qkreltms opened this issue Aug 15, 2021 · 7 comments
Closed

Improvement of notifying sign in error #454

qkreltms opened this issue Aug 15, 2021 · 7 comments
Labels
💬 discussion Discuss issues

Comments

@qkreltms
Copy link

qkreltms commented Aug 15, 2021

Specify project
Client

Is your feature request related to a problem? Please describe.
When I tried to sign in with incorrect email, but correct password, error message displays like "비밀번호를 다시 확인해주세요.".
hackatalk

Describe the solution you'd like
현재 서버, 클라이언트에서 어떤 부분이 틀렸는지 알려줍니다.

어떤 부분이 틀렸는지 알려주는 것은 보안상으로 이슈가 있을 것으로 예상됩니다.

일례로 네이버 로그인 창에서는 아이디 또는 비밀번호가 틀리면 어떤게 틀렸는지 알려주지 않고 "가입하지 않은 아이디이거나, 잘못된 비밀번호입니다.와 같은 메시지를 표시합니다. 그러므로 네이버와 같이 수정하는 것이 좋다고 판단됩니다.

It tells you email or password which part is wrong on client side when sign in button is clicked.

It is expected that there is a security issue for it.

Because of it, for example, when you sign in the Naver, it doesn't tell which is wrong and says "You have not signed up for an ID or an incorrect password." instead.

How about following like that?
Additional context

@qkreltms
Copy link
Author

#436 (comment)

@hyochan hyochan added the 💬 discussion Discuss issues label Aug 16, 2021
@hyochan
Copy link
Member

hyochan commented Aug 16, 2021

Specify project
Client

Is your feature request related to a problem? Please describe.
When I tried to sign in with incorrect email, but correct password, error message displays like "비밀번호를 다시 확인해주세요.".
hackatalk

Describe the solution you'd like
현재 db, 클라이언트에서 어떤 부분이 틀렸는지 알려줍니다.

어떤 부분이 틀렸는지 알려주는 것은 보안상으로 이슈가 있을 것으로 예상됩니다.

일례로 네이버 로그인 창에서는 아이디 또는 비밀번호가 틀리면 어떤게 틀렸는지 알려주지 않고 "가입하지 않은 아이디이거나, 잘못된 비밀번호입니다.와 같은 메시지를 표시합니다.

Additional context

I hope you write in English next time 😅 so that more people around the world lately can participate in our work.
I think this issue is the matter of which string we should provide here since it doesn't tell which is wrong. It just tells that the password is incorrect. It doesn't argue that the email is wrong.

@qkreltms
Copy link
Author

Do you saying it should validate and tells whether email is wrong not only arguing about password if I understand correctly? @hyochan

@hyochan
Copy link
Member

hyochan commented Aug 24, 2021

Do you saying it should validate and tells whether email is wrong not only arguing about password if I understand correctly? @hyochan

No. Looking inside the code, I found that it did not have any scenario on giving hints on the wrong email that does not match the database. Just telling the email format is not correct does not make any security issue.

@qkreltms
Copy link
Author

qkreltms commented Aug 24, 2021

Okay I got this now "No. Looking inside the code, I found that it did not have any scenario on giving hints on the wrong email that does not match the database. Just telling the email format is not correct does not make any security issue."

Than, how about just displaying error message like "가입하지 않은 아이디이거나, 잘못된 비밀번호입니다." not displaying red underline because users may be confused because of it.(email format is wrong, but message is "비밀번호를 다시 확인해주세요." with red underline on password field)

@hyochan
Copy link
Member

hyochan commented Aug 24, 2021

Than, how about just displaying error message like "가입하지 않은 아이디이거나, 잘못된 비밀번호입니다." not displaying red underline because users may be confused because of it.(email format is wrong, but message is "비밀번호를 다시 확인해주세요." with red underline on password field)

I think you've missed the code here https://github.com/dooboolab/hackatalk/blob/ee38d64fba78203f63e13a80425e7e689d383c29/client/src/components/pages/SignIn/index.tsx#L176.

I am still confused about what you are trying to achieve here. How about just give out a proposal if you think something is actually needed? Or it'd be good to bring another idea and focus on that 🤔

@qkreltms
Copy link
Author

qkreltms commented Aug 24, 2021

I've focused on these lines
https://github.com/dooboolab/hackatalk/blob/ee38d64fba78203f63e13a80425e7e689d383c29/client/src/components/pages/SignIn/index.tsx#L217-L220
It only tells password is incorrect If I understand correctly.

Well if you think it is not an issue I will close this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
💬 discussion Discuss issues
Projects
None yet
Development

No branches or pull requests

2 participants