Idea #2659
closed[Workbench] Anonymous user can access publicly shared data using Workbench and curl
Added by Tom Clegg over 10 years ago. Updated over 9 years ago.
Description
This has two parts:
- 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.
- Created automatically, much like the "system group". uuid =
- 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)
Related issues
Updated by Tom Clegg over 10 years ago
- Project changed from 37 to Arvados
- Description updated (diff)
Updated by Tom Clegg about 10 years ago
- Target version set to 2014-06-17 Curating and Crunch
Updated by Radhika Chippada about 10 years ago
- Assigned To changed from Tim Pierce to Radhika Chippada
Updated by Radhika Chippada about 10 years ago
- Status changed from New to In Progress
Updated by Tom Clegg about 10 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)
Updated by Radhika Chippada about 10 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
Updated by Tom Clegg about 10 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.
Updated by Tom Clegg about 10 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.
Updated by Radhika Chippada about 10 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.
Updated by Tom Clegg about 10 years ago
- Target version changed from 2014-06-17 Curating and Crunch to 2014-07-16 Sprint
Updated by Tom Clegg about 10 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.)
Updated by Radhika Chippada about 10 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]
Updated by Tom Clegg about 10 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
Updated by Tom Clegg about 10 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"...?
Updated by Radhika Chippada about 10 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
Updated by Tom Clegg about 10 years ago
At 2659-anonymous-server-side 4d351f0
Is theshare_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 theif !$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 ofcreate
which can fail silently.
Updated by Tom Clegg about 10 years ago
2659-anonymous-server-side @ 60ea080 - looks good to merge, thanks!
Updated by Tom Clegg about 10 years ago
- Target version deleted (
2014-07-16 Sprint)
Updated by Tom Clegg almost 10 years ago
- Target version set to Arvados Future Sprints
Updated by Tom Clegg over 9 years ago
- Target version changed from Arvados Future Sprints to 2015-02-18 sprint
Updated by Ward Vandewege over 9 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
Updated by Jonathan Sheffi over 9 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.
Updated by Tom Clegg over 9 years ago
- 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
- "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).
Updated by Radhika Chippada over 9 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
Updated by Ward Vandewege over 9 years ago
- Status changed from In Progress to Resolved