Bug #16736
Token lifetime options
0%
Description
Add a new option API.MaximumTokenLifetime:
- If no expiration time is given in the create call, it is the "maximum" expiration (new configuration option API.MaximumTokenLifetime)
- For regular users, token expires_at is clamped to MaximumTokenLifetime for create/update
- Admins can create tokens with any expiration time.
- Tokens created to run a container do not have a set expire time (because it will expire when the container ends)
- Tokens created for use on a shell node by arvados-login-sync script have max lifetime, and are rotated by the script on some interval (like MaximumTokenLifetime/2)
Tokens created through login use Login.TokenLifetime (existing behavior).
Subtasks
Related issues
History
#1
Updated by Lucas Di Pentima 5 months ago
Pushed a branch with a test: 16736-expiring-tokens-limits
#2
Updated by Lucas Di Pentima 5 months ago
- Status changed from New to In Progress
#3
Updated by Lucas Di Pentima 5 months ago
- Target version changed from 2020-09-09 Sprint to 2020-08-26 Sprint
#4
Updated by Lucas Di Pentima 5 months ago
- Target version changed from 2020-08-26 Sprint to 2020-09-09 Sprint
#5
Updated by Lucas Di Pentima 4 months ago
- Target version changed from 2020-09-09 Sprint to 2020-09-23 Sprint
#6
Updated by Lucas Di Pentima 4 months ago
Updates at 386b60af1 - branch 16736-expiring-tokens-limits
Test run: https://ci.arvados.org/job/developer-run-tests/2078/
- Adds tests exposing the bug.
- Adds checks related to token's
expires_at
on create/update. - Fixes fixture removing expiration dates that were set far in the future.
#7
Updated by Peter Amstutz 4 months ago
Lucas Di Pentima wrote:
Updates at 386b60af1 - branch
16736-expiring-tokens-limits
Test run: https://ci.arvados.org/job/developer-run-tests/2078/
- Adds tests exposing the bug.
- Adds checks related to token's
expires_at
on create/update.- Fixes fixture removing expiration dates that were set far in the future.
def permission_to_create current_user.andand.is_admin or ((current_user.andand.id == self.user_id) and (current_api_client_authorization.andand.expires_at.nil? or (self.expires_at and current_api_client_authorization.expires_at >= self.expires_at))) end
I don't think this is quite right, this will allow an admin to always create a token even if their token is about to expire. But that's not right. Admins can create tokens for other users, but the tokens are still capped by expires_at.
Also, instead of "current_api_client_authorization.andand", if it is possible for current_api_client_authorization to be nil I don't think we allow tokens to be manipulated (is there a special case, like tests?)
I think this should be:
def permission_to_create (current_user.andand.is_admin || current_user.andand.id == self.user_id) and !current_api_client_authorization.nil? and (current_api_client_authorization.expires_at.nil? || (!self.expires_at.nil? && current_api_client_authorization.expires_at >= self.expires_at))) end
By the way, I looked this up recently, the difference between "and"/"or" and "&&"/"||" in Ruby has to do with operator precedence, "and"/"or" have lower precedence than "&&"/"||" so the convention is to use the and/or for the "outer" clauses and &&/|| for the "inner" clauses although when grouping with parenthesis it doesn't really matter.
Arvados::V1::ApiClientAuthorizationsController#find_objects_for_index should include a filter on expires_at when current_api_client_authorization is non-nil, and there should be a controller test for that.
#8
Updated by Peter Amstutz 4 months ago
update: I think you mentioned on gitter than current_api_client_authorization
is nil in the case of the login process, which makes sense. We are creating a token, we don't have one yet.
However, if there was a bug that allowed some other way of calling token creation without setting current_api_client_authorization, that would be a serious security hole.
I wonder if there's a way to handle login as a special case. The value of current_api_client_authorization comes from Thread.current[:api_client_authorization], so the login code could assign a fake (not persisted to database) ApiClientAuthorization object.
#9
Updated by Peter Amstutz 4 months ago
When Login.TokenLifetime is set:
- Token from login uses "Login.TokenLifetime" expiration (existing behavior)
- (valid) token from login can be used to create new tokens
- If no expiration time is given, it is the "maximum" expiration (new configuration option API.MaximumTokenLifetime)
- SystemRootToken can create tokens with any expiration (or nil)
- Token created to run a container doesn't have a set expire time, because it will expire when the container ends
- Token used on a shell node is max lifetime, when about to expire it is rotated by the arvados-login-sync script
- "Show current token" dialog in workbench should generate a new token instead
- Change the label to something like "Get new user token"
- Auto-logout should coordinate across browser tabs/windows (using the same token?) so that it only logs out when all of them have been idle.
- Figure out which browser feature lets you do that
#10
Updated by Peter Amstutz 4 months ago
- Release deleted (
25)
#11
Updated by Peter Amstutz 4 months ago
- Subject changed from Trusted client token with expiration can create tokens with any expiration date (or none at all) to Token default and maximum lifetime option
#12
Updated by Peter Amstutz 4 months ago
- Target version changed from 2020-09-23 Sprint to 2020-10-07 Sprint
- Assigned To deleted (
Lucas Di Pentima) - Description updated (diff)
- Subject changed from Token default and maximum lifetime option to Token lifetime options
#13
Updated by Peter Amstutz 4 months ago
- Target version changed from 2020-10-07 Sprint to 2020-10-21 Sprint
#14
Updated by Peter Amstutz 3 months ago
- Target version changed from 2020-10-21 Sprint to 2020-11-04 Sprint
#15
Updated by Peter Amstutz 3 months ago
- Related to Story #16520: GxP Qualification added
#16
Updated by Peter Amstutz 3 months ago
- Assigned To set to Peter Amstutz
#17
Updated by Peter Amstutz 3 months ago
- Target version changed from 2020-11-04 Sprint to 2020-11-18
#18
Updated by Peter Amstutz 2 months ago
- Target version changed from 2020-11-18 to 2020-12-02 Sprint
#19
Updated by Peter Amstutz 2 months ago
- Story points set to 3.0
#20
Updated by Peter Amstutz 2 months ago
- Assigned To deleted (
Peter Amstutz)
#21
Updated by Peter Amstutz 2 months ago
- Target version changed from 2020-12-02 Sprint to 2020-12-16 Sprint
#22
Updated by Peter Amstutz about 2 months ago
- Assigned To set to Lucas Di Pentima
#23
Updated by Lucas Di Pentima about 1 month ago
- Target version changed from 2020-12-16 Sprint to 2021-01-06 Sprint
#24
Updated by Lucas Di Pentima 13 days ago
- Target version changed from 2021-01-06 Sprint to 2021-01-20 Sprint