- Notifications
You must be signed in to change notification settings - Fork905
feat: add role-based token lifetime configuration#18179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
feat: add role-based token lifetime configuration#18179
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR introduces role-based token lifetime configuration, allowing administrators to specify different maximum API token durations per user role.
- Exposes a new
role_token_lifetimes
field across SDK types, CLI flags, and API schemas. - Implements parsing, validation, and resolution of the JSON mapping to internal duration values at server startup.
- Integrates role-based checks into token creation and token config endpoints, with comprehensive tests.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
site/src/api/typesGenerated.ts | Addedrole_token_lifetimes to the API SDK type |
codersdk/deployment.go | Added raw and parsed fields for role lifetimes; validation |
codersdk/deployment_test.go | Tests for setting and retrieving role-specific lifetimes |
coderd/config.go | Parses JSON mapping, resolves org names to IDs, populates map |
coderd/config_test.go | Tests forProcessRoleTokenLifetimesConfig |
coderd/apikey.go | Enforces role-based max lifetime invalidateAPIKeyLifetime |
coderd/apidoc/swagger.json & docs.go | Updated API docs/schema for new field |
cli/server.go | CallsProcessRoleTokenLifetimesConfig during startup |
docs/reference/{cli,api}/**/*.md | CLI and API docs for--role-token-lifetimes |
Comments suppressed due to low confidence (1)
codersdk/deployment.go:3152
- Consider adding unit tests for the Validate() method to cover invalid JSON structures and non-string values to ensure validation logic catches all bad inputs.
func (c *DeploymentValues) Validate() error {
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
6f41712
to8b9e310
CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR adds support for role-based token lifetime configuration by introducing a new configuration option (--role-token-lifetimes) that accepts a JSON mapping of roles to maximum token lifetimes. Key changes include new type definitions and documentation updates, the addition of processing logic in server startup and configuration validation, and extensive tests covering both integration and unit scenarios.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
site/src/api/typesGenerated.ts | Added a new field for role token lifetimes |
enterprise/cli/testdata/coder_server_--help.golden | Updated CLI help documentation for new role token lifetimes flag |
docs/reference/cli/server.md | Added documentation section for --role-token-lifetimes |
docs/reference/api/schemas.md & general.md | Updated API schemas and general docs to include role token lifetimes |
codersdk/deployment_test.go & deployment.go | Added tests and logic for role-based token lifetime configuration |
coderd/config.go | Introduced processing of role token lifetimes, including org lookup |
coderd/apikey.go | Updated token validation and retrieval of max token lifetime based on roles |
Other test files (config_test.go, apikey_test.go, apikey_rolelifetime_test.go, cli files) | Added tests and documentation updates covering the new configuration |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
8b9e310
to8e2ed35
CompareReally unfortunate it is a server flag. So you have to start coder, create an organization, then stop coder, update the flag, start again? |
coderd/apikey.go Outdated
if user.ID != uuid.Nil { | ||
authzInfo, err := api.Database.GetAuthorizationUserRoles(ctx, user.ID) | ||
if err != nil { | ||
api.Logger.Error(ctx, "failed to get authorization roles for token config", "user_id", user.ID.String(), "error", err) | ||
// Fallback to global max if roles can't be fetched | ||
userRoles = []string{} | ||
} else { | ||
userRoles = authzInfo.Roles | ||
} | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You should use this function:
Lines 449 to 479 inc7a5d22
// UserRBACSubject fetches a user's rbac.Subject from the database. It pulls all roles from both | |
// site and organization scopes. It also pulls the groups, and the user's status. | |
funcUserRBACSubject(ctx context.Context,db database.Store,userID uuid.UUID,scope rbac.ExpandableScope) (rbac.Subject, database.UserStatus,error) { | |
//nolint:gocritic // system needs to update user roles | |
roles,err:=db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx),userID) | |
iferr!=nil { | |
return rbac.Subject{},"",xerrors.Errorf("get authorization user roles: %w",err) | |
} | |
roleNames,err:=roles.RoleNames() | |
iferr!=nil { | |
return rbac.Subject{},"",xerrors.Errorf("expand role names: %w",err) | |
} | |
//nolint:gocritic // Permission to lookup custom roles the user has assigned. | |
rbacRoles,err:=rolestore.Expand(dbauthz.AsSystemRestricted(ctx),db,roleNames) | |
iferr!=nil { | |
return rbac.Subject{},"",xerrors.Errorf("expand role names: %w",err) | |
} | |
actor:= rbac.Subject{ | |
Type:rbac.SubjectTypeUser, | |
FriendlyName:roles.Username, | |
Email:roles.Email, | |
ID:userID.String(), | |
Roles:rbacRoles, | |
Groups:roles.Groups, | |
Scope:scope, | |
}.WithCachedASTValue() | |
returnactor,roles.Status,nil | |
} |
It is the more correct way to fetch roles. It is a bit more expensive, but worth it imo.
8e2ed35
to3ea7737
CompareUh oh!
There was an error while loading.Please reload this page.
codersdk/deployment.go Outdated
// MaximumTokenDurationExpression is a CEL expression that determines the maximum token lifetime based on user attributes. | ||
// The expression has access to 'subject' (cel.Subject), 'globalMaxDuration' (string), and 'defaultDuration' (string). | ||
// Must return a duration string (e.g., duration("168h")). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can you add a link to CEL here pls?
We might need a separate docs page on how to write CEL expressions with examples if we're gonna go this route.
We could also list the values available to the CEL expression there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I want to write a (mini) RFC regarding this and just wanted to have this/a PR as a PoC and precedent.
I'll add the link to CEL and will write docs once/if there's consensus about this route 👍🏻
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
coderd/apikey.go Outdated
user := httpmw.UserParam(r) | ||
if user.ID != uuid.Nil { | ||
subject, userStatus, err := httpmw.UserRBACSubject(ctx, api.Database, user.ID, rbac.ScopeAll) | ||
if err == nil && userStatus != database.UserStatusSuspended { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If the user is suspended, should we not allow tokens at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
httpmw.UserRBACSubject
rejects the request if the user is not active.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
httpmw.UserRBACSubject
rejects the request if the user is not active.
Is that done on the dbauthz layer? Looking at the query or the description ofUserRBACSubject
I wouldn't be able to tell that it does that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I was actually looking atExtractAPIKey
, sorry. We probably shouldn't allow creating a token for an inactive user though.
Uh oh!
There was an error while loading.Please reload this page.
coderd/apikey_rolelifetime_test.go Outdated
// This test verifies that organization-specific role configurations work with CEL expressions | ||
// Set up a client with organization-specific role configurations using CEL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nit: is thisorganization-specific
? You're only checking thatorgID != ""
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, the idea here was that one could write an expression that is not tied to the default org (the""
one), but instead only applies to organizations.
To make it 'actually' organization aware, one would need to create the org and then restart the coderd with a new server flag value for this expression set, but I didn't want to overengineer this for now.
Lifetime: 721 * time.Hour, | ||
}) | ||
require.Error(t, err) | ||
require.Contains(t, err.Error(), "exceeds the maximum allowed") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should this be a sentinel error?
Uh oh!
There was an error while loading.Please reload this page.
docs/manifest.json Outdated
@@ -1,1733 +1,1796 @@ | |||
{ | |||
"versions": ["main"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What's happening here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Make doing make things and regenerating this, I think
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
coderd/cel/token_lifetime.go Outdated
// TODO: Add timeout protection for CEL evaluation in future iterations | ||
// Example: EvaluateWithTimeout(ctx, program, vars, 100*time.Millisecond) | ||
// TODO: Add expression complexity limits in future iterations | ||
// Example: Check AST node count or expression depth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can you create follow-up issues for these and link them here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Will do once there's consensus on the RFC (creation in process).
Uh oh!
There was an error while loading.Please reload this page.
coderd/cel/token_lifetime_test.go Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Could you add some benchmarks for evaluating some sample CEL expressions here? The compilation cost at startup is less of an issue IMO as long as it's less than a second for most reasonable expressions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sure, added some benchmarks. Here are my results:
❯ gotest -bench=. -benchmem -benchtime=1s ./coderd/cel -run=^$goos: darwingoarch: arm64pkg: github.com/coder/coder/v2/coderd/celcpu: Apple M3 ProBenchmarkTokenLifetimeEvaluation/SimpleRoleCheck-11 963181 1180 ns/op 769 B/op 22 allocs/opBenchmarkTokenLifetimeEvaluation/ConditionalDuration-11 951918 1232 ns/op 785 B/op 24 allocs/opBenchmarkTokenLifetimeEvaluation/ComplexRoleLogic-11 916761 1304 ns/op 785 B/op 24 allocs/opBenchmarkTokenLifetimeEvaluation/GroupMembership-11 5291121 224.7 ns/op 128 B/op 6 allocs/opBenchmarkTokenLifetimeEvaluation/EmailDomainCheck-11 7379955 163.9 ns/op 32 B/op 3 allocs/opBenchmarkTokenLifetimeEvaluation/MultipleConditions-11 837206 1440 ns/op 929 B/op 30 allocs/opBenchmarkTokenLifetimeEvaluation/DurationComparison-11 11288636 107.0 ns/op 32 B/op 4 allocs/opBenchmarkTokenLifetimeEvaluation/ComplexDurationCalculation-11 904186 1299 ns/op 801 B/op 26 allocs/opBenchmarkTokenLifetimeCompilation/Simple-11 46714 25338 ns/op 26023 B/op 471 allocs/opBenchmarkTokenLifetimeCompilation/Medium-11 28944 42751 ns/op 41549 B/op 713 allocs/opBenchmarkTokenLifetimeCompilation/Complex-11 8616 139708 ns/op 136116 B/op 2205 allocs/opBenchmarkEnvironmentCreation-11 129195 9242 ns/op 10065 B/op 138 allocs/opBenchmarkWithResultValidation-11 869743 1374 ns/op 1202 B/op 27 allocs/opBenchmarkDifferentInputs-11 1273315 953.0 ns/op 889 B/op 18 allocs/opPASSok github.com/coder/coder/v2/coderd/cel 19.749s
And here's the output after a futuristic word transformator got to those numbers:
CEL Expression Evaluation Times (after compilation) Maximum observed duration: ~1,440 nanoseconds (1.44 microseconds) Here's the breakdown from fastest to slowest: 1. DurationComparison (107 ns) - Simple comparison of two duration variables 2. EmailDomainCheck (164 ns) - String operation checking email suffix 3. GroupMembership (225 ns) - Checking if a string exists in an array 4. DifferentInputs (953 ns) - Same expression with varying inputs 5. SimpleRoleCheck (1,180 ns) - Checking if user has "owner" role 6. ConditionalDuration (1,232 ns) - If-then-else returning different durations 7. ComplexDurationCalculation (1,299 ns) - Nested conditionals with duration limits 8. ComplexRoleLogic (1,304 ns) - Multiple role checks with conditions 9. WithResultValidation (1,374 ns) - Same as ConditionalDuration but with validation 10. MultipleConditions (1,440 ns) - Combined role and group checks ← SLOWEST Compilation Times (one-time cost at startup) - Simple expressions: ~25 microseconds - Medium complexity: ~43 microseconds - Complex expressions: ~140 microseconds Key Takeaways 1. Evaluation is very fast: Even the slowest expression takes only 1.44 microseconds 2. String operations are fastest: 100-200 nanoseconds 3. Role checking is moderate: 1-1.5 microseconds 4. Compilation is quick: Even complex expressions compile in 0.14 milliseconds Real-world Impact If you evaluate a token lifetime expression on every API request: - Worst case: 1.44 microseconds overhead - Typical case: ~1 microsecond overhead - To put this in perspective: A typical database query takes 1-10 milliseconds (1,000-10,000 microseconds) The CEL evaluation adds negligible overhead - about 0.01-0.1% of a typical database operation.
c5117c4
toe0261c5
CompareChange-Id: Ief7d00a53f20340b36060e7c5f15499a672696f3Signed-off-by: Thomas Kosiewski <tk@coder.com>
Change-Id: I9693c219e2e6268662d2ced209bc75744c07cf67Signed-off-by: Thomas Kosiewski <tk@coder.com>
f080b23
to75432b4
CompareChange-Id: I2dcfa21535a8c5d4b2276622617782f0b2c47603Signed-off-by: Thomas Kosiewski <tk@coder.com>
75432b4
to78e370e
Compare
Uh oh!
There was an error while loading.Please reload this page.
Role-Based Token Lifetime Configuration with Expr Expressions
Summary
This PR implements role-based token lifetime configuration using expr expressions, providing administrators with flexible control over API token lifetimes based on user attributes and roles.
The implementation allows administrators to configure custom expr expressions that evaluate user roles, organization membership, email domains, and other attributes to determine appropriate token lifetimes dynamically.
Key Features
subject.Roles
,subject.Email
,subject.Groups
,globalMaxDuration
, anddefaultDuration
variablesConfiguration
The new
--max-token-lifetime-expression
flag accepts expr expressions that return duration values:coder server --max-token-lifetime-expression='any(subject.Roles, .Name == "owner") ? duration("168h") : duration("24h")'
(Can also be set via an env variable or YAML.)
Example Expressions
Role-based configuration:
Organization-specific roles:
Email domain-based:
Implementation Details
Behavior
Future Enhancements
The following improvements are planned for future PRs:
Breaking Changes
None - this is a purely additive feature with backward compatibility.
Historical Implementation Notes
Note for reviewers: This PR presents the expr-based approach. An alternative JSON-based configuration was also implemented in commit63eaf5c. We chose the expr approach for its flexibility in supporting complex logic, email domains, and other advanced use cases while maintaining simplicity for basic scenarios.
Previous Description:
Fixes#17395
Graphite generated description:
Role-Based Token Lifetime Configuration
This PR adds support for role-based token lifetime configuration, enabling administrators to set distinct maximum API token lifetimes for various user roles.
Key features:
The configuration uses Go duration format (e.g., "720h" for 30 days) and can be set via CLI flag, environment variable, or YAML configuration. When a user creates a token, the system checks their roles and applies the appropriate maximum lifetime.
Example configuration:
This implementation includes comprehensive test coverage for various scenarios, including validation, parsing, and integration with the token creation workflow.