Project

General

Profile

Actions

Bug #12136

closed

"You must login" group file warning shows after log in

Added by Matt Gold about 5 years ago. Updated almost 4 years ago.

Status:
Resolved
Priority name:
Normal
Assignee:
Category name:
Group Files
Target version:
Start date:
2019-11-22
Due date:
% Done:

0%

Estimated time:
Deployment actions:

Description

-- I clicked a link to a file in a private group
-- reached the group page and saw a warning in red that I needed to log in
-- logged in
-- the logged-in view still showed the warning. Screenshot attached


Files

Screen Shot 2019-11-22 at 7.42.00 PM.png (253 KB) Screen Shot 2019-11-22 at 7.42.00 PM.png Matt Gold, 2019-11-22 07:43 PM
Screenshot_2019-11-25_13-36-29.png (64.8 KB) Screenshot_2019-11-25_13-36-29.png wp-login.php message Boone Gorges, 2019-11-25 02:42 PM
Screenshot_2019-11-25_13-41-42.png (54.1 KB) Screenshot_2019-11-25_13-41-42.png success message Boone Gorges, 2019-11-25 02:42 PM
library-wipeout.gif (15.8 KB) library-wipeout.gif Raymond Hoh, 2020-09-25 06:50 PM
Peek 2020-09-29 14-53.gif (156 KB) Peek 2020-09-29 14-53.gif Boone Gorges, 2020-09-29 03:55 PM
Actions #1

Updated by Boone Gorges almost 5 years ago

The only way I could reproduce was by interpreting your third step - "logged in" - as "clicked the "Log In toolbar button at the upper-right". After logging in via this interface, you're redirected back to the URL where you initiated the login, which in this case had the URL params ?status=forbidden&doc_url=.... So the error message appears again.

My first question is: Why did the Commons think you were logged out? Were you clicking a link in a browser tab after the browser'd been idle for a long time, so that you lost your login session? Or perhaps you had multiple tabs; had viewed https://commons.gc.cuny.edu/groups/cac-community-team-project-planning/ while logged in in one tab; logged out in a different tab; then tried clicking a link in the first tab? This seems like a scenario that's hard to reproduce, since you shouldn't see the link if you're not logged in.

Second question: What behavior do we want? Currently, if you try to access a private file URL, you're redirected to the group's home page with the error message in question. This behavior dates back to #646 and #563. Users who are dropped onto the group home page in this fashion must then perform some additional action to get the document: they have to click the 'Log In' link in the red message, or the 'Log In' toolbar button like Matt did, and then they have to navigate to the document. This feels clunky to me. At the same time, simply serving up the document immediately after login also feels weird, because you'd still be looking at the logged-out view while your browser prompts you to download the file. Here's a proposal for a redesigned flow:

a. As a non-logged-in user, visit the file URL
b. Redirect to wp-login.php, with a generic 'You must log in to access the page you requested' message. (a and b together better match what happen when you try to access other kinds of private group content, like Docs and Forums). See screenshot 'wp-login.php message'.
c. On successful login, redirect to the group's Files page. On that files page, show a success message with a download link. See 'success message' mockup. This way, the user can either click the link to download, or if they don't notice the link, they can browse through the file list to find it.
d. Optionally, after the Files list is loaded in c, we can trigger the file download via JS. IMO this could potentially be jarring, but happy to hear what others think.

This is just a proposal, based on making the flow clearer and more consistent with other parts of the Commons. If others have better ideas, I'm all ears.

Actions #2

Updated by Boone Gorges almost 5 years ago

  • Target version set to 1.17.0
Actions #3

Updated by Sonja Leix almost 5 years ago

Thanks Boone for proposing a solution for this odd bug.
I like your approach of redirecting the user to the group's files page to then initiate the download.

From the description in the ticket it seemed that the "you're logged out" message might come up when trying to access a private group even when logged in. Maybe i'm missing something in your response, but shouldn't the user flow be as followed for logged in users:
1. Logged in User ("User" for short in below steps) tries to access a page on a private group she is not a member of
2. User sees a message that the group is private with an appropriate option to "request access / join" the group
3. Once the User has officially joined the private group as a member she can then access the file and download it

Actions #4

Updated by Boone Gorges almost 5 years ago

Thanks, Sonja. I'll review behavior for logged-in users who are not group members.

Actions #5

Updated by Boone Gorges over 4 years ago

  • Target version changed from 1.17.0 to 1.18.0
Actions #6

Updated by Boone Gorges about 4 years ago

  • Assignee changed from Boone Gorges to Raymond Hoh

Ray, could you please take this one on? It looks like we might need a slightly better understanding of the way users are redirected when they're logged in and when they try to access files from private groups of which they aren't a member. Then we'll need to change the way the redirection happens in each relevant case. (Note that groups no longer have a Files tab; we'd be redirecting to Library.)

Actions #7

Updated by Raymond Hoh about 4 years ago

(Note that groups no longer have a Files tab; we'd be redirecting to Library.)

Boone, I tried to add the success message by adding a template notice as in https://redmine.gc.cuny.edu/attachments/13070/Screenshot_2019-11-25_13-41-42.png. However, on the Library tab, the Vue app wipes out any HTML markup after the library is loaded.

Here's a quick GIF: https://redmine.gc.cuny.edu/attachments/15930/library-wipeout.gif.

You will see the template notice very briefly before the Library wipes it out. Since you wrote the Library Vue app, do you have any pointers? Or should I inject the template notice via JS instead?

Actions #8

Updated by Boone Gorges about 4 years ago

Oh, I didn't think about this :) Ray, could you please push your changes up to branch on Github and then reassign this ticket to me? I have a bit of infrastructure in place to handle (my own version of) template notices, but I can't tell you off the top of my head how it'll work, so maybe I can have a first swing at it myself.

Actions #9

Updated by Raymond Hoh about 4 years ago

  • Assignee changed from Raymond Hoh to Boone Gorges

In commit 844e617, I've added some code to address how a private group document is downloaded and accessed.

This is what the commit does:

  • If not logged in and a user is accessing a private group doc, the user is redirected to the WP login page. After login, the user will get redirected to the group's Library page with a message to download.
  • If logged in and a user is accessing a private group doc without group membership, the user is redirected to the group homepage with a notice
  • If logged in and a user is accessing a hidden group doc without group membership, the user is redirected to the group directory with a notice.

cac-group-library requires the 'template_notices' hook so the notice can be shown. I've added a PR here, but feel free to use another approach.

Actions #10

Updated by Boone Gorges about 4 years ago

  • Status changed from Assigned to Staged for Production Release

What you have offered in this PR is about 10x simpler than what I was going to suggest :-D I've merged it and pinned to CAC in https://github.com/cuny-academic-commons/cac/commit/be0bff4a8f76b0d6424440781a8b1e487414a9f1.

I've tested the redirect fixes and they look good, with the exception of a small syntax error in the template notice markup, which I fixed here https://github.com/cuny-academic-commons/cac/commit/ca81bb9798e898874fb03648ac038f1e52edb769

Let's call this one good. Thank you for your quick work on it!

Actions #11

Updated by Raymond Hoh about 4 years ago

  • Status changed from Staged for Production Release to Assigned

The Vue app still needs to be updated so the message from 'template_notices' hook isn't wiped out as in https://redmine.gc.cuny.edu/attachments/15930/library-wipeout.gif.

Need your Vue expertise here!

Actions #12

Updated by Raymond Hoh about 4 years ago

Boone, the scenario you need to test is:

1. Be logged out.
2. From the private group's email notification, go to the file link for a private group. (Should look like https://commons.gc.cuny.edu/?get_group_doc=FILE_ID/FILENAME.)
3. You will be redirected to the login page.
4. After successful login, you should be redirected to the private group library's page and you will see a template notice that is added
5. The template notice is shown briefly as in https://redmine.gc.cuny.edu/attachments/15930/library-wipeout.gif, but is wiped out due to the Vue app. Some type of whitelisting of the #message container in the Vue app or changing the container ID where the Vue app refreshes its content would be needed I'm guessing. Or using your existing JS template notices that you've set up would work somehow.

The easier alternative is we redirect to the private group's homepage instead of the private group's library page after login.

Actions #13

Updated by Boone Gorges about 4 years ago

Thanks for the details, Ray!

Here's my solution: https://github.com/cuny-academic-commons/cac-group-library/commit/885a176c4511a0caf51fbd69eccfd326d4345e89 In short, when the Vue app is mounted, we check to see whether BP has left any template notices in the DOM (id="message"). If so, we get the content of the message, and add it to cac-group-library's own successMessage UI. There's a brief flash of differently-styled content, but I think it's minor. See gif.

Note that I had to make a change to the way that successMessage is populated so that HTML is not escaped. In other words, we are now trusting that the content of the 'message' div is already sanitized.

Actions #14

Updated by Boone Gorges almost 4 years ago

  • Status changed from Staged for Production Release to Resolved
Actions

Also available in: Atom PDF