Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add master configurations for access token max and default lifespans [DET-10464] #10101

Merged
merged 8 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/reference/deploy/master-config-reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1569,6 +1569,18 @@ Integer identifier of a role to be assigned. Defaults to ``2``, which is the rol
Initial password for the built-in ``determined`` and ``admin`` users. Applies on first launch when a
cluster's database is bootstrapped, otherwise it is ignored.

``token``
=========

Applies only to Determined Enterprise Edition. Defines default and maximum lifespan settings for
access tokens. These settings allow administrators to control how long access tokens can remain
Copy link
Contributor

Choose a reason for hiding this comment

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

reading this it is not clear to me the difference between an "access token" and a regular token, and i'm not sure how a user would know there is a difference, but I dont have an immediate idea of how to fix it.

valid, enhancing security while supporting automation.

- ``default_lifespan_days``: Specifies the default lifespan (in days) for new access tokens.
Defaults to 30 days.
- ``max_lifespan_days``: Specifies the maximum allowed lifespan (in days) for access tokens.
Setting this to ``-1`` allows for an infinite token lifespan. Defaults to ``-1``.

ShreyaLnuHpe marked this conversation as resolved.
Show resolved Hide resolved
**************
``webhooks``
**************
Expand Down
13 changes: 8 additions & 5 deletions docs/release-notes/api-cli-access-token.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

**New Features**

- API/CLI: Add support for access tokens. Add the ability create and administer access tokens for
users to authenticate in automated workflows. Users can define the lifespan of these tokens,
making it easier to securely authenticate and run processes. This feature enhances automation
while maintaining strong security protocols by allowing tighter control over token usage and
expiration.
- API/CLI: Add support for access tokens. Add the ability to create and administer access tokens
for users to authenticate in automated workflows. Users can define the lifespan of these tokens,
making it easier to securely authenticate and run processes. Users can set global defaults and
limits for the validity of access tokens by configuring ``default_lifespan_days`` and
``max_lifespan_days`` in the master configuration. Setting ``max_lifespan_days`` to ``-1``
indicates an **infinite** lifespan for the access token. This feature enhances automation while
maintaining strong security protocols by allowing tighter control over token usage and
expiration. This feature requires Determined Enterprise Edition.

- CLI:

Expand Down
7 changes: 5 additions & 2 deletions harness/determined/cli/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,11 @@ def create_token(args: argparse.Namespace) -> None:
raise errors.CliError(f"User '{username}' not found or does not have an ID")

# convert days into hours Go duration format
if args.expiration_days:
expiration_in_hours = str(24 * args.expiration_days) + "h"
expiration_in_hours = None
if args.expiration_days is not None:
expiration_in_hours = (
"-1" if args.expiration_days == -1 else f"{24 * args.expiration_days}h"
Copy link
Contributor

Choose a reason for hiding this comment

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

might be worth just making sure that args.expiration_days is an int here, if it's a float not sure what would happen

Copy link
Contributor

Choose a reason for hiding this comment

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

With the current code, any float input would return an error invalid int value: '5.0'. I think we should keep it as is.

Another option is to explicitly cast the input to an integer. However, the potential issue with this approach is that it could mislead users into thinking that floating-point values are valid, which might give the impression that fractional days are allowed.

expiration_in_hours = None
if args.expiration_days is not None:
    expiration_days = int(args.expiration_days)  # Ensure it's an integer
    expiration_in_hours = "-1" if expiration_days == -1 else f"{24 * expiration_days}h"

)

request = bindings.v1PostAccessTokenRequest(
userId=user.id, lifespan=expiration_in_hours, description=args.description
Expand Down
4 changes: 4 additions & 0 deletions helm/charts/determined/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ useNodePortForMaster: false
# authz option (EE-only) sets the authorization mode.
# authz:
# type: rbac
# access token option (EE-Only) allows to authenticate in automated workflows.
# token:
# max_lifespan_days: -1
# default_lifespan_days: 30

# oidc (EE-only) enables OpenID Connect Integration, which is only available if enterpriseEdition
# is true. It allows users to use single sign-on with their organization’s identity provider.
Expand Down
44 changes: 37 additions & 7 deletions master/internal/api_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"google.golang.org/grpc/status"

"github.com/determined-ai/determined/master/internal/api"
"github.com/determined-ai/determined/master/internal/config"
"github.com/determined-ai/determined/master/internal/db"
"github.com/determined-ai/determined/master/internal/grpcutil"
"github.com/determined-ai/determined/master/internal/token"
Expand All @@ -36,14 +37,43 @@ func (a *apiServer) PostAccessToken(
return nil, status.Error(codes.PermissionDenied, err.Error())
}

tokenExpiration := token.DefaultTokenLifespan
if req.Lifespan != "" {
d, err := time.ParseDuration(req.Lifespan)
if err != nil || d < 0 {
return nil, status.Error(codes.InvalidArgument,
"Lifespan must be a Go-formatted duration string with a positive value")
defaultTokenLifespan, err := a.m.config.Security.Token.GetDefaultLifespan()
if err != nil {
return nil, status.Error(codes.Internal, "Failed to parse default lifespan")
}

maxTokenLifespan, err := a.m.config.Security.Token.GetMaxLifespan()
if err != nil {
return nil, status.Error(codes.Internal, "Failed to parse max lifespan")
}

tokenExpiration := defaultTokenLifespan
if req.Lifespan != nil {
if *req.Lifespan == config.InfiniteTokenLifespanString {
tokenExpiration = maxTokenLifespan
} else {
d, err := time.ParseDuration(*req.Lifespan)
if err != nil {
return nil, status.Error(codes.InvalidArgument,
fmt.Sprintf("Failed to parse lifespan %s, with error: %v", *req.Lifespan,
err.Error()))
}
if d < 0 {
return nil, status.Error(codes.InvalidArgument,
"Lifespan must be a Go-formatted duration string with a positive value")
}
tokenExpiration = d
}
tokenExpiration = d
}

// Ensure the token lifespan does not exceed the maximum allowed or minimum -1 value.
if tokenExpiration > maxTokenLifespan {
return nil, status.Error(codes.InvalidArgument, "Token Lifespan must be less than max"+
" token lifespan")
}
if tokenExpiration < -1 {
return nil, status.Error(codes.InvalidArgument, "token lifespan must be greater than 0 days,"+
" unless set to -1 for infinite lifespan")
}

token, tokenID, err := token.CreateAccessToken(
Expand Down
15 changes: 6 additions & 9 deletions master/internal/api_token_intg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/determined-ai/determined/master/internal/token"
"github.com/determined-ai/determined/master/internal/user"
"github.com/determined-ai/determined/master/pkg/model"
"github.com/determined-ai/determined/master/pkg/ptrs"
"github.com/determined-ai/determined/proto/pkg/apiv1"
)

Expand Down Expand Up @@ -67,11 +68,11 @@ func TestPostAccessTokenWithLifespan(t *testing.T) {
// With lifespan input
resp, err := api.PostAccessToken(ctx, &apiv1.PostAccessTokenRequest{
UserId: int32(userID),
Lifespan: lifespan,
Lifespan: ptrs.Ptr("5s"),
Description: desc,
})
token, tokenID := resp.Token, resp.TokenId
require.NoError(t, err)
token, tokenID := resp.Token, resp.TokenId
require.NotNil(t, token)
require.NotNil(t, tokenID)

Expand Down Expand Up @@ -261,13 +262,9 @@ func getTestUser(ctx context.Context) (model.UserID, error) {
func testSetLifespan(ctx context.Context, t *testing.T, userID model.UserID, lifespan string,
tokenID model.TokenID,
) error {
expLifespan := token.DefaultTokenLifespan
var err error
if lifespan != "" {
expLifespan, err = time.ParseDuration(lifespan)
if err != nil {
return fmt.Errorf("Invalid duration format")
}
expLifespan, err := time.ParseDuration(lifespan)
if err != nil {
return fmt.Errorf("Invalid duration format")
}
var expiry, createdAt time.Time
err = db.Bun().NewSelect().
Expand Down
6 changes: 6 additions & 0 deletions master/internal/api_user_intg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@ func setupAPITest(t *testing.T, pgdb *db.PgDB,
},
TaskContainerDefaults: model.TaskContainerDefaultsConfig{},
ResourceConfig: *config.DefaultResourceConfig(),
Security: config.SecurityConfig{
Token: config.TokenConfig{
MaxLifespanDays: config.MaxAllowedTokenMaxLifespan,
DefaultLifespanDays: config.DefaultTokenLifespanDays,
},
},
},
taskSpec: &tasks.TaskSpec{SSHRsaSize: 1024},
allRms: map[string]rm.ResourceManager{config.DefaultClusterName: mockRM},
Expand Down
81 changes: 80 additions & 1 deletion master/internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ import (
"fmt"
"net/url"
"path/filepath"
"strconv"
"sync"
"time"

"github.com/jinzhu/copier"
log "github.com/sirupsen/logrus"
Expand All @@ -35,11 +37,20 @@ var (
masterConfig *Config
)

// KubernetesDefaultPriority is the default K8 resource manager priority.
const (
// KubernetesDefaultPriority is the default K8 resource manager priority.
KubernetesDefaultPriority = 50
sslModeDisable = "disable"
preemptionScheduler = "preemption"
// InfiniteTokenLifespan is the value to set the token lifespan to infinite.
InfiniteTokenLifespan = -1
// InfiniteTokenLifespanString is the string representation of InfiniteTokenLifespan.
InfiniteTokenLifespanString = "-1"
// DefaultTokenLifespanDays is the default token lifespan in days.
DefaultTokenLifespanDays = 30
// MaxAllowedTokenMaxLifespan is the max allowed lifespan for tokens.
// This is the maximum number of days a go duration can represent.
MaxAllowedTokenMaxLifespan = 106751
ShreyaLnuHpe marked this conversation as resolved.
Show resolved Hide resolved
)

type (
Expand Down Expand Up @@ -111,6 +122,10 @@ func DefaultConfig() *Config {
RsaKeySize: 1024,
},
AuthZ: *DefaultAuthZConfig(),
Token: TokenConfig{
MaxLifespanDays: DefaultTokenLifespanDays,
DefaultLifespanDays: DefaultTokenLifespanDays,
},
},
// If left unspecified, the port is later filled in with 8080 (no TLS) or 8443 (TLS).
Port: 0,
Expand Down Expand Up @@ -394,6 +409,13 @@ func (c *Config) Resolve() error {
c.SAML.GroupsAttributeName = ""
}

if c.Security.Token.MaxLifespanDays == InfiniteTokenLifespan {
c.Security.Token.MaxLifespanDays = MaxAllowedTokenMaxLifespan
}
if c.Security.Token.DefaultLifespanDays == InfiniteTokenLifespan {
c.Security.Token.DefaultLifespanDays = MaxAllowedTokenMaxLifespan
}

return nil
}

Expand Down Expand Up @@ -446,10 +468,67 @@ type SecurityConfig struct {
TLS TLSConfig `json:"tls"`
SSH SSHConfig `json:"ssh"`
AuthZ AuthZConfig `json:"authz"`
Token TokenConfig `json:"token"`

InitialUserPassword string `json:"initial_user_password"`
}

// TokenConfig is the configuration setting for tokens.
type TokenConfig struct {
MaxLifespanDays int `json:"max_lifespan_days"`
DefaultLifespanDays int `json:"default_lifespan_days"`
}

// GetMaxLifespan returns the parsed time.Duration for MaxLifespanDays.
func (t *TokenConfig) GetMaxLifespan() (time.Duration, error) {
return ParseLifespanDays(t.MaxLifespanDays)
}
ShreyaLnuHpe marked this conversation as resolved.
Show resolved Hide resolved

// GetDefaultLifespan returns the parsed time.Duration for DefaultLifespanDays.
func (t *TokenConfig) GetDefaultLifespan() (time.Duration, error) {
return ParseLifespanDays(t.DefaultLifespanDays)
}

// ParseLifespanDays parses a lifespan in days into either a time.Duration or nil if the lifespan is
// infinite.
func ParseLifespanDays(dayDuration int) (time.Duration, error) {
duration, err := time.ParseDuration(strconv.Itoa(dayDuration*24) + "h")
if err != nil {
return 0, err
}
return duration, nil
}

// Validate implements the check.Validatable interface for the TokenConfig.
func (t *TokenConfig) Validate() []error {
var errs []error
if t.MaxLifespanDays < 0 {
errs = append(errs, errors.New("max token lifespan must be greater than 0 days, unless"+
" set to -1 for infinite lifespan"),
)
}
if t.DefaultLifespanDays < 0 {
errs = append(errs, errors.New("default token lifespan must be greater than 0 days,"+
" unless set to -1 for infinite lifespan"),
)
}
if t.DefaultLifespanDays > t.MaxLifespanDays {
errs = append(errs, errors.New("default token lifespan must be less than max token"+
" lifespan"))
}
if t.MaxLifespanDays > MaxAllowedTokenMaxLifespan {
errs = append(errs, fmt.Errorf("max token lifespan should be less than %v, as per"+
" Go standards", MaxAllowedTokenMaxLifespan),
)
}
if t.DefaultLifespanDays > MaxAllowedTokenMaxLifespan {
errs = append(errs, fmt.Errorf("default token lifespan days should be less than %v, as per"+
" Go standards", MaxAllowedTokenMaxLifespan),
)
}
return errs
}

// SSHConfig is the configuration setting for SSH.
type SSHConfig struct {
RsaKeySize int `json:"rsa_key_size"`
Expand Down
12 changes: 5 additions & 7 deletions master/internal/token/postgres_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,11 @@ import (
"github.com/uptrace/bun"
"gopkg.in/guregu/null.v3"

"github.com/determined-ai/determined/master/internal/config"
"github.com/determined-ai/determined/master/internal/db"
"github.com/determined-ai/determined/master/pkg/model"
)

const (
// DefaultTokenLifespan is how long a newly created access token is valid.
DefaultTokenLifespan = 30 * 24 * time.Hour
)

// AccessTokenOption modifies a model.UserSession to apply optional settings to the AccessToken
// object.
type AccessTokenOption func(f *model.UserSession)
Expand All @@ -43,14 +39,16 @@ func WithTokenDescription(description string) AccessTokenOption {
// CreateAccessToken creates a new access token and store in
// user_sessions db.
func CreateAccessToken(
ctx context.Context, userID model.UserID, opts ...AccessTokenOption,
ctx context.Context,
userID model.UserID,
opts ...AccessTokenOption,
) (string, model.TokenID, error) {
now := time.Now().UTC()
// Populate the default values in the model.
accessToken := &model.UserSession{
UserID: userID,
CreatedAt: now,
Expiry: now.Add(DefaultTokenLifespan),
Expiry: now.Add(config.DefaultTokenLifespanDays * 24 * time.Hour),
TokenType: model.TokenTypeAccessToken,
Description: null.StringFromPtr(nil),
RevokedAt: null.Time{},
Expand Down
12 changes: 6 additions & 6 deletions master/internal/token/postgres_token_intg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/o1egl/paseto"
"github.com/stretchr/testify/require"

"github.com/determined-ai/determined/master/internal/config"
"github.com/determined-ai/determined/master/internal/db"
"github.com/determined-ai/determined/master/internal/user"
"github.com/determined-ai/determined/master/pkg/etc"
Expand Down Expand Up @@ -54,8 +55,7 @@ func TestCreateAccessToken(t *testing.T) {
require.NotNil(t, tokenID)

restoredToken := restoreTokenInfo(token, t)

expLifespan := DefaultTokenLifespan
expLifespan := config.DefaultTokenLifespanDays * 24 * time.Hour
actLifespan := restoredToken.Expiry.Sub(restoredToken.CreatedAt)
require.Equal(t, expLifespan, actLifespan)

Expand All @@ -78,7 +78,7 @@ func TestCreateAccessTokenHasExpiry(t *testing.T) {
require.NoError(t, err)

// Add a AccessToken with custom (Now() + 3 Months) Expiry Time.
expLifespan := DefaultTokenLifespan * 3
expLifespan := config.DefaultTokenLifespanDays * 24 * time.Hour
token, tokenID, err := CreateAccessToken(context.TODO(), testUser.ID,
WithTokenExpiry(&expLifespan), WithTokenDescription(desc))
require.NoError(t, err)
Expand Down Expand Up @@ -117,15 +117,15 @@ func TestUpdateAccessToken(t *testing.T) {

// Test before updating Access token
description := "description"
require.True(t, accessToken.RevokedAt.IsZero())
require.False(t, accessToken.Proto().Revoked)
require.NotEqual(t, description, accessToken.Description)

opt := AccessTokenUpdateOptions{Description: &description, SetRevoked: true}
tokenInfo, err := UpdateAccessToken(context.TODO(), model.TokenID(accessToken.ID), opt)
require.NoError(t, err)

// Test after updating access token
require.False(t, tokenInfo.RevokedAt.IsZero())
require.True(t, tokenInfo.Proto().Revoked)
require.Contains(t, description, tokenInfo.Description.String)

// Delete from DB by UserID for cleanup
Expand Down Expand Up @@ -228,8 +228,8 @@ func getAccessToken(ctx context.Context, userID model.UserID) ([]model.UserSessi
err := db.Bun().NewSelect().
Table("user_sessions").
Where("user_id = ?", userID).
Where("revoked_at IS NULL").
Where("token_type = ?", model.TokenTypeAccessToken).
Where("revoked_at IS NULL").
Scan(ctx, &tokenInfos)
if err != nil {
return nil, err
Expand Down
Binary file modified proto/buf.image.bin
Binary file not shown.
Loading
Loading