Merge branch 'main' into VAULT-20402/aws-external-ids

.github/workflows/ci.yml

@@ -316,7 +316,7 @@ jobs:
                   "type": "section",
                   "text": {
                     "type": "mrkdwn",
-                    "text": "${{ needs.test-go.result != 'failure' && ':white_check_mark:' || ':x:' }} Go tests\n${{ needs.test-go-race.result != 'failure' && ':white_check_mark:' || ':x:' }} Go race tests\n\t\t${{ needs.test-go-race.outputs.data-race-result != 'success' && ':x:' || ':white_check_mark:' }} Data races detected\n${{ needs.test-go-testonly.result != 'failure' && ':white_check_mark:' || ':x:' }} Go testonly tests\n${{ needs.test-ui.result != 'failure' && ':white_check_mark:' || ':x:' }} UI tests"
+                    "text": "${{ needs.test-go.result != 'failure' && ':white_check_mark:' || ':x:' }} Go tests\n${{ needs.test-go-race.result != 'failure' && ':white_check_mark:' || ':x:' }} Go race tests\n\t\t${{ needs.test-go-race.outputs.data-race-result != 'success' && ':x: Data race detected' || ':white_check_mark: No race detected' }}\n${{ needs.test-go-testonly.result != 'failure' && ':white_check_mark:' || ':x:' }} Go testonly tests\n${{ needs.test-ui.result != 'failure' && ':white_check_mark:' || ':x:' }} UI tests"
                   },
                   "accessory": {
                     "type": "button",

.github/workflows/test-go.yml

@@ -407,6 +407,11 @@ jobs:
             package_parallelism="-p 2"
           fi
 
+          # If running Go Test 32bit nightly tests, add a flag to rerun failed tests
+          if [[  "${{inputs.name}}" == 'i386' ]]; then
+              RERUN_FAILS="--rerun-fails"
+          fi
+
           VAULT_TEST_LOG_DIR='${{ steps.metadata.outputs.go-test-log-dir-absolute }}'
           export VAULT_TEST_LOG_DIR
           mkdir -p "$VAULT_TEST_LOG_DIR"
@@ -416,6 +421,7 @@ jobs:
               --junitfile '${{ steps.metadata.outputs.gotestsum-junitfile }}' \
               --jsonfile '${{ steps.metadata.outputs.gotestsum-jsonfile }}' \
               --jsonfile-timing-events '${{ steps.metadata.outputs.gotestsum-timing-events }}' \
+              $RERUN_FAILS \
               --packages "$packages" \
               -- \
               $package_parallelism \

Makefile

@@ -174,7 +174,7 @@ hooks:
 
 # bootstrap the build by generating any necessary code and downloading additional tools that may
 # be used by devs.
-bootstrap: prep tools
+bootstrap: tools prep
 
 # Note: if you have plugins in GOPATH you can update all of them via something like:
 # for i in $(ls | grep vault-plugin-); do cd $i; git remote update; git reset --hard origin/master; dep ensure -update; git add .; git commit; git push; cd ..; done

audit/broker.go

@@ -21,6 +21,11 @@ import (
 	"github.com/hashicorp/vault/sdk/logical"
 )
 
+const (
+	// timeout is the duration which should be used for context related timeouts.
+	timeout = 5 * time.Second
+)
+
 var (
 	_ Registrar = (*Broker)(nil)
 	_ Auditor   = (*Broker)(nil)
@@ -276,9 +281,32 @@ func (b *Broker) LogRequest(ctx context.Context, in *logical.LogInput) (ret erro
 
 	e.Data = in
 
+	// Evaluate whether we need a new context for auditing.
+	var auditContext context.Context
+	if isContextViable(ctx) {
+		auditContext = ctx
+	} else {
+		// In cases where we are trying to audit the request, and the existing
+		// context is not viable due to a short deadline, we detach ourselves from
+		// the original context (keeping only the namespace).
+		// This is so that we get a fair run at writing audit entries if Vault
+		// has taken up a lot of time handling the request before audit (request)
+		// is triggered. Pipeline nodes and the eventlogger.Broker may check for a
+		// cancelled context and refuse to process the nodes further.
+		ns, err := namespace.FromContext(ctx)
+		if err != nil {
+			retErr = multierror.Append(retErr, fmt.Errorf("namespace missing from context: %w", err))
+			return retErr.ErrorOrNil()
+		}
+
+		tempContext, auditCancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer auditCancel()
+		auditContext = namespace.ContextWithNamespace(tempContext, ns)
+	}
+
 	var status eventlogger.Status
 	if hasAuditPipelines(b.broker) {
-		status, err = b.broker.Send(ctx, event.AuditType.AsEventType(), e)
+		status, err = b.broker.Send(auditContext, event.AuditType.AsEventType(), e)
 		if err != nil {
 			retErr = multierror.Append(retErr, multierror.Append(err, status.Warnings...))
 			return retErr.ErrorOrNil()
@@ -297,7 +325,7 @@ func (b *Broker) LogRequest(ctx context.Context, in *logical.LogInput) (ret erro
 	}
 
 	// Handle any additional audit that is required (Enterprise/CE dependant).
-	err = b.handleAdditionalAudit(ctx, e)
+	err = b.handleAdditionalAudit(auditContext, e)
 	if err != nil {
 		retErr = multierror.Append(retErr, err)
 	}
@@ -335,21 +363,28 @@ func (b *Broker) LogResponse(ctx context.Context, in *logical.LogInput) (ret err
 
 	e.Data = in
 
-	// In cases where we are trying to audit the response, we detach
-	// ourselves from the original context (keeping only the namespace).
-	// This is so that we get a fair run at writing audit entries if Vault
-	// has taken up a lot of time handling the request before audit (response)
-	// is triggered. Pipeline nodes and the eventlogger.Broker may check for a
-	// cancelled context and refuse to process the nodes further.
-	ns, err := namespace.FromContext(ctx)
-	if err != nil {
-		retErr = multierror.Append(retErr, fmt.Errorf("namespace missing from context: %w", err))
-		return retErr.ErrorOrNil()
-	}
+	// Evaluate whether we need a new context for auditing.
+	var auditContext context.Context
+	if isContextViable(ctx) {
+		auditContext = ctx
+	} else {
+		// In cases where we are trying to audit the response, and the existing
+		// context is not viable due to a short deadline, we detach ourselves from
+		// the original context (keeping only the namespace).
+		// This is so that we get a fair run at writing audit entries if Vault
+		// has taken up a lot of time handling the request before audit (response)
+		// is triggered. Pipeline nodes and the eventlogger.Broker may check for a
+		// cancelled context and refuse to process the nodes further.
+		ns, err := namespace.FromContext(ctx)
+		if err != nil {
+			retErr = multierror.Append(retErr, fmt.Errorf("namespace missing from context: %w", err))
+			return retErr.ErrorOrNil()
+		}
 
-	auditContext, auditCancel := context.WithTimeout(context.Background(), 5*time.Second)
-	defer auditCancel()
-	auditContext = namespace.ContextWithNamespace(auditContext, ns)
+		tempContext, auditCancel := context.WithTimeout(context.Background(), timeout)
+		defer auditCancel()
+		auditContext = namespace.ContextWithNamespace(tempContext, ns)
+	}
 
 	var status eventlogger.Status
 	if hasAuditPipelines(b.broker) {
@@ -424,3 +459,26 @@ func (b *Broker) IsRegistered(name string) bool {
 
 	return b.isRegisteredByName(name)
 }
+
+// isContextViable examines the supplied context to see if its own deadline would
+// occur later than a newly created context with a specific timeout.
+// If the existing context is viable it can be used 'as-is', if not, the caller
+// should consider creating a new context with the relevant deadline and associated
+// context values (e.g. namespace) in order to reduce the likelihood that the
+// audit system believes there is a failure in audit (and updating its metrics)
+// when the root cause is elsewhere.
+func isContextViable(ctx context.Context) bool {
+	if ctx == nil {
+		return false
+	}
+
+	deadline, hasDeadline := ctx.Deadline()
+
+	// If there's no deadline on the context then we don't need to worry about
+	// it being cancelled due to a timeout.
+	if !hasDeadline {
+		return true
+	}
+
+	return deadline.After(time.Now().Add(timeout))
+}

audit/broker_test.go

@@ -158,3 +158,52 @@ func BenchmarkAuditBroker_File_Request_DevNull(b *testing.B) {
 		}
 	})
 }
+
+// TestBroker_isContextViable_basics checks the expected result of isContextViable
+// for basic inputs such as nil and a never-ending context.
+func TestBroker_isContextViable_basics(t *testing.T) {
+	t.Parallel()
+
+	require.False(t, isContextViable(nil))
+	require.True(t, isContextViable(context.Background()))
+}
+
+// TestBroker_isContextViable_timeouts checks the expected result of isContextViable
+// for various timeout durations.
+func TestBroker_isContextViable_timeouts(t *testing.T) {
+	t.Parallel()
+
+	tests := map[string]struct {
+		timeout  time.Duration
+		expected bool
+	}{
+		"2s-smaller-deadline": {
+			timeout:  timeout - 2*time.Second,
+			expected: false,
+		},
+		"same-deadline": {
+			timeout:  timeout,
+			expected: false, // Expected as a near miss
+		},
+		"same-deadline-plus": {
+			timeout:  timeout + 5*time.Millisecond,
+			expected: true,
+		},
+		"2x-longer-deadline": {
+			timeout:  timeout * 2,
+			expected: true,
+		},
+	}
+
+	for name, tc := range tests {
+		name := name
+		tc := tc
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx, cancel := context.WithTimeout(context.Background(), tc.timeout)
+			t.Cleanup(func() { cancel() })
+			require.Equal(t, tc.expected, isContextViable(ctx))
+		})
+	}
+}

builtin/logical/pki/cert_util.go

@@ -253,7 +253,7 @@ func fetchCertBySerial(sc *storageContext, prefix, serial string) (*logical.Stor
 	certCounter := sc.Backend.GetCertificateCounter()
 	certsCounted := certCounter.IsInitialized()
 	if err = sc.Storage.Put(sc.Context, certEntry); err != nil {
-		return nil, errutil.InternalError{Err: fmt.Sprintf("error saving certificate with serial %s to new location", serial)}
+		return nil, errutil.InternalError{Err: fmt.Sprintf("error saving certificate with serial %s to new location: %s", serial, err)}
 	}
 	if err = sc.Storage.Delete(sc.Context, legacyPath); err != nil {
 		// If we fail here, we have an extra (copy) of a cert in storage, add to metrics:

changelog/26616.txt

@@ -0,0 +1,5 @@
+```release-note:bug
+core/audit: Audit logging a Vault request/response will now use a minimum 5 second context timeout. 
+If the existing context deadline occurs later than 5s in the future, it will be used, otherwise a 
+new context, separate from the original will be used.
+```

changelog/26663.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+ui: Show computed values from `sys/internal/ui/mounts` endpoint for auth mount configuration view
+```

helper/testhelpers/testhelpers.go

@@ -728,7 +728,7 @@ func RetryUntilAtCadence(t testing.T, timeout, sleepTime time.Duration, f func()
 	RetryUntilAtCadenceWithHandler(t, timeout, sleepTime, fail, f)
 }
 
-// RetryUntilAtCadence runs f until it returns a nil result or the timeout is reached.
+// RetryUntilAtCadenceWithHandler runs f until it returns a nil result or the timeout is reached.
 // If a nil result hasn't been obtained by timeout, onFailure is called.
 func RetryUntilAtCadenceWithHandler(t testing.T, timeout, sleepTime time.Duration, onFailure func(error), f func() error) {
 	t.Helper()
@@ -743,8 +743,11 @@ func RetryUntilAtCadenceWithHandler(t testing.T, timeout, sleepTime time.Duratio
 	onFailure(err)
 }
 
-// RetryUntil runs f until it returns a nil result or the timeout is reached.
+// RetryUntil runs f with a 100ms pause between calls, until f returns a nil result
+// or the timeout is reached.
 // If a nil result hasn't been obtained by timeout, calls t.Fatal.
+// NOTE: See RetryUntilAtCadence if you want to specify a different wait/sleep
+// duration between calls.
 func RetryUntil(t testing.T, timeout time.Duration, f func() error) {
 	t.Helper()
 	RetryUntilAtCadence(t, timeout, 100*time.Millisecond, f)

ui/app/adapters/auth-method.js

@@ -21,20 +21,26 @@ export default ApplicationAdapter.extend({
 
   findAll(store, type, sinceToken, snapshotRecordArray) {
     const isUnauthenticated = snapshotRecordArray?.adapterOptions?.unauthenticated;
-    if (isUnauthenticated) {
+    // sys/internal/ui/mounts returns the actual value of the system TTL
+    // instead of '0' which just indicates the mount is using system defaults
+    const useMountsEndpoint = snapshotRecordArray?.adapterOptions?.useMountsEndpoint;
+    if (isUnauthenticated || useMountsEndpoint) {
       const url = `/${this.urlPrefix()}/internal/ui/mounts`;
       return this.ajax(url, 'GET', {
-        unauthenticated: true,
+        unauthenticated: isUnauthenticated,
       })
         .then((result) => {
           return {
             data: result.data.auth,
           };
         })
-        .catch(() => {
-          return {
-            data: {},
-          };
+        .catch((e) => {
+          if (isUnauthenticated) return { data: {} };
+
+          if (e instanceof AdapterError) {
+            set(e, 'policyPath', 'sys/internal/ui/mounts');
+          }
+          throw e;
         });
     }
     return this.ajax(this.url(), 'GET').catch((e) => {

ui/app/app.js

@@ -15,7 +15,7 @@ export default class App extends Application {
   engines = {
     'config-ui': {
       dependencies: {
-        services: ['auth', 'flash-messages', 'namespace', 'router', 'store', 'version', 'customMessages'],
+        services: ['auth', 'flash-messages', 'namespace', 'router', 'store', 'version', 'custom-messages'],
       },
     },
     'open-api-explorer': {

ui/app/helpers/tabs-for-auth-section.js

@@ -99,7 +99,7 @@ export function tabsForAuthSection([authMethodModel, sectionType = 'authSettings
     tabs.push({
       label: 'Method Options',
       route: 'vault.cluster.settings.auth.configure.section',
-      routeParams: ['options'],
+      routeParams: [authMethodModel.id, 'options'],
     });
     return tabs;
   }

ui/app/models/identity/group-alias.js

@@ -14,7 +14,7 @@ export default IdentityModel.extend({
   formFields: computed(function () {
     return ['name', 'mountAccessor'];
   }),
-  group: belongsTo('identity/group', { readOnly: true, async: false }),
+  group: belongsTo('identity/group', { readOnly: true, async: false, inverse: 'alias' }),
 
   name: attr('string'),
   canonicalId: attr('string'),

ui/app/routes/vault/cluster/access/method.js

@@ -15,26 +15,28 @@ export default Route.extend({
 
   model(params) {
     const { path } = params;
-    return this.store.findAll('auth-method').then((modelArray) => {
-      const model = modelArray.find((m) => m.id === path);
-      if (!model) {
-        const error = new AdapterError();
-        set(error, 'httpStatus', 404);
-        throw error;
-      }
-      const supportManaged = supportedManagedAuthBackends();
-      if (!supportManaged.includes(model.methodType)) {
-        // do not fetch path-help for unmanaged auth types
-        model.set('paths', {
-          apiPath: model.apiPath,
-          paths: [],
+    return this.store
+      .findAll('auth-method', { adapterOptions: { useMountsEndpoint: true } })
+      .then((modelArray) => {
+        const model = modelArray.find((m) => m.id === path);
+        if (!model) {
+          const error = new AdapterError();
+          set(error, 'httpStatus', 404);
+          throw error;
+        }
+        const supportManaged = supportedManagedAuthBackends();
+        if (!supportManaged.includes(model.methodType)) {
+          // do not fetch path-help for unmanaged auth types
+          model.set('paths', {
+            apiPath: model.apiPath,
+            paths: [],
+          });
+          return model;
+        }
+        return this.pathHelp.getPaths(model.apiPath, path).then((paths) => {
+          model.set('paths', paths);
+          return model;
         });
-        return model;
-      }
-      return this.pathHelp.getPaths(model.apiPath, path).then((paths) => {
-        model.set('paths', paths);
-        return model;
       });
-    });
   },
 });

ui/app/routes/vault/cluster/logout.js

@@ -45,9 +45,8 @@ export default Route.extend(ModelBoundaryRoute, {
       queryParams.namespace = ns;
     }
     if (Ember.testing) {
-      // TODO: cleanup this replaceWith instance. Using router.replaceWith causes test failures
       // Don't redirect on the test
-      this.replaceWith('vault.cluster.auth', { queryParams });
+      this.router.replaceWith('vault.cluster.auth', { queryParams });
     } else {
       const { cluster_name } = this.paramsFor('vault.cluster');
       location.assign(this.router.urlFor('vault.cluster.auth', cluster_name, { queryParams }));

ui/app/templates/components/auth-method/configuration.hbs

@@ -10,12 +10,14 @@
         @alwaysRender={{not (is-empty-value (get @model attr.name))}}
         @label={{or attr.options.label (to-label attr.name)}}
         @value={{stringify (get @model attr.name)}}
+        @formatTtl={{eq attr.options.editType "ttl"}}
       />
     {{else}}
       <InfoTableRow
         @alwaysRender={{not (is-empty-value (get @model attr.name))}}
         @label={{or attr.options.label (to-label attr.name)}}
         @value={{get @model attr.name}}
+        @formatTtl={{eq attr.options.editType "ttl"}}
       />
     {{/if}}
   {{/each}}