From ccf9a512bba46d8d70aa9e2be4c7cfcd7983fbf9 Mon Sep 17 00:00:00 2001 From: Ilija Matoski Date: Fri, 23 Feb 2024 10:37:54 +0100 Subject: [PATCH] feat: add gitlab revoke option so vault doesn't revoke the token when the parent expires (#52) --- README.md | 76 +++++++- entry_config.go | 13 +- entry_role.go | 30 +-- entry_token.go | 48 +++-- path_config.go | 32 +--- path_config_test.go | 109 +---------- path_config_token_autorotate_test.go | 17 +- path_role.go | 52 +++--- path_role_test.go | 269 ++++++--------------------- path_role_ttl_test.go | 193 +++++++++++++++++++ path_token_role.go | 27 ++- path_token_role_test.go | 77 +++++--- secret_access_tokens.go | 48 ++--- utils.go | 13 +- utils_test.go | 75 ++++++++ 15 files changed, 608 insertions(+), 471 deletions(-) create mode 100644 path_role_ttl_test.go diff --git a/README.md b/README.md index d217ffa..ad46eb6 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,65 @@ Before we can use this plugin we need to create an access token that will have r The current authentication model requires providing Vault with a Gitlab Token. +## Configuration + +### Config + +| Property | Required | Default value | Sensitive | Description | +|:-------------------------:|:--------:|:-------------:|:---------:|:----------------------------------------------------------------------------------------------------------------------------------| +| token | yes | n/a | yes | The token to access Gitlab API | +| base_url | yes | n/a | no | The address to access Gitlab | +| auto_rotate_token | no | no | no | Should we autorotate the token when it's close to expiry? (Experimental) | +| revoke_auto_rotated_token | no | no | no | Should we revoke the auto-rotated token after a new one has been generated? | +| auto_rotate_before | no | 24h | no | How much time should be remaining on the token validity before we should rotate it? Minimum can be set to 24h and maximum to 730h | + +### Role + +| Property | Required | Default value | Sensitive | Description | +|:--------------------:|:--------:|:-------------:|:---------:|:---------------------------------------------------------------------------------------------------------------------| +| path | yes | n/a | no | Project/Group path to create an access token for. If the token type is set to personal then write the username here. | +| name | yes | n/a | no | The name of the access token | +| ttl | yes | n/a | no | The TTL of the token | +| access_level | no/yes | n/a | no | Access level of access token (only required for Group and Project access tokens) | +| scopes | no | [] | no | List of scopes | +| token_type | yes | n/a | no | Access token type | +| gitlab_revokes_token | no | no | no | Gitlab revokes the token when it's time. Vault will not revoke the token when the lease expires | + +#### ttl + +Depending on `gitlab_revokes_token` the TTL will change. + +* `true` - 24h <= ttl <= 365 days +* `false` - 1h <= ttl <= 365 days + +#### access_level + +It's not required if `token_type` is set to `personal`. + +For a list of available roles check https://docs.gitlab.com/ee/user/permissions.html + +#### scopes + +Depending on the type of token you have different scopes: + +* `Personal` - https://docs.gitlab.com/ee/user/profile/personal_access_tokens.html#personal-access-token-scopes +* `Project` - https://docs.gitlab.com/ee/user/project/settings/project_access_tokens.html#scopes-for-a-project-access-token +* `Group` - https://docs.gitlab.com/ee/user/group/settings/group_access_tokens.html#scopes-for-a-group-access-token + +#### token_types + +Can be + +* personal +* project +* group + +#### gitlab_revokes_token + +This is a flag that doesn't expire the token when the token used to create the credentials expire. +When the vault token used to create gitlab credentials with a TTL longer than the vault token, the new gitlab credentials will expire at the same time with the parent. +Setting this up will not call the revoke endpoint on gitlab. + ## Examples ### Setup @@ -56,16 +115,21 @@ Due to how Gitlab manages expiration the minimum is 24h and maximum is 365 days. [Remove ability to create deprecated non-expiring access tokens](https://gitlab.com/gitlab-org/gitlab/-/issues/392855). Since Gitlab 16.0 the ability to create non expiring token has been removed. +If you use Vault to manage the tokens the minimal TTL you can use is `1h`, by setting `gitlab_revokes_token=false`. + The command bellow will set up the config backend with a max TTL of 48h. ```shell -$ vault write gitlab/config max_ttl=48h base_url=https://gitlab.example.com token=gitlab-super-secret-token +$ vault write gitlab/config base_url=https://gitlab.example.com token=gitlab-super-secret-token ``` -You may also need to configure the Max TTL for a token that can be issued by setting: +You may also need to configure the Max/Default TTL for a token that can be issued by setting: + +Max TTL: 1 year +Default TTL: 1 week ```shell -$ vault secrets tune -max-lease-ttl=8784h gitlab/ +$ vault secrets tune -max-lease-ttl=8784h -default-lease-ttl=168h gitlab/ ``` Check https://developer.hashicorp.com/vault/docs/commands/secrets/tune for more information. @@ -76,11 +140,11 @@ This will create three roles, one of each type. ```shell # personal access tokens can only be created by Gitlab Administrators (see https://docs.gitlab.com/ee/api/users.html#create-a-personal-access-token) -$ vault write gitlab/roles/personal name=personal-token-name path=username scopes="read_api" token_type=personal token_ttl=24h +$ vault write gitlab/roles/personal name=personal-token-name path=username scopes="read_api" token_type=personal ttl=48h -$ vault write gitlab/roles/project name=project-token-name path=group/project scopes="read_api" access_level=guest token_type=project token_ttl=24h +$ vault write gitlab/roles/project name=project-token-name path=group/project scopes="read_api" access_level=guest token_type=project ttl=48h -$ vault write gitlab/roles/group name=group-token-name path=group/subgroup scopes="read_api" access_level=developer token_type=group token_ttl=24h +$ vault write gitlab/roles/group name=group-token-name path=group/subgroup scopes="read_api" access_level=developer token_type=group ttl=48h ``` ### Get access tokens diff --git a/entry_config.go b/entry_config.go index d1daebb..fa9e78e 100644 --- a/entry_config.go +++ b/entry_config.go @@ -10,7 +10,6 @@ import ( type EntryConfig struct { BaseURL string `json:"base_url" structs:"base_url" mapstructure:"base_url"` Token string `json:"token" structs:"token" mapstructure:"token"` - MaxTTL time.Duration `json:"max_ttl" structs:"max_ttl" mapstructure:"max_ttl"` AutoRotateToken bool `json:"auto_rotate_token" structs:"auto_rotate_token" mapstructure:"auto_rotate_token"` AutoRotateBefore time.Duration `json:"auto_rotate_before" structs:"auto_rotate_before" mapstructure:"auto_rotate_before"` TokenExpiresAt time.Time `json:"token_expires_at" structs:"token_expires_at" mapstructure:"token_expires_at"` @@ -24,13 +23,11 @@ func (e EntryConfig) LogicalResponseData() map[string]any { } return map[string]any{ - "max_ttl": int64(e.MaxTTL / time.Second), - "base_url": e.BaseURL, - "token": e.Token, - "auto_rotate_token": e.AutoRotateToken, - "auto_rotate_before": e.AutoRotateBefore.String(), - "token_expires_at": tokenExpiresAt, - "revoke_auto_rotated_token": e.RevokeAutoRotatedToken, + "base_url": e.BaseURL, + "token": e.Token, + "auto_rotate_token": e.AutoRotateToken, + "auto_rotate_before": e.AutoRotateBefore.String(), + "token_expires_at": tokenExpiresAt, } } diff --git a/entry_role.go b/entry_role.go index bc321b8..7589a63 100644 --- a/entry_role.go +++ b/entry_role.go @@ -9,24 +9,26 @@ import ( ) type entryRole struct { - RoleName string `json:"role_name" structs:"role_name" mapstructure:"role_name"` - TokenTTL time.Duration `json:"token_ttl" structs:"token_ttl" mapstructure:"token_ttl"` - Path string `json:"path" structs:"path" mapstructure:"path"` - Name string `json:"name" structs:"name" mapstructure:"name"` - Scopes []string `json:"scopes" structs:"scopes" mapstructure:"scopes"` - AccessLevel AccessLevel `json:"access_level" structs:"access_level" mapstructure:"access_level,omitempty"` - TokenType TokenType `json:"token_type" structs:"token_type" mapstructure:"token_type"` + RoleName string `json:"role_name" structs:"role_name" mapstructure:"role_name"` + TTL time.Duration `json:"ttl" structs:"ttl" mapstructure:"ttl"` + Path string `json:"path" structs:"path" mapstructure:"path"` + Name string `json:"name" structs:"name" mapstructure:"name"` + Scopes []string `json:"scopes" structs:"scopes" mapstructure:"scopes"` + AccessLevel AccessLevel `json:"access_level" structs:"access_level" mapstructure:"access_level,omitempty"` + TokenType TokenType `json:"token_type" structs:"token_type" mapstructure:"token_type"` + GitlabRevokesTokens bool `json:"gitlab_revokes_token" structs:"gitlab_revokes_token" mapstructure:"gitlab_revokes_token"` } func (e entryRole) LogicalResponseData() map[string]any { return map[string]any{ - "role_name": e.RoleName, - "path": e.Path, - "name": e.Name, - "scopes": e.Scopes, - "access_level": e.AccessLevel.String(), - "token_ttl": int64(e.TokenTTL / time.Second), - "token_type": e.TokenType.String(), + "role_name": e.RoleName, + "path": e.Path, + "name": e.Name, + "scopes": e.Scopes, + "access_level": e.AccessLevel.String(), + "ttl": int64(e.TTL / time.Second), + "token_type": e.TokenType.String(), + "gitlab_revokes_token": e.GitlabRevokesTokens, } } diff --git a/entry_token.go b/entry_token.go index 8f07020..6e8d4c6 100644 --- a/entry_token.go +++ b/entry_token.go @@ -1,19 +1,24 @@ package gitlab -import "time" +import ( + "strconv" + "time" +) type EntryToken struct { - TokenID int `json:"token_id"` - UserID int `json:"user_id"` - ParentID string `json:"parent_id"` - Path string `json:"path"` - Name string `json:"name"` - Token string `json:"token"` - TokenType TokenType `json:"token_type"` - CreatedAt *time.Time `json:"created_at"` - ExpiresAt *time.Time `json:"expires_at"` - Scopes []string `json:"scopes"` - AccessLevel AccessLevel `json:"access_level"` // not used for personal access tokens + TokenID int `json:"token_id"` + UserID int `json:"user_id"` + ParentID string `json:"parent_id"` + Path string `json:"path"` + Name string `json:"name"` + Token string `json:"token"` + TokenType TokenType `json:"token_type"` + CreatedAt *time.Time `json:"created_at"` + ExpiresAt *time.Time `json:"expires_at"` + Scopes []string `json:"scopes"` + AccessLevel AccessLevel `json:"access_level"` // not used for personal access tokens + RoleName string `json:"role_name"` + GitlabRevokesToken bool `json:"gitlab_revokes_token"` } func (e EntryToken) SecretResponse() (map[string]any, map[string]any) { @@ -22,18 +27,21 @@ func (e EntryToken) SecretResponse() (map[string]any, map[string]any) { "token": e.Token, "path": e.Path, "scopes": e.Scopes, + "role_name": e.RoleName, "access_level": e.AccessLevel.String(), "created_at": e.CreatedAt, "expires_at": e.ExpiresAt, }, map[string]any{ - "path": e.Path, - "name": e.Name, - "user_id": e.UserID, - "parent_id": e.ParentID, - "token_id": e.TokenID, - "token_type": e.TokenType.String(), - "scopes": e.Scopes, - "access_level": e.AccessLevel.String(), + "path": e.Path, + "name": e.Name, + "user_id": e.UserID, + "parent_id": e.ParentID, + "token_id": e.TokenID, + "token_type": e.TokenType.String(), + "scopes": e.Scopes, + "access_level": e.AccessLevel.String(), + "role_name": e.RoleName, + "gitlab_revokes_token": strconv.FormatBool(e.GitlabRevokesToken), } } diff --git a/path_config.go b/path_config.go index 291ae75..3cc121e 100644 --- a/path_config.go +++ b/path_config.go @@ -30,13 +30,8 @@ var ( }, "base_url": { Type: framework.TypeString, - Description: `The address to access Gitlab. Default is "https://gitlab.com".`, - Default: "https://gitlab.com", - }, - "max_ttl": { - Type: framework.TypeDurationSecond, - Description: `Maximum lifetime expected generated token will be valid for. If set to 0 it will be set for maximum 8670 hours`, - Default: DefaultConfigFieldAccessTokenMaxTTL, + Required: true, + Description: `The address to access Gitlab.`, }, "auto_rotate_token": { Type: framework.TypeBool, @@ -109,7 +104,6 @@ func (b *Backend) pathConfigRead(ctx context.Context, req *logical.Request, data func (b *Backend) pathConfigWrite(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { var warnings []string - var maxTtlRaw, maxTtlOk = data.GetOk("max_ttl") var autoTokenRotateRaw, autoTokenRotateTtlOk = data.GetOk("auto_rotate_before") var token, tokenOk = data.GetOk("token") var err error @@ -124,25 +118,6 @@ func (b *Backend) pathConfigWrite(ctx context.Context, req *logical.Request, dat RevokeAutoRotatedToken: data.Get("revoke_auto_rotated_token").(bool), } - if maxTtlOk { - maxTtl := maxTtlRaw.(int) - switch { - case maxTtl > 0 && maxTtl < int(DefaultAccessTokenMinTTL.Seconds()): - warnings = append(warnings, "max_ttl is set with less than 24 hours. With current token expiry limitation, this max_ttl is ignored, it's set to 24 hours") - config.MaxTTL = DefaultAccessTokenMinTTL - case maxTtl <= 0: - config.MaxTTL = DefaultAccessTokenMaxPossibleTTL - warnings = append(warnings, "max_ttl is not set. Token wil be generated with expiration date of '8760 hours'") - case maxTtl > int(DefaultAccessTokenMaxPossibleTTL.Seconds()): - warnings = append(warnings, "max_ttl is set to more than '8760 hours'. Token wil be generated with expiration date of '8760 hours'") - config.MaxTTL = DefaultAccessTokenMaxPossibleTTL - default: - config.MaxTTL = time.Duration(maxTtl) * time.Second - } - } else if config.MaxTTL == 0 { - config.MaxTTL = DefaultAccessTokenMaxPossibleTTL - } - if autoTokenRotateTtlOk { atr, _ := convertToInt(autoTokenRotateRaw) if atr > int(DefaultAutoRotateBeforeMaxTTL.Seconds()) { @@ -173,7 +148,6 @@ func (b *Backend) pathConfigWrite(ctx context.Context, req *logical.Request, dat event(ctx, b.Backend, "config-write", map[string]string{ "path": "config", - "max_ttl": config.MaxTTL.String(), "auto_rotate_token": strconv.FormatBool(config.AutoRotateToken), "auto_rotate_before": config.AutoRotateBefore.String(), "base_url": config.BaseURL, @@ -181,7 +155,7 @@ func (b *Backend) pathConfigWrite(ctx context.Context, req *logical.Request, dat }) b.SetClient(nil) - b.Logger().Debug("Wrote new config", "base_url", config.BaseURL, "max_ttl", config.MaxTTL) + b.Logger().Debug("Wrote new config", "base_url", config.BaseURL) return &logical.Response{ Data: config.LogicalResponseData(), Warnings: warnings, diff --git a/path_config_test.go b/path_config_test.go index 3de4479..d87d776 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -3,13 +3,13 @@ package gitlab_test import ( "context" "testing" - "time" "github.com/hashicorp/go-multierror" "github.com/hashicorp/vault/sdk/logical" - gitlab "github.com/ilijamt/vault-plugin-secrets-gitlab" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + gitlab "github.com/ilijamt/vault-plugin-secrets-gitlab" ) func TestPathConfig(t *testing.T) { @@ -48,8 +48,8 @@ func TestPathConfig(t *testing.T) { Operation: logical.UpdateOperation, Path: gitlab.PathConfigStorage, Storage: l, Data: map[string]any{ - "token": "token", - "max_ttl": int((32 * time.Hour).Seconds()), + "token": "token", + "base_url": "https://gitlab.com", }, }) @@ -67,7 +67,6 @@ func TestPathConfig(t *testing.T) { require.NoError(t, resp.Error()) assert.EqualValues(t, "token", resp.Data["token"]) assert.NotEmpty(t, resp.Data["base_url"]) - assert.EqualValues(t, int((32 * time.Hour).Seconds()), resp.Data["max_ttl"]) require.Len(t, events.eventsProcessed, 1) resp, err = b.HandleRequest(context.Background(), &logical.Request{ @@ -113,104 +112,4 @@ func TestPathConfig(t *testing.T) { assert.EqualValues(t, 1, errorMap[gitlab.ErrFieldRequired.Error()]) require.Len(t, errorMap, 1) }) - - t.Run("if max_ttl is less than 24h, should be set to 24h", func(t *testing.T) { - b, l, err := getBackend() - require.NoError(t, err) - - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: gitlab.PathConfigStorage, Storage: l, - Data: map[string]any{ - "token": "token", - "max_ttl": time.Hour * 23, - }, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NotEmpty(t, resp.Warnings) - require.NoError(t, resp.Error()) - - assert.EqualValues(t, (time.Hour * 24).Seconds(), resp.Data["max_ttl"]) - }) - - t.Run("if max_ttl is 0 or less than 0 set max_ttl to 8670 hours", func(t *testing.T) { - b, l, err := getBackend() - require.NoError(t, err) - - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: gitlab.PathConfigStorage, Storage: l, - Data: map[string]any{ - "token": "token", - "max_ttl": 0, - }, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NotEmpty(t, resp.Warnings) - require.NoError(t, resp.Error()) - - assert.EqualValues(t, (365 * 24 * time.Hour).Seconds(), resp.Data["max_ttl"]) - }) - - t.Run("if max_ttl is 0 or less than 0 set max_ttl to 8670 hours", func(t *testing.T) { - b, l, err := getBackend() - require.NoError(t, err) - - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: gitlab.PathConfigStorage, Storage: l, - Data: map[string]any{ - "token": "token", - "max_ttl": 0, - }, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NotEmpty(t, resp.Warnings) - require.NoError(t, resp.Error()) - - assert.EqualValues(t, (365 * 24 * time.Hour).Seconds(), resp.Data["max_ttl"]) - }) - - t.Run("if max_ttl is more than 8670 hours set max_ttl to 8670 hours", func(t *testing.T) { - b, l, err := getBackend() - require.NoError(t, err) - - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: gitlab.PathConfigStorage, Storage: l, - Data: map[string]any{ - "token": "token", - "max_ttl": 366 * 24 * time.Hour, - }, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NotEmpty(t, resp.Warnings) - require.NoError(t, resp.Error()) - - assert.EqualValues(t, (365 * 24 * time.Hour).Seconds(), resp.Data["max_ttl"]) - }) - - t.Run("if max_ttl is between 24h and 8670h", func(t *testing.T) { - b, l, err := getBackend() - require.NoError(t, err) - - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: gitlab.PathConfigStorage, Storage: l, - Data: map[string]any{ - "token": "token", - "max_ttl": 14 * 24 * time.Hour, - }, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NoError(t, resp.Error()) - - assert.EqualValues(t, (14 * 24 * time.Hour).Seconds(), resp.Data["max_ttl"]) - }) - } diff --git a/path_config_token_autorotate_test.go b/path_config_token_autorotate_test.go index 82d878e..fa8e023 100644 --- a/path_config_token_autorotate_test.go +++ b/path_config_token_autorotate_test.go @@ -6,9 +6,10 @@ import ( "time" "github.com/hashicorp/vault/sdk/logical" - gitlab "github.com/ilijamt/vault-plugin-secrets-gitlab" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + gitlab "github.com/ilijamt/vault-plugin-secrets-gitlab" ) func TestPathConfig_AutoRotate(t *testing.T) { @@ -35,7 +36,7 @@ func TestPathConfig_AutoRotate(t *testing.T) { Path: gitlab.PathConfigStorage, Storage: l, Data: map[string]any{ "token": "super-secret-token", - "max_ttl": "48h", + "base_url": "https://gitlab.com", "auto_rotate_before": "2h", }, }) @@ -52,7 +53,7 @@ func TestPathConfig_AutoRotate(t *testing.T) { Path: gitlab.PathConfigStorage, Storage: l, Data: map[string]any{ "token": "super-secret-token", - "max_ttl": "48h", + "base_url": "https://gitlab.com", "auto_rotate_before": (gitlab.DefaultAutoRotateBeforeMaxTTL + time.Hour).String(), }, }) @@ -68,7 +69,7 @@ func TestPathConfig_AutoRotate(t *testing.T) { Path: gitlab.PathConfigStorage, Storage: l, Data: map[string]any{ "token": "super-secret-token", - "max_ttl": "48h", + "base_url": "https://gitlab.com", "auto_rotate_before": "48h", }, }) @@ -85,7 +86,7 @@ func TestPathConfig_AutoRotate(t *testing.T) { Path: gitlab.PathConfigStorage, Storage: l, Data: map[string]any{ "token": "super-secret-token", - "max_ttl": "48h", + "base_url": "https://gitlab.com", "auto_rotate_before": (gitlab.DefaultAutoRotateBeforeMinTTL - time.Hour).String(), }, }) @@ -100,8 +101,8 @@ func TestPathConfig_AutoRotate(t *testing.T) { Operation: logical.UpdateOperation, Path: gitlab.PathConfigStorage, Storage: l, Data: map[string]any{ - "token": "super-secret-token", - "max_ttl": "48h", + "token": "super-secret-token", + "base_url": "https://gitlab.com", }, }) require.NoError(t, err) @@ -117,7 +118,7 @@ func TestPathConfig_AutoRotate(t *testing.T) { Path: gitlab.PathConfigStorage, Storage: l, Data: map[string]any{ "token": "super-secret-token", - "max_ttl": "48h", + "base_url": "https://gitlab.com", "auto_rotate_before": "10h", }, }) diff --git a/path_role.go b/path_role.go index d8e6f18..b51f423 100644 --- a/path_role.go +++ b/path_role.go @@ -53,11 +53,10 @@ var ( }, AllowedValues: allowedValues(append(validTokenScopes, ValidPersonalTokenScopes...)...), }, - "token_ttl": { + "ttl": { Type: framework.TypeDurationSecond, Description: "The TTL of the token", - Required: false, - Default: DefaultRoleFieldAccessTokenMaxTTL, + Required: true, DisplayAttrs: &framework.DisplayAttributes{ Name: "Token TTL", }, @@ -80,6 +79,15 @@ var ( Name: "Token Type", }, }, + "gitlab_revokes_token": { + Type: framework.TypeBool, + Default: false, + Required: false, + Description: `Gitlab revokes the token when it's time. Vault will not revoke the token when the lease expires.`, + DisplayAttrs: &framework.DisplayAttributes{ + Name: "Gitlab revokes token.", + }, + }, } ) @@ -206,13 +214,14 @@ func (b *Backend) pathRolesWrite(ctx context.Context, req *logical.Request, data accessLevel, _ = AccessLevelParse(data.Get("access_level").(string)) var role = entryRole{ - RoleName: roleName, - TokenTTL: time.Duration(data.Get("token_ttl").(int)) * time.Second, - Path: data.Get("path").(string), - Name: data.Get("name").(string), - Scopes: data.Get("scopes").([]string), - AccessLevel: accessLevel, - TokenType: tokenType, + RoleName: roleName, + TTL: time.Duration(data.Get("ttl").(int)) * time.Second, + Path: data.Get("path").(string), + Name: data.Get("name").(string), + Scopes: data.Get("scopes").([]string), + AccessLevel: accessLevel, + TokenType: tokenType, + GitlabRevokesTokens: data.Get("gitlab_revokes_token").(bool), } // validate token type @@ -223,7 +232,8 @@ func (b *Backend) pathRolesWrite(ctx context.Context, req *logical.Request, data // check if all required fields are set for name, field := range fieldSchemaRoles { val, ok, _ := data.GetOkErr(name) - if tokenType == TokenTypePersonal && name == "access_level" { + if (tokenType == TokenTypePersonal && name == "access_level") || + name == "gitlab_revokes_token" { continue } if field.Required && !ok { @@ -233,16 +243,16 @@ func (b *Backend) pathRolesWrite(ctx context.Context, req *logical.Request, data } } - // validate ttl to make sure it's less than max ttl in config, and above 24h - if role.TokenTTL > 0 && role.TokenTTL < DefaultAccessTokenMinTTL { - warnings = append(warnings, "token_ttl is set with less than 24 hours. With current token expiry limitation, this token_ttl is ignored, it will be set to 24 hours") - role.TokenTTL = DefaultAccessTokenMinTTL - } else if role.TokenTTL > config.MaxTTL && config.MaxTTL > 0 { - warnings = append(warnings, fmt.Sprintf("token_ttl needs to be less than %s, setting 'token_ttl' to %s", config.MaxTTL, config.MaxTTL)) - role.TokenTTL = config.MaxTTL - } else if role.TokenTTL <= 0 { - role.TokenTTL = config.MaxTTL - warnings = append(warnings, fmt.Sprintf("token_ttl is set to 0. Tokens without ttls are not supported since Gitlab 16.0 setting to %d based on config max_ttl", config.MaxTTL)) + if role.TTL > DefaultAccessTokenMaxPossibleTTL { + err = multierror.Append(err, fmt.Errorf("ttl = %s [ttl <= max_ttl = %s]: %w", role.TTL.String(), DefaultAccessTokenMaxPossibleTTL, ErrInvalidValue)) + } + + if role.GitlabRevokesTokens && role.TTL < 24*time.Hour { + err = multierror.Append(err, fmt.Errorf("ttl = %s [%s <= ttl <= %s]: %w", role.TTL, DefaultAccessTokenMinTTL, DefaultAccessTokenMaxPossibleTTL, ErrInvalidValue)) + } + + if !role.GitlabRevokesTokens && role.TTL < time.Hour { + err = multierror.Append(err, fmt.Errorf("ttl = %s [ttl >= 1h]: %w", role.TTL, ErrInvalidValue)) } // validate access level diff --git a/path_role_test.go b/path_role_test.go index edfa190..983e722 100644 --- a/path_role_test.go +++ b/path_role_test.go @@ -8,9 +8,10 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/vault/sdk/logical" - gitlab "github.com/ilijamt/vault-plugin-secrets-gitlab" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + gitlab "github.com/ilijamt/vault-plugin-secrets-gitlab" ) func TestPathRolesList(t *testing.T) { @@ -64,11 +65,12 @@ func TestPathRoles(t *testing.T) { Operation: logical.CreateOperation, Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, Data: map[string]any{ - "path": "user", - "name": gitlab.TokenTypePersonal.String(), - "token_type": gitlab.TokenTypePersonal.String(), - "token_ttl": gitlab.DefaultAccessTokenMinTTL, - "scopes": gitlab.ValidPersonalTokenScopes, + "path": "user", + "name": gitlab.TokenTypePersonal.String(), + "token_type": gitlab.TokenTypePersonal.String(), + "ttl": gitlab.DefaultAccessTokenMinTTL, + "scopes": gitlab.ValidPersonalTokenScopes, + "gitlab_revokes_token": false, }, }) require.NoError(t, err) @@ -81,12 +83,13 @@ func TestPathRoles(t *testing.T) { Operation: logical.CreateOperation, Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, Data: map[string]any{ - "path": "user", - "name": gitlab.TokenTypePersonal.String(), - "access_level": gitlab.AccessLevelOwnerPermissions.String(), - "token_type": gitlab.TokenTypePersonal.String(), - "token_ttl": gitlab.DefaultAccessTokenMinTTL, - "scopes": gitlab.ValidPersonalTokenScopes, + "path": "user", + "name": gitlab.TokenTypePersonal.String(), + "access_level": gitlab.AccessLevelOwnerPermissions.String(), + "token_type": gitlab.TokenTypePersonal.String(), + "ttl": gitlab.DefaultAccessTokenMinTTL, + "scopes": gitlab.ValidPersonalTokenScopes, + "gitlab_revokes_token": false, }, }) require.Error(t, err) @@ -101,11 +104,12 @@ func TestPathRoles(t *testing.T) { Operation: logical.CreateOperation, Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, Data: map[string]any{ - "path": "user", - "name": gitlab.TokenTypeProject.String(), - "token_type": gitlab.TokenTypeProject.String(), - "token_ttl": gitlab.DefaultAccessTokenMinTTL, - "scopes": gitlab.ValidProjectTokenScopes, + "path": "user", + "name": gitlab.TokenTypeProject.String(), + "token_type": gitlab.TokenTypeProject.String(), + "ttl": gitlab.DefaultAccessTokenMinTTL, + "scopes": gitlab.ValidProjectTokenScopes, + "gitlab_revokes_token": false, }, }) require.Error(t, err) @@ -117,12 +121,13 @@ func TestPathRoles(t *testing.T) { Operation: logical.CreateOperation, Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, Data: map[string]any{ - "path": "user", - "name": gitlab.TokenTypeProject.String(), - "access_level": gitlab.AccessLevelOwnerPermissions.String(), - "token_type": gitlab.TokenTypeProject.String(), - "token_ttl": gitlab.DefaultAccessTokenMinTTL, - "scopes": gitlab.ValidProjectTokenScopes, + "path": "user", + "name": gitlab.TokenTypeProject.String(), + "access_level": gitlab.AccessLevelOwnerPermissions.String(), + "token_type": gitlab.TokenTypeProject.String(), + "ttl": gitlab.DefaultAccessTokenMinTTL, + "scopes": gitlab.ValidProjectTokenScopes, + "gitlab_revokes_token": false, }, }) require.NoError(t, err) @@ -138,11 +143,12 @@ func TestPathRoles(t *testing.T) { Operation: logical.CreateOperation, Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, Data: map[string]any{ - "path": "user", - "name": gitlab.TokenTypeGroup.String(), - "token_type": gitlab.TokenTypeGroup.String(), - "token_ttl": gitlab.DefaultAccessTokenMinTTL, - "scopes": gitlab.ValidGroupTokenScopes, + "path": "user", + "name": gitlab.TokenTypeGroup.String(), + "token_type": gitlab.TokenTypeGroup.String(), + "ttl": gitlab.DefaultAccessTokenMinTTL, + "scopes": gitlab.ValidGroupTokenScopes, + "gitlab_revokes_token": false, }, }) require.Error(t, err) @@ -154,12 +160,13 @@ func TestPathRoles(t *testing.T) { Operation: logical.CreateOperation, Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, Data: map[string]any{ - "path": "user", - "name": gitlab.TokenTypeGroup.String(), - "access_level": gitlab.AccessLevelOwnerPermissions.String(), - "token_type": gitlab.TokenTypeGroup.String(), - "token_ttl": gitlab.DefaultAccessTokenMinTTL, - "scopes": gitlab.ValidGroupTokenScopes, + "path": "user", + "name": gitlab.TokenTypeGroup.String(), + "access_level": gitlab.AccessLevelOwnerPermissions.String(), + "token_type": gitlab.TokenTypeGroup.String(), + "ttl": gitlab.DefaultAccessTokenMinTTL, + "scopes": gitlab.ValidGroupTokenScopes, + "gitlab_revokes_token": false, }, }) require.NoError(t, err) @@ -183,7 +190,7 @@ func TestPathRoles(t *testing.T) { require.NotNil(t, resp) require.Error(t, resp.Error()) var errorMap = countErrByName(err.(*multierror.Error)) - assert.EqualValues(t, 3, errorMap[gitlab.ErrFieldRequired.Error()]) + assert.EqualValues(t, 4, errorMap[gitlab.ErrFieldRequired.Error()]) assert.EqualValues(t, 2, errorMap[gitlab.ErrFieldInvalidValue.Error()]) }) @@ -198,6 +205,7 @@ func TestPathRoles(t *testing.T) { "path": "user", "name": "Example user personal token", "access_level": gitlab.AccessLevelOwnerPermissions.String(), + "ttl": "48h", "token_type": gitlab.TokenTypeProject.String(), "scopes": gitlab.ValidProjectTokenScopes, }, @@ -213,11 +221,13 @@ func TestPathRoles(t *testing.T) { Operation: logical.CreateOperation, Path: fmt.Sprintf("%s/%d", gitlab.PathRoleStorage, time.Now().UnixNano()), Storage: l, Data: map[string]any{ - "path": "user", - "name": "Example project personal token", - "access_level": gitlab.AccessLevelOwnerPermissions.String(), - "token_type": gitlab.TokenTypeProject.String(), - "scopes": gitlab.ValidPersonalTokenScopes, + "path": "user", + "name": "Example project personal token", + "access_level": gitlab.AccessLevelOwnerPermissions.String(), + "token_type": gitlab.TokenTypeProject.String(), + "ttl": "48h", + "scopes": gitlab.ValidPersonalTokenScopes, + "gitlab_revokes_token": false, }, }) require.Error(t, err) @@ -237,6 +247,7 @@ func TestPathRoles(t *testing.T) { Data: map[string]any{ "path": "user", "name": "Example user personal token", + "ttl": "48h", "token_type": gitlab.TokenTypePersonal.String(), "scopes": gitlab.ValidPersonalTokenScopes, }, @@ -277,6 +288,7 @@ func TestPathRoles(t *testing.T) { Data: map[string]any{ "path": "user", "name": "Example user personal token", + "ttl": "48h", "access_level": gitlab.AccessLevelOwnerPermissions.String(), "token_type": gitlab.TokenTypeGroup.String(), "scopes": gitlab.ValidProjectTokenScopes, @@ -307,163 +319,6 @@ func TestPathRoles(t *testing.T) { }) }) - t.Run("24h > TokenTTL > MaxTTL (10 days)", func(t *testing.T) { - var b, l, err = getBackend() - require.NoError(t, err) - - // create a configuration with max ttl set to 10 days - func() { - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: gitlab.PathConfigStorage, Storage: l, - Data: map[string]any{ - "max_ttl": (10 * 24 * time.Hour).Seconds(), - "token": "token", - }, - }) - require.NoError(t, err) - require.NotNil(t, resp) - }() - - var roleData = map[string]any{ - "path": "user", - "name": "Example user personal token", - "token_type": gitlab.TokenTypePersonal.String(), - "token_ttl": int64((12 * 24 * time.Hour).Seconds()), - "scopes": []string{ - gitlab.TokenScopeApi.String(), - gitlab.TokenScopeReadRegistry.String(), - }, - } - - // create a role - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.CreateOperation, - Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, - Data: roleData, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NotEmpty(t, resp.Warnings) - - // read a role - resp, err = b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.ReadOperation, - Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NoError(t, resp.Error()) - require.EqualValues(t, (10 * 24 * time.Hour).Seconds(), resp.Data["token_ttl"]) - }) - - t.Run("0 > TokenTTL > 24h", func(t *testing.T) { - var b, l, err = getBackendWithConfig(defaultConfig) - require.NoError(t, err) - - var roleData = map[string]any{ - "path": "user", - "name": "Example user personal token", - "token_type": gitlab.TokenTypePersonal.String(), - "token_ttl": (12 * time.Hour).Seconds(), - "scopes": []string{ - gitlab.TokenScopeApi.String(), - gitlab.TokenScopeReadRegistry.String(), - }, - } - - // create a role - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.CreateOperation, - Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, - Data: roleData, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NotEmpty(t, resp.Warnings) - - // read a role - resp, err = b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.ReadOperation, - Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NoError(t, resp.Error()) - require.EqualValues(t, (24 * time.Hour).Seconds(), resp.Data["token_ttl"]) - }) - - t.Run("not set token_ttl should default to 24h", func(t *testing.T) { - var b, l, err = getBackendWithConfig(defaultConfig) - require.NoError(t, err) - - var roleData = map[string]any{ - "path": "user", - "name": "Example user personal token", - "token_type": gitlab.TokenTypePersonal.String(), - "scopes": []string{ - gitlab.TokenScopeApi.String(), - gitlab.TokenScopeReadRegistry.String(), - }, - } - - // create a role - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.CreateOperation, - Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, - Data: roleData, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NotEmpty(t, resp.Warnings) - - // read a role - resp, err = b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.ReadOperation, - Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NoError(t, resp.Error()) - require.EqualValues(t, int64((24 * time.Hour).Seconds()), resp.Data["token_ttl"]) - }) - - t.Run("token_ttl set to 0 should default to config max_ttl", func(t *testing.T) { - var b, l, err = getBackendWithConfig(defaultConfig) - require.NoError(t, err) - - var roleData = map[string]any{ - "path": "user", - "name": "Example user personal token", - "token_type": gitlab.TokenTypePersonal.String(), - "token_ttl": 0, - "scopes": []string{ - gitlab.TokenScopeApi.String(), - gitlab.TokenScopeReadRegistry.String(), - }, - } - - // create a role - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.CreateOperation, - Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, - Data: roleData, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NotEmpty(t, resp.Warnings) - - // read a role - resp, err = b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.ReadOperation, - Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, - }) - require.NoError(t, err) - require.NotNil(t, resp) - require.NoError(t, resp.Error()) - require.EqualValues(t, int64((365 * 24 * time.Hour).Seconds()), resp.Data["token_ttl"]) - }) - t.Run("update handler existence check", func(t *testing.T) { var b, l, err = getBackend() require.NoError(t, err) @@ -487,8 +342,7 @@ func TestPathRoles(t *testing.T) { Operation: logical.UpdateOperation, Path: gitlab.PathConfigStorage, Storage: l, Data: map[string]any{ - "max_ttl": (10 * 24 * time.Hour).Seconds(), - "token": "token", + "token": "token", }, }) require.NoError(t, err) @@ -496,10 +350,11 @@ func TestPathRoles(t *testing.T) { }() var roleData = map[string]any{ - "path": "user", - "name": "Example user personal token", - "token_type": gitlab.TokenTypePersonal.String(), - "token_ttl": int64((5 * 24 * time.Hour).Seconds()), + "path": "user", + "name": "Example user personal token", + "token_type": gitlab.TokenTypePersonal.String(), + "ttl": int64((5 * 24 * time.Hour).Seconds()), + "gitlab_revokes_token": false, "scopes": []string{ gitlab.TokenScopeApi.String(), gitlab.TokenScopeReadRegistry.String(), @@ -526,8 +381,8 @@ func TestPathRoles(t *testing.T) { require.NotNil(t, resp) require.NoError(t, resp.Error()) require.EqualValues(t, "test", resp.Data["role_name"]) - require.Equal(t, int64((5 * 24 * time.Hour).Seconds()), resp.Data["token_ttl"]) - require.Subset(t, resp.Data, roleData) + require.Equal(t, int64((5 * 24 * time.Hour).Seconds()), resp.Data["ttl"]) + assert.Subset(t, resp.Data, roleData) // update a role roleData["name"] = "Example user personal token - updated" @@ -540,7 +395,7 @@ func TestPathRoles(t *testing.T) { require.NoError(t, err) require.NotNil(t, resp) require.NoError(t, resp.Error()) - require.Subset(t, resp.Data, roleData) + assert.Subset(t, resp.Data, roleData) require.EqualValues(t, "test", resp.Data["role_name"]) // read a role @@ -551,7 +406,7 @@ func TestPathRoles(t *testing.T) { require.NoError(t, err) require.NotNil(t, resp) require.NoError(t, resp.Error()) - require.Subset(t, resp.Data, roleData) + assert.Subset(t, resp.Data, roleData) require.EqualValues(t, "test", resp.Data["role_name"]) require.EqualValues(t, "user2", resp.Data["path"]) require.EqualValues(t, "Example user personal token - updated", resp.Data["name"]) diff --git a/path_role_ttl_test.go b/path_role_ttl_test.go new file mode 100644 index 0000000..55172ef --- /dev/null +++ b/path_role_ttl_test.go @@ -0,0 +1,193 @@ +package gitlab_test + +import ( + "context" + "fmt" + "maps" + "testing" + "time" + + "github.com/hashicorp/vault/sdk/logical" + "github.com/stretchr/testify/require" + + gitlab "github.com/ilijamt/vault-plugin-secrets-gitlab" +) + +func TestPathRolesTTL(t *testing.T) { + var defaultConfig = map[string]any{"token": "random-token"} + + t.Run("general ttl limits", func(t *testing.T) { + var b, l, err = getBackendWithConfig(defaultConfig) + require.NoError(t, err) + + var generalRole = map[string]any{ + "path": "user", + "name": "Example user personal token", + "token_type": gitlab.TokenTypePersonal.String(), + "scopes": []string{ + gitlab.TokenScopeApi.String(), + gitlab.TokenScopeReadRegistry.String(), + }, + "gitlab_revokes_token": false, + } + + t.Run("role.TTL > DefaultAccessTokenMaxPossibleTTL", func(t *testing.T) { + var role = maps.Clone(generalRole) + maps.Copy(role, map[string]any{ + "ttl": (gitlab.DefaultAccessTokenMaxPossibleTTL + time.Hour).String(), + }) + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.CreateOperation, + Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, + Data: role, + }) + require.Error(t, err) + require.ErrorIs(t, err, gitlab.ErrInvalidValue) + require.NotNil(t, resp) + require.True(t, resp.IsError()) + require.ErrorContains(t, resp.Error(), "ttl = 8761h0m0s [ttl <= max_ttl = 8760h0m0s]") + }) + + t.Run("ttl = maxTTL", func(t *testing.T) { + var role = maps.Clone(generalRole) + maps.Copy(role, map[string]any{ + "ttl": (gitlab.DefaultAccessTokenMaxPossibleTTL).String(), + }) + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.CreateOperation, + Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, + Data: role, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.Empty(t, resp.Warnings) + + // read a role + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.NoError(t, resp.Error()) + require.EqualValues(t, int64(gitlab.DefaultAccessTokenMaxPossibleTTL.Seconds()), resp.Data["ttl"]) + }) + }) + + t.Run("vault revokes the token", func(t *testing.T) { + var b, l, err = getBackendWithConfig(defaultConfig) + require.NoError(t, err) + + var generalRole = map[string]any{ + "path": "user", + "name": "Example user personal token", + "token_type": gitlab.TokenTypePersonal.String(), + "scopes": []string{ + gitlab.TokenScopeApi.String(), + gitlab.TokenScopeReadRegistry.String(), + }, + "gitlab_revokes_token": false, + } + + t.Run("ttl >= 1h && ttl <= DefaultAccessTokenMaxPossibleTTL", func(t *testing.T) { + var role = maps.Clone(generalRole) + maps.Copy(role, map[string]any{ + "ttl": "1h", + }) + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.CreateOperation, + Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, + Data: role, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.Empty(t, resp.Warnings) + + // read a role + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.NoError(t, resp.Error()) + require.EqualValues(t, int64((1 * time.Hour).Seconds()), resp.Data["ttl"]) + }) + + t.Run("ttl < 1h", func(t *testing.T) { + var role = maps.Clone(generalRole) + maps.Copy(role, map[string]any{ + "ttl": "59m59s", + }) + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.CreateOperation, + Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, + Data: role, + }) + require.Error(t, err) + require.ErrorIs(t, err, gitlab.ErrInvalidValue) + require.NotNil(t, resp) + require.True(t, resp.IsError()) + require.ErrorContains(t, resp.Error(), "ttl = 59m59s [ttl >= 1h]") + }) + }) + + t.Run("gitlab revokes the tokens", func(t *testing.T) { + var b, l, err = getBackendWithConfig(defaultConfig) + require.NoError(t, err) + + var generalRole = map[string]any{ + "path": "user", + "name": "Example user personal token", + "token_type": gitlab.TokenTypePersonal.String(), + "scopes": []string{ + gitlab.TokenScopeApi.String(), + gitlab.TokenScopeReadRegistry.String(), + }, + "gitlab_revokes_token": true, + } + + t.Run("ttl < 24h", func(t *testing.T) { + var role = maps.Clone(generalRole) + maps.Copy(role, map[string]any{ + "ttl": "23h59m59s", + }) + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.CreateOperation, + Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, + Data: role, + }) + require.Error(t, err) + require.ErrorIs(t, err, gitlab.ErrInvalidValue) + require.NotNil(t, resp) + require.True(t, resp.IsError()) + require.ErrorContains(t, resp.Error(), "ttl = 23h59m59s [24h0m0s <= ttl <= 8760h0m0s]") + }) + + t.Run("ttl >= 24h && ttl <= DefaultAccessTokenMaxPossibleTTL", func(t *testing.T) { + var role = maps.Clone(generalRole) + maps.Copy(role, map[string]any{ + "ttl": "24h", + }) + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.CreateOperation, + Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, + Data: role, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.Empty(t, resp.Warnings) + + // read a role + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.NoError(t, resp.Error()) + require.EqualValues(t, int64((24 * time.Hour).Seconds()), resp.Data["ttl"]) + }) + }) + +} diff --git a/path_token_role.go b/path_token_role.go index 5e8b3d9..48d45e6 100644 --- a/path_token_role.go +++ b/path_token_role.go @@ -57,8 +57,17 @@ func (b *Backend) pathTokenRoleCreate(ctx context.Context, req *logical.Request, _, _ = rand.Read(buf) var token *EntryToken var name = strings.ToLower(fmt.Sprintf("vault-generated-%s-access-token-%x", role.TokenType.String(), buf)) - var expiresAt = time.Now().Add(role.TokenTTL) + var expiresAt time.Time + var startTime = time.Now().UTC() + var client Client + var gitlabRevokesTokens = role.GitlabRevokesTokens + var vaultRevokesTokens = !role.GitlabRevokesTokens + + expiresAt = startTime.Add(role.TTL) + if gitlabRevokesTokens { + _, expiresAt, _ = calculateGitlabTTL(role.TTL, startTime) + } client, err = b.getClient(ctx, req.Storage) if err != nil { @@ -87,15 +96,27 @@ func (b *Backend) pathTokenRoleCreate(ctx context.Context, req *logical.Request, return logical.ErrorResponse("invalid token type"), fmt.Errorf("%s: %w", role.TokenType.String(), ErrUnknownTokenType) } + token.RoleName = role.RoleName + token.GitlabRevokesToken = role.GitlabRevokesTokens + + if vaultRevokesTokens { + token.ExpiresAt = &expiresAt + } + var secretData, secretInternal = token.SecretResponse() resp = b.Secret(secretAccessTokenType).Response(secretData, secretInternal) - resp.Secret.MaxTTL = role.TokenTTL - resp.Secret.TTL = token.ExpiresAt.Sub(*token.CreatedAt) + + resp.Secret.MaxTTL = role.TTL + resp.Secret.TTL = role.TTL + if gitlabRevokesTokens { + resp.Secret.TTL = token.ExpiresAt.Sub(*token.CreatedAt) + } event(ctx, b.Backend, "token-write", map[string]string{ "path": fmt.Sprintf("%s/%s", PathRoleStorage, roleName), "name": name, "parent_id": role.Path, + "ttl": resp.Secret.TTL.String(), "role_name": roleName, "token_id": strconv.Itoa(token.TokenID), "token_type": role.TokenType.String(), diff --git a/path_token_role_test.go b/path_token_role_test.go index eb1dc0c..1757a0d 100644 --- a/path_token_role_test.go +++ b/path_token_role_test.go @@ -3,11 +3,13 @@ package gitlab_test import ( "context" "fmt" + "strconv" "testing" "github.com/hashicorp/vault/sdk/logical" - gitlab "github.com/ilijamt/vault-plugin-secrets-gitlab" "github.com/stretchr/testify/require" + + gitlab "github.com/ilijamt/vault-plugin-secrets-gitlab" ) func TestPathTokenRoles(t *testing.T) { @@ -25,7 +27,8 @@ func TestPathTokenRoles(t *testing.T) { require.ErrorIs(t, err, gitlab.ErrRoleNotFound) }) - var generalTokenCreation = func(t *testing.T, tokenType gitlab.TokenType, level gitlab.AccessLevel) { + var generalTokenCreation = func(t *testing.T, tokenType gitlab.TokenType, level gitlab.AccessLevel, gitlabRevokesToken bool) { + t.Logf("token creation, token type: %s, level: %s, gitlab revokes token: %t", tokenType, level, gitlabRevokesToken) var b, l, events, err = getBackendWithEvents() require.NoError(t, err) require.NoError(t, writeBackendConfig(b, l, defaultConfig)) @@ -38,15 +41,22 @@ func TestPathTokenRoles(t *testing.T) { client := newInMemoryClient(true) b.SetClient(client) + var ttl = "1h" + if gitlabRevokesToken { + ttl = "48h" + } + // create a role resp, err := b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.CreateOperation, Path: fmt.Sprintf("%s/test", gitlab.PathRoleStorage), Storage: l, Data: map[string]any{ - "path": "user", - "name": tokenType.String(), - "token_type": tokenType.String(), - "access_level": level, + "path": "user", + "name": tokenType.String(), + "token_type": tokenType.String(), + "access_level": level, + "ttl": ttl, + "gitlab_revokes_token": strconv.FormatBool(gitlabRevokesToken), }, }) require.NoError(t, err) @@ -77,7 +87,12 @@ func TestPathTokenRoles(t *testing.T) { }) require.NoError(t, err) require.Nil(t, resp) - require.NotContains(t, client.accessTokens, fmt.Sprintf("%s_%v", tokenType.String(), tokenId)) + + if gitlabRevokesToken { + require.Contains(t, client.accessTokens, fmt.Sprintf("%s_%v", tokenType.String(), tokenId)) + } else { + require.NotContains(t, client.accessTokens, fmt.Sprintf("%s_%v", tokenType.String(), tokenId)) + } // calling revoke with nil secret resp, err = b.HandleRequest(context.Background(), &logical.Request{ @@ -87,40 +102,48 @@ func TestPathTokenRoles(t *testing.T) { require.Error(t, err) require.Nil(t, resp) - // calling revoke again would return a token not found in internal error - switch tokenType { - case gitlab.TokenTypeProject: - client.projectAccessTokenRevokeError = true - case gitlab.TokenTypePersonal: - client.personalAccessTokenRevokeError = true - case gitlab.TokenTypeGroup: - client.groupAccessTokenRevokeError = true + if !gitlabRevokesToken { + // calling revoke again would return a token not found in internal error + switch tokenType { + case gitlab.TokenTypeProject: + client.projectAccessTokenRevokeError = true + case gitlab.TokenTypePersonal: + client.personalAccessTokenRevokeError = true + case gitlab.TokenTypeGroup: + client.groupAccessTokenRevokeError = true + } + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.RevokeOperation, + Path: fmt.Sprintf("%s/%s", gitlab.PathTokenRoleStorage, leaseId), Storage: l, + Secret: secret, + }) + require.Error(t, err) + require.Error(t, resp.Error()) } - resp, err = b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.RevokeOperation, - Path: fmt.Sprintf("%s/%s", gitlab.PathTokenRoleStorage, leaseId), Storage: l, - Secret: secret, - }) - require.Error(t, err) - require.Error(t, resp.Error()) - events.expectEvents(t, []expectedEvent{ + var expectedEvents = []expectedEvent{ {eventType: "gitlab/config-write"}, {eventType: "gitlab/role-write"}, {eventType: "gitlab/token-write"}, {eventType: "gitlab/token-revoke"}, - }) + } + + events.expectEvents(t, expectedEvents) } t.Run("personal access token", func(t *testing.T) { - generalTokenCreation(t, gitlab.TokenTypePersonal, gitlab.AccessLevelUnknown) + generalTokenCreation(t, gitlab.TokenTypePersonal, gitlab.AccessLevelUnknown, false) + generalTokenCreation(t, gitlab.TokenTypePersonal, gitlab.AccessLevelUnknown, true) }) t.Run("project access token", func(t *testing.T) { - generalTokenCreation(t, gitlab.TokenTypeProject, gitlab.AccessLevelGuestPermissions) + generalTokenCreation(t, gitlab.TokenTypeProject, gitlab.AccessLevelGuestPermissions, false) + generalTokenCreation(t, gitlab.TokenTypeProject, gitlab.AccessLevelGuestPermissions, true) }) t.Run("group access token", func(t *testing.T) { - generalTokenCreation(t, gitlab.TokenTypeGroup, gitlab.AccessLevelGuestPermissions) + generalTokenCreation(t, gitlab.TokenTypeGroup, gitlab.AccessLevelGuestPermissions, false) + generalTokenCreation(t, gitlab.TokenTypeGroup, gitlab.AccessLevelGuestPermissions, true) }) + } diff --git a/secret_access_tokens.go b/secret_access_tokens.go index ceebbb4..5ad2df8 100644 --- a/secret_access_tokens.go +++ b/secret_access_tokens.go @@ -67,12 +67,6 @@ func (b *Backend) secretAccessTokenRevoke(ctx context.Context, req *logical.Requ return nil, fmt.Errorf("secret: %w", ErrNilValue) } - var client Client - client, err = b.getClient(ctx, req.Storage) - if err != nil { - return nil, fmt.Errorf("revoke token: %w", err) - } - var tokenId int tokenId, err = convertToInt(req.Secret.InternalData["token_id"]) if err != nil { @@ -82,32 +76,42 @@ func (b *Backend) secretAccessTokenRevoke(ctx context.Context, req *logical.Requ var parentId = req.Secret.InternalData["parent_id"].(string) var tokenType TokenType var tokenTypeValue = req.Secret.InternalData["token_type"].(string) - + var gitlabRevokesToken, _ = strconv.ParseBool(req.Secret.InternalData["gitlab_revokes_token"].(string)) + var vaultRevokesToken = !gitlabRevokesToken tokenType, err = TokenTypeParse(tokenTypeValue) if err != nil { // shouldn't be possible to hit due to the guards in the creation of the roles return nil, fmt.Errorf("%s: %w", tokenTypeValue, ErrUnknownTokenType) } - switch tokenType { - case TokenTypePersonal: - err = client.RevokePersonalAccessToken(tokenId) - case TokenTypeProject: - err = client.RevokeProjectAccessToken(tokenId, parentId) - case TokenTypeGroup: - err = client.RevokeGroupAccessToken(tokenId, parentId) - } + if vaultRevokesToken { + var client Client + client, err = b.getClient(ctx, req.Storage) + if err != nil { + return nil, fmt.Errorf("revoke token: %w", err) + } + + switch tokenType { + case TokenTypePersonal: + err = client.RevokePersonalAccessToken(tokenId) + case TokenTypeProject: + err = client.RevokeProjectAccessToken(tokenId, parentId) + case TokenTypeGroup: + err = client.RevokeGroupAccessToken(tokenId, parentId) + } - if err != nil && !errors.Is(err, ErrAccessTokenNotFound) { - return logical.ErrorResponse("failed to revoke token"), fmt.Errorf("revoke token: %w", err) + if err != nil && !errors.Is(err, ErrAccessTokenNotFound) { + return logical.ErrorResponse("failed to revoke token"), fmt.Errorf("revoke token: %w", err) + } } event(ctx, b.Backend, "token-revoke", map[string]string{ - "lease_id": secret.LeaseID, - "path": req.Secret.InternalData["path"].(string), - "name": req.Secret.InternalData["name"].(string), - "token_id": strconv.Itoa(tokenId), - "token_type": tokenTypeValue, + "lease_id": secret.LeaseID, + "path": req.Secret.InternalData["path"].(string), + "name": req.Secret.InternalData["name"].(string), + "token_id": strconv.Itoa(tokenId), + "token_type": tokenTypeValue, + "gitlab_revokes_token": strconv.FormatBool(gitlabRevokesToken), }) return nil, nil diff --git a/utils.go b/utils.go index dfe38b5..96490a7 100644 --- a/utils.go +++ b/utils.go @@ -1,6 +1,9 @@ package gitlab -import "fmt" +import ( + "fmt" + "time" +) func allowedValues(values ...string) (ret []any) { for _, value := range values { @@ -28,3 +31,11 @@ func convertToInt(num any) (int, error) { } return 0, fmt.Errorf("%v: %w", num, ErrInvalidValue) } + +func calculateGitlabTTL(duration time.Duration, start time.Time) (ttl time.Duration, exp time.Time, err error) { + const D = 24 * time.Hour + var val = start.Add(duration).Round(0) + exp = val.AddDate(0, 0, 1).Truncate(D) + ttl = exp.Sub(start.Round(0)) + return ttl, exp, nil +} diff --git a/utils_test.go b/utils_test.go index dc89e36..c682171 100644 --- a/utils_test.go +++ b/utils_test.go @@ -2,6 +2,7 @@ package gitlab import ( "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -31,3 +32,77 @@ func TestConvertToInt(t *testing.T) { } } } + +func TestCalculateGitlabTTL(t *testing.T) { + var tests = []struct { + inDuration time.Duration + inTime time.Time + outDuration time.Duration + outExpiry time.Time + outErr error + }{ + // 1h on 2024-02-22T13:06:10.575Z, should expire 2024-02-23 + { + inDuration: time.Hour, + inTime: time.Date(2024, 2, 22, 13, 6, 10, 0, time.UTC), + outDuration: (time.Hour * 10) + (53 * time.Minute) + (50 * time.Second), + outExpiry: time.Date(2024, 2, 23, 0, 0, 0, 0, time.UTC), + outErr: nil, + }, + // 3h + { + inDuration: 3 * time.Hour, + inTime: time.Date(2024, 2, 22, 13, 0, 0, 0, time.UTC), + outDuration: time.Hour * 11, + outExpiry: time.Date(2024, 2, 23, 0, 0, 0, 0, time.UTC), + outErr: nil, + }, + + // 1h1s + { + inDuration: time.Hour + time.Second, + inTime: time.Date(2024, 2, 22, 23, 0, 0, 0, time.UTC), + outDuration: time.Hour * 25, + outExpiry: time.Date(2024, 2, 24, 0, 0, 0, 0, time.UTC), + outErr: nil, + }, + + // 23h on 2024-02-22T20:00:00.000Z, should expire 2024-02-24 + { + inDuration: time.Hour * 23, + inTime: time.Date(2024, 2, 22, 20, 0, 0, 0, time.UTC), + outDuration: 28 * time.Hour, + outExpiry: time.Date(2024, 2, 24, 0, 0, 0, 0, time.UTC), + outErr: nil, + }, + + // 5h on 2024-02-22T20:00:00.000Z, should expire 2024-02-24 + { + inDuration: time.Hour * 5, + inTime: time.Date(2024, 2, 22, 20, 0, 0, 0, time.UTC), + outDuration: time.Hour * 28, + outExpiry: time.Date(2024, 2, 24, 0, 0, 0, 0, time.UTC), + outErr: nil, + }, + + // 45h on 2024-02-22T20:00:00.000Z, should expire 2024-02-25 + { + inDuration: time.Hour * 45, + inTime: time.Date(2024, 2, 22, 20, 0, 0, 0, time.UTC), + outDuration: time.Hour * 52, + outExpiry: time.Date(2024, 2, 25, 0, 0, 0, 0, time.UTC), + outErr: nil, + }, + } + + for _, tst := range tests { + t.Logf("calculateGitlabTTL(%s, %s) = duration %s, expiry %s, error %v", tst.inDuration, tst.inTime.Format(time.RFC3339), tst.outDuration, tst.outExpiry.Format(time.RFC3339), tst.outErr) + dur, exp, err := calculateGitlabTTL(tst.inDuration, tst.inTime) + if err != nil { + assert.ErrorIs(t, err, tst.outErr) + } + assert.EqualValues(t, tst.outExpiry, exp) + assert.WithinDuration(t, tst.outExpiry, exp, time.Minute) + assert.EqualValues(t, tst.outDuration, dur) + } +}