fix(shell): s3.user.provision handles existing users by attaching policy (#9040)

* fix(shell): s3.user.provision handles existing users by attaching policy

Instead of erroring when the user already exists, the command now
creates the policy and attaches it to the existing user via UpdateUser.
Credentials are only generated and displayed for newly created users.

* fix(shell): skip duplicate policy attachment in s3.user.provision

Check if the policy is already attached before appending and calling
UpdateUser, making repeated runs idempotent.

* fix(shell): generate service account ID in s3.serviceaccount.create

The command built a ServiceAccount proto without setting Id, which was
rejected by credential.ValidateServiceAccountId on any real store. Now
generates sa:<parent>:<uuid> matching the format used by the admin UI.

* test(s3): integration tests for s3.* shell commands

Adds TestShell* integration tests covering ~40 previously untested
shell commands: user, accesskey, group, serviceaccount, anonymous,
bucket, policy.attach/detach, config.show, and iam.export/import.

Switches the test cluster's credential store from memory to filer_etc
because the memory store silently drops groups and service accounts
in LoadConfiguration/SaveConfiguration.

* fix(shell): rollback policy on key generation failure in s3.user.provision

If iam.GenerateRandomString or iam.GenerateSecretAccessKey fails after
the policy was persisted, the policy would be left orphaned. Extracts
the rollback logic into a local closure and invokes it on all failure
paths after policy creation for consistency.

* address PR review feedback for s3 shell tests and serviceaccount

- s3.serviceaccount.create: use 16 bytes of randomness (hex-encoded) for
  the service account UUID instead of 4 bytes to eliminate collision risk
- s3.serviceaccount.create: print the actual ID and drop the outdated
  "server-assigned" note (the ID is now client-generated)
- tests: guard createdAK in accesskey rotate/delete subtests so sibling
  failures don't run invalid CLI calls
- tests: requireContains/requireNotContains use t.Fatalf to fail fast
- tests: Provision subtest asserts the "Attached policy" message on the
  second provision call for an existing user
- tests: update extractServiceAccountID comment example to match the
  sa:<parent>:<uuid> format
- tests: drop redundant saID empty-check (extractServiceAccountID fatals)

* test(s3): use t.Fatalf for precondition check in serviceaccount test

test/s3/policy/policy_test.go

@@ -732,10 +732,12 @@ func startMiniCluster(t *testing.T) (*TestCluster, error) {
 	err := os.WriteFile(securityToml, []byte("# Empty security config\n"), 0644)
 	require.NoError(t, err)
 
-	// Configure credential store for IAM tests
+	// Configure credential store for IAM tests.
+	// Use filer_etc instead of memory because the memory store does not
+	// persist groups or service accounts through LoadConfiguration/SaveConfiguration.
 	credentialToml := filepath.Join(testDir, "credential.toml")
 	credentialConfig := `
-[credential.memory]
+[credential.filer_etc]
 enabled = true
 `
 	err = os.WriteFile(credentialToml, []byte(credentialConfig), 0644)

test/s3/policy/shell_accesskey_test.go

@@ -0,0 +1,75 @@
+package policy
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb"
+	"github.com/stretchr/testify/require"
+)
+
+// TestShellAccessKeyLifecycle exercises s3.accesskey.* commands end-to-end.
+func TestShellAccessKeyLifecycle(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+	cluster, err := startMiniCluster(t)
+	require.NoError(t, err)
+	defer cluster.Stop()
+
+	const weedCmd = "weed"
+	master := string(pb.NewServerAddress("127.0.0.1", cluster.masterPort, cluster.masterGrpcPort))
+	filer := string(pb.NewServerAddress("127.0.0.1", cluster.filerPort, cluster.filerGrpcPort))
+
+	userName := uniqueName("akuser")
+	// Create user with explicit key so we know the initial value.
+	initialAK := "INITIALAK1234567890X"
+	initialSK := "initialsecret1234567890abcdefghijklmnop"
+	execShell(t, weedCmd, master, filer,
+		fmt.Sprintf("s3.user.create -name %s -access_key %s -secret_key %s", userName, initialAK, initialSK))
+	defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.delete -name %s", userName))
+
+	t.Run("ListInitialKey", func(t *testing.T) {
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.accesskey.list -user %s", userName))
+		requireContains(t, out, initialAK, "accesskey.list initial")
+	})
+
+	var createdAK string
+	t.Run("CreateAdditionalKey", func(t *testing.T) {
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.accesskey.create -user %s", userName))
+		requireContains(t, out, "Access Key:", "accesskey.create output")
+		requireContains(t, out, "Secret Key:", "accesskey.create output")
+		createdAK = extractFieldAfter(out, "Access Key:")
+		if createdAK == "" {
+			t.Fatalf("failed to extract access key from create output:\n%s", out)
+		}
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.accesskey.list -user %s", userName))
+		requireContains(t, out, initialAK, "list contains original")
+		requireContains(t, out, createdAK, "list contains new key")
+	})
+
+	t.Run("RotateKey", func(t *testing.T) {
+		if createdAK == "" {
+			t.Fatal("createdAK is empty; CreateAdditionalKey must run successfully first")
+		}
+		out := execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.accesskey.rotate -user %s -access_key %s", userName, initialAK))
+		requireContains(t, out, initialAK, "rotate shows old key")
+		requireContains(t, out, "deleted", "rotate marks old key deleted")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.accesskey.list -user %s", userName))
+		requireNotContains(t, out, initialAK, "old key removed")
+		requireContains(t, out, createdAK, "other key still present")
+	})
+
+	t.Run("DeleteKey", func(t *testing.T) {
+		if createdAK == "" {
+			t.Fatal("createdAK is empty; CreateAdditionalKey must run successfully first")
+		}
+		execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.accesskey.delete -user %s -access_key %s", userName, createdAK))
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.accesskey.list -user %s", userName))
+		requireNotContains(t, out, createdAK, "deleted key removed from list")
+	})
+}

test/s3/policy/shell_anonymous_test.go

@@ -0,0 +1,54 @@
+package policy
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb"
+	"github.com/stretchr/testify/require"
+)
+
+// TestShellAnonymousAccess exercises s3.anonymous.* commands end-to-end.
+func TestShellAnonymousAccess(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+	cluster, err := startMiniCluster(t)
+	require.NoError(t, err)
+	defer cluster.Stop()
+
+	const weedCmd = "weed"
+	master := string(pb.NewServerAddress("127.0.0.1", cluster.masterPort, cluster.masterGrpcPort))
+	filer := string(pb.NewServerAddress("127.0.0.1", cluster.filerPort, cluster.filerGrpcPort))
+
+	bucketName := uniqueName("anon-bkt")
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.create -name %s", bucketName))
+	defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.delete -name %s", bucketName))
+
+	t.Run("SetAndGet", func(t *testing.T) {
+		out := execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.anonymous.set -bucket %s -access Read,List", bucketName))
+		requireContains(t, out, bucketName, "anonymous.set output")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.anonymous.get -bucket %s", bucketName))
+		requireContains(t, out, bucketName, "anonymous.get bucket")
+		requireContains(t, out, "Read", "anonymous.get read action")
+		requireContains(t, out, "List", "anonymous.get list action")
+	})
+
+	t.Run("List", func(t *testing.T) {
+		out := execShell(t, weedCmd, master, filer, "s3.anonymous.list")
+		requireContains(t, out, bucketName, "anonymous.list contains bucket")
+	})
+
+	t.Run("SetNone", func(t *testing.T) {
+		execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.anonymous.set -bucket %s -access none", bucketName))
+
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.anonymous.get -bucket %s", bucketName))
+		requireContains(t, out, "none", "anonymous.get after set none")
+
+		out = execShell(t, weedCmd, master, filer, "s3.anonymous.list")
+		requireNotContains(t, out, bucketName, "anonymous.list after clearing")
+	})
+}

test/s3/policy/shell_bucket_test.go

@@ -0,0 +1,128 @@
+package policy
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb"
+	"github.com/stretchr/testify/require"
+)
+
+// TestShellBucketLifecycle exercises s3.bucket.* commands end-to-end:
+// create/list/delete, owner, quota, versioning, lock, quota.enforce.
+func TestShellBucketLifecycle(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+	cluster, err := startMiniCluster(t)
+	require.NoError(t, err)
+	defer cluster.Stop()
+
+	const weedCmd = "weed"
+	master := string(pb.NewServerAddress("127.0.0.1", cluster.masterPort, cluster.masterGrpcPort))
+	filer := string(pb.NewServerAddress("127.0.0.1", cluster.filerPort, cluster.filerGrpcPort))
+
+	t.Run("CreateListDelete", func(t *testing.T) {
+		bucketName := uniqueName("bkt")
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.create -name %s", bucketName))
+		requireContains(t, out, bucketName, "bucket.create output")
+
+		out = execShell(t, weedCmd, master, filer, "s3.bucket.list")
+		requireContains(t, out, bucketName, "bucket.list contains created")
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.delete -name %s", bucketName))
+
+		out = execShell(t, weedCmd, master, filer, "s3.bucket.list")
+		requireNotContains(t, out, bucketName, "bucket.list after delete")
+	})
+
+	t.Run("Owner", func(t *testing.T) {
+		bucketName := uniqueName("bkt-own")
+		ownerName := uniqueName("owner")
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.create -name %s", ownerName))
+		defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.delete -name %s", ownerName))
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.create -name %s", bucketName))
+		defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.delete -name %s", bucketName))
+
+		// Initially no owner.
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.owner -name %s", bucketName))
+		requireContains(t, out, "none", "initial owner none")
+
+		// Set owner.
+		execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.bucket.owner -name %s -owner %s", bucketName, ownerName))
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.owner -name %s", bucketName))
+		requireContains(t, out, ownerName, "owner set")
+
+		// Remove owner.
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.owner -name %s -delete", bucketName))
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.owner -name %s", bucketName))
+		requireContains(t, out, "none", "owner removed")
+	})
+
+	t.Run("Quota", func(t *testing.T) {
+		bucketName := uniqueName("bkt-quota")
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.create -name %s", bucketName))
+		defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.delete -name %s", bucketName))
+
+		execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.bucket.quota -name %s -op=set -sizeMB=1024", bucketName))
+
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.quota -name %s -op=get", bucketName))
+		requireContains(t, out, "1024", "quota.get shows size")
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.quota -name %s -op=disable", bucketName))
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.quota -name %s -op=enable", bucketName))
+
+		// Enforce should run on an empty bucket without error.
+		execShell(t, weedCmd, master, filer, "s3.bucket.quota.enforce -apply")
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.quota -name %s -op=remove", bucketName))
+	})
+
+	t.Run("Versioning", func(t *testing.T) {
+		bucketName := uniqueName("bkt-ver")
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.create -name %s", bucketName))
+		defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.delete -name %s", bucketName))
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.versioning -name %s -enable", bucketName))
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.versioning -name %s", bucketName))
+		requireContains(t, out, "Enabled", "versioning enabled")
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.versioning -name %s -suspend", bucketName))
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.versioning -name %s", bucketName))
+		requireContains(t, out, "Suspended", "versioning suspended")
+	})
+
+	t.Run("Lock", func(t *testing.T) {
+		bucketName := uniqueName("bkt-lock")
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.create -name %s", bucketName))
+		defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.delete -name %s", bucketName))
+
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.lock -name %s", bucketName))
+		requireContains(t, out, "Disabled", "lock initially disabled")
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.lock -name %s -enable", bucketName))
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.lock -name %s", bucketName))
+		requireContains(t, out, "Enabled", "lock enabled")
+
+		// Versioning should have been auto-enabled.
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.versioning -name %s", bucketName))
+		requireContains(t, out, "Enabled", "versioning auto-enabled by lock")
+	})
+
+	t.Run("CreateWithLock", func(t *testing.T) {
+		bucketName := uniqueName("bkt-wlock")
+		out := execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.bucket.create -name %s -withLock", bucketName))
+		// Cleanup may fail if the bucket contains locked objects; we created none.
+		defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.delete -name %s", bucketName))
+
+		requireContains(t, out, "Object Lock", "create -withLock output")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.lock -name %s", bucketName))
+		requireContains(t, out, "Enabled", "lock enabled after create -withLock")
+	})
+}

test/s3/policy/shell_config_test.go

@@ -0,0 +1,86 @@
+package policy
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb"
+	"github.com/stretchr/testify/require"
+)
+
+// TestShellConfigShow verifies s3.config.show outputs a summary of IAM config.
+func TestShellConfigShow(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+	cluster, err := startMiniCluster(t)
+	require.NoError(t, err)
+	defer cluster.Stop()
+
+	const weedCmd = "weed"
+	master := string(pb.NewServerAddress("127.0.0.1", cluster.masterPort, cluster.masterGrpcPort))
+	filer := string(pb.NewServerAddress("127.0.0.1", cluster.filerPort, cluster.filerGrpcPort))
+
+	userName := uniqueName("cfg-user")
+	groupName := uniqueName("cfg-grp")
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.create -name %s", userName))
+	defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.delete -name %s", userName))
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.group.create -name %s", groupName))
+	defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.group.delete -name %s", groupName))
+
+	out := execShell(t, weedCmd, master, filer, "s3.config.show")
+	requireContains(t, out, "S3 IAM Configuration Summary", "config.show header")
+	requireContains(t, out, userName, "config.show contains user")
+	requireContains(t, out, groupName, "config.show contains group")
+}
+
+// TestShellIAMExportImport does a roundtrip: create resources, export, delete, import, verify.
+func TestShellIAMExportImport(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+	cluster, err := startMiniCluster(t)
+	require.NoError(t, err)
+	defer cluster.Stop()
+
+	const weedCmd = "weed"
+	master := string(pb.NewServerAddress("127.0.0.1", cluster.masterPort, cluster.masterGrpcPort))
+	filer := string(pb.NewServerAddress("127.0.0.1", cluster.filerPort, cluster.filerGrpcPort))
+
+	userName := uniqueName("exp-user")
+	groupName := uniqueName("exp-grp")
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.create -name %s", userName))
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.group.create -name %s", groupName))
+
+	exportFile := filepath.Join(t.TempDir(), "iam_export.txt")
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.iam.export -file %s", exportFile))
+
+	data, err := os.ReadFile(exportFile)
+	require.NoError(t, err)
+	content := string(data)
+	requireContains(t, content, userName, "export file contains user")
+	requireContains(t, content, groupName, "export file contains group")
+
+	// Delete the resources.
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.delete -name %s", userName))
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.group.delete -name %s", groupName))
+
+	out := execShell(t, weedCmd, master, filer, "s3.user.list")
+	requireNotContains(t, out, userName, "user gone before import")
+
+	// Import.
+	out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.iam.import -file %s -apply", exportFile))
+	requireContains(t, out, "Imported IAM configuration", "import output")
+
+	// Verify resources restored.
+	out = execShell(t, weedCmd, master, filer, "s3.user.list")
+	requireContains(t, out, userName, "user restored after import")
+	out = execShell(t, weedCmd, master, filer, "s3.group.list")
+	requireContains(t, out, groupName, "group restored after import")
+
+	// Cleanup.
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.delete -name %s", userName))
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.group.delete -name %s", groupName))
+}

test/s3/policy/shell_group_test.go

@@ -0,0 +1,62 @@
+package policy
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb"
+	"github.com/stretchr/testify/require"
+)
+
+// TestShellGroupLifecycle exercises s3.group.* commands end-to-end.
+func TestShellGroupLifecycle(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+	cluster, err := startMiniCluster(t)
+	require.NoError(t, err)
+	defer cluster.Stop()
+
+	const weedCmd = "weed"
+	master := string(pb.NewServerAddress("127.0.0.1", cluster.masterPort, cluster.masterGrpcPort))
+	filer := string(pb.NewServerAddress("127.0.0.1", cluster.filerPort, cluster.filerGrpcPort))
+
+	groupName := uniqueName("grp")
+	userName := uniqueName("grpuser")
+
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.create -name %s", userName))
+	defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.delete -name %s", userName))
+
+	t.Run("CreateShowList", func(t *testing.T) {
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.group.create -name %s", groupName))
+		requireContains(t, out, groupName, "group.create output")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.group.show -name %s", groupName))
+		requireContains(t, out, groupName, "group.show output")
+
+		out = execShell(t, weedCmd, master, filer, "s3.group.list")
+		requireContains(t, out, groupName, "group.list")
+	})
+
+	t.Run("AddRemoveUser", func(t *testing.T) {
+		out := execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.group.add-user -group %s -user %s", groupName, userName))
+		requireContains(t, out, userName, "group.add-user output")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.group.show -name %s", groupName))
+		requireContains(t, out, userName, "group.show after add")
+
+		out = execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.group.remove-user -group %s -user %s", groupName, userName))
+		requireContains(t, out, userName, "group.remove-user output")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.group.show -name %s", groupName))
+		requireNotContains(t, out, fmt.Sprintf("\"%s\"", userName), "group.show after remove")
+	})
+
+	t.Run("Delete", func(t *testing.T) {
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.group.delete -name %s", groupName))
+		out := execShell(t, weedCmd, master, filer, "s3.group.list")
+		requireNotContains(t, out, fmt.Sprintf("\"%s\"", groupName), "group.list after delete")
+	})
+}

test/s3/policy/shell_helpers_test.go

@@ -0,0 +1,73 @@
+package policy
+
+import (
+	"strings"
+	"testing"
+)
+
+// requireContains fails the test if substr is not found in output.
+func requireContains(t *testing.T, output, substr, context string) {
+	t.Helper()
+	if !strings.Contains(output, substr) {
+		t.Fatalf("%s: expected output to contain %q\n--- output ---\n%s\n--- end ---", context, substr, output)
+	}
+}
+
+// requireNotContains fails the test if substr IS found in output.
+func requireNotContains(t *testing.T, output, substr, context string) {
+	t.Helper()
+	if strings.Contains(output, substr) {
+		t.Fatalf("%s: expected output to NOT contain %q\n--- output ---\n%s\n--- end ---", context, substr, output)
+	}
+}
+
+// extractFieldAfter returns the first occurrence of the value after a "Prefix: " line.
+// Example: extractFieldAfter(out, "Access Key:") -> "AKIAXXXX..."
+// Returns "" if not found.
+func extractFieldAfter(output, prefix string) string {
+	for _, line := range strings.Split(output, "\n") {
+		line = strings.TrimSpace(line)
+		if strings.HasPrefix(line, prefix) {
+			return strings.TrimSpace(strings.TrimPrefix(line, prefix))
+		}
+	}
+	return ""
+}
+
+// splitLines splits output into trimmed non-empty lines.
+func splitLines(output string) []string {
+	var lines []string
+	for _, line := range strings.Split(output, "\n") {
+		if trimmed := strings.TrimSpace(line); trimmed != "" {
+			lines = append(lines, trimmed)
+		}
+	}
+	return lines
+}
+
+// fieldsOf splits a line on whitespace.
+func fieldsOf(line string) []string {
+	return strings.Fields(line)
+}
+
+// extractServiceAccountID parses the tab-separated output of `s3.serviceaccount.list`
+// and returns the ID of the first row whose PARENT column matches parentUser.
+// The list output format is:
+//
+//	ID                                         PARENT     STATUS    DESCRIPTION
+//	sa:user-yyy:a1b2c3d4e5f6...                user-yyy   enabled   some desc
+func extractServiceAccountID(t *testing.T, listOutput, parentUser string) string {
+	t.Helper()
+	for _, line := range strings.Split(listOutput, "\n") {
+		line = strings.TrimSpace(line)
+		if line == "" || strings.HasPrefix(line, "ID") || strings.HasPrefix(line, "No service accounts") {
+			continue
+		}
+		fields := strings.Fields(line)
+		if len(fields) >= 2 && fields[1] == parentUser {
+			return fields[0]
+		}
+	}
+	t.Fatalf("could not find service account with parent=%q in output:\n%s", parentUser, listOutput)
+	return ""
+}

test/s3/policy/shell_policy_attach_test.go

@@ -0,0 +1,67 @@
+package policy
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb"
+	"github.com/stretchr/testify/require"
+)
+
+// TestShellPolicyAttachDetach exercises s3.policy.attach and s3.policy.detach.
+func TestShellPolicyAttachDetach(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+	cluster, err := startMiniCluster(t)
+	require.NoError(t, err)
+	defer cluster.Stop()
+
+	const weedCmd = "weed"
+	master := string(pb.NewServerAddress("127.0.0.1", cluster.masterPort, cluster.masterGrpcPort))
+	filer := string(pb.NewServerAddress("127.0.0.1", cluster.filerPort, cluster.filerGrpcPort))
+
+	// Create a policy via file.
+	policyJSON := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:GetObject","Resource":"*"}]}`
+	tmpFile, err := os.CreateTemp("", "test_policy_*.json")
+	require.NoError(t, err)
+	defer os.Remove(tmpFile.Name())
+	_, err = tmpFile.WriteString(policyJSON)
+	require.NoError(t, err)
+	require.NoError(t, tmpFile.Close())
+
+	policyName := uniqueName("attach-pol")
+	userName := uniqueName("attach-user")
+
+	execShell(t, weedCmd, master, filer,
+		fmt.Sprintf("s3.policy -put -name=%s -file=%s", policyName, tmpFile.Name()))
+	defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.policy -delete -name=%s", policyName))
+
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.create -name %s", userName))
+	defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.delete -name %s", userName))
+
+	t.Run("AttachAndVerify", func(t *testing.T) {
+		out := execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.policy.attach -policy %s -user %s", policyName, userName))
+		requireContains(t, out, policyName, "policy.attach output")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.show -name %s", userName))
+		requireContains(t, out, policyName, "user.show after attach")
+	})
+
+	t.Run("AttachIdempotent", func(t *testing.T) {
+		// Should succeed without error per the command's idempotent design.
+		execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.policy.attach -policy %s -user %s", policyName, userName))
+	})
+
+	t.Run("DetachAndVerify", func(t *testing.T) {
+		out := execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.policy.detach -policy %s -user %s", policyName, userName))
+		requireContains(t, out, policyName, "policy.detach output")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.show -name %s", userName))
+		requireNotContains(t, out, fmt.Sprintf("\"%s\"", policyName), "user.show after detach")
+	})
+}

test/s3/policy/shell_serviceaccount_test.go

@@ -0,0 +1,80 @@
+package policy
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb"
+	"github.com/stretchr/testify/require"
+)
+
+// TestShellServiceAccountLifecycle exercises s3.serviceaccount.* commands end-to-end.
+func TestShellServiceAccountLifecycle(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+	cluster, err := startMiniCluster(t)
+	require.NoError(t, err)
+	defer cluster.Stop()
+
+	const weedCmd = "weed"
+	master := string(pb.NewServerAddress("127.0.0.1", cluster.masterPort, cluster.masterGrpcPort))
+	filer := string(pb.NewServerAddress("127.0.0.1", cluster.filerPort, cluster.filerGrpcPort))
+
+	userName := uniqueName("sauser")
+	execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.create -name %s", userName))
+	defer execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.delete -name %s", userName))
+
+	var saID string
+
+	t.Run("CreateAndList", func(t *testing.T) {
+		description := "integration-test-sa"
+		out := execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.serviceaccount.create -user %s -description %s", userName, description))
+		requireContains(t, out, "Created service account", "serviceaccount.create")
+		requireContains(t, out, "Access Key:", "serviceaccount.create credentials")
+		requireContains(t, out, "Secret Key:", "serviceaccount.create credentials")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.serviceaccount.list -user %s", userName))
+		requireContains(t, out, userName, "serviceaccount.list parent column")
+		saID = extractServiceAccountID(t, out, userName)
+	})
+
+	t.Run("Show", func(t *testing.T) {
+		if saID == "" {
+			t.Skip("no saID extracted")
+		}
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.serviceaccount.show -id %s", saID))
+		requireContains(t, out, saID, "show contains id")
+		requireContains(t, out, userName, "show contains parent")
+		requireContains(t, out, "enabled", "show contains status")
+	})
+
+	t.Run("CreateWithActions", func(t *testing.T) {
+		out := execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.serviceaccount.create -user %s -actions Read,List -expiry 24h", userName))
+		requireContains(t, out, "Access Key:", "serviceaccount.create with options")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.serviceaccount.list -user %s", userName))
+		// Should now have at least 2 service accounts — count lines that start with parent in field[1]
+		count := 0
+		for _, line := range splitLines(out) {
+			fields := fieldsOf(line)
+			if len(fields) >= 2 && fields[1] == userName {
+				count++
+			}
+		}
+		if count < 2 {
+			t.Fatalf("expected at least 2 service accounts for %s, got %d\n%s", userName, count, out)
+		}
+	})
+
+	t.Run("Delete", func(t *testing.T) {
+		if saID == "" {
+			t.Skip("no saID extracted")
+		}
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.serviceaccount.delete -id %s", saID))
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.serviceaccount.list -user %s", userName))
+		requireNotContains(t, out, saID, "deleted sa removed from list")
+	})
+}

test/s3/policy/shell_user_test.go

@@ -0,0 +1,110 @@
+package policy
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb"
+	"github.com/stretchr/testify/require"
+)
+
+// TestShellUserLifecycle exercises the s3.user.* commands end-to-end:
+// create, show, list, enable, disable, delete, and provision.
+func TestShellUserLifecycle(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+	cluster, err := startMiniCluster(t)
+	require.NoError(t, err)
+	defer cluster.Stop()
+
+	const weedCmd = "weed"
+	master := string(pb.NewServerAddress("127.0.0.1", cluster.masterPort, cluster.masterGrpcPort))
+	filer := string(pb.NewServerAddress("127.0.0.1", cluster.filerPort, cluster.filerGrpcPort))
+
+	t.Run("CreateShowListDelete", func(t *testing.T) {
+		userName := uniqueName("user")
+
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.create -name %s", userName))
+		requireContains(t, out, userName, "user.create output")
+		requireContains(t, out, "access_key", "user.create JSON")
+		requireContains(t, out, "Secret Key:", "user.create stderr")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.show -name %s", userName))
+		requireContains(t, out, userName, "user.show")
+
+		out = execShell(t, weedCmd, master, filer, "s3.user.list")
+		requireContains(t, out, userName, "user.list")
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.delete -name %s", userName))
+
+		out = execShell(t, weedCmd, master, filer, "s3.user.list")
+		requireNotContains(t, out, userName, "user.list after delete")
+	})
+
+	t.Run("CreateWithExplicitKeys", func(t *testing.T) {
+		userName := uniqueName("user-expl")
+		ak := "TESTAK1234567890ABCD"
+		sk := "testsecretkey1234567890abcdefghijklmnopq"
+
+		out := execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.user.create -name %s -access_key %s -secret_key %s", userName, ak, sk))
+		requireContains(t, out, ak, "user.create with explicit keys")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.show -name %s", userName))
+		requireContains(t, out, ak, "user.show reveals access key")
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.delete -name %s", userName))
+	})
+
+	t.Run("EnableDisable", func(t *testing.T) {
+		userName := uniqueName("user-toggle")
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.create -name %s", userName))
+
+		out := execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.disable -name %s", userName))
+		requireContains(t, out, "disabled", "user.disable")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.show -name %s", userName))
+		requireContains(t, out, "disabled", "user.show after disable")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.enable -name %s", userName))
+		requireContains(t, out, "enabled", "user.enable")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.show -name %s", userName))
+		requireContains(t, out, "enabled", "user.show after enable")
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.delete -name %s", userName))
+	})
+
+	t.Run("Provision", func(t *testing.T) {
+		userName := uniqueName("prov-user")
+		bucketName := uniqueName("prov-bkt")
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.create -name %s", bucketName))
+
+		out := execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.user.provision -name %s -bucket %s -role readwrite", userName, bucketName))
+		requireContains(t, out, "Created policy", "provision output")
+		requireContains(t, out, "Created user", "provision output")
+		requireContains(t, out, "Access Key:", "provision credentials")
+		requireContains(t, out, "Secret Key:", "provision credentials")
+
+		out = execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.show -name %s", userName))
+		requireContains(t, out, userName, "user.show after provision")
+
+		// Second call with same user but different bucket/role should succeed without creating duplicate user.
+		bucket2 := uniqueName("prov-bkt2")
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.create -name %s", bucket2))
+		out = execShell(t, weedCmd, master, filer,
+			fmt.Sprintf("s3.user.provision -name %s -bucket %s -role readonly", userName, bucket2))
+		requireContains(t, out, "already exists", "provision on existing user")
+		requireContains(t, out, "Created policy", "second policy created")
+		requireContains(t, out, "Attached policy", "second policy attached to existing user")
+		requireNotContains(t, out, "Access Key:", "no new credentials for existing user")
+
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.user.delete -name %s", userName))
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.delete -name %s", bucketName))
+		execShell(t, weedCmd, master, filer, fmt.Sprintf("s3.bucket.delete -name %s", bucket2))
+	})
+}

weed/shell/command_s3_serviceaccount_create.go

@@ -2,6 +2,8 @@ package shell
 
 import (
 	"context"
+	"crypto/rand"
+	"encoding/hex"
 	"flag"
 	"fmt"
 	"io"
@@ -63,7 +65,17 @@ func (c *commandS3ServiceAccountCreate) Do(args []string, commandEnv *CommandEnv
 		return fmt.Errorf("generate secret key: %v", err)
 	}
 
+	// Generate a unique service account ID matching the format
+	// required by credential.ValidateServiceAccountId: sa:<parent>:<uuid>.
+	// 16 bytes (128 bits) of randomness makes collisions negligible.
+	var idBytes [16]byte
+	if _, err := rand.Read(idBytes[:]); err != nil {
+		return fmt.Errorf("generate service account id: %v", err)
+	}
+	saId := fmt.Sprintf("sa:%s:%s", *user, hex.EncodeToString(idBytes[:]))
+
 	sa := &iam_pb.ServiceAccount{
+		Id:          saId,
 		ParentUser:  *user,
 		Description: *description,
 		Credential: &iam_pb.Credential{
@@ -117,7 +129,7 @@ func (c *commandS3ServiceAccountCreate) Do(args []string, commandEnv *CommandEnv
 	}
 
 	fmt.Fprintf(writer, "Created service account for user %q\n", *user)
-	fmt.Fprintln(writer, "Note: use s3.serviceaccount.list to find the server-assigned ID.")
+	fmt.Fprintf(writer, "ID:         %s\n", saId)
 	fmt.Fprintf(writer, "Access Key: %s\n", ak)
 	fmt.Fprintf(writer, "Secret Key: %s\n", sk)
 	if *description != "" {

weed/shell/command_s3_user_provision.go

@@ -110,15 +110,8 @@ func (c *commandS3UserProvision) Do(args []string, commandEnv *CommandEnv, write
 		return fmt.Errorf("marshal policy: %v", err)
 	}
 
-	// Generate credentials
-	ak, err := iam.GenerateRandomString(iam.AccessKeyIdLength, iam.CharsetUpper)
-	if err != nil {
-		return fmt.Errorf("generate access key: %v", err)
-	}
-	sk, err := iam.GenerateSecretAccessKey()
-	if err != nil {
-		return fmt.Errorf("generate secret key: %v", err)
-	}
+	var ak, sk string
+	var userCreated bool
 
 	err = pb.WithGrpcClient(false, 0, func(conn *grpc.ClientConn) error {
 		client := iam_pb.NewSeaweedIdentityAccessManagementClient(conn)
@@ -126,8 +119,10 @@ func (c *commandS3UserProvision) Do(args []string, commandEnv *CommandEnv, write
 		defer cancel()
 
 		// Step 0: Check if user already exists
+		var existingIdentity *iam_pb.Identity
 		if resp, getErr := client.GetUser(ctx, &iam_pb.GetUserRequest{Username: *name}); getErr == nil && resp.Identity != nil {
-			return fmt.Errorf("user %q already exists", *name)
+			existingIdentity = resp.Identity
+			fmt.Fprintf(writer, "User %q already exists, adding policy\n", *name)
 		} else if getErr != nil && status.Code(getErr) != codes.NotFound {
 			return fmt.Errorf("check user existence: %w", getErr)
 		}
@@ -142,27 +137,61 @@ func (c *commandS3UserProvision) Do(args []string, commandEnv *CommandEnv, write
 		}
 		fmt.Fprintf(writer, "Created policy %q\n", policyName)
 
-		// Step 2: Create user
-		identity := &iam_pb.Identity{
-			Name: *name,
-			Credentials: []*iam_pb.Credential{
-				{
-					AccessKey: ak,
-					SecretKey: sk,
-					Status:    iam.AccessKeyStatusActive,
-				},
-			},
-			PolicyNames: []string{policyName},
-		}
-		_, err = client.CreateUser(ctx, &iam_pb.CreateUserRequest{Identity: identity})
-		if err != nil {
-			// Rollback: remove the policy we just created
+		// rollbackPolicy removes the policy we just created. Used when a later
+		// step fails, to avoid leaving the policy orphaned.
+		rollbackPolicy := func() {
 			if _, delErr := client.DeletePolicy(ctx, &iam_pb.DeletePolicyRequest{Name: policyName}); delErr != nil {
 				fmt.Fprintf(writer, "Warning: failed to rollback policy %q: %v\n", policyName, delErr)
 			}
-			return fmt.Errorf("create user: %w", err)
 		}
-		fmt.Fprintf(writer, "Created user %q with policy %q attached\n", *name, policyName)
+
+		if existingIdentity != nil {
+			// User exists: attach the new policy if not already present
+			for _, pn := range existingIdentity.PolicyNames {
+				if pn == policyName {
+					fmt.Fprintf(writer, "Policy %q already attached to user %q\n", policyName, *name)
+					return nil
+				}
+			}
+			existingIdentity.PolicyNames = append(existingIdentity.PolicyNames, policyName)
+			_, err = client.UpdateUser(ctx, &iam_pb.UpdateUserRequest{Username: *name, Identity: existingIdentity})
+			if err != nil {
+				rollbackPolicy()
+				return fmt.Errorf("attach policy to existing user: %w", err)
+			}
+			fmt.Fprintf(writer, "Attached policy %q to existing user %q\n", policyName, *name)
+		} else {
+			// Step 2: Create new user with credentials
+			ak, err = iam.GenerateRandomString(iam.AccessKeyIdLength, iam.CharsetUpper)
+			if err != nil {
+				rollbackPolicy()
+				return fmt.Errorf("generate access key: %v", err)
+			}
+			sk, err = iam.GenerateSecretAccessKey()
+			if err != nil {
+				rollbackPolicy()
+				return fmt.Errorf("generate secret key: %v", err)
+			}
+
+			identity := &iam_pb.Identity{
+				Name: *name,
+				Credentials: []*iam_pb.Credential{
+					{
+						AccessKey: ak,
+						SecretKey: sk,
+						Status:    iam.AccessKeyStatusActive,
+					},
+				},
+				PolicyNames: []string{policyName},
+			}
+			_, err = client.CreateUser(ctx, &iam_pb.CreateUserRequest{Identity: identity})
+			if err != nil {
+				rollbackPolicy()
+				return fmt.Errorf("create user: %w", err)
+			}
+			userCreated = true
+			fmt.Fprintf(writer, "Created user %q with policy %q attached\n", *name, policyName)
+		}
 
 		return nil
 	}, commandEnv.option.FilerAddress.ToGrpcAddress(), false, commandEnv.option.GrpcDialOption)
@@ -170,10 +199,12 @@ func (c *commandS3UserProvision) Do(args []string, commandEnv *CommandEnv, write
 		return err
 	}
 
-	fmt.Fprintln(writer)
-	fmt.Fprintf(writer, "Access Key: %s\n", ak)
-	fmt.Fprintf(writer, "Secret Key: %s\n", sk)
-	fmt.Fprintln(writer)
-	fmt.Fprintln(writer, "Save these credentials - the secret key cannot be retrieved later.")
+	if userCreated {
+		fmt.Fprintln(writer)
+		fmt.Fprintf(writer, "Access Key: %s\n", ak)
+		fmt.Fprintf(writer, "Secret Key: %s\n", sk)
+		fmt.Fprintln(writer)
+		fmt.Fprintln(writer, "Save these credentials - the secret key cannot be retrieved later.")
+	}
 	return nil
 }