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

Unconfirmed user messages #355

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

mariha
Copy link
Collaborator

@mariha mariha commented Apr 17, 2020

When user creates an account the e-mail address they provided needs to be verified. Util they confirm, they can log in but they don't have access to most resources so the server returns 403 errors. I changed the code so that they get a message with some hints what is going on. This solves #327.

I tried (quite hard) to write tests for my changes, temporarily gave up though. I hope to come back to it later, by the time please give a look at what I have so far.

It is possible to send requests resulting in 403 when loading users to the Map, Searching for specific users or checking Messages. I updated all these places. I thought also that instead we might send a permissions-verifying request first, when user tries to log in (seems for me more intuitive to fail before going further).

I added filtering to exception handling. It is based on http error codes, so that:

  • 403 Forbidden response results in returning access_denied string from the resources (added), mapped to
    Access denied. Please check your email for account confirmation message. (proposition)
  • 500 Internal server error response results in returning internal_server_error string (added), mapped to
    Oops... This is on us, sorry! Something has gone wrong on the server side. Please try again later.

Also:

  • message_thread_create_failed, messages_reload_failed and message_send_failed are not used anywhere any more so I removed them

I was thinking of changing error handling, moving towards having one place where different exception types and http error codes are translated to messages and then displayed, so that it is more uniform across the app and we could easily hook some more robust logging/analytics. It's in HttpErrorHelper as for now. I also narrowed down Errors to HttpExceptions or Exceptions so we could differentiate exception handling behavior based on ex type and error code.

… is not confirmed yet.

`403 Forbidden` and `500 Internal server error` responses are handled for loading users on a Map and loading threads of Messages. Accordingly `access_denied` and `internal_server_error` strings are added to the resources.
… is not confirmed yet.

`403 Forbidden` and `500 Internal server error` responses are handled for searching for users.
…icable. This way HttpErrorHelper can differentiate exception handling behaviour based on the exception type.
…ad_create_failed and messages_reload_failed)
…t they were doing so instead of simple failure message may be more interested in what actually went wrong.

Remove message_send_failed from the resources as it is not used anywhere any more.
@mariha mariha mentioned this pull request Apr 17, 2020
@mariha
Copy link
Collaborator Author

mariha commented Apr 20, 2020

Looks like currently there is an extra step in the account creation process, after user confirms his e-mail address there are 3 extra days s/he has to wait until they can send messages. From verification e-mail:

New users will be unable to send emails within Warmshowers nor post to the
Forums for 3 days following their verification of membership. We apologize
for the inconvenience, this restriction is necessary to keep spammers off the
site.

When verified user tries to send anything before this waiting period passes, server returns 403 response. It is handled same as access denied for unverified user, same message is displayed. On the website there is no Send Message button until it is possible to do so.

@saemy saemy force-pushed the develop branch 5 times, most recently from 5f99b11 to 4cf4e33 Compare January 10, 2021 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant