feat(accounts): wire AWS Organizations discovery endpoint (closes #208) (#212)

* feat(accounts): wire AWS Organizations discovery endpoint to DiscoverOrgAccounts (closes #208)

POST /api/accounts/discover-org used to return a stub
"not yet implemented" message. Wire it end-to-end against the
already-shipped accounts.DiscoverOrgAccounts function:

1. Parse {account_id} body, validate it's a UUID.
2. Load the named account via config.GetCloudAccount; 404 if missing.
3. Validate provider == "aws" and aws_is_org_root == true; 400
   otherwise.
4. Resolve the org-root's stored credentials via
   credentials.ResolveAWSCredentialProvider and build an aws.Config.
5. Call accounts.DiscoverOrgAccounts (injectable seam:
   Handler.discoverOrgFn for tests; nil-default falls back to the
   real Organizations API call).
6. Dedupe each member account by (provider="aws", external_id)
   against the existing aws cloud_accounts roster.
7. Persist new members with enabled=false, aws_auth_mode=bastion,
   aws_bastion_id pointing at the org root — so the operator must
   review/approve and fill in the target role ARN before any
   discovered account participates in scheduled collection.
8. Return {discovered, created, skipped} counts.

Spec acceptance F-1, F-2, F-3 from
specs/multi-account-execution/acceptance.md.

Test additions in handler_accounts_test.go cover the primary
acceptance dimensions:
- 400 on invalid JSON body
- 400 on invalid account_id (not a UUID)
- 400 on non-AWS account
- 400 on aws_is_org_root=false
- 404 on account not found
- happy path: 3 discovered, 1 already known (skipped), 2 created;
  asserts each persisted row has enabled=false +
  aws_auth_mode=bastion + aws_bastion_id=org-root.ID

The router-level smoke test in router_handlers_test.go is updated
to assert the routing dispatcher reaches the handler (now expects
a ClientError 400 on the empty body it sends, instead of the prior
NotNil result from the stub).

MockConfigStore gains ListCloudAccountsFn + CreateCloudAccountFn
hooks so the org-discovery happy-path test can inject an existing
roster and capture the persisted rows.

* fix(accounts): tighten discoverOrgAccounts per CR pass 1 — admin-only, drop misleading bastion default, drop blanket-400 (#212)

Three CR-pass-1 findings on PR #212 addressed in one commit:

1. Permission scope: switch from requirePermission("create",
   "accounts") to requireAdmin. Org discovery can create N
   cloud_accounts rows in one call and bring unfamiliar accounts
   into the roster — admin scope is the right gate even though
   the rows boot disabled. requireAdmin honours both the admin
   API-key and bearer-token paths used elsewhere.

2. Drop the default AWSAuthMode="bastion" on persisted rows.
   awsAmbientCredResult treats role_arn or bastion modes with an
   empty AWSRoleARN as "ambient host credentials" — semantically
   correct for the CUDly self-account, but WRONG for a
   freshly-discovered member account, which would have falsely
   passed the test-credentials endpoint. Persist with
   AWSAuthMode="" instead; the operator's review step must set
   BOTH the mode AND a non-empty AWSRoleARN before flipping
   enabled=true. AWSBastionID is still pre-filled with the org
   root's ID so the operator's edit is just "add role ARN, set
   mode=bastion". The empty mode also fails
   ResolveAWSCredentialProvider's switch with a clear
   "unsupported aws_auth_mode" error if the row is enabled
   prematurely.

3. buildOrgRootAWSConfig no longer wraps every
   ResolveAWSCredentialProvider error as ClientError(400). The
   resolver mixes definite client-side validation failures
   (missing aws_role_arn) with transient server-side ones
   (credential store unavailable, network errors during
   access-key load); without a sentinel/typed error in the
   credentials package today, blanket-400 was misleading. Wrap
   the error as a regular Go error instead — the handler-default
   surface is 5xx, retries are possible, and the error message
   makes it into the structured log either way. A proper
   400/500 split lives in a future credentials-package
   error-type cleanup; flagged in the doc comment.

Test update: TestDiscoverOrgAccounts_HappyPathDedupesAndPersists
now asserts AWSAuthMode is empty on persisted rows (was checking
"bastion"). The doc above the assertions explains the WHY so a
future maintainer doesn't "fix" it back.

go test ./... clean. No behaviour change other than the three
items above.

* fix(accounts): stamp discovered org accounts metadata

* fix(accounts): skip duplicate org discoveries on insert

* fix(accounts): guard nil org discovery results

Author: cristim

internal/api/handler.go

@@ -9,6 +9,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/LeanerCloud/CUDly/internal/accounts"
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/LeanerCloud/CUDly/internal/credentials"
 	"github.com/LeanerCloud/CUDly/internal/email"
@@ -78,6 +79,12 @@ type Handler struct {
 	// the integration suite runs hermetically.
 	reshapeAccountResolver func(context.Context) (string, error)
 
+	// Optional org-discovery factory used by tests to avoid live AWS
+	// Organizations API calls. When nil (production default), the handler
+	// falls back to accounts.DiscoverOrgAccounts which dials Organizations
+	// via the credentials resolved for the org-root account.
+	discoverOrgFn func(context.Context, aws.Config) (*accounts.OrgDiscoveryResult, error)
+
 	// commitmentOpts discovers which AWS (term, payment) combinations
 	// each service actually sells and validates saves against that data.
 	// Nil is valid: the endpoint returns unavailable and save-side

internal/api/handler_accounts.go

@@ -10,11 +10,14 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
 	"golang.org/x/oauth2"
 
+	"github.com/LeanerCloud/CUDly/internal/accounts"
 	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/LeanerCloud/CUDly/internal/credentials"
 	"github.com/LeanerCloud/CUDly/internal/oidc"
 	"github.com/aws/aws-lambda-go/events"
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/sts"
 	"github.com/google/uuid"
 )
 
@@ -1086,13 +1089,191 @@ func (h *Handler) listPlanAccounts(ctx context.Context, req *events.LambdaFuncti
 	return accounts, nil
 }
 
-// discoverOrgAccounts handles POST /api/accounts/discover-org.
+// DiscoverOrgRequest is the request body for POST /api/accounts/discover-org.
+// AccountID is the UUID of the org-root cloud account whose stored credentials
+// will be used to call AWS Organizations.
+type DiscoverOrgRequest struct {
+	AccountID string `json:"account_id"`
+}
+
+// DiscoverOrgResult is the response shape for POST /api/accounts/discover-org.
+// Discovered is the total number of member accounts the AWS Organizations API
+// returned; Created is the number of new cloud_accounts rows persisted; Skipped
+// is the number that already existed (matched by provider+external_id).
+type DiscoverOrgResult struct {
+	Discovered int `json:"discovered"`
+	Created    int `json:"created"`
+	Skipped    int `json:"skipped"`
+}
+
+// discoverOrgAccounts handles POST /api/accounts/discover-org. The endpoint
+// (1) validates the named cloud account is an AWS org root, (2) resolves its
+// stored credentials, (3) calls AWS Organizations ListAccounts via the
+// credentials, (4) deduplicates against existing aws cloud_accounts rows by
+// external_id, and (5) persists the new ones with enabled=false, an empty
+// aws_auth_mode, and aws_bastion_id pointing at the org root, so an operator
+// must review/approve each discovered account and explicitly choose the
+// bastion auth mode plus role ARN before it'll be picked up by the scheduler.
+// See issue #208 for the spec; see specs/multi-account-execution/
+// acceptance.md F-1..F-3 for the acceptance criteria.
 func (h *Handler) discoverOrgAccounts(ctx context.Context, req *events.LambdaFunctionURLRequest) (any, error) {
-	if _, err := h.requirePermission(ctx, req, "create", "accounts"); err != nil {
+	// Admin-only: org discovery can create N cloud_accounts rows in one call
+	// and may bring unfamiliar accounts into the roster. Even though those
+	// rows boot disabled, the elevated privilege makes admin-scope the right
+	// gate (CR pass 1 on PR #212).
+	if _, err := h.requireAdmin(ctx, req); err != nil {
+		return nil, err
+	}
+
+	root, err := h.parseDiscoverOrgRoot(ctx, req.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	cfg, err := h.buildOrgRootAWSConfig(ctx, root)
+	if err != nil {
+		return nil, err
+	}
+
+	disco, err := h.runOrgDiscovery(ctx, cfg)
+	if err != nil {
 		return nil, err
 	}
+	if disco == nil {
+		return DiscoverOrgResult{}, nil
+	}
 
-	return map[string]string{"message": "org discovery not yet implemented"}, nil
+	return h.persistDiscoveredMembers(ctx, root, disco.Accounts)
+}
+
+// parseDiscoverOrgRoot decodes the request body, loads the named account, and
+// validates it's a usable AWS org root. Returns ClientErrors for the four
+// failure modes the spec enumerates (bad JSON / bad UUID / not-aws / not-root)
+// + 404 for missing-account. Pulled out of discoverOrgAccounts to keep that
+// function under the gocyclo budget.
+func (h *Handler) parseDiscoverOrgRoot(ctx context.Context, rawBody string) (*config.CloudAccount, error) {
+	var body DiscoverOrgRequest
+	if err := json.Unmarshal([]byte(rawBody), &body); err != nil {
+		return nil, NewClientError(400, "invalid JSON body")
+	}
+	if err := validateUUID(body.AccountID); err != nil {
+		return nil, err
+	}
+
+	root, err := h.config.GetCloudAccount(ctx, body.AccountID)
+	if err != nil {
+		return nil, fmt.Errorf("accounts: get cloud account: %w", err)
+	}
+	if root == nil {
+		return nil, NewClientError(404, "cloud account not found")
+	}
+	if root.Provider != "aws" {
+		return nil, NewClientError(400, "discover-org requires an aws account")
+	}
+	if !root.AWSIsOrgRoot {
+		return nil, NewClientError(400, "account is not configured as an org root (set aws_is_org_root=true)")
+	}
+	return root, nil
+}
+
+// buildOrgRootAWSConfig resolves the org-root account's stored credentials and
+// returns an aws.Config configured to call AWS APIs as that account.
+//
+// Returns the resolver's error wrapped as a regular Go error (which the
+// handler-default mapping surfaces as 500) — NOT a 400 ClientError — because
+// ResolveAWSCredentialProvider mixes definite client-side validation
+// failures (e.g., missing aws_role_arn for role_arn mode) with transient
+// server-side failures (credential store unavailable, network errors during
+// access-key load). Without a sentinel/typed error in the credentials
+// package today, blanket-400 was misleading; 5xx is the safer default and
+// retries are possible. Refining this to a proper 400/500 split is tracked
+// in the credentials-package error-type cleanup (out of scope for this PR;
+// see CR pass 1 on #212).
+func (h *Handler) buildOrgRootAWSConfig(ctx context.Context, root *config.CloudAccount) (aws.Config, error) {
+	baseCfg, err := h.getBaseAWSConfig(ctx)
+	if err != nil {
+		return aws.Config{}, fmt.Errorf("accounts: load base aws config: %w", err)
+	}
+	stsClient := sts.NewFromConfig(baseCfg)
+	credProvider, err := credentials.ResolveAWSCredentialProvider(ctx, root, h.credStore, stsClient)
+	if err != nil {
+		return aws.Config{}, fmt.Errorf("accounts: resolve credentials for org root %s: %w", root.ID, err)
+	}
+	cfg := baseCfg.Copy()
+	cfg.Credentials = credProvider
+	return cfg, nil
+}
+
+// runOrgDiscovery dispatches to the configured discovery function — the
+// injectable seam Handler.discoverOrgFn for tests, falling back to the real
+// accounts.DiscoverOrgAccounts in production.
+func (h *Handler) runOrgDiscovery(ctx context.Context, cfg aws.Config) (*accounts.OrgDiscoveryResult, error) {
+	discoverFn := h.discoverOrgFn
+	if discoverFn == nil {
+		discoverFn = accounts.DiscoverOrgAccounts
+	}
+	disco, err := discoverFn(ctx, cfg)
+	if err != nil {
+		return nil, fmt.Errorf("accounts: org discovery failed: %w", err)
+	}
+	return disco, nil
+}
+
+// persistDiscoveredMembers dedupes the discovered list against existing aws
+// rows and persists each new one with the spec-mandated defaults
+// (enabled=false, aws_auth_mode="", aws_bastion_id=root.ID). Returns the
+// {discovered, created, skipped} summary.
+func (h *Handler) persistDiscoveredMembers(ctx context.Context, root *config.CloudAccount, members []config.CloudAccount) (DiscoverOrgResult, error) {
+	awsProvider := "aws"
+	existing, err := h.config.ListCloudAccounts(ctx, config.CloudAccountFilter{Provider: &awsProvider})
+	if err != nil {
+		return DiscoverOrgResult{}, fmt.Errorf("accounts: list existing aws accounts: %w", err)
+	}
+	knownExternal := make(map[string]struct{}, len(existing))
+	for i := range existing {
+		knownExternal[existing[i].ExternalID] = struct{}{}
+	}
+
+	result := DiscoverOrgResult{Discovered: len(members)}
+	now := time.Now()
+	for i := range members {
+		member := members[i]
+		if _, found := knownExternal[member.ExternalID]; found {
+			result.Skipped++
+			continue
+		}
+		// Defaults the spec mandates: persist disabled (operator review gate)
+		// + bastion-id pre-filled with this org root's ID. AWSAuthMode is
+		// LEFT EMPTY on purpose — pre-setting it to "bastion" while
+		// AWSRoleARN is empty would cause awsAmbientCredResult to falsely
+		// classify the row as having valid ambient creds (the role_arn /
+		// bastion + empty-AWSRoleARN branch returns OK with the "ambient
+		// host" message, which is wrong for a discovered member account).
+		// The operator's review step must set both AWSAuthMode="bastion"
+		// AND a non-empty AWSRoleARN before flipping enabled=true; the
+		// scheduler silently skips disabled rows in the meantime, and the
+		// empty AWSAuthMode we persist here also fails
+		// ResolveAWSCredentialProvider's switch with a clear
+		// "unsupported aws_auth_mode" error if the row is enabled
+		// prematurely. (CR pass 1 on PR #212.)
+		member.Enabled = false
+		member.ID = uuid.New().String()
+		member.CreatedAt = now
+		member.UpdatedAt = now
+		member.AWSAuthMode = ""
+		member.AWSBastionID = root.ID
+		if err := h.config.CreateCloudAccount(ctx, &member); err != nil {
+			if isDuplicateKeyError(err) {
+				knownExternal[member.ExternalID] = struct{}{}
+				result.Skipped++
+				continue
+			}
+			return DiscoverOrgResult{}, fmt.Errorf("accounts: persist discovered %s: %w", member.ExternalID, err)
+		}
+		knownExternal[member.ExternalID] = struct{}{}
+		result.Created++
+	}
+	return result, nil
 }
 
 // parseServiceOverridePath parses "uuid/service-overrides/provider/service"