diff --git a/CHANGELOG.md b/CHANGELOG.md index eab358a..4d53d3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,16 +1,22 @@ -## 1.6.0 (April 19, 2023) +## 1.7.0 (May 29, 2024) + +IMPROVEMENTS: + +* Make `access_token` field to `config/admin` optional for path. This allows the plugin to be configured without requiring an admin access token even if the plugin is used only to generate user token using identity token with `config/user_token` path. PR: [#189](https://github.com/jfrog/artifactory-secrets-plugin/pull/189) + +## 1.6.0 (April 19, 2024) IMPROVEMENTS: * Add `force_revocable` field to `config/admin`, `config/user_token`, and `config/user_token/` paths. Issue: [#174](https://github.com/jfrog/artifactory-secrets-plugin/issues/174) PR: [#147](https://github.com/jfrog/artifactory-secrets-plugin/pull/147), [#175](https://github.com/jfrog/artifactory-secrets-plugin/pull/175) -## 1.5.0 (March 13, 2023) +## 1.5.0 (March 13, 2024) IMPROVEMENTS: * Add `allow_scope_override` field to `config/admin` path. This allows override of `scope` field when generating new admin scope token using `artifactory/roles/` path. Issue: [#134](https://github.com/jfrog/artifactory-secrets-plugin/issues/134) PR: [#147](https://github.com/jfrog/artifactory-secrets-plugin/pull/147), [#163](https://github.com/jfrog/artifactory-secrets-plugin/pull/163) -## 1.4.0 (March 11, 2023) +## 1.4.0 (March 11, 2024) IMPROVEMENTS: @@ -20,7 +26,7 @@ BUG FIXES: * Fix `default_ttl` and `max_ttl` for `config/user_token` path fall back logic. Issue: [#159](https://github.com/jfrog/artifactory-secrets-plugin/issues/159) PR: [#162](https://github.com/jfrog/artifactory-secrets-plugin/pull/162) -## 1.3.0 (Feburary 27, 2023) +## 1.3.0 (Feburary 27, 2024) IMPROVEMENTS: @@ -37,7 +43,7 @@ BUG FIXES: PR: [155](https://github.com/jfrog/vault-plugin-secrets-artifactory/pull/155) -## 1.2.0 (January 10, 2023) +## 1.2.0 (January 10, 2024) IMPROVEMENTS: diff --git a/README.md b/README.md index db461a7..ed8fe37 100644 --- a/README.md +++ b/README.md @@ -477,7 +477,7 @@ No renewals or new tokens will be issued if the backend configuration (config/ad #### Parameters * `url` (string) - Address of the Artifactory instance, e.g. https://my.jfrog.io -* `access_token` (stirng) - Administrator token to access Artifactory +* `access_token` (string) - Optional. Administrator token to access Artifactory * `username_template` (string) - Optional. Vault Username Template for dynamically generating usernames. * `use_expiring_tokens` (boolean) - Optional. If Artifactory version >= 7.50.3, set `expires_in` to `max_ttl` (admin token) or `ttl` (user token) and `force_revocable = true`. Default to `false`. * `force_revocable` (boolean) - Optional. When set to true, we will add the `force_revocable` flag to the token's extension. In addition, a new configuration has been added that sets the default for setting the `force_revocable` default when creating a new token - the default of this configuration will be `false` to ensure that the Circle of Trust remains in place. @@ -509,7 +509,7 @@ Configures default values for the `user_token/:user-name` path. The optional `us #### Parameters -* `access_token` (stirng) - Optional. User identity token to access Artifactory. If `username` is not set then this token will be used for *all* users. +* `access_token` (string) - Optional. User identity token to access Artifactory. If `username` is not set then this token will be used for *all* users. * `refresh_token` (string) - Optional. Refresh token for the user access token. If `username` is not set then this token will be used for *all* users. * `audience` (string) - Optional. See the JFrog Platform REST documentation on [Create Token](https://jfrog.com/help/r/jfrog-rest-apis/create-token) for a full and up to date description. Service ID must begin with valid JFrog service type. Options: jfrt, jfxr, jfpip, jfds, jfmc, jfac, jfevt, jfmd, jfcon, or *. For instructions to retrieve the Artifactory Service ID see this [documentation](https://jfrog.com/help/r/jfrog-rest-apis/get-service-id) * `refreshable` (boolean) - Optional. A refreshable access token gets replaced by a new access token, which is not what a consumer of tokens from this backend would be expecting; instead they'd likely just request a new token periodically. Set this to `true` only if your usage requires this. See the JFrog Platform documentation on [Generating Refreshable Tokens](https://jfrog.com/help/r/jfrog-platform-administration-documentation/generating-refreshable-tokens) for a full and up to date description. Defaults to `false`. @@ -552,7 +552,7 @@ vault delete artifactory/config/user_token/myuser #### Parameters -* `grant_type` (stirng) - Optional. Defaults to `client_credentials` when creating the access token. You likely don't need to change this. +* `grant_type` (string) - Optional. Defaults to `client_credentials` when creating the access token. You likely don't need to change this. * `username` (string) - Optional. Defaults to using the username_template. The static username for which the access token is created. If the user does not exist, Artifactory will create a transient user. Note that non-administrative access tokens can only create tokens for themselves. * `scope` (string) - Space-delimited list. See the JFrog Artifactory REST documentation on ["Create Token"](https://jfrog.com/help/r/jfrog-rest-apis/create-token) for a full and up to date description. * `refreshable` (boolean) - Optional. A refreshable access token gets replaced by a new access token, which is not what a consumer of tokens from this backend would be expecting; instead they'd likely just request a new token periodically. Set this to `true` only if your usage requires this. See the JFrog Platform documentation on [Generating Refreshable Tokens](https://jfrog.com/help/r/jfrog-platform-administration-documentation/generating-refreshable-tokens) for a full and up to date description. Defaults to `false`. diff --git a/artifactory.go b/artifactory.go index 92c22ff..154f786 100644 --- a/artifactory.go +++ b/artifactory.go @@ -43,18 +43,24 @@ type errorResponse struct { } func (b *backend) RevokeToken(config baseConfiguration, tokenId, accessToken string) error { + if config.AccessToken == "" { + return fmt.Errorf("empty access token not allowed") + } + + logger := b.Logger().With("func", "RevokeToken") + u, err := url.Parse(config.ArtifactoryURL) if err != nil { - b.Logger().Error("could not parse artifactory url", "url", u, "err", err) + logger.Error("could not parse artifactory url", "url", u, "err", err) return err } var resp *http.Response - if b.useNewAccessAPI() { + if b.useNewAccessAPI(config) { resp, err = b.performArtifactoryDelete(config, "/access/api/v1/tokens/"+tokenId) if err != nil { - b.Logger().Error("error deleting access token", "tokenId", tokenId, "response", resp, "err", err) + logger.Error("error deleting access token", "tokenId", tokenId, "response", resp, "err", err) return err } } else { @@ -63,7 +69,7 @@ func (b *backend) RevokeToken(config baseConfiguration, tokenId, accessToken str resp, err = b.performArtifactoryPost(config, u.Path+"/artifactory/api/security/token/revoke", values) if err != nil { - b.Logger().Error("error deleting token", "tokenId", tokenId, "response", resp, "err", err) + logger.Error("error deleting token", "tokenId", tokenId, "response", resp, "err", err) return err } } @@ -73,10 +79,10 @@ func (b *backend) RevokeToken(config baseConfiguration, tokenId, accessToken str if resp.StatusCode >= http.StatusBadRequest { body, err := io.ReadAll(resp.Body) if err != nil { - b.Logger().Error("revokenToken could not read error response body", "err", err) + logger.Error("revokenToken could not read error response body", "err", err) return fmt.Errorf("could not parse response body. Err: %v", err) } - b.Logger().Error("revokenToken got non-200 status code", "statusCode", resp.StatusCode, "body", string(body)) + 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) } @@ -122,6 +128,10 @@ func (b *backend) CreateToken(config baseConfiguration, role artifactoryRole) (* } 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") } @@ -135,17 +145,23 @@ func (b *backend) RefreshToken(config baseConfiguration, role artifactoryRole) ( } 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 { return nil, fmt.Errorf("empty username not allowed, possibly a template error") } + 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 // I'm testing against will actually delete a token when you ask it to revoke by token_id, // but the token is still usable even after it's deleted. See RTFACT-15293. request.ExpiresIn = 0 // never expires - if config.UseExpiringTokens && b.supportForceRevocable() && expiresIn > 0 { + if config.UseExpiringTokens && b.supportForceRevocable(config) && expiresIn > 0 { request.ExpiresIn = int64(expiresIn.Seconds()) if config.ForceRevocable != nil { request.ForceRevocable = *config.ForceRevocable @@ -155,13 +171,13 @@ func (b *backend) createToken(config baseConfiguration, expiresIn time.Duration, } u, err := url.Parse(config.ArtifactoryURL) if err != nil { - b.Logger().Error("could not parse artifactory url", "url", config.ArtifactoryURL, "err", err) + logger.Error("could not parse artifactory url", "url", config.ArtifactoryURL, "err", err) return nil, err } path := "" - if b.useNewAccessAPI() { + if b.useNewAccessAPI(config) { path = "/access/api/v1/tokens" } else { path = u.Path + "/artifactory/api/security/token" @@ -174,7 +190,7 @@ func (b *backend) createToken(config baseConfiguration, expiresIn time.Duration, resp, err := b.performArtifactoryPostWithJSON(config, path, jsonReq) if err != nil { - b.Logger().Error("error making token request", "response", resp, "err", err) + logger.Error("error making token request", "response", resp, "err", err) return nil, err } @@ -188,7 +204,7 @@ func (b *backend) createToken(config baseConfiguration, expiresIn time.Duration, var errResp createTokenErrorResponse err := json.NewDecoder(resp.Body).Decode(&errResp) if err != nil { - b.Logger().Error("could not parse error response", "response", resp, "err", err) + logger.Error("could not parse error response", "response", resp, "err", err) return nil, fmt.Errorf("could not create access token. Err: %v", err) } @@ -204,16 +220,16 @@ func (b *backend) createToken(config baseConfiguration, expiresIn time.Duration, body, err := io.ReadAll(resp.Body) if err != nil { - b.Logger().Error("createToken could not read error response body", "err", err) + logger.Error("createToken could not read error response body", "err", err) return nil, fmt.Errorf("could not parse response body. Err: %v", e) } - b.Logger().Error("createToken got non-200 status code", "statusCode", resp.StatusCode, "body", string(body)) + 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) } var createdToken createTokenResponse if err := json.NewDecoder(resp.Body).Decode(&createdToken); err != nil { - b.Logger().Error("could not parse response", "response", resp, "err", err) + logger.Error("could not parse response", "response", resp, "err", err) return nil, fmt.Errorf("could not create access token. Err: %v", err) } @@ -223,53 +239,63 @@ 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() bool { - return b.checkVersion("7.50.3") +func (b *backend) supportForceRevocable(config baseConfiguration) bool { + return b.checkVersion("7.50.3", config) } // useNewAccessAPI verifies whether or not the Artifactory version is 7.21.1 or higher. // 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() bool { - return b.checkVersion("7.21.1") +func (b *backend) useNewAccessAPI(config baseConfiguration) bool { + return b.checkVersion("7.21.1", config) } // getVersion will fetch the current Artifactory version and store it in the backend -func (b *backend) getVersion(config baseConfiguration) (err error) { +func (b *backend) getVersion(config baseConfiguration) (version string, err error) { + logger := b.Logger().With("func", "getVersion") + resp, err := b.performArtifactoryGet(config, "/artifactory/api/system/version") if err != nil { - b.Logger().Error("error making system version request", "response", resp, "err", err) - return + logger.Error("error making system version request", "response", resp, "err", err) + return "", err } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - b.Logger().Error("got non-200 status code", "statusCode", resp.StatusCode) - return fmt.Errorf("could not get the system version: HTTP response %v", resp.StatusCode) + 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 systemVersion systemVersionResponse if err = json.NewDecoder(resp.Body).Decode(&systemVersion); err != nil { - b.Logger().Error("could not parse system version response", "response", resp, "err", err) - return + logger.Error("could not parse system version response", "response", resp, "err", err) + return "", err } - b.version = systemVersion.Version - return + + 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) (compatible bool) { - v1, err := version.NewVersion(b.version) +func (b *backend) checkVersion(ver string, config baseConfiguration) (compatible bool) { + logger := b.Logger().With("func", "checkVersion") + + artifactoryVersion, err := b.getVersion(config) if err != nil { - b.Logger().Error("could not parse Artifactory system version", "ver", b.version, "err", err) + 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) + return + } + + v1, err := version.NewVersion(artifactoryVersion) + if err != nil { + logger.Error("could not parse Artifactory system version", "ver", artifactoryVersion, "err", err) return } v2, err := version.NewVersion(ver) if err != nil { - b.Logger().Error("could not parse provided version", "ver", ver, "err", err) + logger.Error("could not parse provided version", "ver", ver, "err", err) return } @@ -282,15 +308,21 @@ func (b *backend) checkVersion(ver string) (compatible bool) { // 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) { - b.Logger().Error("outdated artifactory, unable to retrieve root cert, skipping token validation") + logger.Error("outdated artifactory, unable to retrieve root cert, skipping token validation") validate = false } else { - b.Logger().Error("error retrieving root cert", "err", err.Error()) + logger.Error("error retrieving root cert", "err", err.Error()) return } } @@ -330,6 +362,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) { + if config.AccessToken == "" { + return nil, fmt.Errorf("empty access token not allowed") + } + // Parse Current Token (to get tokenID/scope) jwtToken, err := b.parseJWT(config, token) if err != nil { @@ -349,6 +385,8 @@ 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: @@ -358,7 +396,7 @@ func (b *backend) getTokenInfo(config baseConfiguration, token string) (info *To case json.Number: v, err := exp.Int64() if err != nil { - b.Logger().Error("error parsing token exp as json.Number", "err", err) + logger.Error("error parsing token exp as json.Number", "err", err) } info.Expires = v } @@ -368,28 +406,34 @@ func (b *backend) getTokenInfo(config baseConfiguration, token string) (info *To // 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) { + 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") { + if !b.checkVersion("7.12.0", config) { return cert, ErrIncompatibleVersion } + logger := b.Logger().With("func", "getRootCert") + resp, err := b.performArtifactoryGet(config, "/access/api/v1/cert/root") if err != nil { - b.Logger().Error("error requesting cert/root", "response", resp, "err", err) + logger.Error("error requesting cert/root", "response", resp, "err", err) return } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - b.Logger().Error("got non-200 status code", "statusCode", resp.StatusCode) + logger.Error("got non-200 status code", "statusCode", resp.StatusCode) return cert, fmt.Errorf("could not get the certificate: HTTP response %v", resp.StatusCode) } body, err := io.ReadAll(resp.Body) if err != nil { - b.Logger().Error("error reading root cert response body", "err", err) + logger.Error("error reading root cert response body", "err", err) return } @@ -397,13 +441,13 @@ func (b *backend) getRootCert(config baseConfiguration) (cert *x509.Certificate, binCert := make([]byte, len(body)) n, err := base64.StdEncoding.Decode(binCert, body) if err != nil { - b.Logger().Error("error decoding body", "err", err) + logger.Error("error decoding body", "err", err) return } cert, err = x509.ParseCertificate(binCert[0:n]) if err != nil { - b.Logger().Error("error parsing certificate", "err", err) + logger.Error("error parsing certificate", "err", err) return } return @@ -419,6 +463,13 @@ type Usage struct { } func (b *backend) sendUsage(config baseConfiguration, featureId string) { + logger := b.Logger().With("func", "sendUsage") + + if config.AccessToken == "" { + logger.Info("access token is empty in config") + return + } + features := []Feature{ { FeatureId: featureId, @@ -432,13 +483,13 @@ func (b *backend) sendUsage(config baseConfiguration, featureId string) { jsonReq, err := json.Marshal(usage) if err != nil { - b.Logger().Info("error marshalling call home request", "err", err) + logger.Info("error marshalling call home request", "err", err) return } resp, err := b.performArtifactoryPostWithJSON(config, "artifactory/api/system/usage", jsonReq) if err != nil { - b.Logger().Info("error making call home request", "response", resp, "err", err) + logger.Info("error making call home request", "response", resp, "err", err) return } @@ -447,6 +498,10 @@ func (b *backend) sendUsage(config baseConfiguration, featureId string) { } func (b *backend) performArtifactoryGet(config baseConfiguration, path string) (*http.Response, error) { + if config.AccessToken == "" { + return nil, fmt.Errorf("empty access token not allowed") + } + u, err := parseURLWithDefaultPort(config.ArtifactoryURL) if err != nil { return nil, err @@ -468,6 +523,10 @@ func (b *backend) performArtifactoryGet(config baseConfiguration, path string) ( // performArtifactoryPost will HTTP POST values to the Artifactory API. func (b *backend) performArtifactoryPost(config baseConfiguration, path string, values url.Values) (*http.Response, error) { + if config.AccessToken == "" { + return nil, fmt.Errorf("empty access token not allowed") + } + u, err := parseURLWithDefaultPort(config.ArtifactoryURL) if err != nil { return nil, err @@ -490,6 +549,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) { + if config.AccessToken == "" { + return nil, fmt.Errorf("empty access token not allowed") + } + u, err := parseURLWithDefaultPort(config.ArtifactoryURL) if err != nil { return nil, err @@ -514,6 +577,9 @@ 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) { + if config.AccessToken == "" { + return nil, fmt.Errorf("empty access token not allowed") + } u, err := parseURLWithDefaultPort(config.ArtifactoryURL) if err != nil { diff --git a/backend.go b/backend.go index 3982184..e9783df 100644 --- a/backend.go +++ b/backend.go @@ -22,7 +22,6 @@ type backend struct { rolesMutex sync.RWMutex httpClient *http.Client usernameProducer template.StringTemplate - version string } // UsernameMetadata defines the metadata that a user_template can use to dynamically create user account in Artifactory @@ -96,11 +95,6 @@ func (b *backend) initialize(ctx context.Context, req *logical.InitializationReq b.InitializeHttpClient(config) - err = b.getVersion(config.baseConfiguration) - if err != nil { - return err - } - if len(config.UsernameTemplate) != 0 { up, err := testUsernameTemplate(config.UsernameTemplate) if err != nil { diff --git a/path_config.go b/path_config.go index 577c4d8..3710859 100644 --- a/path_config.go +++ b/path_config.go @@ -18,7 +18,6 @@ func (b *backend) pathConfig() *framework.Path { Fields: map[string]*framework.FieldSchema{ "access_token": { Type: framework.TypeString, - Required: true, Description: "Administrator token to access Artifactory", }, "url": { @@ -160,11 +159,9 @@ func (b *backend) pathConfigUpdate(ctx context.Context, req *logical.Request, da } if val, ok := data.GetOk("force_revocable"); ok { - temp := val.(bool) config.ForceRevocable = &temp } else { - config.ForceRevocable = nil } @@ -180,10 +177,6 @@ func (b *backend) pathConfigUpdate(ctx context.Context, req *logical.Request, da config.RevokeOnDelete = val.(bool) } - if config.AccessToken == "" { - return logical.ErrorResponse("access_token is required"), nil - } - if config.ArtifactoryURL == "" { return logical.ErrorResponse("url is required"), nil } @@ -192,11 +185,6 @@ func (b *backend) pathConfigUpdate(ctx context.Context, req *logical.Request, da go b.sendUsage(config.baseConfiguration, "pathConfigRotateUpdate") - err = b.getVersion(config.baseConfiguration) - if err != nil { - return logical.ErrorResponse("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'"), err - } - entry, err := logical.StorageEntryJSON(configAdminPath, config) if err != nil { return nil, err @@ -263,13 +251,18 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, _ *f go b.sendUsage(config.baseConfiguration, "pathConfigRead") + version, err := b.getVersion(config.baseConfiguration) + if err != nil { + b.Logger().Warn("failed to get system version", "err", err) + } + // I'm not sure if I should be returning the access token, so I'll hash it. accessTokenHash := sha256.Sum256([]byte(config.AccessToken)) configMap := map[string]interface{}{ "access_token_sha256": fmt.Sprintf("%x", accessTokenHash[:]), "url": config.ArtifactoryURL, - "version": b.version, + "version": version, "use_expiring_tokens": config.UseExpiringTokens, "force_revocable": config.ForceRevocable, "bypass_artifactory_tls_verification": config.BypassArtifactoryTLSVerification, @@ -297,7 +290,7 @@ func (b *backend) pathConfigRead(ctx context.Context, req *logical.Request, _ *f } } - if b.supportForceRevocable() { + if b.supportForceRevocable(config.baseConfiguration) { configMap["use_expiring_tokens"] = config.UseExpiringTokens } diff --git a/path_config_rotate.go b/path_config_rotate.go index 8deedd0..ba38a76 100644 --- a/path_config_rotate.go +++ b/path_config_rotate.go @@ -2,6 +2,7 @@ package artifactory import ( "context" + "errors" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" @@ -44,6 +45,10 @@ func (b *backend) pathConfigRotateWrite(ctx context.Context, req *logical.Reques return logical.ErrorResponse("backend not configured"), nil } + if config.AccessToken == "" { + return logical.ErrorResponse("missing access token"), errors.New("missing access token") + } + go b.sendUsage(config.baseConfiguration, "pathConfigRotateWrite") oldAccessToken := config.AccessToken @@ -84,7 +89,7 @@ func (b *backend) pathConfigRotateWrite(ctx context.Context, req *logical.Reques // Set new token and set revoke_on_delete to true config.AccessToken = resp.AccessToken - b.Logger().Info("set config.RevokeOnDelete to 'true'") + b.Logger().With("func", "pathConfigRotateWrite").Info("set config.RevokeOnDelete to 'true'") config.RevokeOnDelete = true // Save new config diff --git a/path_config_rotate_test.go b/path_config_rotate_test.go index 81f8cde..016e272 100644 --- a/path_config_rotate_test.go +++ b/path_config_rotate_test.go @@ -28,6 +28,7 @@ func TestAcceptanceBackend_PathRotate(t *testing.T) { e.Cleanup(t) // Failure Tests + t.Run("MissingAccessTokenErr", e.PathConfigRotateMissingAccessTokenErr) t.Run("CreateTokenErr", e.PathConfigRotateCreateTokenErr) t.Run("badAccessToken", e.PathConfigRotateBadAccessToken) } @@ -67,6 +68,17 @@ func (e *accTestEnv) PathConfigRotateWithDetails(t *testing.T) { // Not testing Description, because it is not returned in the token (yet) } +func (e *accTestEnv) PathConfigRotateMissingAccessTokenErr(t *testing.T) { + e.UpdateConfigAdmin(t, testData{ + "access_token": "", + "url": e.URL, + }) + resp, err := e.update("config/rotate", testData{}) + assert.NotNil(t, resp) + assert.Contains(t, resp.Data["error"], "missing access token") + assert.ErrorContains(t, err, "missing access token") +} + func (e *accTestEnv) PathConfigRotateCreateTokenErr(t *testing.T) { tokenId, accessToken := e.createNewNonAdminTestToken(t) e.UpdateConfigAdmin(t, testData{ @@ -92,7 +104,6 @@ func (e *accTestEnv) PathConfigRotateBadAccessToken(t *testing.T) { err = e.Storage.Put(e.Context, entry) assert.NoError(t, err) resp, err := e.update("config/rotate", testData{}) - // assert.Contains(t, resp.Data["error"], "error parsing existing AccessToken") - assert.Contains(t, resp.Data["error"], "could not get the certificate") + assert.Contains(t, resp.Data["error"], "error parsing existing access token") assert.Error(t, err) } diff --git a/path_config_test.go b/path_config_test.go index 76c3324..988beb8 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -146,31 +146,14 @@ func (e *accTestEnv) PathConfigUpdateUsernameTemplate(t *testing.T) { // most of these were covered by unit tests, but we want test coverage for acceptance func (e *accTestEnv) PathConfigUpdateErrors(t *testing.T) { - // Access Token Required - resp, err := e.update(configAdminPath, testData{ - "url": e.URL, - }) - assert.NoError(t, err) - assert.NotNil(t, resp) - assert.True(t, resp.IsError()) - assert.Contains(t, resp.Error().Error(), "access_token") // URL Required - resp, err = e.update(configAdminPath, testData{ + resp, err := e.update(configAdminPath, testData{ "access_token": "test-access-token", }) assert.NoError(t, err) assert.NotNil(t, resp) assert.True(t, resp.IsError()) assert.Contains(t, resp.Error().Error(), "url") - // Bad Token - resp, err = e.update(configAdminPath, testData{ - "access_token": "test-access-token", - "url": e.URL, - }) - assert.NotNil(t, resp) - assert.True(t, resp.IsError()) - assert.Contains(t, resp.Error().Error(), "Unable to get Artifactory Version") - assert.ErrorContains(t, err, "could not get the system version") } func (e *accTestEnv) PathConfigReadBadAccessToken(t *testing.T) { @@ -191,26 +174,6 @@ func (e *accTestEnv) PathConfigReadBadAccessToken(t *testing.T) { // Otherwise success, we don't need to re-test for this } -func TestBackend_AccessTokenRequired(t *testing.T) { - b, config := makeBackend(t) - - adminConfig := map[string]interface{}{ - "url": "https://127.0.0.1", - } - - resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: configAdminPath, - Storage: config.StorageView, - Data: adminConfig, - }) - assert.NoError(t, err) - - assert.NotNil(t, resp) - assert.True(t, resp.IsError()) - assert.Contains(t, resp.Error().Error(), "access_token") -} - func TestBackend_URLRequired(t *testing.T) { b, config := makeBackend(t) diff --git a/path_config_user_token.go b/path_config_user_token.go index 9c0621d..43a8e12 100644 --- a/path_config_user_token.go +++ b/path_config_user_token.go @@ -107,8 +107,10 @@ func (b *backend) fetchUserTokenConfiguration(ctx context.Context, storage logic return nil, err } + logger := b.Logger().With("func", "fetchUserTokenConfiguration") + if entry == nil && len(username) > 0 { - b.Logger().Info(fmt.Sprintf("no configuration for username %s. Fetching default user token configuration", username), "path", configUserTokenPath) + logger.Info(fmt.Sprintf("no configuration for username %s. Fetching default user token configuration", username), "path", configUserTokenPath) e, err := storage.Get(ctx, configUserTokenPath) if err != nil { return nil, err @@ -120,7 +122,7 @@ func (b *backend) fetchUserTokenConfiguration(ctx context.Context, storage logic } if entry == nil { - b.Logger().Warn("no configuration found. Using system default configuration.") + logger.Warn("no configuration found. Using system default configuration.") return &userTokenConfiguration{ DefaultTTL: b.Backend.System().DefaultLeaseTTL(), MaxTTL: b.Backend.System().MaxLeaseTTL(), @@ -147,7 +149,7 @@ func (b *backend) storeUserTokenConfiguration(ctx context.Context, req *logical. return err } - b.Logger().Info("saving user token configuration", "path", path) + b.Logger().With("func", "storeUserTokenConfiguration").Info("saving user token configuration", "path", path) err = req.Storage.Put(ctx, entry) if err != nil { return err @@ -284,7 +286,7 @@ 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) if err != nil { - b.Logger().Warn("Error parsing AccessToken", "err", err.Error()) + b.Logger().With("func", "pathConfigUserTokenRead").Warn("Error parsing AccessToken", "err", err.Error()) } else { configMap["token_id"] = token.TokenID configMap["username"] = token.Username diff --git a/path_token_create.go b/path_token_create.go index c24c9e4..03cb08d 100644 --- a/path_token_create.go +++ b/path_token_create.go @@ -77,6 +77,10 @@ func (b *backend) pathTokenCreatePerform(ctx context.Context, req *logical.Reque return logical.ErrorResponse("backend not configured"), nil } + if config.AccessToken == "" { + return logical.ErrorResponse("missing access token"), errors.New("missing access token") + } + go b.sendUsage(config.baseConfiguration, "pathTokenCreatePerform") // Read in the requested role @@ -102,11 +106,13 @@ func (b *backend) pathTokenCreatePerform(ctx context.Context, req *logical.Reque } } + logger := b.Logger().With("func", "pathTokenCreatePerform") + maxLeaseTTL := b.Backend.System().MaxLeaseTTL() - b.Logger().Debug("initialize maxLeaseTTL to system value", "maxLeaseTTL", maxLeaseTTL) + logger.Debug("initialize maxLeaseTTL to system value", "maxLeaseTTL", maxLeaseTTL) if value, ok := data.GetOk("max_ttl"); ok && value.(int) > 0 { - b.Logger().Debug("max_ttl is set", "max_ttl", value) + logger.Debug("max_ttl is set", "max_ttl", value) maxTTL := time.Second * time.Duration(value.(int)) // use override max TTL if set and less than maxLeaseTTL @@ -114,26 +120,26 @@ func (b *backend) pathTokenCreatePerform(ctx context.Context, req *logical.Reque maxLeaseTTL = maxTTL } } else if role.MaxTTL > 0 && role.MaxTTL < maxLeaseTTL { - b.Logger().Debug("using role MaxTTL", "role.MaxTTL", role.MaxTTL) + logger.Debug("using role MaxTTL", "role.MaxTTL", role.MaxTTL) maxLeaseTTL = role.MaxTTL } - b.Logger().Debug("Max lease TTL (sec)", "maxLeaseTTL", maxLeaseTTL) + logger.Debug("Max lease TTL (sec)", "maxLeaseTTL", maxLeaseTTL) ttl := b.Backend.System().DefaultLeaseTTL() if value, ok := data.GetOk("ttl"); ok && value.(int) > 0 { - b.Logger().Debug("ttl is set", "ttl", value) + logger.Debug("ttl is set", "ttl", value) ttl = time.Second * time.Duration(value.(int)) } else if role.DefaultTTL != 0 { - b.Logger().Debug("using role DefaultTTL", "role.DefaultTTL", role.DefaultTTL) + logger.Debug("using role DefaultTTL", "role.DefaultTTL", role.DefaultTTL) ttl = role.DefaultTTL } // cap ttl to maxLeaseTTL if ttl > maxLeaseTTL { - b.Logger().Debug("ttl is longer than maxLeaseTTL", "ttl", ttl, "maxLeaseTTL", maxLeaseTTL) + logger.Debug("ttl is longer than maxLeaseTTL", "ttl", ttl, "maxLeaseTTL", maxLeaseTTL) ttl = maxLeaseTTL } - b.Logger().Debug("TTL (sec)", "ttl", ttl) + logger.Debug("TTL (sec)", "ttl", ttl) // Set the role.ExpiresIn based on maxLeaseTTL if use_expiring_tokens is set to tru in config // - This value will be passed to createToken and used as expires_in for versions of Artifactory 7.50.3 or higher diff --git a/path_token_create_test.go b/path_token_create_test.go index d53c73b..5690455 100644 --- a/path_token_create_test.go +++ b/path_token_create_test.go @@ -40,3 +40,22 @@ func TestAcceptanceBackend_PathTokenCreate_overrides(t *testing.T) { t.Run("delete role", accTestEnv.DeletePathRole) t.Run("cleanup backend", accTestEnv.DeletePathConfig) } + +func TestAcceptanceBackend_PathTokenCreate_no_access_token(t *testing.T) { + if !runAcceptanceTests { + t.SkipNow() + } + + accTestEnv, err := newAcceptanceTestEnv() + if err != nil { + t.Fatal(err) + } + + accTestEnv.AccessToken = "" + + t.Run("configure backend with no access token", accTestEnv.UpdatePathConfig) + t.Run("create role", accTestEnv.CreatePathRole) + t.Run("create token for role", accTestEnv.CreatePathToken_no_access_token) + t.Run("delete role", accTestEnv.DeletePathRole) + t.Run("cleanup backend", accTestEnv.DeletePathConfig) +} diff --git a/path_user_token_create.go b/path_user_token_create.go index 93ec692..09ef36c 100644 --- a/path_user_token_create.go +++ b/path_user_token_create.go @@ -2,6 +2,7 @@ package artifactory import ( "context" + "errors" "fmt" "time" @@ -93,6 +94,10 @@ func (b *backend) pathUserTokenCreatePerform(ctx context.Context, req *logical.R baseConfig.AccessToken = userTokenConfig.baseConfiguration.AccessToken } + if baseConfig.AccessToken == "" { + return logical.ErrorResponse("missing access token"), errors.New("missing access token") + } + baseConfig.UseExpiringTokens = userTokenConfig.UseExpiringTokens if value, ok := data.GetOk("use_expiring_tokens"); ok { baseConfig.UseExpiringTokens = value.(bool) @@ -113,11 +118,13 @@ func (b *backend) pathUserTokenCreatePerform(ctx context.Context, req *logical.R RefreshToken: userTokenConfig.RefreshToken, } + logger := b.Logger().With("func", "pathUserTokenCreatePerform") + maxLeaseTTL := b.Backend.System().MaxLeaseTTL() - b.Logger().Debug("initialize maxLeaseTTL to system value", "maxLeaseTTL", maxLeaseTTL) + logger.Debug("initialize maxLeaseTTL to system value", "maxLeaseTTL", maxLeaseTTL) if value, ok := data.GetOk("max_ttl"); ok && value.(int) > 0 { - b.Logger().Debug("max_ttl is set", "max_ttl", value) + logger.Debug("max_ttl is set", "max_ttl", value) maxTTL := time.Second * time.Duration(value.(int)) // use override max TTL if set and is less than maxLeaseTTL @@ -125,27 +132,27 @@ func (b *backend) pathUserTokenCreatePerform(ctx context.Context, req *logical.R maxLeaseTTL = maxTTL } } else if userTokenConfig.MaxTTL > 0 && userTokenConfig.MaxTTL < maxLeaseTTL { - b.Logger().Debug("using user token config MaxTTL", "userTokenConfig.MaxTTL", userTokenConfig.MaxTTL) + logger.Debug("using user token config MaxTTL", "userTokenConfig.MaxTTL", userTokenConfig.MaxTTL) // use max TTL from user config if set and is less than system max lease TTL maxLeaseTTL = userTokenConfig.MaxTTL } - b.Logger().Debug("Max lease TTL (sec)", "maxLeaseTTL", maxLeaseTTL) + logger.Debug("Max lease TTL (sec)", "maxLeaseTTL", maxLeaseTTL) ttl := b.Backend.System().DefaultLeaseTTL() if value, ok := data.GetOk("ttl"); ok && value.(int) > 0 { - b.Logger().Debug("ttl is set", "ttl", value) + logger.Debug("ttl is set", "ttl", value) ttl = time.Second * time.Duration(value.(int)) } else if userTokenConfig.DefaultTTL != 0 { - b.Logger().Debug("using user config DefaultTTL", "userTokenConfig.DefaultTTL", userTokenConfig.DefaultTTL) + logger.Debug("using user config DefaultTTL", "userTokenConfig.DefaultTTL", userTokenConfig.DefaultTTL) ttl = userTokenConfig.DefaultTTL } // cap ttl to maxLeaseTTL if maxLeaseTTL > 0 && ttl > maxLeaseTTL { - b.Logger().Debug("ttl is longer than maxLeaseTTL", "ttl", ttl, "maxLeaseTTL", maxLeaseTTL) + logger.Debug("ttl is longer than maxLeaseTTL", "ttl", ttl, "maxLeaseTTL", maxLeaseTTL) ttl = maxLeaseTTL } - b.Logger().Debug("TTL (sec)", "ttl", ttl) + logger.Debug("TTL (sec)", "ttl", ttl) // now ttl is determined, we set role.ExpiresIn so this value so expirable token has the correct expiration if baseConfig.UseExpiringTokens { @@ -171,12 +178,12 @@ func (b *backend) pathUserTokenCreatePerform(ctx context.Context, req *logical.R resp, err := b.CreateToken(baseConfig, role) if err != nil { if _, ok := err.(*TokenExpiredError); ok { - b.Logger().Info("access token expired. Attempt to refresh using the refresh token.") + 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) } - b.Logger().Info("access token refresh successful") + logger.Info("access token refresh successful") userTokenConfig.AccessToken = refreshResp.AccessToken userTokenConfig.RefreshToken = refreshResp.RefreshToken @@ -186,7 +193,7 @@ func (b *backend) pathUserTokenCreatePerform(ctx context.Context, req *logical.R role.RefreshToken = userTokenConfig.RefreshToken // try again after token was refreshed - b.Logger().Info("attempt to create user token again after access token refresh") + logger.Info("attempt to create user token again after access token refresh") resp, err = b.CreateToken(baseConfig, role) if err != nil { return nil, err diff --git a/path_user_token_create_test.go b/path_user_token_create_test.go index 1b7c632..789f1e2 100644 --- a/path_user_token_create_test.go +++ b/path_user_token_create_test.go @@ -33,3 +33,20 @@ func TestAcceptanceBackend_PathUserTokenCreate_overrides(t *testing.T) { t.Run("create token for admin user", accTestEnv.CreatePathUserToken_overrides) t.Run("cleanup backend", accTestEnv.DeletePathConfig) } + +func TestAcceptanceBackend_PathUserTokenCreate_no_access_token(t *testing.T) { + if !runAcceptanceTests { + t.SkipNow() + } + + accTestEnv, err := newAcceptanceTestEnv() + if err != nil { + t.Fatal(err) + } + + accTestEnv.AccessToken = "" + + t.Run("configure backend with no access token", accTestEnv.UpdatePathConfig) + t.Run("create token for admin user", accTestEnv.CreatePathUserToken_no_access_token) + t.Run("cleanup backend", accTestEnv.DeletePathConfig) +} diff --git a/test_utils.go b/test_utils.go index 98e3d9b..ff69697 100644 --- a/test_utils.go +++ b/test_utils.go @@ -49,11 +49,6 @@ func (e *accTestEnv) createNewTestToken(t *testing.T) (string, string) { e.Backend.(*backend).InitializeHttpClient(&config) - err := e.Backend.(*backend).getVersion(config.baseConfiguration) - if err != nil { - t.Fatal(err) - } - resp, err := e.Backend.(*backend).CreateToken(config.baseConfiguration, role) if err != nil { t.Fatal(err) @@ -80,11 +75,6 @@ func (e *accTestEnv) createNewNonAdminTestToken(t *testing.T) (string, string) { e.Backend.(*backend).InitializeHttpClient(&config) - err := e.Backend.(*backend).getVersion(config.baseConfiguration) - if err != nil { - t.Fatal(err) - } - resp, err := e.Backend.(*backend).CreateToken(config.baseConfiguration, role) if err != nil { t.Fatal(err) @@ -99,12 +89,7 @@ func (e *accTestEnv) revokeTestToken(t *testing.T, accessToken string, tokenID s ArtifactoryURL: e.URL, } - err := e.Backend.(*backend).getVersion(config) - if err != nil { - t.Fatal(err) - } - - err = e.Backend.(*backend).RevokeToken(config, tokenID, accessToken) + err := e.Backend.(*backend).RevokeToken(config, tokenID, accessToken) if err != nil { t.Fatal(err) } @@ -325,6 +310,19 @@ func (e *accTestEnv) CreatePathToken_overrides(t *testing.T) { assert.Equal(t, 60, resp.Data["expires_in"]) } +func (e *accTestEnv) CreatePathToken_no_access_token(t *testing.T) { + resp, err := e.Backend.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: "token/admin-role", + Storage: e.Storage, + }) + + assert.Error(t, err, "missing access token") + assert.NotNil(t, resp) + assert.Empty(t, resp.Data["access_token"]) + assert.Empty(t, resp.Data["token_id"]) +} + func (e *accTestEnv) CreatePathScopedDownToken(t *testing.T) { resp, err := e.Backend.HandleRequest(context.Background(), &logical.Request{ Operation: logical.ReadOperation, @@ -436,6 +434,35 @@ func (e *accTestEnv) CreatePathUserToken_overrides(t *testing.T) { assert.NotEmpty(t, resp.Data["reference_token"]) } +func (e *accTestEnv) CreatePathUserToken_no_access_token(t *testing.T) { + resp, err := e.Backend.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: configUserTokenPath + "/admin", + Storage: e.Storage, + Data: map[string]interface{}{ + "default_description": "foo", + "refreshable": true, + "include_reference_token": true, + "use_expiring_tokens": true, + "default_ttl": 60, + }, + }) + + assert.NoError(t, err) + assert.Nil(t, resp) + + resp, err = e.Backend.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: createUserTokenPath + "admin", + Storage: e.Storage, + }) + + assert.Error(t, err, "missing access token") + assert.NotNil(t, resp) + assert.Empty(t, resp.Data["access_token"]) + assert.Empty(t, resp.Data["token_id"]) +} + // Cleanup will delete the admin configuration and revoke the token func (e *accTestEnv) Cleanup(t *testing.T) { data := e.ReadConfigAdmin(t)