Skip to content
This repository has been archived by the owner. It is now read-only.

Add wantsauth handling to My Shots #2414

Closed
ianb opened this issue Mar 17, 2017 · 7 comments
Closed

Add wantsauth handling to My Shots #2414

ianb opened this issue Mar 17, 2017 · 7 comments
Assignees

Comments

@ianb
Copy link
Contributor

@ianb ianb commented Mar 17, 2017

In #2413 there is handling for authenticating requests after page load. But that ticket only works for individual shot pages, so if you haven't saved from Page Shot in a session and you visit My Shots you will appear unauthenticated.

To fix this we need to wire My Shots authentication failures to wantsauth. Right now My Shots redirects, but I would propose we change it to a 404. Then the 404 page should include wantsauth and try to request authentication. If it does receive authentication then probably we could just reload the page, since the cookie is now set.

@ianb ianb added this to the Screenshots in 54 milestone Mar 20, 2017
@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Mar 21, 2017

second time now that I've assigned this to myself, mistakenly thinking it was an addon bug :-P

@jaredhirsch jaredhirsch removed their assignment Mar 21, 2017
@ianb
Copy link
Contributor Author

@ianb ianb commented Mar 21, 2017

I suppose it is site-only, though the chance of it requiring add-on refactoring are relatively high.

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Mar 21, 2017

Aight, I'll take a swing at it. I've mostly avoided the site code up till now, but the usage of wantsauth seems straightforward.

@jaredhirsch jaredhirsch self-assigned this Mar 21, 2017
@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Mar 22, 2017

@ianb OK, I think I understand the issue here with the auth flow: shotindex/server checks for req.deviceId, but we can't lazily set the deviceId until server.js has actually rendered the page. What I don't understand is why we'd want the 404 page to handle the auth check: if you are a screenshots user, and get sent to someone else's bad screenshot URL, we wouldn't want to bounce you back to your shots, we'd want to leave you at that 404 page.

Two suggestions / ideas:

Might it make sense to change the refreshModel code on the search page to include updating the wantsauth state? That way, My Shots could just show an introductory "looking for your shots" gif if a user isn't logged in, then show shots when the user gets authed.

Or, we could bounce un-authed users to an auth check page, oauth-style, where we'd just say something like "Logging you in, click here if this page doesn't refresh in N seconds...", do the wantsauth check, then either redirect from that page back to My Shots, or redirect to the landing page for non-screenshots users?

@ianb
Copy link
Contributor Author

@ianb ianb commented Mar 22, 2017

Whether it's a 404 page or an empty/loading My Shots page, I guess doesn't matter? My Shots already has support to dynamically refresh itself, so we could make use of that... or reload.

But, if I send the My Shots link to someone without Screenshots, they should get something other than a loading page, like a 404 or a bounce to the homepage.

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Mar 22, 2017

Cool, thanks. I guess I'll try something like this approach:

  • sniff the UA when My Shots is loaded
    • If it's any browser other than desktop Firefox >= 54, bounce to the homepage.
    • Otherwise, if they have a deviceId, show their shots
    • Otherwise, give wantsauth a few seconds to connect with the addon:
      • If wantsauth sets the deviceId, just reload the page (or the model, tbd)
      • if not, the user has opted out; bounce them to the homepage as well.

I'll try 5-10 sec as a wantsauth timeout on my ancient loaner windows laptop, see if that's a reasonable upper bound for older hardware.

@ianb
Copy link
Contributor Author

@ianb ianb commented Mar 22, 2017

Note that we want to support other browsers soonish, so ideally the UA bounce would happen after a possible login.

jaredhirsch added a commit that referenced this issue Mar 27, 2017
Fix #2414, lazily check auth state on My Shots page
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants