Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Draft
ThomasK33 wants to merge3 commits intomain
base:main
Choose a base branch
Loading
fromthomask33/feat_add_role-based_token_lifetime_configuration

Conversation

ThomasK33
Copy link
Member

@ThomasK33ThomasK33 commentedJun 2, 2025
edited by blink-sobot
Loading

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

  • Expr Expression Support: Configure token lifetimes using powerful expr expressions with access to user subject data
  • Role-Based Logic: Support for both site-wide roles (e.g., "owner", "member") and organization-specific roles
  • Flexible Evaluation: Access tosubject.Roles,subject.Email,subject.Groups,globalMaxDuration, anddefaultDuration variables
  • Backward Compatibility: Falls back to global maximum when no expression is configured
  • Graceful Error Handling: Falls back to default duration on expression evaluation errors
  • Comprehensive Testing: Full test coverage including integration tests and edge cases

Configuration

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:

any(subject.Roles, .Name == "owner") ? duration("720h") :any(subject.Roles, .Name == "user-admin") ? duration("168h") : duration("24h")

Organization-specific roles:

any(subject.Roles, .Name == "organization-admin" && .OrgID != "") ? duration("168h") :any(subject.Roles, .Name == "organization-member" && .OrgID != "") ? duration("72h") :duration(defaultDuration)

Email domain-based:

subject.Email endsWith "@company.com" ? duration("720h") : duration("24h")

Implementation Details

  • Expr Environment: Custom expr environment with native type support for Subject and Role structs
  • Duration Function: Built-in duration() function for parsing duration strings
  • Expression Compilation: Expressions are compiled once at server startup for performance
  • Error Handling: Graceful fallback to default duration on evaluation errors with logging
  • Security: Input validation and type checking for all expr expressions
  • Type Safety: Uses int64 representation for durations to work with expr's type system

Behavior

  • Startup: Expressions are compiled during server startup; invalid expressions prevent server start
  • Evaluation: For each token creation, the expression is evaluated with the user's current attributes
  • Fallback: On evaluation errors, the system falls back to the default token duration and logs the error
  • Precedence: When configured, the expression result takes precedence over the global maximum token lifetime

Future Enhancements

The following improvements are planned for future PRs:

  • Expression evaluation timeout protection
  • Expression complexity limits
  • Hot-reloading of expressions without server restart

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:

  • Added a new --role-token-lifetimes configuration option that accepts a JSON mapping of roles to maximum token lifetimes
  • Supports both site-wide roles (e.g., "owner", "member") and organization-specific roles (e.g., "MyOrg/admin")
  • Automatically resolves organization names to IDs during server startup
  • Uses the most generous lifetime a user is entitled to based on their roles
  • Maintains backward compatibility with the global maximum token lifetime setting

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:

{"owner":"720h","user-admin":"168h","MyOrg/template-admin":"72h"}

This implementation includes comprehensive test coverage for various scenarios, including validation, parsing, and integration with the token creation workflow.

@ThomasK33Graphite App
Copy link
MemberAuthor

This stack of pull requests is managed byGraphite. Learn more aboutstacking.

@ThomasK33ThomasK33 marked this pull request as ready for reviewJune 2, 2025 14:27
Copy link
Contributor

@CopilotCopilotAI left a 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 newrole_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
FileDescription
site/src/api/typesGenerated.tsAddedrole_token_lifetimes to the API SDK type
codersdk/deployment.goAdded raw and parsed fields for role lifetimes; validation
codersdk/deployment_test.goTests for setting and retrieving role-specific lifetimes
coderd/config.goParses JSON mapping, resolves org names to IDs, populates map
coderd/config_test.goTests forProcessRoleTokenLifetimesConfig
coderd/apikey.goEnforces role-based max lifetime invalidateAPIKeyLifetime
coderd/apidoc/swagger.json & docs.goUpdated API docs/schema for new field
cli/server.goCallsProcessRoleTokenLifetimesConfig during startup
docs/reference/{cli,api}/**/*.mdCLI 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 {

@ThomasK33ThomasK33force-pushed thethomask33/feat_add_role-based_token_lifetime_configuration branch from6f41712 to8b9e310CompareJune 2, 2025 14:34
@ThomasK33ThomasK33 requested a review fromCopilotJune 2, 2025 14:37
Copy link
Contributor

@CopilotCopilotAI left a 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
FileDescription
site/src/api/typesGenerated.tsAdded a new field for role token lifetimes
enterprise/cli/testdata/coder_server_--help.goldenUpdated CLI help documentation for new role token lifetimes flag
docs/reference/cli/server.mdAdded documentation section for --role-token-lifetimes
docs/reference/api/schemas.md & general.mdUpdated API schemas and general docs to include role token lifetimes
codersdk/deployment_test.go & deployment.goAdded tests and logic for role-based token lifetime configuration
coderd/config.goIntroduced processing of role token lifetimes, including org lookup
coderd/apikey.goUpdated 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

@ThomasK33ThomasK33 requested a review fromEmyrkJune 2, 2025 14:59
@ThomasK33ThomasK33force-pushed thethomask33/feat_add_role-based_token_lifetime_configuration branch from8b9e310 to8e2ed35CompareJune 2, 2025 15:01
@Emyrk
Copy link
Member

Really unfortunate it is a server flag. So you have to start coder, create an organization, then stop coder, update the flag, start again?

Comment on lines 349 to 355
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 {
Copy link
Member

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:

// 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.

@ThomasK33ThomasK33force-pushed thethomask33/feat_add_role-based_token_lifetime_configuration branch from8e2ed35 to3ea7737CompareJune 3, 2025 17:08

// 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")).
Copy link
Contributor

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.

ThomasK33 reacted with thumbs up emoji
Copy link
MemberAuthor

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 👍🏻

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 {
Copy link
Contributor

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?

Copy link
Member

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.

Copy link
MemberAuthor

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.

Copy link
Member

@johnstcnjohnstcnJun 4, 2025
edited
Loading

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.

ThomasK33 reacted with thumbs up emoji

// This test verifies that organization-specific role configurations work with CEL expressions

// Set up a client with organization-specific role configurations using CEL
Copy link
Contributor

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 != "".

Copy link
MemberAuthor

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")
Copy link
Contributor

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?

@@ -1,1733 +1,1796 @@
{
"versions": ["main"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

What's happening here?

Copy link
MemberAuthor

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

Comment on lines 97 to 102
// 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
Copy link
Member

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?

ThomasK33 reacted with thumbs up emoji
Copy link
MemberAuthor

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).

Copy link
Member

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.

Copy link
MemberAuthor

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.

@ThomasK33ThomasK33force-pushed thethomask33/feat_add_role-based_token_lifetime_configuration branch 3 times, most recently fromc5117c4 toe0261c5CompareJune 4, 2025 14:39
@ThomasK33ThomasK33 marked this pull request as draftJune 5, 2025 10:19
Change-Id: Ief7d00a53f20340b36060e7c5f15499a672696f3Signed-off-by: Thomas Kosiewski <tk@coder.com>
Change-Id: I9693c219e2e6268662d2ced209bc75744c07cf67Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33ThomasK33force-pushed thethomask33/feat_add_role-based_token_lifetime_configuration branch fromf080b23 to75432b4CompareJune 5, 2025 14:47
Change-Id: I2dcfa21535a8c5d4b2276622617782f0b2c47603Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33ThomasK33force-pushed thethomask33/feat_add_role-based_token_lifetime_configuration branch from75432b4 to78e370eCompareJune 5, 2025 18:13
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@dannykoppingdannykoppingdannykopping left review comments

@johnstcnjohnstcnjohnstcn left review comments

@EmyrkEmyrkEmyrk left review comments

Copilot code reviewCopilotCopilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@ThomasK33ThomasK33

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support finer control on token lifetime
4 participants
@ThomasK33@Emyrk@dannykopping@johnstcn

[8]ページ先頭

©2009-2025 Movatter.jp