Skip to content

Conversation

tyamagu2
Copy link

I recently wrote a strategy for a service that only accepts state parameter shorter than 48 characters. The state length is currently hard-coded, so I had to overwrite options[:state] in setup_phase, but it would be nice if omniauth-oauth2 had an option to change state parameter length.

@coveralls
Copy link

coveralls commented Jul 15, 2016

Coverage Status

Coverage decreased (-4.7%) to 80.247% when pulling 9520239 on tyamagu2:state_length into 464fcef on intridea:master.

def authorize_params
options.authorize_params[:state] = SecureRandom.hex(24)
state_length = options[:state_length]
options.authorize_params[:state] = SecureRandom.hex(state_length / 2 + 1)[0...state_length]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe will be better to use

params = options.authorize_params.merge(options_for("authorize"))

and update

       def options_for(option)
         hash = {}
         options.send(:"#{option}_options").select { |key| options[key] }.each do |key|
           hash[key.to_sym] = options[key].respond_to?(:call) ? options[key].call(env) : options[key]
         end
         hash
       end

so, allow to use a Proc for generation of state

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tyamagu2 , see: #97

example with rails + devise:

  config.omniauth :instagram,
    'client_id',
    'client_secret',
    state:             Proc.new { |env| FooBar.state(env) },
    authorize_options: [:scope, :state]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants