Bug #17785

[controller/api] "Forbidden: this API client cannot manipulate other clients' access tokens." on federated login clusters (2.2.0 regression)

Added by Ward Vandewege 12 months ago. Updated 2 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
11/23/2021
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

This happens on tordo (2.3.0~dev20210608145247) (login federation with ce8i5) but not on 2xpu4 (2.2.0) (directly configured for login through google).

Bug observed in multiple places:

  • go to workbench.tordo, log in as admin, view a user in the admin user list, and click the "Log in as ..." button. The result is a fiddlesticks with the error "Forbidden: this API client cannot manipulate other clients' access tokens.", e.g.:
{
  ":errors":[
    "Forbidden: this API client cannot manipulate other clients' access tokens. (req-ckw5smn0dfhygvcgk5h6)" 
  ],
  ":error_token":"1625590529+e5031a85" 
}
  • on shell.ce8i5, the `arvados-login-sync` script (which runs with a token belonging to an admin user) throws this output on every iteration:
Error setting token for STRIPPED: ["Forbidden: this API client cannot manipulate other clients' access tokens. (req-ddhir3er6zg31hszw9o1)"]
Error setting token for STRIPPED: ["Forbidden: this API client cannot manipulate other clients' access tokens. (req-u61v42jybvqur0ygz5x3)"]
Error setting token for STRIPPED: ["Forbidden: this API client cannot manipulate other clients' access tokens. (req-1dtnzyr2oo2sfp6e8pjz)"]
Error setting token for STRIPPED: ["Forbidden: this API client cannot manipulate other clients' access tokens. (req-h05j0ififv2t8ksfekhd)"]
Error setting token for STRIPPED: ["Forbidden: this API client cannot manipulate other clients' access tokens. (req-1w89ttespwuf51azgjt1)"]
Error setting token for STRIPPED: ["Forbidden: this API client cannot manipulate other clients' access tokens. (req-1773gy0yhdoo11t74emp)"]

Subtasks

Task #17869: Review 17785-federated-token-regressionResolvedLucas Di Pentima

Task #17905: investigateResolvedLucas Di Pentima


Related issues

Related to Arvados - Feature #17583: Remote controller forwards trusted client aware calls on a federated scenarioResolved01/21/2022

Blocks Arvados - Bug #17754: [wb] merge account problemResolved02/18/2022

History

#1 Updated by Ward Vandewege 12 months ago

  • Description updated (diff)

#2 Updated by Ward Vandewege 12 months ago

  • Target version changed from 2021-06-09 sprint to 2021-06-23 sprint
  • Description updated (diff)

#3 Updated by Ward Vandewege 12 months ago

  • Related to Bug #17754: [wb] merge account problem added

#4 Updated by Ward Vandewege 12 months ago

  • Description updated (diff)

#5 Updated by Peter Amstutz 12 months ago

  • Target version changed from 2021-06-23 sprint to 2021-07-07 sprint

#6 Updated by Peter Amstutz 11 months ago

  • Assigned To set to Nico César

#7 Updated by Nico César 11 months ago

  • File 2021-06-24_10-57.png added
  • File 17785_error.png added
  • File 2021-06-24_10-53.png added

#8 Updated by Nico César 11 months ago

  • File deleted (2021-06-24_10-57.png)

#9 Updated by Nico César 11 months ago

  • File deleted (17785_error.png)

#10 Updated by Nico César 11 months ago

  • File deleted (2021-06-24_10-53.png)

#11 Updated by Ward Vandewege 11 months ago

  • Description updated (diff)
  • Subject changed from [workbench] log in as another user broken to [controller/api] "Forbidden: this API client cannot manipulate other clients' access tokens." on federated login clusters (2.2.0 regression)

#12 Updated by Ward Vandewege 11 months ago

  • Description updated (diff)

#13 Updated by Ward Vandewege 11 months ago

  • Related to Feature #17583: Remote controller forwards trusted client aware calls on a federated scenario added

#15 Updated by Peter Amstutz 11 months ago

  • Target version changed from 2021-07-07 sprint to 2021-07-21 sprint

#16 Updated by Peter Amstutz 11 months ago

  • Assigned To deleted (Nico César)

#17 Updated by Peter Amstutz 11 months ago

  • Assigned To set to Peter Amstutz

#18 Updated by Peter Amstutz 11 months ago

  • Related to deleted (Bug #17754: [wb] merge account problem)

#19 Updated by Peter Amstutz 11 months ago

  • Blocks Bug #17754: [wb] merge account problem added

#20 Updated by Peter Amstutz 10 months ago

  • Target version changed from 2021-07-21 sprint to 2021-08-04 sprint

#21 Updated by Peter Amstutz 10 months ago

  • Target version changed from 2021-08-04 sprint to 2021-08-18 sprint

#22 Updated by Peter Amstutz 10 months ago

  • Target version changed from 2021-08-18 sprint to 2021-09-01 sprint

#23 Updated by Peter Amstutz 9 months ago

  • Assigned To changed from Peter Amstutz to Lucas Di Pentima

#24 Updated by Lucas Di Pentima 9 months ago

  • Status changed from New to In Progress

#25 Updated by Lucas Di Pentima 9 months ago

While trying to reproduce this error, I kept getting another error message when asking tordo for its user's list:

$ arv user list
Error: error updating local user records: //railsapi.internal/arvados/v1/users/batch_update: 422 Unprocessable Entity: #<ActiveRecord::RecordInvalid: Validation failed: Username has already been taken> (req-1iksissnw490d1thn1q9)

After re-requesting the list with -b (to bypass federation) and comparing users with ce8i5, I realized an issue with a couple of users sharing the same ward2 username. The one cached on tordo's side doesn't exist on ce8i5 (even though its UUID is from that cluster), so when tordo's controller attempts to refresh the local cache, the username is already taken by a user record that won't get updated anymore.

#26 Updated by Lucas Di Pentima 9 months ago

  • Target version changed from 2021-09-01 sprint to 2021-09-15 sprint

#27 Updated by Lucas Di Pentima 9 months ago

For the "Login as <user>" case, the issue is that wb1 tries to create an ApiClientAuthorization resource owned by the target user, and that fails because the current api client record isn't trusted because:

$ arv user current
{
 "created_at":"2020-01-21T14:39:17.181611000Z",
 "first_name":"Lucas ",
 "full_name":"Lucas  Di Pentima",
 "identity_url":"",
 "is_active":true,
 "is_admin":true,
 "is_invited":true,
 "kind":"arvados#user",
 "last_name":"Di Pentima",
 "uuid":"ce8i5-tpzed-o4njwilpp4ov286",
[...]
}
$ arv api_client_authorization current
{
 "href":"/api_client_authorizations/ce8i5-gj3su-vobk909hrbmmsrl",
 "uuid":"ce8i5-gj3su-vobk909hrbmmsrl",
 "owner_uuid":"ce8i5-tpzed-o4njwilpp4ov286",
 "api_token":"xxxxtokenxxxx",
 "api_client_id":0,
[...]
}

#28 Updated by Lucas Di Pentima 9 months ago

Updates at deca285 - branch 17785-federated-token-regression
Test run: developer-run-tests-remainder: #2789

  • Adds an integration test that exposes the issue.

#29 Updated by Lucas Di Pentima 9 months ago

For the arvados-login-sync case, maybe what's needed is to set up the LOGINCLUSTER_ARVADOS_API_HOST and LOGINCLUSTER_ARVADOS_API_TOKEN envvars so that the api client authorization creation is done on the LoginCluster?

#30 Updated by Lucas Di Pentima 8 months ago

  • Target version changed from 2021-09-15 sprint to 2021-09-29 sprint

#31 Updated by Peter Amstutz 8 months ago

  • Release set to 42

#32 Updated by Peter Amstutz 8 months ago

  • Target version changed from 2021-09-29 sprint to 2021-10-13 sprint

#33 Updated by Peter Amstutz 8 months ago

  • Release deleted (42)

#34 Updated by Lucas Di Pentima 7 months ago

  • Target version changed from 2021-10-13 sprint to 2021-10-27 sprint

#35 Updated by Lucas Di Pentima 7 months ago

  • Target version changed from 2021-10-27 sprint to 2021-11-10 sprint

#36 Updated by Peter Amstutz 7 months ago

  • Target version changed from 2021-11-10 sprint to 2021-11-24 sprint

#37 Updated by Peter Amstutz 7 months ago

  • Target version changed from 2021-11-24 sprint to 2021-11-10 sprint

#38 Updated by Peter Amstutz 7 months ago

  • Target version changed from 2021-11-10 sprint to 2021-11-24 sprint

#39 Updated by Lucas Di Pentima 6 months ago

Updates at a98916d - branch 17785-federated-token-regression
Test run: developer-run-tests: #2812

  • Migrates the api_client_authorization handling to controller's "new code path", forwarding CREATE and UPDATE calls to remote clusters if needed.

This fixes the test commented on #17785-28

#40 Updated by Tom Clegg 6 months ago

In sdk/go/arvados/api_client_authorization.go
  • The *string fields should be string since there is no semantic difference between "" and NULL
  • I don't think we need the Href field. It looks like we are currently using two different approaches for this. Container and Group have Href fields, but User and Collection don't -- instead "href" is exempt from the response-comparison test for User and Collection in lib/controller/handler_test.go. Unless something actually needs this, I think we should drop it.
In test
  • Looks like this could be simplified
    -       for cls := range s.testClusters {
    -               if cls == "z1111" || cls == "z3333" {
    +       for _, cls := range []string{"z1111", "z3333"} {
    
  • After creating the "log in as..." token, might as well check that it works... if resp is an APIClientAuthorization then resp.TokenV2() might be convenient.

#41 Updated by Lucas Di Pentima 6 months ago

  • Target version changed from 2021-11-24 sprint to 2021-12-08 sprint

#42 Updated by Lucas Di Pentima 6 months ago

Updates at 47982d3
Test run: developer-run-tests: #2814

  • Enhances IntegrationSuite.TestFederatedApiClientAuthHandling test by confirming that the created token does work.
  • Fixes arvados.ApiClientAuthorization's member types. Updates tests & related code.

#43 Updated by Tom Clegg 6 months ago

LGTM, thanks!

#44 Updated by Lucas Di Pentima 6 months ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved

Applied in changeset arvados-private:commit:arvados|9c8a812148e6b989fd7ab6aac49168276f5d5b9f.

#45 Updated by Peter Amstutz 2 months ago

  • Release set to 46

Also available in: Atom PDF