diff --git a/.github/workflows/acceptance-tests.yml b/.github/workflows/acceptance-tests.yml index f6c06d5..e526804 100644 --- a/.github/workflows/acceptance-tests.yml +++ b/.github/workflows/acceptance-tests.yml @@ -31,7 +31,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: 1.21 + go-version: 1.22 - name: Install Helm uses: azure/setup-helm@v4.2.0 - name: Install GoReleaser diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 483a67a..29897d4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -19,7 +19,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: 1.21 + go-version: 1.22 - name: Import GPG key id: import_gpg uses: crazy-max/ghaction-import-gpg@v6 diff --git a/.goreleaser.yaml b/.goreleaser.yaml index ec8a346..9bd0f06 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -32,7 +32,7 @@ builds: goarch: '386' binary: artifactory-secrets-plugin snapshot: - name_template: '{{ .Version }}' + version_template: '{{ .Version }}' archives: - format: binary checksum: diff --git a/CHANGELOG.md b/CHANGELOG.md index ef6461f..86c7e40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 1.8.1 (November 8, 2024). Tested on Artifactory 7.98.8 with Vault v1.18.1 and OpenBao v2.0.0 + +BUG FIXES: + +* Fix error when reading `config/user_token` with expired access token and failed to refresh. Issue: [#217](https://github.com/jfrog/artifactory-secrets-plugin/issues/217) PR: [#223](https://github.com/jfrog/artifactory-secrets-plugin/pull/223) + ## 1.8.0 (June 7, 2024) IMPROVEMENTS: diff --git a/artifactory.go b/artifactory.go index 154f786..bc6599b 100644 --- a/artifactory.go +++ b/artifactory.go @@ -2,6 +2,7 @@ package artifactory import ( "bytes" + "context" "crypto/x509" "encoding/base64" "encoding/json" @@ -13,11 +14,11 @@ import ( "net/url" "regexp" "strings" - "time" jwt "github.com/golang-jwt/jwt/v4" "github.com/hashicorp/go-version" "github.com/hashicorp/vault/sdk/helper/template" + "github.com/hashicorp/vault/sdk/logical" "github.com/samber/lo" ) @@ -34,6 +35,7 @@ type baseConfiguration struct { ArtifactoryURL string `json:"artifactory_url"` UseExpiringTokens bool `json:"use_expiring_tokens,omitempty"` ForceRevocable *bool `json:"force_revocable,omitempty"` + UseNewAccessAPI bool `json:"use_new_access_api,omitempty"` } type errorResponse struct { @@ -42,7 +44,7 @@ type errorResponse struct { Detail string `json:"detail"` } -func (b *backend) RevokeToken(config baseConfiguration, tokenId, accessToken string) error { +func (b *backend) RevokeToken(config baseConfiguration, tokenId string) error { if config.AccessToken == "" { return fmt.Errorf("empty access token not allowed") } @@ -56,18 +58,23 @@ func (b *backend) RevokeToken(config baseConfiguration, tokenId, accessToken str } var resp *http.Response - - if b.useNewAccessAPI(config) { + if config.UseNewAccessAPI { resp, err = b.performArtifactoryDelete(config, "/access/api/v1/tokens/"+tokenId) if err != nil { logger.Error("error deleting access token", "tokenId", tokenId, "response", resp, "err", err) return err } } else { + path, err := url.JoinPath(u.Path, "/artifactory/api/security/token/revoke") + if err != nil { + logger.Error("error joining url path", "err", err) + return err + } + values := url.Values{} - values.Set("token", accessToken) + values.Set("token_id", tokenId) - resp, err = b.performArtifactoryPost(config, u.Path+"/artifactory/api/security/token/revoke", values) + resp, err = b.performArtifactoryPost(config, path, values) if err != nil { logger.Error("error deleting token", "tokenId", tokenId, "response", resp, "err", err) return err @@ -83,7 +90,7 @@ func (b *backend) RevokeToken(config baseConfiguration, tokenId, accessToken str return fmt.Errorf("could not parse response body. Err: %v", err) } logger.Error("revokenToken got non-200 status code", "statusCode", resp.StatusCode, "body", string(body)) - return fmt.Errorf("could not revoke tokenID: %v - HTTP response %v", tokenId, body) + return fmt.Errorf("could not revoke tokenID: %v - HTTP response %v", tokenId, string(body)) } return nil @@ -102,17 +109,32 @@ type CreateTokenRequest struct { RefreshToken string `json:"refresh_token,omitempty"` } -type createTokenErrorResponse struct { +type artifactoryErrorResponse struct { Errors []errorResponse `json:"errors"` } +func (r artifactoryErrorResponse) String() string { + return lo.Reduce(r.Errors, func(agg string, e errorResponse, _ int) string { + if agg == "" { + return e.Message + } + return fmt.Sprintf("%s, %s", agg, e.Message) + }, "") +} + type TokenExpiredError struct{} func (e *TokenExpiredError) Error() string { return "token has expired" } +var expiredTokenRegex = regexp.MustCompile(`.*Invalid token, expired.*`) + func (b *backend) CreateToken(config baseConfiguration, role artifactoryRole) (*createTokenResponse, error) { + if config.AccessToken == "" { + return nil, fmt.Errorf("empty access token not allowed") + } + request := CreateTokenRequest{ GrantType: role.GrantType, Username: role.Username, @@ -124,36 +146,11 @@ func (b *backend) CreateToken(config baseConfiguration, role artifactoryRole) (* RefreshToken: role.RefreshToken, } - return b.createToken(config, role.ExpiresIn, request) -} - -func (b *backend) RefreshToken(config baseConfiguration, role artifactoryRole) (*createTokenResponse, error) { - if config.AccessToken == "" { - return nil, fmt.Errorf("empty access token not allowed") - } - - if role.RefreshToken == "" { - return nil, fmt.Errorf("no refresh token supplied") - } - - request := CreateTokenRequest{ - GrantType: grantTypeRefreshToken, - RefreshToken: role.RefreshToken, - } - - return b.createToken(config, role.ExpiresIn, request) -} - -func (b *backend) createToken(config baseConfiguration, expiresIn time.Duration, request CreateTokenRequest) (*createTokenResponse, error) { - if config.AccessToken == "" { - return nil, fmt.Errorf("empty access token not allowed") - } - - if request.GrantType == "client_credentials" && len(request.Username) == 0 { + if request.GrantType == grantTypeClientCredentials && len(request.Username) == 0 { return nil, fmt.Errorf("empty username not allowed, possibly a template error") } - logger := b.Logger().With("func", "createToken") + logger := b.Logger().With("func", "CreateToken") // Artifactory will not let you revoke a token that has an expiry unless it also meets // criteria that can only be set in its configuration file. The version of Artifactory @@ -161,8 +158,14 @@ func (b *backend) createToken(config baseConfiguration, expiresIn time.Duration, // but the token is still usable even after it's deleted. See RTFACT-15293. request.ExpiresIn = 0 // never expires - if config.UseExpiringTokens && b.supportForceRevocable(config) && expiresIn > 0 { - request.ExpiresIn = int64(expiresIn.Seconds()) + supportForceRevocable, err := b.supportForceRevocable(config) + if err != nil { + logger.Error("failed to determine if force_revocable is supported", "err", err) + return nil, err + } + + if config.UseExpiringTokens && supportForceRevocable && role.ExpiresIn > 0 { + request.ExpiresIn = int64(role.ExpiresIn.Seconds()) if config.ForceRevocable != nil { request.ForceRevocable = *config.ForceRevocable } else { @@ -177,60 +180,142 @@ func (b *backend) createToken(config baseConfiguration, expiresIn time.Duration, path := "" - if b.useNewAccessAPI(config) { + var resp *http.Response + var createErr error + + if config.UseNewAccessAPI { path = "/access/api/v1/tokens" + + jsonReq, err := json.Marshal(request) + if err != nil { + return nil, err + } + + resp, createErr = b.performArtifactoryPostWithJSON(config, path, jsonReq) } else { - path = u.Path + "/artifactory/api/security/token" - } + path, err = url.JoinPath(u.Path, "/artifactory/api/security/token") + if err != nil { + logger.Error("error joining url path", "err", err) + return nil, err + } - jsonReq, err := json.Marshal(request) - if err != nil { - return nil, err + values := url.Values{ + "grant_type": []string{grantTypeClientCredentials}, + "username": []string{request.Username}, + "scope": []string{request.Scope}, + "expires_in": []string{fmt.Sprintf("%d", request.ExpiresIn)}, + "refreshable": []string{fmt.Sprintf("%t", request.Refreshable)}, + "audience": []string{request.Audience}, + } + + resp, createErr = b.performArtifactoryPost(config, path, values) } - resp, err := b.performArtifactoryPostWithJSON(config, path, jsonReq) - if err != nil { - logger.Error("error making token request", "response", resp, "err", err) - return nil, err + if createErr != nil { + logger.Error("error making token request", "response", resp, "err", createErr) + return nil, createErr } //noinspection GoUnhandledErrorResult defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - e := fmt.Errorf("could not create access token: HTTP response %v", resp.StatusCode) - - if resp.StatusCode == http.StatusUnauthorized { - var errResp createTokenErrorResponse - err := json.NewDecoder(resp.Body).Decode(&errResp) - if err != nil { - logger.Error("could not parse error response", "response", resp, "err", err) - return nil, fmt.Errorf("could not create access token. Err: %v", err) - } + var errResp artifactoryErrorResponse + err := json.NewDecoder(resp.Body).Decode(&errResp) + if err != nil { + logger.Error("could not parse error response", "response", resp, "err", err) + return nil, fmt.Errorf("could not create access token. Err: %v", err) + } - errMessages := lo.Reduce(errResp.Errors, func(agg string, e errorResponse, _ int) string { - return fmt.Sprintf("%s, %s", agg, e.Message) - }, "") + if resp.StatusCode == http.StatusUnauthorized && expiredTokenRegex.MatchString(errResp.String()) { + return nil, &TokenExpiredError{} + } - expiredTokenRe := regexp.MustCompile(`.*Invalid token, expired.*`) - if expiredTokenRe.MatchString(errMessages) { - return nil, &TokenExpiredError{} - } + logger.Error("got non-200 status code", "statusCode", resp.StatusCode, "message", errResp.String()) + return nil, fmt.Errorf("could not create access token: %s", errResp) + } + + var createdToken createTokenResponse + if err := json.NewDecoder(resp.Body).Decode(&createdToken); err != nil { + logger.Error("could not parse response", "response", resp, "err", err) + return nil, fmt.Errorf("could not create access token. Err: %v", err) + } + + return &createdToken, nil +} + +func (b *backend) RefreshToken(config baseConfiguration, refreshToken string) (*createTokenResponse, error) { + if config.AccessToken == "" { + return nil, fmt.Errorf("empty access token not allowed") + } + + if refreshToken == "" { + return nil, fmt.Errorf("no refresh token supplied") + } + + logger := b.Logger().With("func", "RefreshToken") + + u, err := url.Parse(config.ArtifactoryURL) + if err != nil { + logger.Error("could not parse artifactory url", "url", config.ArtifactoryURL, "err", err) + return nil, err + } + + request := CreateTokenRequest{ + GrantType: grantTypeRefreshToken, + RefreshToken: refreshToken, + } + + var resp *http.Response + var refreshErr error + + if config.UseNewAccessAPI { + jsonReq, err := json.Marshal(request) + if err != nil { + return nil, err } - body, err := io.ReadAll(resp.Body) + resp, refreshErr = b.performArtifactoryPostWithJSON(config, "/access/api/v1/tokens", jsonReq) + } else { + path, err := url.JoinPath(u.Path, "/artifactory/api/security/token") if err != nil { - logger.Error("createToken could not read error response body", "err", err) - return nil, fmt.Errorf("could not parse response body. Err: %v", e) + logger.Error("error joining url path", "err", err) + return nil, err + } + + values := url.Values{ + "grant_type": []string{grantTypeRefreshToken}, + "refresh_token": []string{refreshToken}, + "access_token": []string{config.AccessToken}, + } + + resp, refreshErr = b.performArtifactoryPost(config, path, values) + } + + if refreshErr != nil { + logger.Error("error making token request", "response", resp, "err", refreshErr) + return nil, refreshErr + } + + //noinspection GoUnhandledErrorResult + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + var errResp artifactoryErrorResponse + err := json.NewDecoder(resp.Body).Decode(&errResp) + if err != nil { + logger.Error("could not parse error response", "response", resp, "err", err) + return nil, fmt.Errorf("could not refresh access token. Err: %v", err) } - logger.Error("createToken got non-200 status code", "statusCode", resp.StatusCode, "body", string(body)) - return nil, fmt.Errorf("could not create access token. HTTP response: %s", body) + + logger.Error("got non-200 status code", "statusCode", resp.StatusCode, "message", errResp.String()) + return nil, fmt.Errorf("could not refresh access token: %s", errResp.String()) } var createdToken createTokenResponse if err := json.NewDecoder(resp.Body).Decode(&createdToken); err != nil { logger.Error("could not parse response", "response", resp, "err", err) - return nil, fmt.Errorf("could not create access token. Err: %v", err) + return nil, fmt.Errorf("could not refresh access token. Err: %w", err) } return &createdToken, nil @@ -239,7 +324,7 @@ func (b *backend) createToken(config baseConfiguration, expiresIn time.Duration, // supportForceRevocable verifies whether or not the Artifactory version is 7.50.3 or higher. // The access API changes in v7.50.3 to support force_revocable to allow us to set the expiration for the tokens. // REF: https://www.jfrog.com/confluence/display/JFROG/JFrog+Platform+REST+API#JFrogPlatformRESTAPI-CreateToken -func (b *backend) supportForceRevocable(config baseConfiguration) bool { +func (b *backend) supportForceRevocable(config baseConfiguration) (bool, error) { return b.checkVersion("7.50.3", config) } @@ -247,13 +332,53 @@ func (b *backend) supportForceRevocable(config baseConfiguration) bool { // The access API changed in v7.21.1 // REF: https://www.jfrog.com/confluence/display/JFROG/Artifactory+REST+API#ArtifactoryRESTAPI-AccessTokens func (b *backend) useNewAccessAPI(config baseConfiguration) bool { - return b.checkVersion("7.21.1", config) + compatible, err := b.checkVersion("7.21.1", config) + if err != nil { + b.Logger(). + With("func", "useNewAccessAPI"). + Warn("failed to check for Artifactory version. Default to 'true'", "err", err) + return true // default to use new API + } + + return compatible +} + +func (b *backend) refreshExpiredAccessToken(ctx context.Context, req *logical.Request, config *baseConfiguration, userTokenConfig *userTokenConfiguration, username string) error { + logger := b.Logger().With("func", "RefreshExpiredAccessToken") + + // check if user access token is expired or not + // if so, refresh it with new tokens + logger.Debug("use checkVersion to see if access token is expired") + _, err := b.checkVersion("7.50.3", *config) // doesn't matter which version to check + if err != nil { + logger.Debug("failed checkVersion", "err", err) + + if _, ok := err.(*TokenExpiredError); ok { + logger.Info("access token expired. Attempt to refresh using the refresh token.", "err", err) + refreshErr := userTokenConfig.RefreshAccessToken(ctx, req, username, b, *config) + if refreshErr != nil { + logger.Error("failed to refresh access token.", "err", refreshErr) + return refreshErr + } + + config.AccessToken = userTokenConfig.AccessToken + return nil + } + + return err + } + + return nil } +var tokenFailedValidationRegex = regexp.MustCompile(`.*Token failed verification: expired.*`) + // getVersion will fetch the current Artifactory version and store it in the backend func (b *backend) getVersion(config baseConfiguration) (version string, err error) { logger := b.Logger().With("func", "getVersion") + logger.Debug("fetching Artifactory version") + resp, err := b.performArtifactoryGet(config, "/artifactory/api/system/version") if err != nil { logger.Error("error making system version request", "response", resp, "err", err) @@ -264,7 +389,19 @@ func (b *backend) getVersion(config baseConfiguration) (version string, err erro if resp.StatusCode != http.StatusOK { logger.Error("got non-200 status code", "statusCode", resp.StatusCode) - return "", fmt.Errorf("could not get the system version: HTTP response %v", resp.StatusCode) + + var errResp artifactoryErrorResponse + err := json.NewDecoder(resp.Body).Decode(&errResp) + if err != nil { + logger.Error("could not parse error response", "response", resp, "err", err) + return "", fmt.Errorf("could not get Artifactory version. Err: %v", err) + } + + if resp.StatusCode == http.StatusUnauthorized && tokenFailedValidationRegex.MatchString(errResp.String()) { + return "", &TokenExpiredError{} + } + + return "", fmt.Errorf("could not get the system version: HTTP response %v", errResp.String()) } var systemVersion systemVersionResponse @@ -273,14 +410,18 @@ func (b *backend) getVersion(config baseConfiguration) (version string, err erro return "", err } + logger.Debug("found Artifactory version", "version", systemVersion.Version) + return systemVersion.Version, nil } // checkVersion will return a boolean and error to check compatibility before making an API call // -- This was formerly "checkSystemStatus" but that was hard-coded, that method now calls this one -func (b *backend) checkVersion(ver string, config baseConfiguration) (compatible bool) { +func (b *backend) checkVersion(ver string, config baseConfiguration) (compatible bool, err error) { logger := b.Logger().With("func", "checkVersion") + compatible = false + artifactoryVersion, err := b.getVersion(config) if err != nil { logger.Error("Unable to get Artifactory Version. Check url and access_token fields. TLS connection verification with Artifactory can be skipped by setting bypass_artifactory_tls_verification field to 'true'", "ver", artifactoryVersion, "err", err) @@ -299,6 +440,7 @@ func (b *backend) checkVersion(ver string, config baseConfiguration) (compatible return } + logger.Trace("comparing versions", "v1", v1.String(), "v2", v2.String()) if v1.GreaterThanOrEqual(v2) { compatible = true } @@ -306,53 +448,6 @@ func (b *backend) checkVersion(ver string, config baseConfiguration) (compatible return } -// parseJWT will parse a JWT token string from Artifactory and return a *jwt.Token, err -func (b *backend) parseJWT(config baseConfiguration, token string) (jwtToken *jwt.Token, err error) { - if config.AccessToken == "" { - return nil, fmt.Errorf("empty access token not allowed") - } - - validate := true - - logger := b.Logger().With("func", "parseJWT") - - cert, err := b.getRootCert(config) - if err != nil { - if errors.Is(err, ErrIncompatibleVersion) { - logger.Error("outdated artifactory, unable to retrieve root cert, skipping token validation") - validate = false - } else { - logger.Error("error retrieving root cert", "err", err.Error()) - return - } - } - - // Parse Token - if validate { - jwtToken, err = jwt.Parse(token, - func(token *jwt.Token) (interface{}, error) { return cert.PublicKey, nil }, - jwt.WithValidMethods([]string{"RS256"})) - if err != nil { - return - } - if !jwtToken.Valid { - return - } - } else { // SKIP Validation - // -- NOTE THIS IGNORES THE SIGNATURE, which is probably bad, - // but it is artifactory's job to validate the token, right? - // p := jwt.Parser{} - // token, _, err := p.ParseUnverified(oldAccessToken, jwt.MapClaims{}) - jwtToken, err = jwt.Parse(token, nil, jwt.WithoutClaimsValidation()) - if err != nil { - return - } - } - - // If we got here, we should have a jwtToken and nil err - return -} - type TokenInfo struct { TokenID string `json:"token_id"` Scope string `json:"scope"` @@ -362,7 +457,10 @@ type TokenInfo struct { // getTokenInfo will parse the provided token to return useful information about it func (b *backend) getTokenInfo(config baseConfiguration, token string) (info *TokenInfo, err error) { + logger := b.Logger().With("func", "getTokenInfo") + if config.AccessToken == "" { + logger.Error("config.AccessToken is empty") return nil, fmt.Errorf("empty access token not allowed") } @@ -385,8 +483,6 @@ func (b *backend) getTokenInfo(config baseConfiguration, token string) (info *To Username: strings.Join(sub[2:], "/"), // 3rd+ elements (incase username has / in it) } - logger := b.Logger().With("func", "getTokenInfo") - // exp -> expires at (unixtime) - may not be present switch exp := claims["exp"].(type) { case int64: @@ -404,20 +500,72 @@ func (b *backend) getTokenInfo(config baseConfiguration, token string) (info *To return } +// parseJWT will parse a JWT token string from Artifactory and return a *jwt.Token, err +func (b *backend) parseJWT(config baseConfiguration, token string) (jwtToken *jwt.Token, err error) { + logger := b.Logger().With("func", "parseJWT") + + if config.AccessToken == "" { + logger.Error("config.AccessToken is empty") + return nil, fmt.Errorf("empty access token not allowed") + } + + validate := true + + cert, err := b.getRootCert(config) + if err != nil { + if errors.Is(err, ErrIncompatibleVersion) { + logger.Error("outdated artifactory, unable to retrieve root cert, skipping token validation") + validate = false + } else { + logger.Error("error retrieving root cert", "err", err.Error()) + return + } + } + + // Parse Token + if validate { + jwtToken, err = jwt.Parse(token, + func(token *jwt.Token) (interface{}, error) { return cert.PublicKey, nil }, + jwt.WithValidMethods([]string{"RS256"})) + if err != nil { + return + } + if !jwtToken.Valid { + return + } + } else { // SKIP Validation + // -- NOTE THIS IGNORES THE SIGNATURE, which is probably bad, + // but it is artifactory's job to validate the token, right? + // p := jwt.Parser{} + // token, _, err := p.ParseUnverified(oldAccessToken, jwt.MapClaims{}) + jwtToken, err = jwt.Parse(token, nil, jwt.WithoutClaimsValidation()) + if err != nil { + return + } + } + + // If we got here, we should have a jwtToken and nil err + return +} + // getRootCert will return the Artifactory access root certificate's public key, for validating token signatures func (b *backend) getRootCert(config baseConfiguration) (cert *x509.Certificate, err error) { + logger := b.Logger().With("func", "getRootCert") + if config.AccessToken == "" { return nil, fmt.Errorf("empty access token not allowed") } // Verify Artifactory version is at 7.12.0 or higher, prior versions will not work // REF: https://jfrog.com/help/r/jfrog-rest-apis/get-root-certificate - if !b.checkVersion("7.12.0", config) { + valid, err := b.checkVersion("7.12.0", config) + if err != nil { + return nil, err + } + if !valid { return cert, ErrIncompatibleVersion } - logger := b.Logger().With("func", "getRootCert") - resp, err := b.performArtifactoryGet(config, "/access/api/v1/cert/root") if err != nil { logger.Error("error requesting cert/root", "response", resp, "err", err) @@ -427,8 +575,19 @@ func (b *backend) getRootCert(config baseConfiguration) (cert *x509.Certificate, defer resp.Body.Close() if resp.StatusCode != http.StatusOK { + var errResp artifactoryErrorResponse + err := json.NewDecoder(resp.Body).Decode(&errResp) + if err != nil { + logger.Error("could not parse error response", "response", resp, "err", err) + return nil, fmt.Errorf("could not create access token. Err: %v", err) + } + + if resp.StatusCode == http.StatusUnauthorized && tokenFailedValidationRegex.MatchString(errResp.String()) { + return nil, &TokenExpiredError{} + } + logger.Error("got non-200 status code", "statusCode", resp.StatusCode) - return cert, fmt.Errorf("could not get the certificate: HTTP response %v", resp.StatusCode) + return cert, fmt.Errorf("could not get the certificate: HTTP response %v", errResp.String()) } body, err := io.ReadAll(resp.Body) @@ -498,7 +657,10 @@ func (b *backend) sendUsage(config baseConfiguration, featureId string) { } func (b *backend) performArtifactoryGet(config baseConfiguration, path string) (*http.Response, error) { + logger := b.Logger().With("func", "performArtifactoryGet") + if config.AccessToken == "" { + logger.Error("config.AccessToken is empty") return nil, fmt.Errorf("empty access token not allowed") } @@ -549,7 +711,10 @@ func (b *backend) performArtifactoryPost(config baseConfiguration, path string, // performArtifactoryPost will HTTP POST data to the Artifactory API. func (b *backend) performArtifactoryPostWithJSON(config baseConfiguration, path string, postData []byte) (*http.Response, error) { + logger := b.Logger().With("func", "performArtifactoryPostWithJSON") + if config.AccessToken == "" { + logger.Error("config.AccessToken is empty") return nil, fmt.Errorf("empty access token not allowed") } @@ -577,7 +742,10 @@ func (b *backend) performArtifactoryPostWithJSON(config baseConfiguration, path // performArtifactoryDelete will HTTP DELETE to the Artifactory API. // The path will be appended to the configured configured URL Path (usually /artifactory) func (b *backend) performArtifactoryDelete(config baseConfiguration, path string) (*http.Response, error) { + logger := b.Logger().With("func", "performArtifactoryDelete") + if config.AccessToken == "" { + logger.Error("config.AccessToken is empty") return nil, fmt.Errorf("empty access token not allowed") } diff --git a/go.mod b/go.mod index 39b38bc..04c4fc1 100644 --- a/go.mod +++ b/go.mod @@ -1,7 +1,6 @@ module github.com/jfrog/vault-plugin-secrets-artifactory -go 1.21 -toolchain go1.22.5 +go 1.22 require ( github.com/golang-jwt/jwt/v4 v4.5.0 diff --git a/path_config.go b/path_config.go index 3710859..eda0d76 100644 --- a/path_config.go +++ b/path_config.go @@ -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 { @@ -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 @@ -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 @@ -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 @@ -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. @@ -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 @@ -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 } diff --git a/path_config_rotate.go b/path_config_rotate.go index ba38a76..5881224 100644 --- a/path_config_rotate.go +++ b/path_config_rotate.go @@ -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 } diff --git a/path_config_rotate_test.go b/path_config_rotate_test.go index 016e272..81607b6 100644 --- a/path_config_rotate_test.go +++ b/path_config_rotate_test.go @@ -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) { diff --git a/path_config_test.go b/path_config_test.go index 988beb8..5d06a86 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -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 } diff --git a/path_config_user_token.go b/path_config_user_token.go index 43a8e12..5e66cc1 100644 --- a/path_config_user_token.go +++ b/path_config_user_token.go @@ -3,6 +3,7 @@ package artifactory import ( "context" "crypto/sha256" + "errors" "fmt" "strings" "time" @@ -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 @@ -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) @@ -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 @@ -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) @@ -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) } @@ -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 @@ -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 @@ -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 { @@ -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)) @@ -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 diff --git a/path_config_user_token_test.go b/path_config_user_token_test.go index 80d282f..8b5702b 100644 --- a/path_config_user_token_test.go +++ b/path_config_user_token_test.go @@ -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) @@ -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"]) diff --git a/path_user_token_create.go b/path_user_token_create.go index 7db0c07..14d6275 100644 --- a/path_user_token_create.go +++ b/path_user_token_create.go @@ -3,7 +3,6 @@ package artifactory import ( "context" "errors" - "fmt" "time" "github.com/hashicorp/vault/sdk/framework" @@ -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) @@ -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) @@ -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{}{ diff --git a/secret_access_token.go b/secret_access_token.go index d949092..3fd0945 100644 --- a/secret_access_token.go +++ b/secret_access_token.go @@ -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 } diff --git a/test_utils.go b/test_utils.go index eadea35..1bd8043 100644 --- a/test_utils.go +++ b/test_utils.go @@ -36,8 +36,9 @@ type testData map[string]interface{} func (e *accTestEnv) createNewTestToken(t *testing.T) (string, string) { config := adminConfiguration{ baseConfiguration: baseConfiguration{ - AccessToken: e.AccessToken, - ArtifactoryURL: e.URL, + AccessToken: e.AccessToken, + ArtifactoryURL: e.URL, + UseNewAccessAPI: true, }, } @@ -62,8 +63,9 @@ func (e *accTestEnv) createNewTestToken(t *testing.T) (string, string) { func (e *accTestEnv) createNewNonAdminTestToken(t *testing.T) (string, string) { config := adminConfiguration{ baseConfiguration: baseConfiguration{ - AccessToken: e.AccessToken, - ArtifactoryURL: e.URL, + AccessToken: e.AccessToken, + ArtifactoryURL: e.URL, + UseNewAccessAPI: true, }, } @@ -83,13 +85,13 @@ func (e *accTestEnv) createNewNonAdminTestToken(t *testing.T) (string, string) { return resp.TokenId, resp.AccessToken } -func (e *accTestEnv) revokeTestToken(t *testing.T, accessToken string, tokenID string) { +func (e *accTestEnv) revokeTestToken(t *testing.T, tokenID string) { config := baseConfiguration{ AccessToken: e.AccessToken, ArtifactoryURL: e.URL, } - err := e.Backend.(*backend).RevokeToken(config, tokenID, accessToken) + err := e.Backend.(*backend).RevokeToken(config, tokenID) if err != nil { t.Fatal(err) } @@ -505,7 +507,7 @@ func (e *accTestEnv) Cleanup(t *testing.T) { e.DeleteConfigAdmin(t) // revoke the test token - e.revokeTestToken(t, e.AccessToken, data["token_id"].(string)) + e.revokeTestToken(t, data["token_id"].(string)) } func newAcceptanceTestEnv() (*accTestEnv, error) { diff --git a/ttl_test.go b/ttl_test.go index 20d7bbe..7d19690 100644 --- a/ttl_test.go +++ b/ttl_test.go @@ -218,7 +218,7 @@ func TestBackend_UserTokenConfigMaxTTLUseConfigMaxTTL(t *testing.T) { configPath := configUserTokenPath + "/admin" backend_max_ttl := b.System().MaxLeaseTTL() - user_token_config_ttl := backend_max_ttl - 1*time.Minute + user_token_config_ttl := backend_max_ttl - 10*time.Minute SetUserTokenMaxTTL(t, b, config.StorageView, configPath, user_token_config_ttl) resp, err := b.HandleRequest(context.Background(), &logical.Request{