Skip to content

Commit

Permalink
Handle expired access token when reading user config
Browse files Browse the repository at this point in the history 
And attempt to refresh the token

Split off code for refreshing token from create token.

Move code that determine whether to use new or old API to earlier when config is being created. The value is then stored in the config for later use.

Use token ID only when revoking token

More logging with func context
  • Loading branch information
@alexhung
alexhung committed Nov 6, 2024
1 parent 42b8c2a commit 974c557
Show file tree
Hide file tree
Showing 11 changed files with 400 additions and 183 deletions.
420 changes: 294 additions & 126 deletions artifactory.go
View file

Large diffs are not rendered by default.

25 changes: 20 additions & 5 deletions path_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,11 @@ func (b *backend) pathConfigUpdate(ctx context.Context, req *logical.Request, da
}

if config == nil {
config = &adminConfiguration{}
config = &adminConfiguration{
baseConfiguration: baseConfiguration{
UseNewAccessAPI: true,
},
}
}

if val, ok := data.GetOk("url"); ok {
Expand Down Expand Up @@ -185,6 +189,8 @@ func (b *backend) pathConfigUpdate(ctx context.Context, req *logical.Request, da

go b.sendUsage(config.baseConfiguration, "pathConfigRotateUpdate")

config.UseNewAccessAPI = b.useNewAccessAPI(config.baseConfiguration)

entry, err := logical.StorageEntryJSON(configAdminPath, config)
if err != nil {
return nil, err
Expand Down Expand Up @@ -226,7 +232,7 @@ func (b *backend) pathConfigDelete(ctx context.Context, req *logical.Request, _
return nil, nil
}

err = b.RevokeToken(config.baseConfiguration, token.TokenID, config.AccessToken)
err = b.RevokeToken(config.baseConfiguration, token.TokenID)
if err != nil {
b.Logger().Warn("error revoking existing access token", "tokenId", token.TokenID, "err", err)
return nil, nil
Expand All @@ -240,6 +246,8 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, _ *f
b.configMutex.RLock()
defer b.configMutex.RUnlock()

logger := b.Logger().With("func", "pathConfigRead")

config, err := b.fetchAdminConfiguration(ctx, req.Storage)
if err != nil {
return nil, err
Expand All @@ -253,7 +261,8 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, _ *f

version, err := b.getVersion(config.baseConfiguration)
if err != nil {
b.Logger().Warn("failed to get system version", "err", err)
logger.Error("failed to get system version", "err", err)
return nil, err
}

// I'm not sure if I should be returning the access token, so I'll hash it.
Expand All @@ -278,7 +287,7 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, _ *f
// Optionally include token info if it parses properly
token, err := b.getTokenInfo(config.baseConfiguration, config.AccessToken)
if err != nil {
b.Logger().Warn("Error parsing AccessToken", "err", err.Error())
logger.Warn("Error parsing AccessToken", "err", err.Error())
} else {
configMap["token_id"] = token.TokenID
configMap["username"] = token.Username
Expand All @@ -290,7 +299,13 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, _ *f
}
}

if b.supportForceRevocable(config.baseConfiguration) {
supportForceRevocable, err := b.supportForceRevocable(config.baseConfiguration)
if err != nil {
logger.Warn("failed to determine if force_revocable is supported. Set 'supportForceRevocable' to 'false'.", "err", err)
supportForceRevocable = false
}

if supportForceRevocable {
configMap["use_expiring_tokens"] = config.UseExpiringTokens
}

Expand Down
2 changes: 1 addition & 1 deletion path_config_rotate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (b *backend) pathConfigRotateWrite(ctx context.Context, req *logical.Reques
}

// Invalidate Old Token
err = b.RevokeToken(config.baseConfiguration, token.TokenID, oldAccessToken)
err = b.RevokeToken(config.baseConfiguration, token.TokenID)
if err != nil {
return logical.ErrorResponse("error revoking existing access token %s", token.TokenID), err
}
Expand Down
2 changes: 1 addition & 1 deletion path_config_rotate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (e *accTestEnv) PathConfigRotateCreateTokenErr(t *testing.T) {
assert.NotNil(t, resp)
assert.Contains(t, resp.Data["error"], "error creating new access token")
assert.ErrorContains(t, err, "could not create access token")
e.revokeTestToken(t, e.AccessToken, tokenId)
e.revokeTestToken(t, tokenId)
}

func (e *accTestEnv) PathConfigRotateBadAccessToken(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions path_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ func (e *accTestEnv) PathConfigReadBadAccessToken(t *testing.T) {
assert.NoError(t, err)
resp, err := e.read(configAdminPath)

assert.NoError(t, err)
assert.NotNil(t, resp)
assert.Error(t, err)
assert.Nil(t, resp)
// Otherwise success, we don't need to re-test for this
}

Expand Down
62 changes: 53 additions & 9 deletions path_config_user_token.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package artifactory
import (
"context"
"crypto/sha256"
"errors"
"fmt"
"strings"
"time"
Expand Down Expand Up @@ -92,6 +93,25 @@ type userTokenConfiguration struct {
DefaultDescription string `json:"default_description,omitempty"`
}

func (c *userTokenConfiguration) RefreshAccessToken(ctx context.Context, req *logical.Request, username string, b *backend, adminBaseConfig baseConfiguration) error {
logger := b.Logger().With("func", "RefreshAccessToken")

if c.RefreshToken == "" {
return fmt.Errorf("refresh_token is empty")
}

refreshResp, err := b.RefreshToken(adminBaseConfig, c.RefreshToken)
if err != nil {
return err
}
logger.Info("access token refresh successful")

c.AccessToken = refreshResp.AccessToken
c.RefreshToken = refreshResp.RefreshToken

return b.storeUserTokenConfiguration(ctx, req, username, c)
}

// fetchAdminConfiguration will return nil,nil if there's no configuration
func (b *backend) fetchUserTokenConfiguration(ctx context.Context, storage logical.Storage, username string) (*userTokenConfiguration, error) {
// If username is not empty, then append to the path to fetch username specific configuration
Expand All @@ -100,15 +120,15 @@ func (b *backend) fetchUserTokenConfiguration(ctx context.Context, storage logic
path = fmt.Sprintf("%s/%s", path, username)
}

logger := b.Logger().With("func", "fetchUserTokenConfiguration")

// Read in the backend configuration
b.Logger().Info("fetching user token configuration", "path", path)
logger.Info("fetching user token configuration", "path", path)
entry, err := storage.Get(ctx, path)
if err != nil {
return nil, err
}

logger := b.Logger().With("func", "fetchUserTokenConfiguration")

if entry == nil && len(username) > 0 {
logger.Info(fmt.Sprintf("no configuration for username %s. Fetching default user token configuration", username), "path", configUserTokenPath)
e, err := storage.Get(ctx, configUserTokenPath)
Expand All @@ -124,6 +144,9 @@ func (b *backend) fetchUserTokenConfiguration(ctx context.Context, storage logic
if entry == nil {
logger.Warn("no configuration found. Using system default configuration.")
return &userTokenConfiguration{
baseConfiguration: baseConfiguration{
UseNewAccessAPI: true,
},
DefaultTTL: b.Backend.System().DefaultLeaseTTL(),
MaxTTL: b.Backend.System().MaxLeaseTTL(),
}, nil
Expand Down Expand Up @@ -171,8 +194,6 @@ func (b *backend) pathConfigUserTokenUpdate(ctx context.Context, req *logical.Re
return logical.ErrorResponse("backend not configured"), nil
}

go b.sendUsage(adminConfig.baseConfiguration, "pathConfigUserTokenUpdate")

username := ""
if val, ok := data.GetOk("username"); ok {
username = val.(string)
Expand All @@ -193,6 +214,8 @@ func (b *backend) pathConfigUserTokenUpdate(ctx context.Context, req *logical.Re
userTokenConfig.AccessToken = adminConfig.AccessToken
}

go b.sendUsage(userTokenConfig.baseConfiguration, "pathConfigUserTokenUpdate")

if val, ok := data.GetOk("refresh_token"); ok {
userTokenConfig.RefreshToken = val.(string)
}
Expand Down Expand Up @@ -234,6 +257,8 @@ func (b *backend) pathConfigUserTokenUpdate(ctx context.Context, req *logical.Re
userTokenConfig.DefaultDescription = val.(string)
}

userTokenConfig.UseNewAccessAPI = b.useNewAccessAPI(userTokenConfig.baseConfiguration)

err = b.storeUserTokenConfiguration(ctx, req, username, userTokenConfig)
if err != nil {
return nil, err
Expand All @@ -246,6 +271,8 @@ func (b *backend) pathConfigUserTokenRead(ctx context.Context, req *logical.Requ
b.configMutex.RLock()
defer b.configMutex.RUnlock()

baseConfig := baseConfiguration{}

adminConfig, err := b.fetchAdminConfiguration(ctx, req.Storage)
if err != nil {
return nil, err
Expand All @@ -255,7 +282,7 @@ func (b *backend) pathConfigUserTokenRead(ctx context.Context, req *logical.Requ
return logical.ErrorResponse("backend not configured"), nil
}

go b.sendUsage(adminConfig.baseConfiguration, "pathConfigUserTokenRead")
baseConfig = adminConfig.baseConfiguration

username := ""
if val, ok := data.GetOk("username"); ok {
Expand All @@ -267,6 +294,21 @@ func (b *backend) pathConfigUserTokenRead(ctx context.Context, req *logical.Requ
return nil, err
}

if userTokenConfig.baseConfiguration.AccessToken != "" {
baseConfig.AccessToken = userTokenConfig.baseConfiguration.AccessToken
}

if baseConfig.AccessToken == "" {
return logical.ErrorResponse("missing access token"), errors.New("missing access token")
}

err = b.refreshExpiredAccessToken(ctx, req, &baseConfig, userTokenConfig, username)
if err != nil {
return logical.ErrorResponse("failed to refresh access token"), err
}

go b.sendUsage(baseConfig, "pathConfigUserTokenRead")

accessTokenHash := sha256.Sum256([]byte(userTokenConfig.AccessToken))
refreshTokenHash := sha256.Sum256([]byte(userTokenConfig.RefreshToken))

Expand All @@ -284,10 +326,12 @@ func (b *backend) pathConfigUserTokenRead(ctx context.Context, req *logical.Requ
}

// Optionally include token info if it parses properly
token, err := b.getTokenInfo(adminConfig.baseConfiguration, userTokenConfig.AccessToken)
token, err := b.getTokenInfo(baseConfig, userTokenConfig.AccessToken)
if err != nil {
b.Logger().With("func", "pathConfigUserTokenRead").Warn("Error parsing AccessToken", "err", err.Error())
} else {
return logical.ErrorResponse("failed to get token info"), err
}

if token != nil {
configMap["token_id"] = token.TokenID
configMap["username"] = token.Username
configMap["scope"] = token.Scope
Expand Down
12 changes: 10 additions & 2 deletions path_config_user_token_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,18 @@ func TestAcceptanceBackend_PathConfigUserToken(t *testing.T) {
}

func (e *accTestEnv) PathConfigAccessTokenUpdate(t *testing.T) {
_, accessToken1 := e.createNewTestToken(t)

e.UpdateConfigUserToken(t, "", testData{
"access_token": "test123",
"access_token": accessToken1,
})
data := e.ReadConfigUserToken(t, "")
accessTokenHash := data["access_token_sha256"]
assert.NotEmpty(t, "access_token_sha256")

_, accessToken2 := e.createNewTestToken(t)
e.UpdateConfigUserToken(t, "", testData{
"access_token": "test456",
"access_token": accessToken2,
})
data = e.ReadConfigUserToken(t, "")
assert.NotEqual(t, data["access_token_sha256"], accessTokenHash)
Expand Down Expand Up @@ -139,6 +142,11 @@ func TestAcceptanceBackend_PathConfigUserToken_UseSystemDefault(t *testing.T) {
}
accTestEnv := NewConfiguredAcceptanceTestEnv(t)

_, accessToken1 := accTestEnv.createNewTestToken(t)
accTestEnv.UpdateConfigUserToken(t, "", testData{
"access_token": accessToken1,
})

data := accTestEnv.ReadConfigUserToken(t, "")
assert.Equal(t, accTestEnv.Backend.System().DefaultLeaseTTL().Seconds(), data["default_ttl"])
assert.Equal(t, accTestEnv.Backend.System().MaxLeaseTTL().Seconds(), data["max_ttl"])
Expand Down
35 changes: 8 additions & 27 deletions path_user_token_create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package artifactory
import (
"context"
"errors"
"fmt"
"time"

"github.com/hashicorp/vault/sdk/framework"
Expand Down Expand Up @@ -83,8 +82,6 @@ func (b *backend) pathUserTokenCreatePerform(ctx context.Context, req *logical.R
return logical.ErrorResponse("backend not configured"), nil
}

go b.sendUsage(adminConfig.baseConfiguration, "pathUserTokenCreatePerform")

baseConfig = adminConfig.baseConfiguration

username := data.Get("username").(string)
Expand All @@ -102,6 +99,13 @@ func (b *backend) pathUserTokenCreatePerform(ctx context.Context, req *logical.R
return logical.ErrorResponse("missing access token"), errors.New("missing access token")
}

err = b.refreshExpiredAccessToken(ctx, req, &baseConfig, userTokenConfig, username)
if err != nil {
return logical.ErrorResponse("failed to refresh access token"), err
}

go b.sendUsage(baseConfig, "pathUserTokenCreatePerform")

baseConfig.UseExpiringTokens = userTokenConfig.UseExpiringTokens
if value, ok := data.GetOk("use_expiring_tokens"); ok {
baseConfig.UseExpiringTokens = value.(bool)
Expand Down Expand Up @@ -191,30 +195,7 @@ func (b *backend) pathUserTokenCreatePerform(ctx context.Context, req *logical.R

resp, err := b.CreateToken(baseConfig, role)
if err != nil {
if _, ok := err.(*TokenExpiredError); ok {
logger.Info("access token expired. Attempt to refresh using the refresh token.")
refreshResp, err := b.RefreshToken(baseConfig, role)
if err != nil {
return nil, fmt.Errorf("failed to refresh access token. err: %v", err)
}
logger.Info("access token refresh successful")

userTokenConfig.AccessToken = refreshResp.AccessToken
userTokenConfig.RefreshToken = refreshResp.RefreshToken
b.storeUserTokenConfiguration(ctx, req, username, userTokenConfig)

baseConfig.AccessToken = userTokenConfig.AccessToken
role.RefreshToken = userTokenConfig.RefreshToken

// try again after token was refreshed
logger.Info("attempt to create user token again after access token refresh")
resp, err = b.CreateToken(baseConfig, role)
if err != nil {
return nil, err
}
} else {
return nil, err
}
return logical.ErrorResponse("failed to create new token"), err
}

response := b.Secret(SecretArtifactoryAccessTokenType).Response(map[string]interface{}{
Expand Down
3 changes: 1 addition & 2 deletions secret_access_token.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ func (b *backend) secretAccessTokenRevoke(ctx context.Context, req *logical.Requ
}

tokenId := req.Secret.InternalData["token_id"].(string)
accessToken := req.Secret.InternalData["access_token"].(string)
if err := b.RevokeToken(config.baseConfiguration, tokenId, accessToken); err != nil {
if err := b.RevokeToken(config.baseConfiguration, tokenId); err != nil {
return nil, err
}

Expand Down
Loading

0 comments on commit 974c557

Please sign in to comment.