remove redirect for setup wazard

[thomaseude]

Currently if we :set_steps with a route look like : flows_onboarding_user_path(Wicked::FIRST_STEP, new_business: "yes"). It'll redirect_to first_step and overwrite my :set_steps.

The problem is Wicked::FIRST_STEP, it'll work perfectly if I replace by the step_id

Below the code that I try to achieve :

CONTROLLER

class Flows::OnboardingUserController < ApplicationController
  include Wicked::Wizard
  before_action :set_steps
  before_action :setup_wizard

  def show
     ...
  end

  def update
    ...
  end
  
   def set_steps
    if params[:steps]
      self.steps = params[:steps]
    elsif params[:new_business] == "yes"
      self.steps = [:platforms, :business_activity, :links, :reply]
    else
        self.steps = [:reply, :platforms, :business_activity, :company, :user]
    end
  end
end

ROUTE
flows_onboarding_user_path(**Wicked::FIRST_STEP**, new_business: "yes")

[schneems]

This code would create an open render vulnerability where any attacker could manipulate your app to display every file on disk, including your environment variables or other secrets files

self.steps = params[:steps]

I would not accept a patch to fix this specific example use case as it is insecure.

[schneems]

Looks like the tests are failing. Is main working or is it from your patch?

I would accept a patch to run tests on main on a chron if we don’t already have that in there.

[thomaseude]

Thanks for your feedback !

I correct and do something like that, it should fix the vulnerability problem

class Flows::OnboardingUserController < ApplicationController
  include Wicked::Wizard
  before_action :set_steps
  before_action :setup_wizard

  VALID_STEPS = [:platforms, :business_activity, :links, :reply, :company, :user].freeze
  ....
   def set_steps
    if params[:steps]
      self.steps = params[:steps].select { |step| VALID_STEPS.include?(step.to_sym) }
    elsif params[:new_business] == "yes"
      self.steps = [:platforms, :business_activity, :links, :reply]
    else
        self.steps = [:reply, :platforms, :business_activity, :company, :user]
    end
  end
end

Test are falling here, I don't know why :
/wicked/test/integration/navigation_test.rb

test 'pointer to first' do
    visit(bar_path(Wicked::FIRST_STEP))
    assert_has_content?('first')
  end

  test 'pointer to last' do
    visit(bar_path(Wicked::LAST_STEP))
    assert_has_content?('last_step')
  end

On main branch (without my update), test work well

Maybe instead of to change the code, we could update the doc, I find a way to make my use case working. I add step = :platforms

class Flows::OnboardingUserController < ApplicationController

  VALID_STEPS = [:platforms, :business_activity, :links, :reply, :company, :user].freeze
  ...
   def set_steps
    if params[:steps]
      self.steps = params[:steps].select { |step| VALID_STEPS.include?(step.to_sym) }
    elsif params[:new_business] == "yes"
      step = :platforms
      self.steps = [:platforms, :business_activity, :links, :reply]
    end
   end
end

Also. something that I'd like to confirm with you, every time that my page will be reload, :set_steps and :setup_wizard will be called. There is no better way to keep in mind the steps that set as params like I do above ?