Create additional authorization-state-specific routing rules

VERIFIED FIXED in 2013-04-18

Status

VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: hoosteeno, Assigned: giorgos)

Tracking

other
2013-04-18
Dependency tree / graph

Details

(Whiteboard: [kb=998212] [regression])

When we add privacy states to profile fields, we need to anticipate some new interactions between users with a particular authorization state ("public users", "vouched users") and pages with particular visibility rules ("visible to public users," "visible to vouched users"). 

Specifically:

We should do the exact same thing (send a 404? send a "No Permissions" message? redirect to homepage?) when someone requests a URL for...
* A profile that they do not have permissions to see any fields on, or
* A profile that doesn't exist

We should redirect a public user to some public place, or send an error message, if s/he
* Requests a group or tag (or any other non-profile) page
* Submits a search query

giorgos has been mapping specific HTTP responses to URLs + authorization states in an etherpad. We may want to copy it over here when it's ready.
Blocks: 844162
Blocks: 844733
The below spreadsheet contains DRAFT spec for routing requests based on their authZ status and the visibility state of the content they're requesting.

https://docs.google.com/a/mozilla.com/spreadsheet/ccc?key=0Algxlsc2cLlldFFsNldKNzI5YXIwMzFNb1A2MEszYVE#gid=0

Ping me if you want to edit it.
Target Milestone: 2013-03-28 → 2013-03-14
Target Milestone: 2013-03-14 → 2013-03-21
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Reopening:
If I'm following this correctly, with an unvouched account I am receiving a HTTP 302 for a the following urls - the spec suggests the final status code should be a HTTP 301 (comment 1):

With an Unvouched user follow the below urls:
https://mozillians.allizom.org/en-US/country/us/
https://mozillians.allizom.org/en-US/country/us/region/California/
https://mozillians.allizom.org/en-US/country/us/city/
https://mozillians.allizom.org/en-US/group/mozilla-mexico
https://mozillians.allizom.org/en-US/group/258-l10n/toggle

https://mozillians.allizom.org/en-US/u/mbrandt (valid user)
https://mozillians.allizom.org/en-US/country/broccoli (invalid country)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 4

6 years ago
I would argue that the spec is wrong, since 301 is Permanent Redirect, while we want Temporary Redirect, which is 302 (or at least what the web falsely uses for TempRedirect).
That wfm -- I admit that I had to refresh my memory on what the intended difference was between a 301 and 302 but didn't think through the implications. 

hoosteeno how think you?
Yes, I think it should be a 302. Which, I think, means this is again resolved.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Pl(In reply to Justin Crawford [:hoosteeno] from comment #6)
> Yes, I think it should be a 302. Which, I think, means this is again
> resolved.

Please review the spec sheet and update it accordingly :)
Done!
I'm reopening this bug because the /u/ behavior is not conforming to the spec (comment 1).

Here's a user with no public fields:
https://mozillians.org/en-US/u/4234aef0ab/

Expected behavior (according to the spec):
* As a logged out user, if I visit a valid user account with no public fields I should get a 302.
* As a logged out user, if I visit a user URL that doesn't exist, I should get a 302.

Actual behavior: 
In both instances I get a 404.
Blocks: 755376
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

5 years ago
Whiteboard: [regression]
(Assignee)

Comment 10

5 years ago
https://github.com/mozilla/mozillians/pull/451

Pull request with a few tests to cover all scenarios.

Anonymous, unvouched, vouched, incomplete users request:
 - Public profile
 - Private profile
 - Non-existent profile
 - Own profile
 - Pending (unvouched) profile
 - Incomplete profile
(Assignee)

Updated

5 years ago
Assignee: nobody → giorgos

Comment 11

5 years ago
Commits pushed to master at https://github.com/mozilla/mozillians

https://github.com/mozilla/mozillians/commit/8ae87a41ae77cef6cef8579d4b31a38ed2f8602a
[fix bug 846039] Authorization routes for Profile View.

* Allow all users to visit public profiles.
* Allow all users visit own profile.
* Redirect anonymous and unvouched users when requesting private or non-existent profile
* Raise 404 when vouched user requests non-existent profile

https://github.com/mozilla/mozillians/commit/2d974799f9d2c51414ae18e2f45251c5c0e98aff
Merge pull request #451 from glogiotatidis/routes

[fix bug 846039] Authorization routes for Profile View.

Updated

5 years ago
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: 2013-03-21 → 2013-04-18
QA verified - automation is passing. Thanks all!
Status: RESOLVED → VERIFIED

Comment 13

5 years ago
Reopening this bug because the behavior is not conforming to the spec.

These two urls:
https://mozillians-dev.allizom.org/pl/opensearch.xml and 
https://mozillians-dev.allizom.org/nl/u/MozilliansUser/ should return 200 as in "comment 1".

Actual code retuned is 302.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(In reply to Andrei Hutusoru from comment #13)
> Reopening this bug because the behavior is not conforming to the spec.
> 
> These two urls:
> https://mozillians-dev.allizom.org/pl/opensearch.xml and 
> https://mozillians-dev.allizom.org/nl/u/MozilliansUser/ should return 200 as
> in "comment 1".
> 
> Actual code retuned is 302.

Odd, it looks like this was a small hiccup, the automated tests are passing again for dev. Anonymous user reveive a HTTP 200 for https://mozillians-dev.allizom.org/pl/opensearch.xml and https://mozillians-dev.allizom.org/nl/u/MozilliansUser/

For reference here is the report showing the failures - http://qa-selenium.mv.mozilla.com:8080/view/Mozillians/job/mozillians.dev/3152/testReport/tests.test_redirects/TestRedirects/test_200_for_anonymous_users/
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED

Comment 15

5 years ago
Thanks Matt! Verified FIXED.
Status: RESOLVED → VERIFIED

Comment 16

5 years ago
Sorry about this Matt. My mistake. The automated test is xfailed, but the fail is still there. Reopening this.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 17

5 years ago
I can't confirm this error. I get 200 - OK for both urls provided in comment 13.
Bumping to fixed per comment 17
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Bumping to QA verified on stage and prod:

The tests manually pass for me on both stage and prod. I'll investigate our automation to see if there is an incorrect assumption in the code that is causing the test to falsely report a failure.

There appears to be a separate issue occurring on dev (bug 864362) where the server is down, I'm not sure if this is what you were experiencing.

AndreiH, please provide more information - which environment were you seeing the failure and are you able to manually reproduce the errors that automation is hitting.
Status: RESOLVED → VERIFIED
I'm seeing a deviation from spec.

Steps to reproduce:

1. Login.
2. Visit a user who exists but has no public fields. Example: https://mozillians-dev.allizom.org/en-US/?next=/en-US/u/timw/

Actual behavior:

3. Get redirected to logged-out homepage and see error, "You must be logged in to continue"

Expected behavior:

3. D18 here: https://docs.google.com/a/mozilla.com/spreadsheet/ccc?key=0Algxlsc2cLlldFFsNldKNzI5YXIwMzFNb1A2MEszYVE#gid=0
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Whiteboard: [regression] → [kb=998212] [regression]
Er, this is user error, not a regression. Closing again.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Bumping back to verified http://f.cl.ly/items/0q0C1O1W3A313g3W0e1R/yeah!.gif
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.