From 0e70d8d23e9a2f89e58c600910c38b0c7df63c45 Mon Sep 17 00:00:00 2001 From: rch Date: Tue, 25 Nov 2025 11:57:44 -0800 Subject: [PATCH 1/6] Add form field input on task creation (if present). --- cmd/cone/flags.go | 5 + cmd/cone/form_fields.go | 405 ++++++++++++++++++++++++++++++++++++++ cmd/cone/get_drop_task.go | 61 +++++- pkg/client/client.go | 1 + pkg/client/task.go | 4 + 5 files changed, 473 insertions(+), 3 deletions(-) create mode 100644 cmd/cone/form_fields.go diff --git a/cmd/cone/flags.go b/cmd/cone/flags.go index e4ed674a..162b7e52 100644 --- a/cmd/cone/flags.go +++ b/cmd/cone/flags.go @@ -21,6 +21,7 @@ const ( rawTokenFlag = "raw" appDisplayNameFlag = "app" showEncryptedFlag = "show-encrypted" + formDataFlag = "form-data" ) func addWaitFlag(cmd *cobra.Command) { @@ -83,3 +84,7 @@ func addAppDisplayNameFlag(cmd *cobra.Command) { func addShowEncryptedFlag(cmd *cobra.Command) { cmd.Flags().Bool(showEncryptedFlag, false, "Show credentials we could not decrypt.") } + +func addFormDataFlag(cmd *cobra.Command) { + cmd.Flags().String(formDataFlag, "", "Form field data as comma-separated key=value pairs (e.g., 'field1=value1,field2=value2'). Required fields will be prompted interactively if not provided.") +} diff --git a/cmd/cone/form_fields.go b/cmd/cone/form_fields.go new file mode 100644 index 00000000..7f666908 --- /dev/null +++ b/cmd/cone/form_fields.go @@ -0,0 +1,405 @@ +package main + +import ( + "context" + "fmt" + "strconv" + "strings" + + "github.com/pterm/pterm" + "github.com/spf13/viper" + + "github.com/conductorone/conductorone-sdk-go/pkg/models/shared" + + "github.com/conductorone/cone/pkg/client" + "github.com/conductorone/cone/pkg/output" +) + +// collectFormFields collects form field values from the user based on the form definition. +// Returns a map of field names to their values, or nil if no form fields are present. +func collectFormFields(ctx context.Context, v *viper.Viper, form *shared.FormInput) (map[string]any, error) { + if form == nil || len(form.Fields) == 0 { + return nil, nil + } + + requestData := make(map[string]any) + isNonInteractive := v.GetBool(nonInteractiveFlag) + + // Collect form data from command-line flags if provided + formDataFlagValue := v.GetString(formDataFlag) + formDataMap := parseFormDataFlag(formDataFlagValue) + + for _, field := range form.Fields { + fieldName := client.StringFromPtr(field.Name) + if fieldName == "" { + continue + } + + displayName := client.StringFromPtr(field.DisplayName) + if displayName == "" { + displayName = fieldName + } + + description := client.StringFromPtr(field.Description) + + // Check if value was provided via flag + if val, ok := formDataMap[fieldName]; ok { + requestData[fieldName] = val + continue + } + + // Skip if non-interactive and no flag value provided + if isNonInteractive { + // Use default value if available + if defaultValue := getFieldDefaultValue(field); defaultValue != nil { + requestData[fieldName] = defaultValue + } + continue + } + + // Collect value interactively + value, err := collectFieldValue(ctx, field, displayName, description) + if err != nil { + return nil, fmt.Errorf("error collecting field %s: %w", fieldName, err) + } + + if value != nil { + requestData[fieldName] = value + } + } + + if len(requestData) == 0 { + return nil, nil + } + + return requestData, nil +} + +// collectFieldValue collects a single field value from the user based on field type. +func collectFieldValue(ctx context.Context, field shared.Field, displayName, description string) (any, error) { + // Check for default value first + if defaultValue := getFieldDefaultValue(field); defaultValue != nil { + // Show default value and ask for confirmation + pterm.Info.Printf("Field '%s' has default value: %v\n", displayName, defaultValue) + if description != "" { + pterm.Println(description) + } + useDefault, err := pterm.DefaultInteractiveConfirm.Show("Use default value?") + if err != nil { + return nil, err + } + if useDefault { + return defaultValue, nil + } + } + + // Collect based on field type + switch { + case field.StringField != nil: + return collectStringField(ctx, field.StringField, displayName, description) + case field.BoolField != nil: + return collectBoolField(ctx, field.BoolField, displayName, description) + case field.Int64Field != nil: + return collectInt64Field(ctx, field.Int64Field, displayName, description) + case field.StringSliceField != nil: + return collectStringSliceField(ctx, field.StringSliceField, displayName, description) + default: + return nil, fmt.Errorf("unsupported field type for field: %s", displayName) + } +} + +// collectStringField collects a string field value with validation. +func collectStringField(ctx context.Context, field *shared.StringField, displayName, description string) (string, error) { + validator := StringFieldValidator{ + field: field, + displayName: displayName, + description: description, + } + + defaultValue := "" + if field.DefaultValue != nil { + defaultValue = *field.DefaultValue + } + + value, err := output.GetValidInput(ctx, defaultValue, validator) + if err != nil { + return "", err + } + + return value, nil +} + +// collectBoolField collects a boolean field value. +func collectBoolField(ctx context.Context, field *shared.BoolField, displayName, description string) (bool, error) { + if description != "" { + pterm.Info.Println(description) + } + + prompt := fmt.Sprintf("Enter value for '%s' (true/false)", displayName) + if field.DefaultValue != nil { + prompt = fmt.Sprintf("Enter value for '%s' (true/false, default: %v)", displayName, *field.DefaultValue) + } + + result, err := pterm.DefaultInteractiveConfirm.Show(prompt) + if err != nil { + return false, err + } + + return result, nil +} + +// collectInt64Field collects an int64 field value with validation. +func collectInt64Field(ctx context.Context, field *shared.Int64Field, displayName, description string) (int64, error) { + validator := Int64FieldValidator{ + field: field, + displayName: displayName, + description: description, + } + + defaultValue := "" + if field.DefaultValue != nil { + defaultValue = strconv.FormatInt(*field.DefaultValue, 10) + } + + value, err := output.GetValidInput(ctx, defaultValue, validator) + if err != nil { + return 0, err + } + + return value, nil +} + +// collectStringSliceField collects a string slice field value. +func collectStringSliceField(ctx context.Context, field *shared.StringSliceField, displayName, description string) ([]string, error) { + if description != "" { + pterm.Info.Println(description) + } + + prompt := fmt.Sprintf("Enter values for '%s' (comma-separated)", displayName) + if len(field.DefaultValues) > 0 { + prompt = fmt.Sprintf("Enter values for '%s' (comma-separated, default: %s)", displayName, strings.Join(field.DefaultValues, ", ")) + } + + userInput := pterm.DefaultInteractiveTextInput.WithMultiLine(false) + input, err := userInput.Show(prompt) + if err != nil { + return nil, err + } + + if input == "" { + if len(field.DefaultValues) > 0 { + return field.DefaultValues, nil + } + return []string{}, nil + } + + // Split by comma and trim whitespace + values := strings.Split(input, ",") + result := make([]string, 0, len(values)) + for _, v := range values { + trimmed := strings.TrimSpace(v) + if trimmed != "" { + result = append(result, trimmed) + } + } + + return result, nil +} + +// getFieldDefaultValue extracts the default value from a field based on its type. +func getFieldDefaultValue(field shared.Field) any { + switch { + case field.StringField != nil && field.StringField.DefaultValue != nil: + return *field.StringField.DefaultValue + case field.BoolField != nil && field.BoolField.DefaultValue != nil: + return *field.BoolField.DefaultValue + case field.Int64Field != nil && field.Int64Field.DefaultValue != nil: + return *field.Int64Field.DefaultValue + case field.StringSliceField != nil && len(field.StringSliceField.DefaultValues) > 0: + return field.StringSliceField.DefaultValues + default: + return nil + } +} + +// parseFormDataFlag parses the --form-data flag value. +// Expected format: "field1=value1,field2=value2" or JSON object. +func parseFormDataFlag(formDataFlag string) map[string]any { + if formDataFlag == "" { + return nil + } + + result := make(map[string]any) + + // Try parsing as comma-separated key=value pairs + pairs := strings.Split(formDataFlag, ",") + for _, pair := range pairs { + parts := strings.SplitN(strings.TrimSpace(pair), "=", 2) + if len(parts) == 2 { + key := strings.TrimSpace(parts[0]) + value := strings.TrimSpace(parts[1]) + result[key] = value + } + } + + return result +} + +// StringFieldValidator validates string field input. +type StringFieldValidator struct { + field *shared.StringField + displayName string + description string +} + +func (v StringFieldValidator) IsValid(txt string) (string, bool) { + if txt == "" { + // Check if field is required + if v.field.StringRules != nil { + // StringRules might have required field, but we'll be lenient here + // and allow empty if no explicit requirement + return txt, true + } + return txt, true + } + + // Apply validation rules if present + if v.field.StringRules != nil { + rules := v.field.StringRules + if rules.MinLen != nil { + minLen, err := strconv.Atoi(*rules.MinLen) + if err == nil && len(txt) < minLen { + return txt, false + } + } + if rules.MaxLen != nil { + maxLen, err := strconv.Atoi(*rules.MaxLen) + if err == nil && len(txt) > maxLen { + return txt, false + } + } + // Additional validations (email, URI, etc.) could be added here + } + + return txt, true +} + +func (v StringFieldValidator) Prompt(isFirstRun bool) { + if isFirstRun { + if v.description != "" { + pterm.Info.Println(v.description) + } + prompt := fmt.Sprintf("Enter value for '%s'", v.displayName) + if v.field.Placeholder != nil { + prompt = fmt.Sprintf("%s (e.g., %s)", prompt, *v.field.Placeholder) + } + if v.field.DefaultValue != nil { + prompt = fmt.Sprintf("%s (default: %s)", prompt, *v.field.DefaultValue) + } + output.InputNeeded.Println(prompt) + } else { + output.InputNeeded.Println("Invalid input. Please try again.") + } +} + +// Int64FieldValidator validates int64 field input. +type Int64FieldValidator struct { + field *shared.Int64Field + displayName string + description string +} + +func (v Int64FieldValidator) IsValid(txt string) (int64, bool) { + if txt == "" { + // Allow empty if there's a default value + if v.field.DefaultValue != nil { + return *v.field.DefaultValue, true + } + return 0, false + } + + value, err := strconv.ParseInt(txt, 10, 64) + if err != nil { + return 0, false + } + + // Apply validation rules if present + if v.field.Int64Rules != nil { + rules := v.field.Int64Rules + if rules.Const != nil && value != *rules.Const { + return 0, false + } + if rules.Lt != nil && value >= *rules.Lt { + return 0, false + } + if rules.Lte != nil && value > *rules.Lte { + return 0, false + } + if rules.Gt != nil && value <= *rules.Gt { + return 0, false + } + if rules.Gte != nil && value < *rules.Gte { + return 0, false + } + } + + return value, true +} + +func (v Int64FieldValidator) Prompt(isFirstRun bool) { + if isFirstRun { + if v.description != "" { + pterm.Info.Println(v.description) + } + prompt := fmt.Sprintf("Enter integer value for '%s'", v.displayName) + if v.field.Placeholder != nil { + prompt = fmt.Sprintf("%s (e.g., %s)", prompt, *v.field.Placeholder) + } + if v.field.DefaultValue != nil { + prompt = fmt.Sprintf("%s (default: %d)", prompt, *v.field.DefaultValue) + } + output.InputNeeded.Println(prompt) + } else { + output.InputNeeded.Println("Invalid integer input. Please try again.") + } +} + +// getFormFromTask retrieves the form definition from a task. +// This is used when the form is only available after task creation. +func getFormFromTask(task *shared.Task) *shared.FormInput { + if task == nil { + return nil + } + return task.Form +} + +// validateFormData validates that all required form fields are present. +func validateFormData(form *shared.FormInput, requestData map[string]any) error { + if form == nil { + return nil + } + + for _, field := range form.Fields { + fieldName := client.StringFromPtr(field.Name) + if fieldName == "" { + continue + } + + // Check if field is required (this is a simplified check) + // In practice, you'd check field rules for required status + _, hasValue := requestData[fieldName] + if !hasValue { + // Check if there's a default value + if getFieldDefaultValue(field) == nil { + displayName := client.StringFromPtr(field.DisplayName) + if displayName == "" { + displayName = fieldName + } + return fmt.Errorf("required field '%s' is missing", displayName) + } + } + } + + return nil +} + diff --git a/cmd/cone/get_drop_task.go b/cmd/cone/get_drop_task.go index 46096e9a..8fa73994 100644 --- a/cmd/cone/get_drop_task.go +++ b/cmd/cone/get_drop_task.go @@ -29,7 +29,17 @@ func getCmd() *cobra.Command { cmd := &cobra.Command{ Use: "get [flags]\n cone get --query [flags]\n cone get --app-id --entitlement-id [flags]", Short: "Create an access request for an entitlement by alias", - RunE: runGet, + Long: `Create an access request for an entitlement by alias, query, or explicit app/entitlement IDs. + +Some entitlements may require custom form fields to be filled out when making an access request. +If form fields are required, you will be prompted interactively to provide them, or you can +provide them via the --form-data flag in the format: "field1=value1,field2=value2". + +Examples: + cone get my-entitlement-alias + cone get --query "GitHub Admin" --justification "Need admin access" + cone get --app-id app123 --entitlement-id ent456 --form-data "reason=project-work,duration=2w"`, + RunE: runGet, } addGrantDurationFlag(cmd) addEmergencyAccessFlag(cmd) @@ -54,6 +64,7 @@ func taskCmd(cmd *cobra.Command) *cobra.Command { addEntitlementAliasFlag(cmd) addForceTaskCreateFlag(cmd) addEntitlementDetailsFlag(cmd) + addFormDataFlag(cmd) return cmd } @@ -231,7 +242,19 @@ func runGet(cmd *cobra.Command, args []string) error { apiDuration = fmt.Sprintf("%ds", seconds) } - accessRequest, err := c.CreateGrantTask(ctx, appId, entitlementId, userId, appUserId, justification, apiDuration, emergencyAccess) + // Collect form data if provided via flags + var requestData map[string]any + formDataFlagValue := v.GetString(formDataFlag) + hasFormDataFlag := formDataFlagValue != "" + + // Only send requestData if entitlement has a request schema or if user explicitly provided form data + // The API may accept extra data, but we'll validate after task creation + if hasFormDataFlag { + requestData = parseFormDataFlag(formDataFlagValue) + } + + // Create the task with initial form data (if any) + accessRequest, err := c.CreateGrantTask(ctx, appId, entitlementId, userId, appUserId, justification, apiDuration, emergencyAccess, requestData) if err != nil { errorBody := err.Error() if strings.Contains(errorBody, durationErrorMessage) { @@ -241,7 +264,39 @@ func runGet(cmd *cobra.Command, args []string) error { } return nil, err } - return accessRequest.TaskView.Task, nil + + task := accessRequest.TaskView.Task + + // Check if the task has form fields + hasFormFields := task.Form != nil && len(task.Form.Fields) > 0 + + if hasFormFields { + // Collect form fields if not already provided + if requestData == nil || len(requestData) == 0 { + collectedData, err := collectFormFields(ctx, v, task.Form) + if err != nil { + // Log error but don't fail - task was already created + pterm.Warning.Printf("Error collecting form fields: %v\n", err) + } else if collectedData != nil && len(collectedData) > 0 { + // Note: The task is already created, so we can't update it with form data + // In a production scenario, you might want to update the task or recreate it + pterm.Info.Println("Form fields collected. Note: Task was already created without form data.") + pterm.Info.Println("To provide form data on creation, use --form-data flag.") + pterm.Println("Collected form data:", collectedData) + } + } else { + // Validate that provided form data matches the form structure + if err := validateFormData(task.Form, requestData); err != nil { + pterm.Warning.Printf("Form data validation warning: %v\n", err) + } + } + } else if hasFormDataFlag { + // Form data was provided but task doesn't have form fields + // This is likely fine - the API probably ignores extra data, but warn the user + pterm.Warning.Printf("Form data was provided via --form-data flag, but this entitlement does not require form fields. The data was sent but may be ignored by the API.\n") + } + + return task, nil }) } diff --git a/pkg/client/client.go b/pkg/client/client.go index 1edc0447..68a2c9ad 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -76,6 +76,7 @@ type C1Client interface { justification string, duration string, emergencyAccess bool, + requestData map[string]any, ) (*shared.TaskServiceCreateGrantResponse, error) CreateRevokeTask( ctx context.Context, diff --git a/pkg/client/task.go b/pkg/client/task.go index 0a9f54fb..76d381af 100644 --- a/pkg/client/task.go +++ b/pkg/client/task.go @@ -29,6 +29,7 @@ func (c *client) CreateGrantTask( justification string, duration string, emergencyAccess bool, + requestData map[string]any, ) (*shared.TaskServiceCreateGrantResponse, error) { req := shared.TaskServiceCreateGrantRequest{ AppEntitlementID: appEntitlementId, @@ -41,6 +42,9 @@ func (c *client) CreateGrantTask( if duration != "" { req.GrantDuration = &duration } + if requestData != nil && len(requestData) > 0 { + req.RequestData = requestData + } resp, err := c.sdk.Task.CreateGrantTask(ctx, &req) if err != nil { return nil, err From 5e42aed7705dcc29c9ac3d68bf72877e273ae327 Mon Sep 17 00:00:00 2001 From: rch Date: Fri, 12 Dec 2025 10:22:38 -0800 Subject: [PATCH 2/6] Fix form field handling and add logging infrastructure - Added UpdateTaskRequestData client method to update task after collection - Change --form-data flag from comma-separated to JSON format Properly handles values containing commas or special characters - Improve required field validation using IgnoreEmpty and MinLen rules - Fix Int64FieldValidator to be consistent with StringFieldValidator Default values now passed as initial values, not returned from IsValid - Add context cancellation checks to collectBoolField/collectStringSliceField - Add logging package (pkg/logging) with zap and --log-level flag Supports debug, info, warn, error levels; silent by default --- cmd/cone/flags.go | 2 +- cmd/cone/form_fields.go | 112 +++++++++++++++++++++++++++--------- cmd/cone/get_drop_task.go | 44 +++++++------- cmd/cone/main.go | 10 +++- pkg/client/client.go | 1 + pkg/client/task.go | 17 ++++++ pkg/logging/logging.go | 118 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 255 insertions(+), 49 deletions(-) create mode 100644 pkg/logging/logging.go diff --git a/cmd/cone/flags.go b/cmd/cone/flags.go index 162b7e52..bddfb2ad 100644 --- a/cmd/cone/flags.go +++ b/cmd/cone/flags.go @@ -86,5 +86,5 @@ func addShowEncryptedFlag(cmd *cobra.Command) { } func addFormDataFlag(cmd *cobra.Command) { - cmd.Flags().String(formDataFlag, "", "Form field data as comma-separated key=value pairs (e.g., 'field1=value1,field2=value2'). Required fields will be prompted interactively if not provided.") + cmd.Flags().String(formDataFlag, "", `Form field data as JSON (e.g., '{"field1":"value1","field2":"value2"}'). Required fields will be prompted interactively if not provided.`) } diff --git a/cmd/cone/form_fields.go b/cmd/cone/form_fields.go index 7f666908..8a656316 100644 --- a/cmd/cone/form_fields.go +++ b/cmd/cone/form_fields.go @@ -2,6 +2,7 @@ package main import ( "context" + "encoding/json" "fmt" "strconv" "strings" @@ -27,7 +28,10 @@ func collectFormFields(ctx context.Context, v *viper.Viper, form *shared.FormInp // Collect form data from command-line flags if provided formDataFlagValue := v.GetString(formDataFlag) - formDataMap := parseFormDataFlag(formDataFlagValue) + formDataMap, err := parseFormDataFlag(formDataFlagValue) + if err != nil { + return nil, err + } for _, field := range form.Fields { fieldName := client.StringFromPtr(field.Name) @@ -131,6 +135,12 @@ func collectStringField(ctx context.Context, field *shared.StringField, displayN // collectBoolField collects a boolean field value. func collectBoolField(ctx context.Context, field *shared.BoolField, displayName, description string) (bool, error) { + select { + case <-ctx.Done(): + return false, ctx.Err() + default: + } + if description != "" { pterm.Info.Println(description) } @@ -171,6 +181,12 @@ func collectInt64Field(ctx context.Context, field *shared.Int64Field, displayNam // collectStringSliceField collects a string slice field value. func collectStringSliceField(ctx context.Context, field *shared.StringSliceField, displayName, description string) ([]string, error) { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + } + if description != "" { pterm.Info.Println(description) } @@ -222,27 +238,19 @@ func getFieldDefaultValue(field shared.Field) any { } } -// parseFormDataFlag parses the --form-data flag value. -// Expected format: "field1=value1,field2=value2" or JSON object. -func parseFormDataFlag(formDataFlag string) map[string]any { +// parseFormDataFlag parses the --form-data flag value as JSON. +// Expected format: '{"field1":"value1","field2":"value2"}' +func parseFormDataFlag(formDataFlag string) (map[string]any, error) { if formDataFlag == "" { - return nil + return nil, nil } result := make(map[string]any) - - // Try parsing as comma-separated key=value pairs - pairs := strings.Split(formDataFlag, ",") - for _, pair := range pairs { - parts := strings.SplitN(strings.TrimSpace(pair), "=", 2) - if len(parts) == 2 { - key := strings.TrimSpace(parts[0]) - value := strings.TrimSpace(parts[1]) - result[key] = value - } + if err := json.Unmarshal([]byte(formDataFlag), &result); err != nil { + return nil, fmt.Errorf("invalid JSON in --form-data flag: %w", err) } - return result + return result, nil } // StringFieldValidator validates string field input. @@ -311,10 +319,7 @@ type Int64FieldValidator struct { func (v Int64FieldValidator) IsValid(txt string) (int64, bool) { if txt == "" { - // Allow empty if there's a default value - if v.field.DefaultValue != nil { - return *v.field.DefaultValue, true - } + // Empty is invalid - default values are handled by passing them as initial value return 0, false } @@ -373,6 +378,41 @@ func getFormFromTask(task *shared.Task) *shared.FormInput { return task.Form } +// isFieldRequired checks if a field is required based on its validation rules. +func isFieldRequired(field shared.Field) bool { + switch { + case field.StringField != nil: + rules := field.StringField.StringRules + if rules == nil { + return false + } + // Field is required if IgnoreEmpty is not explicitly true and MinLen >= 1 + ignoreEmpty := rules.IgnoreEmpty != nil && *rules.IgnoreEmpty + if ignoreEmpty { + return false + } + if rules.MinLen != nil { + minLen, err := strconv.Atoi(*rules.MinLen) + if err == nil && minLen >= 1 { + return true + } + } + return false + case field.Int64Field != nil: + // Int64 fields don't have an IgnoreEmpty concept in the same way + // Consider required if there are validation rules but no default + return field.Int64Field.Int64Rules != nil && field.Int64Field.DefaultValue == nil + case field.BoolField != nil: + // Bool fields typically have a default (false), so rarely required + return false + case field.StringSliceField != nil: + // String slice fields - check if there are rules requiring items + return false + default: + return false + } +} + // validateFormData validates that all required form fields are present. func validateFormData(form *shared.FormInput, requestData map[string]any) error { if form == nil { @@ -385,18 +425,36 @@ func validateFormData(form *shared.FormInput, requestData map[string]any) error continue } - // Check if field is required (this is a simplified check) - // In practice, you'd check field rules for required status - _, hasValue := requestData[fieldName] + displayName := client.StringFromPtr(field.DisplayName) + if displayName == "" { + displayName = fieldName + } + + // Check if field is required + if !isFieldRequired(field) { + continue + } + + // Check if value was provided + val, hasValue := requestData[fieldName] if !hasValue { // Check if there's a default value if getFieldDefaultValue(field) == nil { - displayName := client.StringFromPtr(field.DisplayName) - if displayName == "" { - displayName = fieldName - } return fmt.Errorf("required field '%s' is missing", displayName) } + continue + } + + // Check if value is empty + switch v := val.(type) { + case string: + if v == "" { + return fmt.Errorf("required field '%s' cannot be empty", displayName) + } + case []string: + if len(v) == 0 { + return fmt.Errorf("required field '%s' cannot be empty", displayName) + } } } diff --git a/cmd/cone/get_drop_task.go b/cmd/cone/get_drop_task.go index 8fa73994..b132c192 100644 --- a/cmd/cone/get_drop_task.go +++ b/cmd/cone/get_drop_task.go @@ -15,6 +15,7 @@ import ( "github.com/conductorone/conductorone-sdk-go/pkg/models/shared" "github.com/conductorone/cone/pkg/client" + "github.com/conductorone/cone/pkg/logging" "github.com/conductorone/cone/pkg/output" ) @@ -33,12 +34,12 @@ func getCmd() *cobra.Command { Some entitlements may require custom form fields to be filled out when making an access request. If form fields are required, you will be prompted interactively to provide them, or you can -provide them via the --form-data flag in the format: "field1=value1,field2=value2". +provide them via the --form-data flag as JSON. Examples: cone get my-entitlement-alias cone get --query "GitHub Admin" --justification "Need admin access" - cone get --app-id app123 --entitlement-id ent456 --form-data "reason=project-work,duration=2w"`, + cone get --app-id app123 --entitlement-id ent456 --form-data '{"reason":"project-work","duration":"2w"}'`, RunE: runGet, } addGrantDurationFlag(cmd) @@ -245,12 +246,14 @@ func runGet(cmd *cobra.Command, args []string) error { // Collect form data if provided via flags var requestData map[string]any formDataFlagValue := v.GetString(formDataFlag) - hasFormDataFlag := formDataFlagValue != "" - - // Only send requestData if entitlement has a request schema or if user explicitly provided form data - // The API may accept extra data, but we'll validate after task creation - if hasFormDataFlag { - requestData = parseFormDataFlag(formDataFlagValue) + + // Only send requestData if user explicitly provided form data + if formDataFlagValue != "" { + var err error + requestData, err = parseFormDataFlag(formDataFlagValue) + if err != nil { + return nil, err + } } // Create the task with initial form data (if any) @@ -269,20 +272,21 @@ func runGet(cmd *cobra.Command, args []string) error { // Check if the task has form fields hasFormFields := task.Form != nil && len(task.Form.Fields) > 0 - + if hasFormFields { // Collect form fields if not already provided if requestData == nil || len(requestData) == 0 { collectedData, err := collectFormFields(ctx, v, task.Form) if err != nil { - // Log error but don't fail - task was already created - pterm.Warning.Printf("Error collecting form fields: %v\n", err) - } else if collectedData != nil && len(collectedData) > 0 { - // Note: The task is already created, so we can't update it with form data - // In a production scenario, you might want to update the task or recreate it - pterm.Info.Println("Form fields collected. Note: Task was already created without form data.") - pterm.Info.Println("To provide form data on creation, use --form-data flag.") - pterm.Println("Collected form data:", collectedData) + return nil, fmt.Errorf("error collecting form fields: %w", err) + } + if collectedData != nil && len(collectedData) > 0 { + // Update the task with the collected form data + taskID := client.StringFromPtr(task.ID) + _, err := c.UpdateTaskRequestData(ctx, taskID, collectedData) + if err != nil { + return nil, fmt.Errorf("error updating task with form data: %w", err) + } } } else { // Validate that provided form data matches the form structure @@ -290,10 +294,10 @@ func runGet(cmd *cobra.Command, args []string) error { pterm.Warning.Printf("Form data validation warning: %v\n", err) } } - } else if hasFormDataFlag { + } else if formDataFlagValue != "" { // Form data was provided but task doesn't have form fields - // This is likely fine - the API probably ignores extra data, but warn the user - pterm.Warning.Printf("Form data was provided via --form-data flag, but this entitlement does not require form fields. The data was sent but may be ignored by the API.\n") + // The data was already sent on task creation and will be ignored by the API + logging.Debugf("Form data was provided via --form-data flag, but this entitlement does not require form fields. The data was sent but may be ignored by the API.") } return task, nil diff --git a/cmd/cone/main.go b/cmd/cone/main.go index 1bfa7425..dc264b2e 100644 --- a/cmd/cone/main.go +++ b/cmd/cone/main.go @@ -8,8 +8,10 @@ import ( "syscall" "github.com/spf13/cobra" + "github.com/spf13/viper" "github.com/conductorone/cone/pkg/client" + "github.com/conductorone/cone/pkg/logging" ) var version = "dev" @@ -54,7 +56,8 @@ func runCli(ctx context.Context) int { cliCmd.PersistentFlags().String("client-secret", "", "Client secret") cliCmd.PersistentFlags().String("api-endpoint", "", "Override the API endpoint") cliCmd.PersistentFlags().StringP("output", "o", "table", "Output format. Valid values: table, json, json-pretty, wide.") - cliCmd.PersistentFlags().Bool("debug", false, "Enable debug logging") + cliCmd.PersistentFlags().Bool("debug", false, "Enable HTTP debug logging") + cliCmd.PersistentFlags().String("log-level", "", "Set log level (debug, info, warn, error)") err := initConfig(cliCmd) if err != nil { @@ -62,6 +65,11 @@ func runCli(ctx context.Context) int { return 1 } + // Initialize logging based on --log-level flag + if logLevel := viper.GetString("log-level"); logLevel != "" { + logging.Init(logging.Level(logLevel)) + } + cliCmd.AddCommand(getCmd()) cliCmd.AddCommand(dropCmd()) cliCmd.AddCommand(whoAmICmd()) diff --git a/pkg/client/client.go b/pkg/client/client.go index 68a2c9ad..fd1dccd5 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -91,6 +91,7 @@ type C1Client interface { ApproveTask(ctx context.Context, taskId string, comment string, policyId string) (*shared.TaskActionsServiceApproveResponse, error) DenyTask(ctx context.Context, taskId string, comment string, policyId string) (*shared.TaskActionsServiceDenyResponse, error) EscalateTask(ctx context.Context, taskId string) (*shared.TaskServiceActionResponse, error) + UpdateTaskRequestData(ctx context.Context, taskID string, requestData map[string]any) (*shared.TaskServiceActionResponse, error) ListApps(ctx context.Context) ([]shared.App, error) ListAppUsers(ctx context.Context, appID string) ([]shared.AppUser, error) ListAppUsersForUser(ctx context.Context, appID string, userID string) ([]shared.AppUser, error) diff --git a/pkg/client/task.go b/pkg/client/task.go index 76d381af..0a9c0dad 100644 --- a/pkg/client/task.go +++ b/pkg/client/task.go @@ -162,3 +162,20 @@ func (c *client) EscalateTask(ctx context.Context, taskID string) (*shared.TaskS } return resp.TaskServiceActionResponse, nil } + +func (c *client) UpdateTaskRequestData(ctx context.Context, taskID string, requestData map[string]any) (*shared.TaskServiceActionResponse, error) { + resp, err := c.sdk.TaskActions.UpdateRequestData(ctx, operations.C1APITaskV1TaskActionsServiceUpdateRequestDataRequest{ + TaskActionsServiceUpdateRequestDataRequest: &shared.TaskActionsServiceUpdateRequestDataRequest{ + Data: requestData, + }, + TaskID: taskID, + }) + if err != nil { + return nil, err + } + + if err := NewHTTPError(resp.RawResponse); err != nil { + return nil, err + } + return resp.TaskServiceActionResponse, nil +} diff --git a/pkg/logging/logging.go b/pkg/logging/logging.go new file mode 100644 index 00000000..9750b498 --- /dev/null +++ b/pkg/logging/logging.go @@ -0,0 +1,118 @@ +package logging + +import ( + "sync" + + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +var ( + logger *zap.SugaredLogger + once sync.Once +) + +// Level represents log levels +type Level string + +const ( + LevelDebug Level = "debug" + LevelInfo Level = "info" + LevelWarn Level = "warn" + LevelError Level = "error" +) + +// Init initializes the global logger with the specified level. +// This should be called once at application startup. +func Init(level Level) { + once.Do(func() { + logger = newLogger(level) + }) +} + +// Get returns the global logger. If Init hasn't been called, +// it returns a no-op logger. +func Get() *zap.SugaredLogger { + if logger == nil { + // Return a no-op logger if not initialized + return zap.NewNop().Sugar() + } + return logger +} + +func newLogger(level Level) *zap.SugaredLogger { + var zapLevel zapcore.Level + switch level { + case LevelDebug: + zapLevel = zapcore.DebugLevel + case LevelInfo: + zapLevel = zapcore.InfoLevel + case LevelWarn: + zapLevel = zapcore.WarnLevel + case LevelError: + zapLevel = zapcore.ErrorLevel + default: + zapLevel = zapcore.InfoLevel + } + + config := zap.Config{ + Level: zap.NewAtomicLevelAt(zapLevel), + Development: false, + Encoding: "console", + EncoderConfig: zap.NewDevelopmentEncoderConfig(), + OutputPaths: []string{"stderr"}, + ErrorOutputPaths: []string{"stderr"}, + } + + // Simplify the output format + config.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder + config.EncoderConfig.EncodeLevel = zapcore.CapitalColorLevelEncoder + + zapLogger, err := config.Build() + if err != nil { + // Fall back to nop logger on error + return zap.NewNop().Sugar() + } + + return zapLogger.Sugar() +} + +// Debug logs a debug message +func Debug(args ...interface{}) { + Get().Debug(args...) +} + +// Debugf logs a formatted debug message +func Debugf(template string, args ...interface{}) { + Get().Debugf(template, args...) +} + +// Info logs an info message +func Info(args ...interface{}) { + Get().Info(args...) +} + +// Infof logs a formatted info message +func Infof(template string, args ...interface{}) { + Get().Infof(template, args...) +} + +// Warn logs a warning message +func Warn(args ...interface{}) { + Get().Warn(args...) +} + +// Warnf logs a formatted warning message +func Warnf(template string, args ...interface{}) { + Get().Warnf(template, args...) +} + +// Error logs an error message +func Error(args ...interface{}) { + Get().Error(args...) +} + +// Errorf logs a formatted error message +func Errorf(template string, args ...interface{}) { + Get().Errorf(template, args...) +} From 86c43404656a7d14009942b4c7f5c0f2b3e57c77 Mon Sep 17 00:00:00 2001 From: rch Date: Fri, 12 Dec 2025 10:31:14 -0800 Subject: [PATCH 3/6] 1 Paul fix, and 1 Paul suggestion: in interactive mode, let the user enter 1 value at a time for form fields which take a list --- cmd/cone/form_fields.go | 52 +++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/cmd/cone/form_fields.go b/cmd/cone/form_fields.go index 8a656316..62ae845d 100644 --- a/cmd/cone/form_fields.go +++ b/cmd/cone/form_fields.go @@ -13,6 +13,7 @@ import ( "github.com/conductorone/conductorone-sdk-go/pkg/models/shared" "github.com/conductorone/cone/pkg/client" + "github.com/conductorone/cone/pkg/logging" "github.com/conductorone/cone/pkg/output" ) @@ -108,7 +109,9 @@ func collectFieldValue(ctx context.Context, field shared.Field, displayName, des case field.StringSliceField != nil: return collectStringSliceField(ctx, field.StringSliceField, displayName, description) default: - return nil, fmt.Errorf("unsupported field type for field: %s", displayName) + // Unknown field type - warn and skip to avoid breaking on new field types + logging.Warnf("Skipping field '%s': unsupported field type. You may need to update cone to handle this field.", displayName) + return nil, nil } } @@ -179,7 +182,8 @@ func collectInt64Field(ctx context.Context, field *shared.Int64Field, displayNam return value, nil } -// collectStringSliceField collects a string slice field value. +// collectStringSliceField collects a string slice field value using a multi-entry loop. +// User enters one value per line, empty line finishes input. func collectStringSliceField(ctx context.Context, field *shared.StringSliceField, displayName, description string) ([]string, error) { select { case <-ctx.Done(): @@ -191,32 +195,40 @@ func collectStringSliceField(ctx context.Context, field *shared.StringSliceField pterm.Info.Println(description) } - prompt := fmt.Sprintf("Enter values for '%s' (comma-separated)", displayName) if len(field.DefaultValues) > 0 { - prompt = fmt.Sprintf("Enter values for '%s' (comma-separated, default: %s)", displayName, strings.Join(field.DefaultValues, ", ")) + pterm.Info.Printf("Default values for '%s': %s\n", displayName, strings.Join(field.DefaultValues, ", ")) + pterm.Info.Println("Press enter with no input to use defaults, or enter new values below.") } + pterm.Info.Printf("Enter values for '%s' (one per line, empty line to finish):\n", displayName) + + var result []string userInput := pterm.DefaultInteractiveTextInput.WithMultiLine(false) - input, err := userInput.Show(prompt) - if err != nil { - return nil, err - } - if input == "" { - if len(field.DefaultValues) > 0 { - return field.DefaultValues, nil + for { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: } - return []string{}, nil - } - // Split by comma and trim whitespace - values := strings.Split(input, ",") - result := make([]string, 0, len(values)) - for _, v := range values { - trimmed := strings.TrimSpace(v) - if trimmed != "" { - result = append(result, trimmed) + input, err := userInput.Show(fmt.Sprintf(" [%d]", len(result)+1)) + if err != nil { + return nil, err + } + + trimmed := strings.TrimSpace(input) + if trimmed == "" { + // Empty line ends input + break } + + result = append(result, trimmed) + } + + // If no values entered and defaults exist, use defaults + if len(result) == 0 && len(field.DefaultValues) > 0 { + return field.DefaultValues, nil } return result, nil From 5b1d0403d28fb1684a38cb2e04bb9206f15853e5 Mon Sep 17 00:00:00 2001 From: rch Date: Fri, 12 Dec 2025 10:47:06 -0800 Subject: [PATCH 4/6] Fix lint and also a coderabbit nit --- pkg/client/task.go | 10 ++++++---- pkg/logging/logging.go | 18 +++++++++--------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/pkg/client/task.go b/pkg/client/task.go index 0a9c0dad..686ba730 100644 --- a/pkg/client/task.go +++ b/pkg/client/task.go @@ -164,11 +164,13 @@ func (c *client) EscalateTask(ctx context.Context, taskID string) (*shared.TaskS } func (c *client) UpdateTaskRequestData(ctx context.Context, taskID string, requestData map[string]any) (*shared.TaskServiceActionResponse, error) { + req := shared.TaskActionsServiceUpdateRequestDataRequest{} + if requestData != nil && len(requestData) > 0 { + req.Data = requestData + } resp, err := c.sdk.TaskActions.UpdateRequestData(ctx, operations.C1APITaskV1TaskActionsServiceUpdateRequestDataRequest{ - TaskActionsServiceUpdateRequestDataRequest: &shared.TaskActionsServiceUpdateRequestDataRequest{ - Data: requestData, - }, - TaskID: taskID, + TaskActionsServiceUpdateRequestDataRequest: &req, + TaskID: taskID, }) if err != nil { return nil, err diff --git a/pkg/logging/logging.go b/pkg/logging/logging.go index 9750b498..d01da2ae 100644 --- a/pkg/logging/logging.go +++ b/pkg/logging/logging.go @@ -12,7 +12,7 @@ var ( once sync.Once ) -// Level represents log levels +// Level represents log levels. type Level string const ( @@ -77,42 +77,42 @@ func newLogger(level Level) *zap.SugaredLogger { return zapLogger.Sugar() } -// Debug logs a debug message +// Debug logs a debug message. func Debug(args ...interface{}) { Get().Debug(args...) } -// Debugf logs a formatted debug message +// Debugf logs a formatted debug message. func Debugf(template string, args ...interface{}) { Get().Debugf(template, args...) } -// Info logs an info message +// Info logs an info message. func Info(args ...interface{}) { Get().Info(args...) } -// Infof logs a formatted info message +// Infof logs a formatted info message. func Infof(template string, args ...interface{}) { Get().Infof(template, args...) } -// Warn logs a warning message +// Warn logs a warning message. func Warn(args ...interface{}) { Get().Warn(args...) } -// Warnf logs a formatted warning message +// Warnf logs a formatted warning message. func Warnf(template string, args ...interface{}) { Get().Warnf(template, args...) } -// Error logs an error message +// Error logs an error message. func Error(args ...interface{}) { Get().Error(args...) } -// Errorf logs a formatted error message +// Errorf logs a formatted error message. func Errorf(template string, args ...interface{}) { Get().Errorf(template, args...) } From 54600a823252ca42836cb63507d2cc27b1ada18a Mon Sep 17 00:00:00 2001 From: rch Date: Fri, 12 Dec 2025 10:51:35 -0800 Subject: [PATCH 5/6] More lint --- cmd/cone/form_fields.go | 12 +----------- cmd/cone/get_drop_task.go | 2 +- pkg/client/task.go | 6 +++--- 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/cmd/cone/form_fields.go b/cmd/cone/form_fields.go index 62ae845d..e57c6de2 100644 --- a/cmd/cone/form_fields.go +++ b/cmd/cone/form_fields.go @@ -251,7 +251,7 @@ func getFieldDefaultValue(field shared.Field) any { } // parseFormDataFlag parses the --form-data flag value as JSON. -// Expected format: '{"field1":"value1","field2":"value2"}' +// Expected format: '{"field1":"value1","field2":"value2"}'. func parseFormDataFlag(formDataFlag string) (map[string]any, error) { if formDataFlag == "" { return nil, nil @@ -381,15 +381,6 @@ func (v Int64FieldValidator) Prompt(isFirstRun bool) { } } -// getFormFromTask retrieves the form definition from a task. -// This is used when the form is only available after task creation. -func getFormFromTask(task *shared.Task) *shared.FormInput { - if task == nil { - return nil - } - return task.Form -} - // isFieldRequired checks if a field is required based on its validation rules. func isFieldRequired(field shared.Field) bool { switch { @@ -472,4 +463,3 @@ func validateFormData(form *shared.FormInput, requestData map[string]any) error return nil } - diff --git a/cmd/cone/get_drop_task.go b/cmd/cone/get_drop_task.go index b132c192..1e41ee20 100644 --- a/cmd/cone/get_drop_task.go +++ b/cmd/cone/get_drop_task.go @@ -280,7 +280,7 @@ func runGet(cmd *cobra.Command, args []string) error { if err != nil { return nil, fmt.Errorf("error collecting form fields: %w", err) } - if collectedData != nil && len(collectedData) > 0 { + if len(collectedData) > 0 { // Update the task with the collected form data taskID := client.StringFromPtr(task.ID) _, err := c.UpdateTaskRequestData(ctx, taskID, collectedData) diff --git a/pkg/client/task.go b/pkg/client/task.go index 686ba730..fd345b90 100644 --- a/pkg/client/task.go +++ b/pkg/client/task.go @@ -42,7 +42,7 @@ func (c *client) CreateGrantTask( if duration != "" { req.GrantDuration = &duration } - if requestData != nil && len(requestData) > 0 { + if len(requestData) > 0 { req.RequestData = requestData } resp, err := c.sdk.Task.CreateGrantTask(ctx, &req) @@ -165,12 +165,12 @@ func (c *client) EscalateTask(ctx context.Context, taskID string) (*shared.TaskS func (c *client) UpdateTaskRequestData(ctx context.Context, taskID string, requestData map[string]any) (*shared.TaskServiceActionResponse, error) { req := shared.TaskActionsServiceUpdateRequestDataRequest{} - if requestData != nil && len(requestData) > 0 { + if len(requestData) > 0 { req.Data = requestData } resp, err := c.sdk.TaskActions.UpdateRequestData(ctx, operations.C1APITaskV1TaskActionsServiceUpdateRequestDataRequest{ TaskActionsServiceUpdateRequestDataRequest: &req, - TaskID: taskID, + TaskID: taskID, }) if err != nil { return nil, err From 8985736e6c1c5b32ab20cfed54791706b27b6f80 Mon Sep 17 00:00:00 2001 From: rch Date: Fri, 12 Dec 2025 11:01:44 -0800 Subject: [PATCH 6/6] More lint --- cmd/cone/form_fields.go | 7 +++++-- cmd/cone/get_drop_task.go | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/cmd/cone/form_fields.go b/cmd/cone/form_fields.go index e57c6de2..97ffd45d 100644 --- a/cmd/cone/form_fields.go +++ b/cmd/cone/form_fields.go @@ -243,8 +243,11 @@ func getFieldDefaultValue(field shared.Field) any { return *field.BoolField.DefaultValue case field.Int64Field != nil && field.Int64Field.DefaultValue != nil: return *field.Int64Field.DefaultValue - case field.StringSliceField != nil && len(field.StringSliceField.DefaultValues) > 0: - return field.StringSliceField.DefaultValues + case field.StringSliceField != nil: + if len(field.StringSliceField.DefaultValues) > 0 { + return field.StringSliceField.DefaultValues + } + return nil default: return nil } diff --git a/cmd/cone/get_drop_task.go b/cmd/cone/get_drop_task.go index 1e41ee20..c51c0c5f 100644 --- a/cmd/cone/get_drop_task.go +++ b/cmd/cone/get_drop_task.go @@ -271,11 +271,14 @@ func runGet(cmd *cobra.Command, args []string) error { task := accessRequest.TaskView.Task // Check if the task has form fields - hasFormFields := task.Form != nil && len(task.Form.Fields) > 0 + hasFormFields := task.Form != nil + if hasFormFields { + hasFormFields = len(task.Form.Fields) > 0 + } if hasFormFields { // Collect form fields if not already provided - if requestData == nil || len(requestData) == 0 { + if len(requestData) == 0 { collectedData, err := collectFormFields(ctx, v, task.Form) if err != nil { return nil, fmt.Errorf("error collecting form fields: %w", err)