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

Describe Remember Me goals? #52

Open
heymarkreeves opened this issue Nov 20, 2014 · 20 comments
Open

Describe Remember Me goals? #52

heymarkreeves opened this issue Nov 20, 2014 · 20 comments
Assignees

Comments

@heymarkreeves
Copy link
Member

Hi, @desigonz & @mailbackwards!

Can you describe your goals for Remember Me functionality at a high level?

Some questions that occur to us:

  • Does this automatically log the user in indefinitely? A perpetual session?
  • If so, how long (days) should that session last?
  • Or does it prefill a sign-in screen and bypass the Sign Up modal?
  • I'm assuming that a perpetual session would store the auth token in persistent cookie as well. Any concerns there?
  • Would you want to persist all values until a Log Out took place?
  • Would this be a setting in the settings screen as well?
  • If we enable Remember Me and a user shares their device with another user, do we need some sort of visual cue that the second user is accessing a logged-in session belonging to another user? (I.e., "Logged in as [email protected]")

Thanks!

Mark

@heymarkreeves
Copy link
Member Author

Noting that this is under the following bullet:

Caching username and password (on both add-to-homescreen and through Safari)

@desigonz
Copy link

Hi Mark,

Here's what we're thinking. We'd love your thoughts on these points, as this is new territory for us!

  • We'd like a user to be logged indefinitely, until they log out
  • Under Settings, could we have a field that says "Keep me logged in" that is checked by default (so users can opt out of Remember Me)?
  • No concerns about storing the auth token in a persistent cookie (but again, let us know if you have thoughts on this)
  • Can you clarify what you mean by "persist all values until a Log Out"?
  • Having a visual cue to indicate who is logged in is a great idea. We'd like to get your advice on where we might be able to do this. Liam and I were thinking that on the History, Favorites, and Interests pages, we could do something like "Oh hi, [email protected]!" and then the rest of the text. But is there a way to include a visual cue on other pages? Possibly in the menu or on the Discovery page?

Thanks! Let us know if you have any questions.

@heymarkreeves
Copy link
Member Author

Can you clarify what you mean by "persist all values until a Log Out"?

Based on what you've described, I think we'll just assume this. It was a catch-all statement.

But is there a way to include a visual cue on other pages? Possibly in the menu or on the Discovery page?

I was thinking in the menu. It doesn't need to compete with content on any of the screens, just be very accessible.

Thanks!

@SherriAlexander
Copy link
Collaborator

Hey there, @mailbackwards!

It sounds like the Remember Me checkbox is something that we'd have to add into the Settings API and add to the Settings screen. Is that something that you could put in place on the back end for the API, and then let me know what values it expects so I can implement the front-end piece? Thanks! :)

@mailbackwards
Copy link
Collaborator

Sure, I'll get working on this and let you know when it's pushed through. Thanks.

@mailbackwards
Copy link
Collaborator

I've just pushed up some code that should allow for a Remember Me checkbox in settings. Any PATCH or PUT to /preferences with a remember_me boolean argument will update the user's remember_created_at field, which is serialized in the user object. If remember_me is true, remember_created_at will be set to now (or will persist). If remember_me is false, remember_created_at will be set to nil. So to check whether a user wants to be remembered, you should be able to just check whether or not remember_created_at is nil.

"Remember Me" defaults to true, so if no remember_me is passed into registration or preferences, remember_created_at will be auto-set if it is not already.

This should be documented in API_ENDPOINTS.md. Let me know if you have any questions about it. Thanks!

@SherriAlexander
Copy link
Collaborator

Hey there, Liam!

I've been making progress on the Remember Me functionality today as well, working on the localStorage aspect. I had assumed that most of the Remember Me functionality would be done via a localStorage variable that was keyed to the user's email (so for example, remember_me = {"[email protected]" : true, "[email protected]": true, "[email protected]": false}), so that we didn't have to keep pinging the Preferences API and nesting AJAX calls.

LocalStorage variables are assumed to be permanent unless the user deletes them manually -- we don't have any mechanism in place to calculate expiration of the remember_me localStorage variable, and hopefully that's okay.

To document, my in-progress logic looks like this:

On page load, the site checks for an existing localStorage variable called remember_me:

  • If the remember_me variable doesn't exist yet and there is a current logged in user, it will create the remember_me variable with the username as the key and "true" as the default Boolean value.
  • If the remember_me variable does exist and there is a current logged in user, it checks to see if that user is already in it (to account for multi-user situations). If the user is not already in the variable, it adds a new entry with the username as the key and "true" as the default Boolean value.
  • If the remember_me variable already exists and the current logged in user is in there, that value is left alone.

This remember_me localStorage variable is used mainly to determine whether the auth token and current username are stored in localStorage (persisting) or sessionStorage (session) variables.

Logging in, the system will check remember_me to know which to use, and create the appropriate variables for auth token and username.

Logging out, the system will remove both localStorage and sessionStorage versions of the auth token and username variables (which should provide a clean slate for the next login).

Most of the functions that need an auth token and username will check sessionStorage variables first and then localStorage if there aren't sessionStorage variables defined.

The only time we'd need to check the API, I believe, would be if they went into the Settings screen to change it. Then I'd ping the Settings API to make sure the checkbox is reflecting the correct value (just in case somehow the localStorage got deleted or something, and I'd have the checkbox send any changes to the Settings API and change the remember_me localStorage variable upon success (and switch the existing auth token and username variables from one type to the other).

Let me know if this all sounds good to the team -- thanks! :)

@mailbackwards
Copy link
Collaborator

Hi Sherri-- Yes, I think this works for us! As long as you send a remember_me boolean to the /preferences API whenever the user updates their settings, it should update accordingly on our end. If the user wants to be remembered, the user's remember_created_at field will have a date value. If they don't, their remember_created_at will be nil. You can ping the API to get this information via GET /preferences.

It's no problem on our end that there's no way to calculate expiration.
Let me know if that all works, or if there's something I'm missing. Thanks!

@SherriAlexander
Copy link
Collaborator

Sounds good -- I'll start on the Settings page functionality today then. Thanks!

@SherriAlexander
Copy link
Collaborator

Hey there, Liam!

Hmm...you had mentioned that "Remember Me" defaults to true, so if no remember_me is passed into registration or preferences, remember_created_at should be auto-set if it is not already.

But when I ping my [email protected] user data to find out what it is, it's returning null for the remember_created_at value, so it's actually defaulting to false. Possibly because I'm an already existing user?

Is there something we can do to assign it to pre-existing users? Thoughts? Thanks!

@mailbackwards
Copy link
Collaborator

Oh yes, I hadn't thought of that. I just went in and set a remember_created_at for pre-existing users, so you should be set now. Let me know if it doesn't work. Thanks!

@SherriAlexander
Copy link
Collaborator

Perfect! I was actually testing at the time, so I saw it come through. :) Thanks!

SherriAlexander added a commit that referenced this issue Jan 9, 2015
…dating, swapping from localStorage to sessionStorage is in place (Github issue #52)
SherriAlexander added a commit that referenced this issue Jan 9, 2015
@heymarkreeves
Copy link
Member Author

This one's ready for the team to test on http://staging.artx.clearbold.com/

You'll want to clear out your cache first.

Mark

@desigonz
Copy link

desigonz commented Feb 3, 2015

Hi Mark,

Thanks for working on this! I tested the Remember Me functionality on an Android (4.2.2) and it worked great. Liam also reports it working well on iOS 8, both Chrome and Safari.

A few things were wonky on my iOS7, which may or may not be related to the Remember Me changes:

  • In Chrome, whether or not I'm signed in, if I leave Chrome and come back to it, the Discover page doesn't load and has a # in the URL (ex. staging.artx.clearbold.com/#/index.html)
  • In Chrome, I'm logged in and try to un-star an event; the star turns green, but the icon never changes to fully un-starred (although when I leave the Favorites page and return, that event no longer appears)
  • I'm having general difficulties in Safari—the Discover page loads, but none of the hyperlinks work except for the menu and the sign up overlay

@heymarkreeves
Copy link
Member Author

Hi, @desigonz!

Did you clear all private data/cache/history in both Safari and Chrome on iOS 7 before testing?

I had some issues in Safari when I first tested, and when I switched to Chrome and reset private data, things were working correctly. There's updated JS in this release, so that may be a factor.

Mark

@desigonz
Copy link

desigonz commented Feb 3, 2015

Hi Mark—yes, I did all of the cache and history in both Safari and Chrome before testing, but I can try it again.

@heymarkreeves
Copy link
Member Author

@desigonz Can you confirm the general difficulties in Safari? I'm not able to reproduce that in iOS 7.

We have reproduced the Chrome items and are investigating.

Thanks!

Mark

@desigonz
Copy link

desigonz commented Feb 3, 2015

Hi Mark—yes, the Safari difficulties are still happening for me after deleting history/cache/cookies several times. I'll see if I can get my hands on another iOS7 and reproduce it.

SherriAlexander added a commit that referenced this issue Feb 4, 2015
…querystring function more specific in case of multiple pages, adding .blur() to deselect star after it's tapped/clicked (#52)
@SherriAlexander
Copy link
Collaborator

Hey there! A quick update on progress:

  • The problem where Chrome iOS wouldn't load up the Discover page was indeed due to it pointing you at the /#/index.html URL. I have fixed all links to the homepage within the app so that they should go to / or /#/, both of which seem to load up fine in Chrome iOS. You'll need to make sure to remove that part of the URL manually next time you go into Chrome iOS for testing, clear all Chrome iOS data, and reload.
  • There are two separate issues going on with "unfavoriting" an event. One is the highlighting of the star, which I believe I've fixed. The other is an actual error being thrown by the Ajax request (406 Not Acceptable) that's only happening in Chrome iOS, though the request is still completed. Liam and I are collaborating on that in a separate ticket on the API Github project.
  • Neither Mark nor I were able to reproduce the Safari difficulties you described -- please keep us in the loop and let us know if it's happening on other devices, and what steps are needed to reproduce the issue, etc.

I should also note that none of these issues were caused by the recent Remember Me and Forgot Password coding, as far as I can tell. Thanks as always for your keen eyes!

@SherriAlexander
Copy link
Collaborator

Hey there, all! Another quick update -- I believe that Liam and I have fixed the 406 errors that were happening in Chrome for iOS now, so hopefully that will take care of the Favorite star issues (and the Interests list not loading, which we noticed while testing) in Chrome iOS! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants