New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 753699 link

Starred by 1 user
Status: Fixed
Owner:
st...@chromium.org
Closed: Aug 2017
Cc:
█ robert...@chromium.org
chanli@chromium.org
lijeffrey@chromium.org
liaoyuke@chromium.org
st...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: ----

Blocking:
issue 757517



Sign in to add a comment

ACL verification should be in api endpoints

Project Member Reported by st...@chromium.org, Aug 9 2017 
What is the bug or feature:
In trigger_flake_swarming_task_service_pipeline.py, we have logic to verify whether the user can trigger a new analysis. However, auth_util.IsCurrentUserAdmin() was executed within the context of a internal task and the result is based on the findit-for-me@appspot account. That's unexpected.


Context or examples:


Expected:
ACL verification should happen up front at the API endpoint where the end-user requests are received, e.g. in findit_api.py

Comment 1 by st...@chromium.org, Aug 28 2017

Owner: st...@chromium.org

Comment 2 by st...@chromium.org, Aug 28 2017

Blocking: 757517
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 28 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/aa6b81c14c0303a1417f0d689f0503a5af58001c

commit aa6b81c14c0303a1417f0d689f0503a5af58001c
Author: Shuotao Gao <stgao@chromium.org>
Date: Mon Aug 28 00:37:47 2017

[Findit] Separate oauth-based and cookie-based authentication.

1. In auth_util.py, separate oauth-based and cookie-based authentication.
2. For all webpage endpoints, use cookie-based auth.
3. For all Google Cloud Endpoints, use oauth-based auth:
   -- Verify email address only for service accounts.
   -- Verify both email address and client id for user accounts.

Bug:  757547 ,  753699 
Change-Id: Ie6efc54775f083d5351d99e71b33dabbe87ccdc8
Reviewed-on: https://chromium-review.googlesource.com/627110
Commit-Queue: Shuotao Gao <stgao@chromium.org>
Reviewed-by: Chan Li <chanli@chromium.org>
Reviewed-by: Jeffrey Li <lijeffrey@chromium.org>

[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/test/findit_api_test.py
[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/handlers/test/process_flake_swarming_task_request_test.py
[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/gae_libs/http/auth_util.py
[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/common/test/acl_test.py
[add] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/common/exceptions.py
[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/handlers/process_flake_swarming_task_request.py
[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/findit_api.py
[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/handlers/flake/check_flake.py
[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/common/constants.py
[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/common/acl.py
[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/waterfall/flake/trigger_flake_swarming_task_service_pipeline.py
[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/handlers/flake/test/check_flake_test.py
[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/gae_libs/http/test/auth_util_test.py
[modify] https://crrev.com/aa6b81c14c0303a1417f0d689f0503a5af58001c/appengine/findit/waterfall/flake/test/trigger_flake_swarming_task_service_pipeline_test.py

Comment 4 by st...@chromium.org, Aug 28 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment