search

icebreaker-science / backend

The backend (Spring Boot) part of the icebreaker.science application
Apache License 2.0
2 stars 0 forks source link

Improve login for non-activated accounts #63

Open dajenet opened 3 years ago

dajenet commented 3 years ago

If an account is not activated on login, but the credentials are valid, the backend should send a different exception to show that the activation is missing.

Furthermore there should be an api call to resend the activation mail.

michael-kamel commented 3 years ago

I don't think it would be a good idea to have different behavior for non activated accounts in general. plus I think we already agreed that the non-activated accounts shouldn't be recognized until activated

chaoran-chen commented 3 years ago

There is already an API to resend the activation mail at /account/resend-confirmation-email. I think that it would be useful since it seems not unlikely that a user will lose the confirmation mail and it would be good if the user wouldn't be "banned" for life because of that.

Or why do you think that it wouldn't be a good idea, @michael-kamel?

michael-kamel commented 3 years ago

There is already an API to resend the activation mail at /account/resend-confirmation-email. I think that it would be useful since it seems not unlikely that a user will lose the confirmation mail and it would be good if the user wouldn't be "banned" for life because of that.

Or why do you think that it wouldn't be a good idea, @michael-kamel?

I'm just disagreeing with notifying the user if his account is still not verified. One reason which is not crucial is that we already agreed against that and therefore implemented it in a way that would make adding this feature a little bit untrivial. The second reason is that I prefer to keep the login functionality as simple as possible to avoid possible bugs and security related issues. This is more of a delighter feature in my opinion cos of the reasons I mentioned before and cos of the fact that unverified tokens/accounts should be verified shortly anyway. I'm not completely against it but I would just notify the user of the possibility that he forgot to activate his account.

dajenet commented 3 years ago

I think this could avoid confusion, if the user forgot to activate his account.

It should be quite easy to implement by removing the enabled check from the login query and throw an exception in the login method, if the account is not activated.

vordemann commented 3 years ago

Agreed in Meeting -> Add different exception

michael-kamel commented 3 years ago

please read the OWASP entry on this before proceeding with implementation, It'll give you a full overview on the trade-off being made here.