From 1c8d1adee45b77ed69dc6dac6456aac27cdc83c5 Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core Date: Thu, 29 Aug 2024 11:56:47 -0400 Subject: [PATCH] Prepare for 1.17.4 release (#28220) * backport of commit 78e1cceccc58d9d6fd7dfa651e49d876afcbccde * Revert "backport of commit a5262e08bbab23c723f191fc6c89506f6b02521a (#28210)" This reverts commit ff2e104a3e39c23de793635c3c89a2c9074e212f. * Revert "UI: remove renew self call after login (#28204) (#28211)" This reverts commit d2d795815f1f2daa472b60fe70ad7855feccd672. --------- Co-authored-by: Ryan Cragun --- Dockerfile | 3 + changelog/28204.txt | 3 - changelog/28207.txt | 3 - .../cache/cachememdb/cache_memdb.go | 2 +- .../cache/cachememdb/index.go | 9 - command/agentproxyshared/cache/lease_cache.go | 154 +--------- .../cache/lease_cache_test.go | 4 +- .../cache/static_secret_cache_updater.go | 213 +------------- .../cache/static_secret_cache_updater_test.go | 262 +++--------------- ui/app/components/auth-jwt.js | 2 - ui/app/services/auth.js | 2 +- ui/tests/acceptance/auth-test.js | 15 - 12 files changed, 69 insertions(+), 603 deletions(-) delete mode 100644 changelog/28204.txt delete mode 100644 changelog/28207.txt diff --git a/Dockerfile b/Dockerfile index 98f200c7d836..254a264319f9 100644 --- a/Dockerfile +++ b/Dockerfile @@ -104,6 +104,9 @@ ENV VERSION=$VERSION # Copy the license file as per Legal requirement COPY LICENSE /usr/share/doc/$NAME/LICENSE.txt +# We must have a copy of the license in this directory to comply with the HasLicense Redhat requirement +COPY LICENSE /licenses/LICENSE.txt + # Set up certificates, our base tools, and Vault. Unlike the other version of # this (https://github.com/hashicorp/docker-vault/blob/master/ubi/Dockerfile), # we copy in the Vault binary from CRT. diff --git a/changelog/28204.txt b/changelog/28204.txt deleted file mode 100644 index beaef7968c73..000000000000 --- a/changelog/28204.txt +++ /dev/null @@ -1,3 +0,0 @@ -```release-note:bug -ui: fixes renew-self being called right after login for non-renewable tokens -``` \ No newline at end of file diff --git a/changelog/28207.txt b/changelog/28207.txt deleted file mode 100644 index 0c3e07b517e8..000000000000 --- a/changelog/28207.txt +++ /dev/null @@ -1,3 +0,0 @@ -```release-note:bug -proxy/cache (enterprise): Fixed an issue where Proxy with static secret caching enabled would not correctly handle requests to older secret versions for KVv2 secrets. Proxy's static secret cache now properly handles all requests relating to older versions for KVv2 secrets. -``` diff --git a/command/agentproxyshared/cache/cachememdb/cache_memdb.go b/command/agentproxyshared/cache/cachememdb/cache_memdb.go index 9746374593ec..ed2cd0ac8001 100644 --- a/command/agentproxyshared/cache/cachememdb/cache_memdb.go +++ b/command/agentproxyshared/cache/cachememdb/cache_memdb.go @@ -240,7 +240,7 @@ func (c *CacheMemDB) SetCapabilitiesIndex(index *CapabilitiesIndex) error { // EvictCapabilitiesIndex removes a capabilities index from the cache based on index name and value. func (c *CacheMemDB) EvictCapabilitiesIndex(indexName string, indexValues ...interface{}) error { index, err := c.GetCapabilitiesIndex(indexName, indexValues...) - if errors.Is(err, ErrCacheItemNotFound) { + if err == ErrCacheItemNotFound { return nil } if err != nil { diff --git a/command/agentproxyshared/cache/cachememdb/index.go b/command/agentproxyshared/cache/cachememdb/index.go index 6297b3df5f2a..484409a57954 100644 --- a/command/agentproxyshared/cache/cachememdb/index.go +++ b/command/agentproxyshared/cache/cachememdb/index.go @@ -53,15 +53,6 @@ type Index struct { // Required: true, Unique: false RequestPath string - // Versions are the versions of the secret for KVv2 static secrets only. This is - // a map of version to response, where version is the version number and response is the - // serialized cached response for that secret version. - // We could have chosen to put index.Response as Versions[0], but opted not to for consistency, - // and also to elevate the fact that the current version/representation of the path being - // cached here is stored there, not here. - // Required: false, Unique: false - Versions map[int][]byte - // Lease is the identifier of the lease in Vault, that belongs to the // response held by this index. // Required: false, Unique: true diff --git a/command/agentproxyshared/cache/lease_cache.go b/command/agentproxyshared/cache/lease_cache.go index ee135e51a047..a9ea65806d5e 100644 --- a/command/agentproxyshared/cache/lease_cache.go +++ b/command/agentproxyshared/cache/lease_cache.go @@ -14,7 +14,6 @@ import ( "io" "net/http" "net/url" - "strconv" "strings" "sync" "time" @@ -221,7 +220,6 @@ func (c *LeaseCache) PersistentStorage() *cacheboltdb.BoltStorage { // checkCacheForDynamicSecretRequest checks the cache for a particular request based on its // computed ID. It returns a non-nil *SendResponse if an entry is found. func (c *LeaseCache) checkCacheForDynamicSecretRequest(id string) (*SendResponse, error) { - c.logger.Trace("checking cache for dynamic secret request", "id", id) return c.checkCacheForRequest(id, nil) } @@ -231,7 +229,6 @@ func (c *LeaseCache) checkCacheForDynamicSecretRequest(id string) (*SendResponse // cache entry, and return nil if it isn't. It will also evict the cache if this is a non-GET // request. func (c *LeaseCache) checkCacheForStaticSecretRequest(id string, req *SendRequest) (*SendResponse, error) { - c.logger.Trace("checking cache for static secret request", "id", id) return c.checkCacheForRequest(id, req) } @@ -272,28 +269,15 @@ func (c *LeaseCache) checkCacheForRequest(id string, req *SendRequest) (*SendRes } } - var response []byte - version := getStaticSecretVersionFromRequest(req) - if version == 0 { - response = index.Response - } else { - response = index.Versions[version] - } - - // We don't have this response as either a current or older version. - if response == nil { - return nil, nil - } - // Cached request is found, deserialize the response - reader := bufio.NewReader(bytes.NewReader(response)) + reader := bufio.NewReader(bytes.NewReader(index.Response)) resp, err := http.ReadResponse(reader, nil) if err != nil { c.logger.Error("failed to deserialize response", "error", err) return nil, err } - sendResp, err := NewSendResponse(&api.Response{Response: resp}, response) + sendResp, err := NewSendResponse(&api.Response{Response: resp}, index.Response) if err != nil { c.logger.Error("failed to create new send response", "error", err) return nil, err @@ -498,8 +482,8 @@ func (c *LeaseCache) Send(ctx context.Context, req *SendRequest) (*SendResponse, // included in the request path. index.RequestPath = getStaticSecretPathFromRequest(req) - c.logger.Trace("attempting to cache static secret with following request path", "request path", index.RequestPath, "version", getStaticSecretVersionFromRequest(req)) - err := c.cacheStaticSecret(ctx, req, resp, index, secret) + c.logger.Trace("attempting to cache static secret with following request path", "request path", index.RequestPath) + err := c.cacheStaticSecret(ctx, req, resp, index) if err != nil { return nil, err } @@ -633,19 +617,16 @@ func (c *LeaseCache) Send(ctx context.Context, req *SendRequest) (*SendResponse, return resp, nil } -func (c *LeaseCache) cacheStaticSecret(ctx context.Context, req *SendRequest, resp *SendResponse, index *cachememdb.Index, secret *api.Secret) error { +func (c *LeaseCache) cacheStaticSecret(ctx context.Context, req *SendRequest, resp *SendResponse, index *cachememdb.Index) error { // If a cached version of this secret exists, we now have access, so // we don't need to re-cache, just update index.Tokens indexFromCache, err := c.db.Get(cachememdb.IndexNameID, index.ID) - if err != nil && !errors.Is(err, cachememdb.ErrCacheItemNotFound) { + if err != nil && err != cachememdb.ErrCacheItemNotFound { return err } - version := getStaticSecretVersionFromRequest(req) - // The index already exists, so all we need to do is add our token - // to the index's allowed token list, and if necessary, the new version, - // then re-store it. + // to the index's allowed token list, then re-store it. if indexFromCache != nil { // We must hold a lock for the index while it's being updated. // We keep the two locking mechanisms distinct, so that it's only writes @@ -654,45 +635,6 @@ func (c *LeaseCache) cacheStaticSecret(ctx context.Context, req *SendRequest, re defer indexFromCache.IndexLock.Unlock() indexFromCache.Tokens[req.Token] = struct{}{} - // Are we looking for a version that's already cached? - haveVersion := false - if version != 0 { - _, ok := indexFromCache.Versions[version] - if ok { - haveVersion = true - } - } else { - if indexFromCache.Response != nil { - haveVersion = true - } - } - - if !haveVersion { - var respBytes bytes.Buffer - err = resp.Response.Write(&respBytes) - if err != nil { - c.logger.Error("failed to serialize response", "error", err) - return err - } - - // Reset the response body for upper layers to read - if resp.Response.Body != nil { - resp.Response.Body.Close() - } - resp.Response.Body = io.NopCloser(bytes.NewReader(resp.ResponseBody)) - - // Set the index's Response - if version == 0 { - indexFromCache.Response = respBytes.Bytes() - // For current KVv2 secrets, see if we can add the version that the secret is - // to the versions map, too. If we got the latest version and the version is #2, - // also update Versions[2] - c.addToVersionListForCurrentVersionKVv2Secret(indexFromCache, secret) - } else { - indexFromCache.Versions[version] = respBytes.Bytes() - } - } - return c.storeStaticSecretIndex(ctx, req, indexFromCache) } @@ -710,19 +652,8 @@ func (c *LeaseCache) cacheStaticSecret(ctx context.Context, req *SendRequest, re } resp.Response.Body = io.NopCloser(bytes.NewReader(resp.ResponseBody)) - // Initialize the versions - index.Versions = map[int][]byte{} - // Set the index's Response - if version == 0 { - index.Response = respBytes.Bytes() - // For current KVv2 secrets, see if we can add the version that the secret is - // to the versions map, too. If we got the latest version and the version is #2, - // also update Versions[2] - c.addToVersionListForCurrentVersionKVv2Secret(index, secret) - } else { - index.Versions[version] = respBytes.Bytes() - } + index.Response = respBytes.Bytes() // Initialize the token map and add this token to it. index.Tokens = map[string]struct{}{req.Token: {}} @@ -730,53 +661,12 @@ func (c *LeaseCache) cacheStaticSecret(ctx context.Context, req *SendRequest, re // Set the index type index.Type = cacheboltdb.StaticSecretType - // Store the index: return c.storeStaticSecretIndex(ctx, req, index) } -// addToVersionListForCurrentVersionKVv2Secret takes a secret index and, if it's -// a KVv2 secret, adds the given response to the corresponding version for it. -// This function fails silently, as we could be parsing arbitrary JSON. -// This function can store a version for a KVv1 secret iff: -// - It has 'data' in the path -// - It has a numerical 'metadata.version' field -// However, this risk seems very small, and the negatives of such a secret being -// stored in the cache aren't worth additional mitigations to check if it's a KVv1 -// or KVv2 mount (such as doing a 'preflight' request like the CLI). -// There's no way to access it and it's just a couple of extra bytes, in the -// case that this does happen to a KVv1 secret. -func (c *LeaseCache) addToVersionListForCurrentVersionKVv2Secret(index *cachememdb.Index, secret *api.Secret) { - if secret != nil { - // First do an imperfect but lightweight check. This saves parsing the secret in the case that the secret isn't KVv2. - // KVv2 secrets always contain /data/, but KVv1 secrets can too, so we can't rely on this. - if strings.Contains(index.RequestPath, "/data/") { - metadata, ok := secret.Data["metadata"] - if ok { - metaDataAsMap, ok := metadata.(map[string]interface{}) - if ok { - versionJson, ok := metaDataAsMap["version"].(json.Number) - if ok { - versionInt64, err := versionJson.Int64() - if err == nil { - version := int(versionInt64) - c.logger.Trace("adding response for current KVv2 secret to index's Versions map", "path", index.RequestPath, "version", version) - - if index.Versions == nil { - index.Versions = map[int][]byte{} - } - - index.Versions[version] = index.Response - } - } - } - } - } - } -} - func (c *LeaseCache) storeStaticSecretIndex(ctx context.Context, req *SendRequest, index *cachememdb.Index) error { // Store the index in the cache - c.logger.Debug("storing static secret response into the cache", "path", index.RequestPath, "id", index.ID) + c.logger.Debug("storing static secret response into the cache", "method", req.Request.Method, "path", index.RequestPath, "id", index.ID) err := c.Set(ctx, index) if err != nil { c.logger.Error("failed to cache the proxied response", "error", err) @@ -805,7 +695,7 @@ func (c *LeaseCache) storeStaticSecretIndex(ctx context.Context, req *SendReques return err } - // Lastly, ensure that we start renewing this index, if it's new. + // Lastly, ensure that we start renewing this index, if it's new. // We require the 'created' check so that we don't renew the same // index multiple times. if c.capabilityManager != nil && created { @@ -822,7 +712,7 @@ func (c *LeaseCache) retrieveOrCreateTokenCapabilitiesEntry(token string) (*cach // The index ID is a hash of the token. indexId := hashStaticSecretIndex(token) indexFromCache, err := c.db.GetCapabilitiesIndex(cachememdb.IndexNameID, indexId) - if err != nil && !errors.Is(err, cachememdb.ErrCacheItemNotFound) { + if err != nil && err != cachememdb.ErrCacheItemNotFound { return nil, false, err } @@ -995,25 +885,6 @@ func canonicalizeStaticSecretPath(requestPath string, ns string) string { return path } -// getStaticSecretVersionFromRequest gets the version of a secret -// from a request. For the latest secret and for KVv1 secrets, -// this will return 0. -func getStaticSecretVersionFromRequest(req *SendRequest) int { - if req == nil || req.Request == nil { - return 0 - } - version := req.Request.FormValue("version") - if version == "" { - return 0 - } - versionInt, err := strconv.Atoi(version) - if err != nil { - // It's not a valid version. - return 0 - } - return versionInt -} - // getStaticSecretPathFromRequest gets the canonical path for a // request, taking into account intricacies relating to /v1/ and namespaces // in the header. @@ -1052,7 +923,6 @@ func computeStaticSecretCacheIndex(req *SendRequest) string { if path == "" { return path } - return hashStaticSecretIndex(path) } @@ -1094,7 +964,7 @@ func (c *LeaseCache) HandleCacheClear(ctx context.Context) http.Handler { // Default to 500 on error, unless the user provided an invalid type, // which would then be a 400. httpStatus := http.StatusInternalServerError - if errors.Is(err, errInvalidType) { + if err == errInvalidType { httpStatus = http.StatusBadRequest } logical.RespondError(w, httpStatus, fmt.Errorf("failed to clear cache: %w", err)) diff --git a/command/agentproxyshared/cache/lease_cache_test.go b/command/agentproxyshared/cache/lease_cache_test.go index d88171150631..7bb454828659 100644 --- a/command/agentproxyshared/cache/lease_cache_test.go +++ b/command/agentproxyshared/cache/lease_cache_test.go @@ -511,7 +511,7 @@ func TestLeaseCache_StoreCacheableStaticSecret(t *testing.T) { // We expect two entries to be stored by this: // 1. The actual static secret // 2. The capabilities index - err := lc.cacheStaticSecret(context.Background(), request, response, index, nil) + err := lc.cacheStaticSecret(context.Background(), request, response, index) if err != nil { return } @@ -577,7 +577,7 @@ func TestLeaseCache_StaticSecret_CacheClear_All(t *testing.T) { // We expect two entries to be stored by this: // 1. The actual static secret // 2. The capabilities index - err := lc.cacheStaticSecret(context.Background(), request, response, index, nil) + err := lc.cacheStaticSecret(context.Background(), request, response, index) if err != nil { return } diff --git a/command/agentproxyshared/cache/static_secret_cache_updater.go b/command/agentproxyshared/cache/static_secret_cache_updater.go index 0af90f923e24..3d79f9eff464 100644 --- a/command/agentproxyshared/cache/static_secret_cache_updater.go +++ b/command/agentproxyshared/cache/static_secret_cache_updater.go @@ -12,7 +12,6 @@ import ( "io" "net/http" "net/url" - "strconv" "strings" "sync/atomic" "time" @@ -29,7 +28,7 @@ import ( "nhooyr.io/websocket" ) -// Example write event (this does not contain all possible fields): +// Example Event: //{ // "id": "a3be9fb1-b514-519f-5b25-b6f144a8c1ce", // "source": "https://vaultproject.io/", @@ -59,37 +58,6 @@ import ( // "time": "2023-09-12T15:19:49.394915-07:00" //} -// Example event with namespaces for an undelete (this does not contain all possible fields): -// { -// "id": "6c6b13fd-f133-f351-3cf0-b09ae6a417b1", -// "source": "vault://hostname", -// "specversion": "1.0", -// "type": "*", -// "data": { -// "event": { -// "id": "6c6b13fd-f133-f351-3cf0-b09ae6a417b1", -// "metadata": { -// "current_version": "3", -// "destroyed_versions": "[2,3]", -// "modified": "true", -// "oldest_version": "0", -// "operation": "destroy", -// "path": "secret-v2/destroy/my-secret" -// } -// }, -// "event_type": "kv-v2/destroy", -// "plugin_info": { -// "mount_class": "secret", -// "mount_accessor": "kv_b27b3cad", -// "mount_path": "secret-v2/", -// "plugin": "kv", -// "version": "2" -// } -// }, -// "datacontentype": "application/cloudevents", -// "time": "2024-08-27T12:46:01.373097-04:00" -//} - // StaticSecretCacheUpdater is a struct that utilizes // the event system to keep the static secret cache up to date. type StaticSecretCacheUpdater struct { @@ -197,47 +165,15 @@ func (updater *StaticSecretCacheUpdater) streamStaticSecretEvents(ctx context.Co } modified, ok := metadata["modified"].(string) if ok && modified == "true" { - // If data_path were in every event, we'd get that instead, but unfortunately it isn't. path, ok := metadata["path"].(string) if !ok { - return fmt.Errorf("unexpected event format when decoding 'data_path' element, message: %s\nerror: %w", string(message), err) + return fmt.Errorf("unexpected event format when decoding 'path' element, message: %s\nerror: %w", string(message), err) } namespace, ok := data["namespace"].(string) if ok { path = namespace + path } - - deletedOrDestroyedVersions, newPath := checkForDeleteOrDestroyEvent(messageMap) - if len(deletedOrDestroyedVersions) > 0 { - path = newPath - err = updater.handleDeleteDestroyVersions(path, deletedOrDestroyedVersions) - if err != nil { - // While we are kind of 'missing' an event this way, re-calling this function will - // result in the secret remaining up to date. - return fmt.Errorf("error handling delete/destroy versions for static secret: path: %q, message: %s error: %w", path, message, err) - } - } - - // For all other operations, we *only* care about the latest version. - // However, if we know the current version, we should update that too - currentVersion := 0 - currentVersionString, ok := metadata["current_version"].(string) - if ok { - versionInt, err := strconv.Atoi(currentVersionString) - if err != nil { - return fmt.Errorf("unexpected event format when decoding 'current_version' element, message: %s\nerror: %w", string(message), err) - } - currentVersion = versionInt - } - - // Note: For delete/destroy events, we continue through to updating the secret itself, too. - // This means that if the latest version of the secret gets deleted, then the cache keeps - // knowledge of which the latest version is. - // One intricacy of e.g. destroyed events is that if the latest secret is destroyed, continuing - // to update the secret will 404. This is consistent with other behaviour. For Proxy, this means - // the secret may be evicted. That's okay. - - err = updater.updateStaticSecret(ctx, path, currentVersion) + err := updater.updateStaticSecret(ctx, path) if err != nil { // While we are kind of 'missing' an event this way, re-calling this function will // result in the secret remaining up to date. @@ -254,97 +190,6 @@ func (updater *StaticSecretCacheUpdater) streamStaticSecretEvents(ctx context.Co return nil } -// checkForDeleteOrDestroyEvent checks an event message for delete/destroy events and if there -// are any, returns the versions to be deleted or destroyed, as well as the path to -// If none can be found, returns empty array and empty string. -// We have to do this since events do not always return data_path for all events. If they did, -// we could rely on that instead of doing string manipulation. -// Example return value: [1, 2, 3], "secrets/data/my-secret". -func checkForDeleteOrDestroyEvent(eventMap map[string]interface{}) ([]int, string) { - var versions []int - - data, ok := eventMap["data"].(map[string]interface{}) - if !ok { - return versions, "" - } - - event, ok := data["event"].(map[string]interface{}) - if !ok { - return versions, "" - } - - metadata, ok := event["metadata"].(map[string]interface{}) - if !ok { - return versions, "" - } - - // We should have only one of these: - deletedVersions, ok := metadata["deleted_versions"].(string) - if ok { - err := json.Unmarshal([]byte(deletedVersions), &versions) - if err != nil { - return versions, "" - } - } - - destroyedVersions, ok := metadata["destroyed_versions"].(string) - if ok { - err := json.Unmarshal([]byte(destroyedVersions), &versions) - if err != nil { - return versions, "" - } - } - - undeletedVersions, ok := metadata["undeleted_versions"].(string) - if ok { - err := json.Unmarshal([]byte(undeletedVersions), &versions) - if err != nil { - return versions, "" - } - } - - // We have neither deleted_versions nor destroyed_versions, return early - if len(versions) == 0 { - return versions, "" - } - - path, ok := metadata["path"].(string) - if !ok { - return versions, "" - } - - namespace, ok := data["namespace"].(string) - if ok { - path = namespace + path - } - - pluginInfo, ok := data["plugin_info"].(map[string]interface{}) - if !ok { - return versions, "" - } - - mountPath := pluginInfo["mount_path"].(string) - if !ok { - return versions, "" - } - - // We get the path without the mount path for safety, just in case the namespace or mount path - // have 'data' inside. - namespaceMountPathOnly := namespace + mountPath - pathWithoutMountPath := strings.TrimPrefix(path, namespaceMountPathOnly) - - // We need to trim destroy or delete to add the correct path for where the secret - // is stored. - trimmedPath := strings.TrimPrefix(pathWithoutMountPath, "delete") - trimmedPath = strings.TrimPrefix(trimmedPath, "destroy") - trimmedPath = strings.TrimPrefix(trimmedPath, "undelete") - - // This is how we form the ID of the cached secrets - fixedPath := namespaceMountPathOnly + "data" + trimmedPath - - return versions, fixedPath -} - // preEventStreamUpdate is called after successful connection to the event system, but before // we process any events, to ensure we don't miss any updates. // In some cases, this will result in multiple processing of the same updates, but @@ -363,7 +208,7 @@ func (updater *StaticSecretCacheUpdater) preEventStreamUpdate(ctx context.Contex if index.Type != cacheboltdb.StaticSecretType { continue } - err = updater.updateStaticSecret(ctx, index.RequestPath, 0) + err = updater.updateStaticSecret(ctx, index.RequestPath) if err != nil { errs = multierror.Append(errs, err) } @@ -374,46 +219,9 @@ func (updater *StaticSecretCacheUpdater) preEventStreamUpdate(ctx context.Contex return errs.ErrorOrNil() } -// handleDeleteDestroyVersions will handle calls to deleteVersions and destroyVersions for a given cached -// secret. The handling is simple: remove them from the cache. We do the same for undeletes, as this will -// also affect the cache, but we don't re-grab the secret for undeletes. -func (updater *StaticSecretCacheUpdater) handleDeleteDestroyVersions(path string, versions []int) error { - indexId := hashStaticSecretIndex(path) - // received delete/destroy versions request: path=secret-v2/delete/my-secret - updater.logger.Debug("received delete/undelete/destroy versions request", "path", path, "indexId", indexId, "versions", versions) - - index, err := updater.leaseCache.db.Get(cachememdb.IndexNameID, indexId) - if errors.Is(err, cachememdb.ErrCacheItemNotFound) { - // This event doesn't correspond to a secret in our cache - // so this is a no-op. - return nil - } - if err != nil { - return err - } - - // Hold the lock as we're modifying the secret - index.IndexLock.Lock() - defer index.IndexLock.Unlock() - - for _, version := range versions { - delete(index.Versions, version) - } - - // Lastly, store the secret - updater.logger.Debug("storing updated secret as result of delete/undelete/destroy", "path", path, "deletedVersions", versions) - err = updater.leaseCache.db.Set(index) - if err != nil { - return err - } - - return nil -} - // updateStaticSecret checks for updates for a static secret on the path given, -// and updates the cache if appropriate. If currentVersion is not 0, we will also update -// will also update the version at index.Versions[currentVersion] with the same data. -func (updater *StaticSecretCacheUpdater) updateStaticSecret(ctx context.Context, path string, currentVersion int) error { +// and updates the cache if appropriate +func (updater *StaticSecretCacheUpdater) updateStaticSecret(ctx context.Context, path string) error { // We clone the client, as we won't be using the same token. client, err := updater.client.Clone() if err != nil { @@ -517,16 +325,9 @@ func (updater *StaticSecretCacheUpdater) updateStaticSecret(ctx context.Context, // Set the index's Response index.Response = respBytes.Bytes() index.LastRenewed = time.Now().UTC() - if currentVersion != 0 { - // It should always be non-nil, but avoid a panic just in case. - if index.Versions == nil { - index.Versions = map[int][]byte{} - } - index.Versions[currentVersion] = index.Response - } // Lastly, store the secret - updater.logger.Debug("storing response into the cache due to update", "path", path, "currentVersion", currentVersion) + updater.logger.Debug("storing response into the cache due to update", "path", path) err = updater.leaseCache.db.Set(index) if err != nil { return err diff --git a/command/agentproxyshared/cache/static_secret_cache_updater_test.go b/command/agentproxyshared/cache/static_secret_cache_updater_test.go index 69154975ec10..91158d954c09 100644 --- a/command/agentproxyshared/cache/static_secret_cache_updater_test.go +++ b/command/agentproxyshared/cache/static_secret_cache_updater_test.go @@ -536,7 +536,9 @@ func Test_StreamStaticSecretEvents_UpdatesCacheWithNewSecrets(t *testing.T) { runStreamStaticSecretEvents := func() { wg.Add(1) err := updater.streamStaticSecretEvents(context.Background()) - require.NoError(t, err) + if err != nil { + t.Fatal(err) + } } go runStreamStaticSecretEvents() @@ -548,7 +550,6 @@ func Test_StreamStaticSecretEvents_UpdatesCacheWithNewSecrets(t *testing.T) { index := &cachememdb.Index{ Namespace: "root/", RequestPath: path, - Versions: map[int][]byte{}, LastRenewed: initialTime, ID: indexId, // Valid token provided, so update should work. @@ -556,7 +557,9 @@ func Test_StreamStaticSecretEvents_UpdatesCacheWithNewSecrets(t *testing.T) { Response: []byte{}, } err := leaseCache.db.Set(index) - require.NoError(t, err) + if err != nil { + t.Fatal(err) + } secretData := map[string]interface{}{ "foo": "bar", @@ -565,7 +568,9 @@ func Test_StreamStaticSecretEvents_UpdatesCacheWithNewSecrets(t *testing.T) { err = client.Sys().Mount("secret-v2", &api.MountInput{ Type: "kv-v2", }) - require.NoError(t, err) + if err != nil { + t.Fatal(err) + } // Wait for the event stream to be fully up and running. Should be faster than this in reality, but // we make it five seconds to protect against CI flakiness. @@ -573,7 +578,9 @@ func Test_StreamStaticSecretEvents_UpdatesCacheWithNewSecrets(t *testing.T) { // Put a secret, which should trigger an event _, err = client.KVv2("secret-v2").Put(context.Background(), "foo", secretData) - require.NoError(t, err) + if err != nil { + t.Fatal(err) + } // Wait for the event to arrive. Events are usually much, much faster // than this, but we make it five seconds to protect against CI flakiness. @@ -581,19 +588,15 @@ func Test_StreamStaticSecretEvents_UpdatesCacheWithNewSecrets(t *testing.T) { // Then, do a GET to see if the index got updated by the event newIndex, err := leaseCache.db.Get(cachememdb.IndexNameID, indexId) - require.NoError(t, err) + if err != nil { + t.Fatal(err) + } require.NotNil(t, newIndex) require.NotEqual(t, []byte{}, newIndex.Response) require.Truef(t, initialTime.Before(newIndex.LastRenewed), "last updated time not updated on index") require.Equal(t, index.RequestPath, newIndex.RequestPath) require.Equal(t, index.Tokens, newIndex.Tokens) - // Assert that the corresponding version got updated too - require.Len(t, newIndex.Versions, 1) - require.NotNil(t, newIndex.Versions) - require.NotNil(t, newIndex.Versions[1]) - require.Equal(t, newIndex.Versions[1], newIndex.Response) - wg.Done() } @@ -619,13 +622,14 @@ func TestUpdateStaticSecret(t *testing.T) { RequestPath: "secret/foo", LastRenewed: initialTime, ID: indexId, - Versions: map[int][]byte{}, // Valid token provided, so update should work. Tokens: map[string]struct{}{client.Token(): {}}, Response: []byte{}, } err := leaseCache.db.Set(index) - require.NoError(t, err) + if err != nil { + t.Fatal(err) + } secretData := map[string]interface{}{ "foo": "bar", @@ -633,20 +637,25 @@ func TestUpdateStaticSecret(t *testing.T) { // create the secret in Vault. n.b. the test cluster has already mounted the KVv1 backend at "secret" err = client.KVv1("secret").Put(context.Background(), "foo", secretData) - require.NoError(t, err) + if err != nil { + t.Fatal(err) + } // attempt the update - err = updater.updateStaticSecret(context.Background(), path, 0) - require.NoError(t, err) + err = updater.updateStaticSecret(context.Background(), path) + if err != nil { + t.Fatal(err) + } newIndex, err := leaseCache.db.Get(cachememdb.IndexNameID, indexId) - require.NoError(t, err) + if err != nil { + t.Fatal(err) + } require.NotNil(t, newIndex) require.Truef(t, initialTime.Before(newIndex.LastRenewed), "last updated time not updated on index") require.NotEqual(t, []byte{}, newIndex.Response) require.Equal(t, index.RequestPath, newIndex.RequestPath) require.Equal(t, index.Tokens, newIndex.Tokens) - require.Len(t, newIndex.Versions, 0) } // TestUpdateStaticSecret_EvictsIfInvalidTokens tests that updateStaticSecret will @@ -691,7 +700,7 @@ func TestUpdateStaticSecret_EvictsIfInvalidTokens(t *testing.T) { } // attempt the update - err = updater.updateStaticSecret(context.Background(), path, 0) + err = updater.updateStaticSecret(context.Background(), path) if err != nil { t.Fatal(err) } @@ -715,14 +724,11 @@ func TestUpdateStaticSecret_HandlesNonCachedPaths(t *testing.T) { path := "secret/foo" - // Attempt the update for with currentVersion 0 - err := updater.updateStaticSecret(context.Background(), path, 0) - require.NoError(t, err) - require.Nil(t, err) - - // Attempt a higher currentVersion just to be sure - err = updater.updateStaticSecret(context.Background(), path, 100) - require.NoError(t, err) + // attempt the update + err := updater.updateStaticSecret(context.Background(), path) + if err != nil { + t.Fatal(err) + } require.Nil(t, err) } @@ -752,14 +758,15 @@ func TestPreEventStreamUpdate(t *testing.T) { RequestPath: path, LastRenewed: initialTime, ID: indexId, - Versions: map[int][]byte{}, // Valid token provided, so update should work. Tokens: map[string]struct{}{client.Token(): {}}, Response: []byte{}, Type: cacheboltdb.StaticSecretType, } err := leaseCache.db.Set(index) - require.NoError(t, err) + if err != nil { + t.Fatal(err) + } secretData := map[string]interface{}{ "foo": "bar", @@ -768,11 +775,15 @@ func TestPreEventStreamUpdate(t *testing.T) { err = client.Sys().Mount("secret-v2", &api.MountInput{ Type: "kv-v2", }) - require.NoError(t, err) + if err != nil { + t.Fatal(err) + } // Put a secret (with different values to what's currently in the cache) _, err = client.KVv2("secret-v2").Put(context.Background(), "foo", secretData) - require.NoError(t, err) + if err != nil { + t.Fatal(err) + } // perform the pre-event stream update: err = updater.preEventStreamUpdate(context.Background()) @@ -786,7 +797,6 @@ func TestPreEventStreamUpdate(t *testing.T) { require.Truef(t, initialTime.Before(newIndex.LastRenewed), "last updated time not updated on index") require.Equal(t, index.RequestPath, newIndex.RequestPath) require.Equal(t, index.Tokens, newIndex.Tokens) - require.Equal(t, index.Versions, newIndex.Versions) } // TestPreEventStreamUpdateErrorUpdating tests that preEventStreamUpdate correctly responds @@ -853,189 +863,3 @@ func TestPreEventStreamUpdateErrorUpdating(t *testing.T) { _, err = leaseCache.db.Get(cachememdb.IndexNameID, indexId) require.Equal(t, cachememdb.ErrCacheItemNotFound, err) } - -// TestCheckForDeleteOrDestroyEvent tests the behaviour of checkForDeleteOrDestroyEvent -// and assures it gives the right responses for different events. -func TestCheckForDeleteOrDestroyEvent(t *testing.T) { - t.Parallel() - - expectedVersions := []int{1, 3, 5} - jsonFormatExpectedVersions := "[1,3,5]" - expectedPath := "secret-v2/data/my-secret" - deletedVersionEventMap := map[string]interface{}{ - "id": "abc", - "source": "abc", - "data": map[string]interface{}{ - "event": map[string]interface{}{ - "id": "bar", - "metadata": map[string]interface{}{ - "current_version": "2", - "deleted_versions": jsonFormatExpectedVersions, - "modified": true, - "operation": "delete", - "path": "secret-v2/delete/my-secret", - }, - }, - "event_type": "kv-v2/delete", - "plugin_info": map[string]interface{}{ - "mount_path": "secret-v2/", - "plugin": "kv", - "version": 2, - }, - }, - } - - undeletedVersionEventMap := map[string]interface{}{ - "id": "abc", - "source": "abc", - "data": map[string]interface{}{ - "event": map[string]interface{}{ - "id": "bar", - "metadata": map[string]interface{}{ - "current_version": "2", - "undeleted_versions": jsonFormatExpectedVersions, - "modified": true, - "operation": "undelete", - "path": "secret-v2/undelete/my-secret", - }, - }, - "event_type": "kv-v2/undelete", - "plugin_info": map[string]interface{}{ - "mount_path": "secret-v2/", - "plugin": "kv", - "version": 2, - }, - }, - } - - destroyedVersionEventMap := map[string]interface{}{ - "id": "abc", - "source": "abc", - "data": map[string]interface{}{ - "event": map[string]interface{}{ - "id": "bar", - "metadata": map[string]interface{}{ - "current_version": "2", - "destroyed_versions": jsonFormatExpectedVersions, - "modified": true, - "operation": "destroy", - "path": "secret-v2/destroy/my-secret", - }, - }, - "event_type": "kv-v2/destroy", - "plugin_info": map[string]interface{}{ - "mount_path": "secret-v2/", - "plugin": "kv", - "version": 2, - }, - }, - } - - actualVersions, actualPath := checkForDeleteOrDestroyEvent(deletedVersionEventMap) - require.Equal(t, expectedVersions, actualVersions) - require.Equal(t, expectedPath, actualPath) - - actualVersions, actualPath = checkForDeleteOrDestroyEvent(undeletedVersionEventMap) - require.Equal(t, expectedVersions, actualVersions) - require.Equal(t, expectedPath, actualPath) - - actualVersions, actualPath = checkForDeleteOrDestroyEvent(destroyedVersionEventMap) - require.Equal(t, expectedVersions, actualVersions) - require.Equal(t, expectedPath, actualPath) -} - -// TestCheckForDeleteOrDestroyNamespacedEvent tests the behaviour of checkForDeleteOrDestroyEvent -// with namespaces in paths. -func TestCheckForDeleteOrDestroyNamespacedEvent(t *testing.T) { - t.Parallel() - - expectedVersions := []int{1, 3, 5} - jsonFormatExpectedVersions := "[1,3,5]" - expectedPath := "ns/secret-v2/data/my-secret" - deletedVersionEventMap := map[string]interface{}{ - "id": "abc", - "source": "abc", - "data": map[string]interface{}{ - "event": map[string]interface{}{ - "id": "bar", - "metadata": map[string]interface{}{ - "current_version": "2", - "deleted_versions": jsonFormatExpectedVersions, - "modified": true, - "operation": "delete", - "data_path": "secret-v2/data/my-secret", - "path": "secret-v2/delete/my-secret", - }, - }, - "namespace": "ns/", - "event_type": "kv-v2/delete", - "plugin_info": map[string]interface{}{ - "mount_path": "secret-v2/", - "plugin": "kv", - "version": 2, - }, - }, - } - - undeletedVersionEventMap := map[string]interface{}{ - "id": "abc", - "source": "abc", - "data": map[string]interface{}{ - "event": map[string]interface{}{ - "id": "bar", - "metadata": map[string]interface{}{ - "current_version": "2", - "undeleted_versions": jsonFormatExpectedVersions, - "modified": true, - "operation": "undelete", - "data_path": "secret-v2/data/my-secret", - "path": "secret-v2/undelete/my-secret", - }, - }, - "namespace": "ns/", - "event_type": "kv-v2/undelete", - "plugin_info": map[string]interface{}{ - "mount_path": "secret-v2/", - "plugin": "kv", - "version": 2, - }, - }, - } - - destroyedVersionEventMap := map[string]interface{}{ - "id": "abc", - "source": "abc", - "data": map[string]interface{}{ - "event": map[string]interface{}{ - "id": "bar", - "metadata": map[string]interface{}{ - "current_version": "2", - "destroyed_versions": jsonFormatExpectedVersions, - "modified": true, - "operation": "destroy", - "data_path": "secret-v2/data/my-secret", - "path": "secret-v2/destroy/my-secret", - }, - }, - "namespace": "ns/", - "event_type": "kv-v2/destroy", - "plugin_info": map[string]interface{}{ - "mount_path": "secret-v2/", - "plugin": "kv", - "version": 2, - }, - }, - } - - actualVersions, actualPath := checkForDeleteOrDestroyEvent(deletedVersionEventMap) - require.Equal(t, expectedVersions, actualVersions) - require.Equal(t, expectedPath, actualPath) - - actualVersions, actualPath = checkForDeleteOrDestroyEvent(undeletedVersionEventMap) - require.Equal(t, expectedVersions, actualVersions) - require.Equal(t, expectedPath, actualPath) - - actualVersions, actualPath = checkForDeleteOrDestroyEvent(destroyedVersionEventMap) - require.Equal(t, expectedVersions, actualVersions) - require.Equal(t, expectedPath, actualPath) -} diff --git a/ui/app/components/auth-jwt.js b/ui/app/components/auth-jwt.js index 9cf587d585ae..610b6d09933b 100644 --- a/ui/app/components/auth-jwt.js +++ b/ui/app/components/auth-jwt.js @@ -87,8 +87,6 @@ export default Component.extend({ this.onError(err); }, - // NOTE TO DEVS: Be careful when updating the OIDC flow and ensure the updates - // work with implicit flow. See issue https://github.com/hashicorp/vault-plugin-auth-jwt/pull/192 prepareForOIDC: task(function* (oidcWindow) { const thisWindow = this.getWindow(); // show the loading animation in the parent diff --git a/ui/app/services/auth.js b/ui/app/services/auth.js index e3f91aa24098..cc369c819fce 100644 --- a/ui/app/services/auth.js +++ b/ui/app/services/auth.js @@ -389,7 +389,7 @@ export default Service.extend({ const now = this.now(); this.set('lastFetch', timestamp); // if expiration was allowed and we're over half the ttl we want to go ahead and renew here - if (this.allowExpiration && this.renewAfterEpoch && now >= this.renewAfterEpoch) { + if (this.allowExpiration && now >= this.renewAfterEpoch) { this.renew(); } this.set('allowExpiration', false); diff --git a/ui/tests/acceptance/auth-test.js b/ui/tests/acceptance/auth-test.js index 6cb3cd979f76..c0f8c6d1aca7 100644 --- a/ui/tests/acceptance/auth-test.js +++ b/ui/tests/acceptance/auth-test.js @@ -11,11 +11,9 @@ import authForm from '../pages/components/auth-form'; import jwtForm from '../pages/components/auth-jwt'; import { create } from 'ember-cli-page-object'; import { setupMirage } from 'ember-cli-mirage/test-support'; -import VAULT_KEYS from 'vault/tests/helpers/vault-keys'; const component = create(authForm); const jwtComponent = create(jwtForm); -const { rootToken } = VAULT_KEYS; module('Acceptance | auth', function (hooks) { setupApplicationTest(hooks); @@ -143,17 +141,4 @@ module('Acceptance | auth', function (hooks) { await component.selectMethod('token'); await click('[data-test-auth-submit]'); }); - - test('it does not call renew-self after successful login with non-renewable token', async function (assert) { - this.server.post( - '/auth/token/renew-self', - () => new Error('should not call renew-self directly after logging in') - ); - - await visit('/vault/auth'); - await component.selectMethod('token'); - await component.token(rootToken); - await click('[data-test-auth-submit]'); - assert.strictEqual(currentURL(), '/vault/dashboard'); - }); });