fix: retrieving state outputs only requires read role (#603)

Fixes #601.

Retrieving the current state outputs should not require organization
ownership, only a read role on the workspace or a workspace admin role.
This PR addresses that.

Also refactors the output retrieval code: instead of retrieving each
output one by one, all outputs are retrieved at once.

internal/rbac/role.go

@@ -32,6 +32,7 @@ var (
 			GetPlanFileAction:                    true,
 			GetWorkspaceAction:                   true,
 			GetStateVersionAction:                true,
+			GetStateVersionOutputAction:          true,
 			DownloadStateAction:                  true,
 			DownloadConfigurationVersionAction:   true,
 			GetRunAction:                         true,

internal/state/factory.go

@@ -130,39 +130,3 @@ func (f *factory) rollback(ctx context.Context, svID string) (*Version, error) {
 	}
 	return clone, nil
 }
-
-func newVersion(opts newVersionOptions) (Version, error) {
-	sv := Version{
-		ID:          internal.NewID("sv"),
-		CreatedAt:   internal.CurrentTimestamp(),
-		Serial:      opts.serial,
-		State:       opts.state,
-		WorkspaceID: opts.workspaceID,
-	}
-
-	var f File
-	if err := json.Unmarshal(opts.state, &f); err != nil {
-		return Version{}, err
-	}
-
-	// extract outputs from state file
-	outputs := make(map[string]*Output, len(f.Outputs))
-	for k, v := range f.Outputs {
-		typ, err := v.Type()
-		if err != nil {
-			return Version{}, err
-		}
-
-		outputs[k] = &Output{
-			ID:             internal.NewID("wsout"),
-			Name:           k,
-			Type:           typ,
-			Value:          v.Value,
-			Sensitive:      v.Sensitive,
-			StateVersionID: sv.ID,
-		}
-	}
-	sv.Outputs = outputs
-
-	return sv, nil
-}

internal/state/tfe.go

@@ -362,18 +362,20 @@ func (*tfe) toOutput(from *Output, scrubSensitive bool) *types.StateVersionOutpu
 
 // https://developer.hashicorp.com/terraform/cloud-docs/api-docs/state-versions#outputs
 func (a *tfe) includeOutputs(ctx context.Context, v any) ([]any, error) {
-	sv, ok := v.(*types.StateVersion)
+	to, ok := v.(*types.StateVersion)
 	if !ok {
 		return nil, nil
 	}
-	var include []any
-	for _, outWithOnlyID := range sv.Outputs {
-		out, err := a.GetStateVersionOutput(ctx, outWithOnlyID.ID)
-		if err != nil {
-			return nil, err
-		}
+	// re-retrieve the state version, because the tfe state version only
+	// possesses the IDs of the outputs, whereas we need the full output structs
+	from, err := a.GetStateVersion(ctx, to.ID)
+	if err != nil {
+		return nil, err
+	}
+	include := make([]any, len(from.Outputs))
+	for i, out := range maps.Values(from.Outputs) {
 		// do not scrub sensitive values for included outputs
-		include = append(include, a.toOutput(out, false))
+		include[i] = a.toOutput(out, false)
 	}
 	return include, nil
 }

internal/state/version.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"time"
 
+	"github.com/leg100/otf/internal"
 	"github.com/leg100/otf/internal/resource"
 )
 
@@ -44,6 +45,42 @@ type (
 	}
 )
 
+func newVersion(opts newVersionOptions) (Version, error) {
+	sv := Version{
+		ID:          internal.NewID("sv"),
+		CreatedAt:   internal.CurrentTimestamp(),
+		Serial:      opts.serial,
+		State:       opts.state,
+		WorkspaceID: opts.workspaceID,
+	}
+
+	var f File
+	if err := json.Unmarshal(opts.state, &f); err != nil {
+		return Version{}, err
+	}
+
+	// extract outputs from state file
+	outputs := make(map[string]*Output, len(f.Outputs))
+	for k, v := range f.Outputs {
+		typ, err := v.Type()
+		if err != nil {
+			return Version{}, err
+		}
+
+		outputs[k] = &Output{
+			ID:             internal.NewID("wsout"),
+			Name:           k,
+			Type:           typ,
+			Value:          v.Value,
+			Sensitive:      v.Sensitive,
+			StateVersionID: sv.ID,
+		}
+	}
+	sv.Outputs = outputs
+
+	return sv, nil
+}
+
 func (v *Version) String() string { return v.ID }
 
 // Clone makes a copy albeit with new identifiers.

internal/workspace/authorizer.go

@@ -27,6 +27,6 @@ func (a *authorizer) CanAccess(ctx context.Context, action rbac.Action, workspac
 	if subj.CanAccessWorkspace(action, policy) {
 		return subj, nil
 	}
-	a.Error(nil, "unauthorized action", "workspace_id", workspaceID, "organization", policy.Organization, "action", action, "subject", subj)
+	a.Error(nil, "unauthorized action", "workspace_id", workspaceID, "organization", policy.Organization, "action", action.String(), "subject", subj)
 	return nil, internal.ErrAccessNotPermitted
 }