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

Use session cookies #64

Open
4 tasks
glciampaglia opened this issue May 10, 2021 · 16 comments
Open
4 tasks

Use session cookies #64

glciampaglia opened this issue May 10, 2021 · 16 comments
Assignees
Labels
backend Issues related to the backend of Rockwell frontend Issues related to the frontend of Rockwell high priority needs discussion
Milestone

Comments

@glciampaglia
Copy link
Collaborator

glciampaglia commented May 10, 2021

To-do (Feb 1, 2022):

  • We want to set a timer on the browser equal to the expiration of the cookie (for example using setTimeout()) and when that has expired, we would show a modal with a refresh button in it and with blurred background (meaning it blurs the app). Notice that the JS would be executed in the browser, so it needs to be added into pug react.
  • Instead of the above, we will check the cookie expiration on any action (next button, retweet, like, etc.), and if so, trigger feed reload.
  • When the refresh happens, the user should be sent back to the same page in the task (not necessarily to feed 1 of 5), but with content that has been re-fetched from the API. However we don't want to reload the page when the user is shown the attention check, since it would put the user in a situation that is impossible to complete. Instead, we would allow the user to complete the check, and then start displaying the refreshed tweets from the next feed on.
  • When refreshing, need to make a call to batch compliance endpoint

Original issue

Currently, whenever the user reloads the page, we go through the Twitter authorizer again and a new session is created in the database. This multiplies the sessions and the complexity of the database.

Instead, we will set a session cookie (using Python requests). using Pug.

Here is the workflow:

  • If the cookie is still valid, we will simply display the latest 20 tweets from the database, regardless of whether they have been deleted or not on Twitter.
  • If the cookie has expired, there are two options:
    • option A: the Twitter token is still valid. Then we will call again the home_timeline endpoint setting since_id to the smallest tweet ID we have. This ensure that Twitter will return us only the same tweets or newer ones. We will then compare those IDs with the ones in the DB, and if any is missing, we will mark those as deleted (see Alter tweet table #63).
    • option B: the Twitter token has expired, then we show the authorizer again and have the user go through it. (Then we do the same steps of option A.)

Last but not least, we need to specify that a cookie older than 10 minutes is expired. (This needs to be set in Requests Pug.)

@RobertAndion
Copy link
Collaborator

RobertAndion commented May 10, 2021

We need to make sure on the twitter credentials expiring in guest access twitter that we redirect back to the authorize (Not sure if this is happening automatically as it is an "edge" case that hasn't been tested.)
Thoughts on implementation:
-Place a cookie in the authorizer if one does not exist. If one exists (expired or not, leave it as is)
-Inside of guest access on each call check our cookie, if it is valid, load from database. If it is not, remake the cookie now and
then reload the home-timeline as stated above and check for deleted tweets and handle accordingly. (If our session has expired and our twitter key expired we need to leave the cookie alone and redirect back to the authorizer)

Possible Bug: Using since_id has one edge case that could break it. If we only have 19 tweets in the since_by (one since then was deleted and no new tweets are on the timeline) We will unable to get the full 20.

@glciampaglia
Copy link
Collaborator Author

Adding needs discussion to see how to handle possible bug mentioned above.

@RobertAndion
Copy link
Collaborator

RobertAndion commented May 16, 2021

We also need to handle the case where a user blocks cookies (Default on some browsers) We need to assume we the cookie is invalid if we do not have a cookie. This way it is safe and assumes we have to do the full workflow. Additionally do we need to ask the users permission to use cookies? (Not sure what the regulation is)

@glciampaglia
Copy link
Collaborator Author

glciampaglia commented May 17, 2021

We discussed the edge cases:

  1. When calling the home_timeline with since_id parameters, and we find that some IDs are missing, we will treat those as deleted ONLY if they are IDs of older tweets. This ensures that the tweet have been deleted, and not that the home timeline had simply updated completely (the home timeline returns the latest 800 tweets, and so for some people that follow a lot of accounts, it could change completely very often).
  2. If the browser blocks cookies, we assume it is the same as if the cookie was always expired.
  3. We will include wording in the informed consent statements that says that if you sign in, you agree to having a cookie stored in your computer. (See Display informed consent message #70.)
  4. Related to this, we need a <noscript> tag with a message saying the browser needs to run javascript. (See Display informed consent message #70.)

@glciampaglia
Copy link
Collaborator Author

We discussed the issue again. We figured out that even though the value of the cookie is determined in get_feed, this function is not the one that returns directly to the participant's browser. Instead, it returns a JSON object to Pug, which then uses this JSON to render feed. Therefore, the cookie must be set in Pug not in Python.

@saumyabhadani95
Copy link
Collaborator

saumyabhadani95 commented Jun 25, 2021

The workflow for this is almost complete, but the only problem is that I am not able to get the cookies. In fact I am not able to get the webpage http://127.0.0.1:3000/ (on which Truman feed is running) using either requests or urllib. python requests just freezes and urllib gives error Remote end closed connection without response. However I am able to get http://127.0.0.1:5000/ (on which Twitter authenticator is running).

@glciampaglia
Copy link
Collaborator Author

The reverse proxy settings was out of sync due to a change of IP of the AWS instance, so we haven't had the chance to fix the above problem. Now the reverse proxy is working again, so we should be able to fix this and close the issue.

@glciampaglia
Copy link
Collaborator Author

glciampaglia commented Jul 27, 2021

It seems that the browser is refusing to set the first cookie because it has the "SameSite" attribute set to None, but it is missing the "secure" attribute, and it seems some browsers like chrome do not allow this. Other browsers like Firefox are still allowing it, but with a warning that soon they will reject it, see here for more info: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite

We will try to add the "secure" attribute and see if it work. We also want to make sure that the node.js endpoint works with the reverse proxy when serving the feed.

@glciampaglia
Copy link
Collaborator Author

glciampaglia commented Jul 29, 2021

We are now including the "secure" attribute to the cookie, however it is still not being set by the browser because this attribute requires to be received over a secure connection (https).

One possible solution, besides switching to an https connection, could be to fix the domain name, so that the cookie is not seen as from a different domain anymore.

For more information, see here:

https://www.chromium.org/updates/same-site

and here:

https://web.dev/samesite-cookies-explained/

@glciampaglia
Copy link
Collaborator Author

Now that we have attention checks, we have decided to increase the expiration time of the cookie to 30 minutes. This is to make sure that the cookie does not expire during the task. When the cookie has expired, the user will be take back to the start of the task (page 0 and attention 0). This way we ensure that if people navigate back to a stale survey the restart from scratch instead of the resuming of they left. This will not affect the task itself, since it will be shorter than the expiration of the cookie. However it will ensure that users are not able to view tweets that have been deleted in the meanwhile, which was the rationale for having session cookies in the first place.

@glciampaglia
Copy link
Collaborator Author

After fixing the reverse proxy, the cookies now work. Right now the cookie expiration is set to 2 minutes. We need to make sure that the expiration is set to 30 minutes.

@glciampaglia glciampaglia reopened this Aug 31, 2021
@saumyabhadani95
Copy link
Collaborator

Cookie expiration set to 30 mins

@glciampaglia
Copy link
Collaborator Author

glciampaglia commented Sep 2, 2021

Right now, when the cookie has expired, the user is sent back to the first feed (feed 1 of 5) ONLY when they refresh the browser. Instead, we would like things to happen differently:

(move to the top)

@glciampaglia
Copy link
Collaborator Author

We discussed this issue again, and we decided we are going implement this directly in React.

@glciampaglia glciampaglia added frontend Issues related to the frontend of Rockwell backend Issues related to the backend of Rockwell labels Oct 5, 2021
@glciampaglia
Copy link
Collaborator Author

We also want to make sure we use the new batch compliance endpoint when we need to refresh the tweets.

@glciampaglia
Copy link
Collaborator Author

We discussed this again, we decided that at the moment this is not a critical issue, as long as we make sure that participants cannot re-access the app after the survey has completed. This way we should not run into compliance issues. We will move this task to Utopia and re-consider it later on, resources permitting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Issues related to the backend of Rockwell frontend Issues related to the frontend of Rockwell high priority needs discussion
Projects
None yet
Development

No branches or pull requests

5 participants