Page MenuHomePhabricator

If not logged in, users clicking a link they're not allowed to view should be asked to log in
Closed, ResolvedPublic

Description

The primary example of this happening is when users click an application link in a "there's a comment for you" email. If they weren't logged in before clicking, they just get Access Denied with no opportunity to log in and continue. See https://wikipedialibrary.wmflabs.org/applications/evaluate/120/ for users discussing this happening.

Event Timeline

Tasking as high priority as this affects current account distribution.

In the short term, would it be possible to reword the Access Denied page to add "You may be able to view this page if you log in: <link>"?

Samwalton9-WMF lowered the priority of this task from High to Medium.Jun 13 2017, 10:22 PM

@jsn.sherman I think this should be a rollover blocker - do you have a sense of feasibility + timescale?

We'll basically get this one for free if we do:
T167194
that one is a 1 week task approximately, and will kill a few other small issues along the way.

This should be totally resolved on staging. Please verify.

Tried to verify but I'm getting Internal Server Errors when logging in/out (it sometimes lets me log in).

I'm betting that I don't always have all the state information that I'm wanting in all situations. This is a good time for me to go ahead and implement more detailed logging.

I was able to reproduce this on staging, so I added more detailed logging to the auth process. I deployed that change, and now I'm unable to reproduce the error at the moment. Since it's a transient issue, it may come up again after it sits for a bit. I'll have better logs to look at if it does.

I haven't been able to reproduce any login/out errors this morning.

'Permission denied' screen looks great, but could we make a further distinction between users who are logged in and don't have permission vs those who are logged out? In the latter case the most likely scenario is someone has clicked a link they should be able to view, if only they logged in. In that case (permission denied + logged out), could an additional note be added saying "you may be able to view this page if you log in" with "log in" linked?

sort of. we can't make any determination about what permissions a user has if they aren't logged in. We can do something different based on logged out vs inadequate permissions, which is what T167194 is.

I'm trying to eliminate access denied for logged out users (eg authentication-driven errors) by simply redirecting them to login anywhere it comes up. Any remaining errors should be for logged in users with inadequate permissions (eg. authorization-driven errors) which I'll try to handle gracefully as part of T167194.

The big question is: are you still seeing access denied anywhere due to not being logged in on staging? If so, let me know the URLs, because I just need to roll them up into the login redirection.

Right, that makes sense. I think I might partly have been confused by accidentally checking a link on the live site.

I clicked https://twlight-staging.wmflabs.org/applications/evaluate/120/ while logged out, and got logged in and returned to that page, which is very neat!

I think this one is truly fixed on staging, and ready to go to prod.