Story #2659

[Workbench] Anonymous user can access publicly shared data using Workbench and curl

Added by Tom Clegg about 6 years ago. Updated over 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
-
Target version:
Start date:
02/02/2015
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
2.0

Description

This has two parts:

  1. Implement a special "Anonymous" group and user
    • Created automatically, much like the "system group". uuid = xyzzy-j7d0g-anonymouspublic?
    • In permission checks, make sure anonymous_group_uuid is always in the list of readable groups.
    • This should produce the desired result if someone shares an object with the Anonymous group -- at least for users who are logged in.
  2. Provide a mechanism for clients to get "anonymous" privileges without using the usual OAuth/SSO procedure.
    • Add apiserver script to generate a token for the special anonymous user
    • Careful in API server not to let an anonymous user modify its own User object (or anything else normally allowed by permission system). Setting token scopes to ["GET /"] should be enough?
    • Add Workbench configuration item for "anonymous user token". (When this is nil, just use the current behavior.)
    • Workbench has to act a bit differently when deciding to show a login page, redirect to authentication procedure, or just proceed as anonymous user. E.g., bookmark a private page, log out, go back to the private page → show "not found or not authorized" page, and provide an option to log in.
    • Workbench should avoid showing "create new folder" links when operating in anonymous mode. (Probably need to fix some assumptions that "everyone can do X", and improve the "editable?" and "creatable?" methods so they return correct answers when current_user is the anonymous user.)

Expected behavior in Workbench:

Root url, no anon config Root url, anon config Top nav email/acct menu
Not logged in Redirect to /users/welcome Projects#index1 "Log in" button
Logged in, active=false, invited=false Redirect to /users/inactive Projects#index2 Logged-in user's email, with flag indicating "not activated". Click to see /users/welcome
Logged in, active=false, invited=true Redirect to /user_agreements Projects#index3 Logged-in user's email, with flag indicating "not activated". Click to see /user_agreements
Logged in, active=true Projects#index Projects#index Logged-in user's email

1 Instead of "my projects", there is a notification panel saying "welcome to arvados" (same as the usual "please login" stuff on front page)

2 Instead of "my projects", there is a notification panel saying "your account is not yet activated" (same as the usual "inactive" page)

3 Instead of "my projects", there is a notification panel saying "please accept user agreements in order to activate your account" (link/button to /user_agreements)


Subtasks

Task #5125: Review branch: 2659-anonymous-share-projectsResolvedTom Clegg


Related issues

Related to Arvados - Feature #4728: [SDKs] 'arv-get' should be able to do anonymous collection downloads using scoped token from "share" linkClosed12/05/2014

Associated revisions

Revision 6e86ca6f
Added by Radhika Chippada about 6 years ago

refs #2659
Merge branch '2659-anonymous-server-side'

Revision c58a2c83
Added by Tom Clegg over 5 years ago

Merge branch '2659-anonymous-share-projects' refs #2659

Revision 36d6d160 (diff)
Added by Tom Clegg over 5 years ago

Avoid optional disk cache usage if it is owned by a different user. refs #2659.

History

#1 Updated by Tom Clegg about 6 years ago

  • Project changed from Umbrella Project to Arvados
  • Description updated (diff)

#2 Updated by Tom Clegg about 6 years ago

  • Target version set to 2014-06-17 Curating and Crunch

#3 Updated by Tim Pierce about 6 years ago

  • Assigned To set to Tim Pierce

#4 Updated by Radhika Chippada about 6 years ago

  • Assigned To changed from Tim Pierce to Radhika Chippada

#5 Updated by Radhika Chippada about 6 years ago

  • Status changed from New to In Progress

#6 Updated by Tom Clegg about 6 years ago

  • Description updated (diff)

#7 Updated by Tom Clegg about 6 years ago

At e27463c

I think Workbench shouldn't know anything about the special UUID for the "anonymous" user. It needs a config setting; it needs to use that token instead of the usual session token when ((user is inactive) or (user is not logged in)) and (not in one of the user-oriented controllers like sessions and user_agreements); and it needs to act differently in some cases based on the knowledge that it's using its configured token instead of a regular session token. (E.g., if I give it a real user's token as anonymous_user_token, it should still refuse to do post/put operations.)

Remove non-essential config stuff from application.yml.example.

Set anonymous_user_token to false in default config, and just check "if anonymous_user_token" instead of "....size==50".

I don't really like having helper methods that are only used by tests, not by the application. Shouldn't we test this behavior by using fixtures to make a "shared with anonymous" project? (That way we can add API server test cases as well, e.g., to ensure even a badly-behaved client cannot use an "anonymous" token to modify anything.)

(to be continued)

#8 Updated by Radhika Chippada about 6 years ago

Tom,
1. Regarding the helper methods: I implemented them with the idea that we will use those method when we implement the UI button to "share" resources. When the UI is implemented, the framework would be ready, and it just needs to call these methods to create the un/share links. I wanted to make sure these are tested, and hence added the calls in the tests.

2. Can you please clarify about "Workbench shouldn't know anything about the special UUID for the "anonymous" user. It needs a config setting; it needs to use that token instead of the usual session token when . . . ".

Thanks.
--Radhika

#9 Updated by Tom Clegg about 6 years ago

(continued)

Instructions in workbench application.default.yml should be a bit more specific, like "...by running `bundle exec ./script/get_anonymous_user_token.rb` in the directory where your API server is running."

The session/logout stuff seems a bit fragile (using the session this way can cause occasional weird behavior in other tabs in the same browser). I think the thread stuff will also need to change to make the "inactive user can act as anonymous user" behavior possible. Perhaps ApplicationController can have a "switch to anonymous user if necessary" method. Controllers that don't desire this (like sessions, api_tokens, user_agreements) can override it and make it a no-op. That method can also set the appropriate "anonymous / can't act" flag in current_user, rather than having can_act look for the magic uuid string...

BTW I haven't managed to see the new behavior yet -- Workbench doesn't notice my config, or something. Haven't figured out exactly what I'm doing wrong.

#10 Updated by Tom Clegg about 6 years ago

Radhika Chippada wrote:

Tom,
1. Regarding the helper methods: I implemented them with the idea that we will use those method when we implement the UI button to "share" resources. When the UI is implemented, the framework would be ready, and it just needs to call these methods to create the un/share links. I wanted to make sure these are tested, and hence added the calls in the tests.

OK. I think we'll use a more generic system with no special code path for the anonymous case, but this does at least start to show how share links should work. So, no harm leaving it in. Test fixtures still desirable too, for testing on the API server side.

2. Can you please clarify about "Workbench shouldn't know anything about the special UUID for the "anonymous" user. It needs a config setting; it needs to use that token instead of the usual session token when . . . ".

Workbench shouldn't rely on api server to always use "*-anonymouspublic" as the anonymous user's uuid. Rather, Workbench should behave differently according to whether Workbench itself is actually using its configured "anonymous token" right now.

#11 Updated by Radhika Chippada about 6 years ago

Tom,
Please take another look at this branch. It is not 100% done, but the following are in and I want to make sure we both are on the same page with the flow:

- anonymous user is configured as inactive (earlier I had it as active user)

- all references are now checking if user is active instead of can_act? (removed the can_act? method)

- anonymous user check is performed by comparing token to that configured (no knowledge of uuid in workbench)

- instead of using session, I introduced a new Thread.current parameter to keep track of the token and let login take place

- for the time being, I suppressed inactive user / agreement check (this is yet to be implemented)

What is still pending:
- inactive user agreement check. Currently, inactive user page, agreement check is suppressed. This needs to be addressed next

- test fixtures are yet to be added. couple tests are still failing and I will look into them tomorrow

Thanks.

#12 Updated by Tom Clegg about 6 years ago

  • Description updated (diff)

#13 Updated by Tom Clegg about 6 years ago

  • Target version changed from 2014-06-17 Curating and Crunch to 2014-07-16 Sprint

#14 Updated by Tom Clegg about 6 years ago

Notes

  • session['arv-referrer'] still seems to be there
  • one check for magic uuid still seems to be there

check_anonymous_token is confusing me a bit. It seems like use_anonymous_token_if_necessary could be simpler, like

  • if anonymous config enabled, and current_user is nil/inactive
    • set Thread.current[:arvados_anonymous_api_token] to configured anonymous token
    • yield
    • unset Thread.current[:arvados_anonymous_api_token]
  • else, yield

This around_filter can be skipped by UserAgreementsController.

Then, ArvadosApiClient can automatically use the anonymous token if it is set in Thread.current and method=='GET'.

(I think everything will be less confusing if we can arrange for current_user to always be the currently logged-in user, regardless of what's happening with the anonymous token.)

#15 Updated by Radhika Chippada about 6 years ago

1. session['arv-referrer'] -- With anonymous configuration, when a user visits the home page, they will see shared content instead of a login page. If a user were to login by clicking the "Log in" link and then click the Logout button, without this they will again see the shared content instead of Login page. I think this would be confusing to users. But, if this is the behavior we want, I can remove it. Please confirm.

2. Thanks for catching the spurious magic anonymous uuid usage. Fixed it. Along with it, I was able to cleanup a bit more around determining if is_anonymous.

3. The around_filter use_anonymous_token_if_necessary alone does not work. The application_controller -> thread_with_mandatory_api_token method invokes thread_with_api_token method, which needs to check the validity of the token and set it. Simply setting it if anonymous config enabled, and current_user is nil/inactive does not work, because we need to address the case where an invalid anonymous token might be configured. So, I just made a method than doing inline.

4 The around_filter is skipped by UserAgreementsController, though it would result in invoking the check_anonymous_token method in earlier filters.

5. current_user to always be the currently logged-in user: This is what the case is. I am no longer juggling with session tokens and instead using Thread.current[:arvados_anonymous_api_token]

#16 Updated by Tom Clegg about 6 years ago

Sketch of possible way to arrange/name existing/new filters so they make a bit more sense:

class ApplicationController
  before_filter :thread_clear
  before_filter :permit_anonymous_browsing
  around_filter :thread_with_optional_api_token
  before_filter :require_api_token
  before_filter :require_active_user

  def permit_anonymous_browsing
    @permit_anonymous_browsing = true
  end

  def require_api_token
    if !@permit_anonymous_browsing and !Thread.current[:arvados_api_token]
      redirect_to_login
    end
  end
end

class UserAgreementsController
  skip_before_filter :permit_anonymous_browsing
  skip_before_filter :require_active_user
end

#17 Updated by Tom Clegg about 6 years ago

I think this (in app/models/arvados_base.rb) should just be current_user && current_user.is_active:

-    current_user
+    current_user && !current_user.is_anonymous

This (in app/models/arvados_api_client.rb) is the API server's URL that makes logout happen, not a Workbench URL. (I don't think we want to change it.)

-    arvados_login_url(params).sub('/login','/logout')
+    "/user_sessions/logged_out" 

In the views/layouts, change current_user.is_anonymous to !current_user.is_active.

Rename anonymous_login_enabled to anonymous_browsing_enabled (I think "anonymous login" is confusing when compared to how it's really implemented) and it cannot depend on Thread.current[:arvados_anonymous_api_token] because that depends on which action is running. For example, the layout shouldn't lose the "projects" dropdown just because you happen to be looking at the user_agreements page where anonymous token usage is disabled.

I don't quite see why the long permit_anonymous_browsing_if_no_thread_token and permit_anonymous_browsing_for_inactive_user methods are better than the one-line permit_anonymous_browsing above...

I think replacing the stuff like "render 'user_agreements/index'" with "redirect_to user_agreements_path" could make the anonymous behavior easier to accomplish. Worth a try?

The User.is_anonymous method should probably go away: It's hard or impossible to make it correct, and it shouldn't be necessary. (The current implementation actually tells you "would an API call use the anonymous token right now?" not "is this particular user model the anonymous user?".)

Edited from above:

ArvadosApiClient should use the anonymous token if it is set in Thread.current and method=='GET' and !current_user.is_active.

If we do that, we can avoid overloading current_user to mean "current user, or sometimes anonymous user, depending on the situation" -- and avoid overloading Thread.current[:arvados_api_token] to mean "user's token, or perhaps the anonymous token"...?

#18 Updated by Radhika Chippada about 6 years ago

Tom,
Thanks for catching the issue with the logout url. I addressed most of these comments. Please feel free to take another look. Have a few questions / unsure of some of the other comments.

1. "don't quite see why the long permit_anonymous_browsing_if_no_thread_token and permit_anonymous_browsing_for_inactive_user methods are better than the one-line permit_anonymous_browsing above" : Your permit method does not do any validation against the configured anonymous token. Do we not want to make sure the configured anonymous token is valid? I now made the permit_anonymous_browsing_if_no_thread_token method a single liner by letting the next filter (set_thread_api_token) do the validation.

2. "In the views/layouts, change current_user.is_anonymous to !current_user.is_active" : The view is customizing the login button display when it is anonymous user. For example, if an anonymous user is the context, the top nav does not show the email and just shows the login button. Whereas, if it is an active or in_active user, instead of Login button, we see the user's email and the dropdown with Logout button (and others as needed) is displayed.

3. "ArvadosApiClient should use the anonymous token if it is set in Thread.current and method=='GET' and !current_user.is_active" : Please clarify this. I added this logic to use arvados_anonymous_api_token instead of arvados_api_token in these specific cases.

4. "I think replacing the stuff like "render 'user_agreements/index'" with "redirect_to user_agreements_path" could make the anonymous behavior easier to accomplish" : I prefer to wrap this story up and merge into master asap given the amount of time it has already taken. I can revisit this suggestion as part "Workbench UI improvements" story this sprint.

Thanks.
--Radhika

#19 Updated by Tom Clegg about 6 years ago

At 2659-anonymous-server-side 4d351f0

Is the share_the_aproject_with_all_groups fixture needed?
  • It seems like making "aproject" anonymously accessible could detract from the effectiveness of other test cases that test permissions of "aproject". Better to just use the new "anonymously_accessible_project" for testing anonymous behavior?
  • If needed, share_aproject_with_anonymous_group might be a clearer name.

In get_anonymous_user_token.rb, the get_existing block should verify that scopes is restricted as we expect. Checking expiry time (while not exactly a security fix) might be nice too. Perhaps like

if get_existing
  api_client_auth = ApiClientAuthorization.
    where('user_id=?', anonymous_user.id.to_i).
    where('expires_at > ?', Time.now).
    select { |auth| auth.scopes == ['GET /'] }.
    first
end
In current_api_client.rb
  • Could fix indentation at group_perm = Link.create(...)
  • I think it would be better to look up (and create if necessary) the "anon user can_read anon group" permission link in the if not $anonymous_user block, even if we don't create a new User record (i.e., after the if !$anonymous_user block). This mirrors the behavior for the User and Group records, and lets us recover automatically (instead of staying in a mysterious troubleshooting situation) if somehow the database ends up with an anonymous_user but no permission link.
  • Use create!, instead of create which can fail silently.

#20 Updated by Tom Clegg about 6 years ago

2659-anonymous-server-side @ 60ea080 - looks good to merge, thanks!

#21 Updated by Tom Clegg almost 6 years ago

  • Target version deleted (2014-07-16 Sprint)

#22 Updated by Tom Clegg over 5 years ago

  • Target version set to Arvados Future Sprints

#23 Updated by Tom Clegg over 5 years ago

  • Target version changed from Arvados Future Sprints to 2015-02-18 sprint

#24 Updated by Ward Vandewege over 5 years ago

  • Subject changed from Anonymous user can access publicly shared data using Workbench and curl to [Workbench] Anonymous user can access publicly shared data using Workbench and curl

#25 Updated by Jonathan Sheffi over 5 years ago

There is now an active customer request around this. Customer would like to create a project to share with his team. Customer would like to be able to create a project with a pipeline and some data and some analysis. Then wants to be able to create a shareable link that can be sent around. Anyone with the link can log in and view the project (including provenance, etc) for "free." Of course, running the analysis requires logging in, etc.

See me for details. I can set up a meeting between engineering and customer to refine the story if necessary.

#26 Updated by Tom Clegg over 5 years ago

To make projects#show work for anonymous users:
  • ApplicationController#check_user_agreements should return true if current_user.nil?
  • ApplicationController#check_user_profile should return true if current_user.nil?
  • ProjectsController should skip_before_filter :require_thread_api_token, only: :show
  • ArvadosApiClient should use the configured anonymous token if Thread.current[:arvados_api_token].nil?
  • Presumably, various parts of the view will need to be wrapped in if current_user
CollectionsController#show should work for a publicly shared collection (e.g., in a publicly shared project).
  • "Create/remove sharing link" shouldn't be there (but the existing sharing link, if any, could be shown).
  • Downloading and viewing files should work.

Hiding the jobs/pipelines/other tabs on projects#show might be an acceptable short term measure (as an alternative to making the pipeline/job #show pages work well in the anonymous case).

#27 Updated by Radhika Chippada over 5 years ago

Branch '2659-anonymous-share-projects' implemented workbench support for anonymous users. Some details:

  • Updated arvados_api_client to use the anonymous token when no current user is found. This would now make it possible to see the objects shared with anonymous users.
  • For a shared project, it would be desirable that pipeline inputs selections are from within this project’s collection set. Similarly, is is desirable that we “run” pipelines in this project as opposed to move them from elsewhere so that the pipeline outputs are also in the shared projects.
  • It appears that the job log is saved in the user’s Home project rather than the project in which they are run. Hence, suppressed the Log tab in job#show page for anonymous users.
  • I updated link_to_if_arvados_object to display raw text when it is a User object for inactive users. Are there other object types that we want to do this for anonymous user case? I only found Collection, Job, PipelineInstance, PipelineTemplate and Group types in the three allowed project tabs, which the anonymous user is allowed to access.
  • Should we update the 404 page to display (1) 404 when the object actually does not exist, (2) permission denied when object cannot be displayed because of access control? I found this to be very confusing in the past and an anonymous user might run into it more often than other regular / non-admin users

#28 Updated by Ward Vandewege over 5 years ago

  • Status changed from In Progress to Resolved

Also available in: Atom PDF